[PATCH AUTOSEL 5.17 01/22] ASoC: soc-pcm: use GFP_KERNEL when the code is sleepable
From: Christophe JAILLET christophe.jaillet@wanadoo.fr
[ Upstream commit fb6d679fee95d272c0a94912c4e534146823ee89 ]
At the kzalloc() call in dpcm_be_connect(), there is no spin lock involved. It's merely protected by card->pcm_mutex, instead. The spinlock is applied at the later call with snd_soc_pcm_stream_lock_irq() only for the list manipulations. (See it's *_irq(), not *_irqsave(); that means the context being sleepable at that point.) So, we can use GFP_KERNEL safely there.
This patch revert commit d8a9c6e1f676 ("ASoC: soc-pcm: use GFP_ATOMIC for dpcm structure") which is no longer needed since commit b7898396f4bb ("ASoC: soc-pcm: Fix and cleanup DPCM locking").
Signed-off-by: Christophe JAILLET christophe.jaillet@wanadoo.fr Link: https://lore.kernel.org/r/e740f1930843060e025e3c0f17ec1393cfdafb26.164875796... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 9a954680d492..11c9853e9e80 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1214,7 +1214,7 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, be_substream->pcm->nonatomic = 1; }
- dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_ATOMIC); + dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_KERNEL); if (!dpcm) return -ENOMEM;
From: Hui Wang hui.wang@canonical.com
[ Upstream commit 0b3d5d2e358ca6772fc3662fca27acb12a682fbf ]
We enabled UBSAN in the ubuntu kernel, and the cs35l41 driver triggers a warning calltrace like below:
cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: bitoffset= 8, word_offset=23, bit_sum mod 32=0, otp_map[i].size = 24 cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: bitoffset= 0, word_offset=24, bit_sum mod 32=24, otp_map[i].size = 0 ================================================================================ UBSAN: shift-out-of-bounds in linux-kernel-src/sound/soc/codecs/cs35l41-lib.c:836:8 shift exponent 64 is too large for 64-bit type 'long unsigned int' CPU: 10 PID: 595 Comm: systemd-udevd Not tainted 5.15.0-23-generic #23 Hardware name: LENOVO \x02MFG_IN_GO/\x02MFG_IN_GO, BIOS N3GET19W (1.00 ) 03/11/2022 Call Trace: <TASK> show_stack+0x52/0x58 dump_stack_lvl+0x4a/0x5f dump_stack+0x10/0x12 ubsan_epilogue+0x9/0x45 __ubsan_handle_shift_out_of_bounds.cold+0x61/0xef ? regmap_unlock_mutex+0xe/0x10 cs35l41_otp_unpack.cold+0x1c6/0x2b2 [snd_soc_cs35l41_lib] cs35l41_hda_probe+0x24f/0x33a [snd_hda_scodec_cs35l41] cs35l41_hda_i2c_probe+0x65/0x90 [snd_hda_scodec_cs35l41_i2c]
When both bitoffset and otp_map[i].size are 0, the line 836 will result in GENMASK(-1, 0), this triggers the shift-out-of-bounds calltrace.
Here add a checking, if both bitoffset and otp_map[i].size are 0, do not run GENMASK() and directly set otp_val to 0, this will not bring any function change on the driver but could avoid the calltrace.
Signed-off-by: Hui Wang hui.wang@canonical.com Link: https://lore.kernel.org/r/20220324081839.62009-2-hui.wang@canonical.com Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/codecs/cs35l41-lib.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c index e5a56bcbb223..281a710a4123 100644 --- a/sound/soc/codecs/cs35l41-lib.c +++ b/sound/soc/codecs/cs35l41-lib.c @@ -831,12 +831,14 @@ int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap) GENMASK(bit_offset + otp_map[i].size - 33, 0)) << (32 - bit_offset); bit_offset += otp_map[i].size - 32; - } else { + } else if (bit_offset + otp_map[i].size - 1 >= 0) { otp_val = (otp_mem[word_offset] & GENMASK(bit_offset + otp_map[i].size - 1, bit_offset) ) >> bit_offset; bit_offset += otp_map[i].size; - } + } else /* both bit_offset and otp_map[i].size are 0 */ + otp_val = 0; + bit_sum += otp_map[i].size;
if (bit_offset == 32) {
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
[ Upstream commit 770f3d992a3f7330f801dfeee98429b2885c9fdb ]
This patch takes a defensive programming and paranoid approach in case the parent device (SoundWire) is pm_runtime resumed but the rt711 device is not. In that case, during the attachment and initialization, a jack detection workqueue can be scheduled. Since the pm_runtime suspend routines will not be invoked, the sequence to cancel all deferred work is not executed, and the jack detection could happen after the bus stops operating, leading to a timeout.
This patch applies the same solution to rt5682, based on the similarities between codec drivers. The race condition with rt5682 was not detected experimentally though.
BugLink: https://github.com/thesofproject/linux/issues/3459 Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Link: https://lore.kernel.org/r/20220406192005.262996-1-pierre-louis.bossart@linux... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/codecs/rt5682.c | 9 +++++++++ sound/soc/codecs/rt711.c | 7 +++++++ 2 files changed, 16 insertions(+)
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c index be68d573a490..e6f7e9f82511 100644 --- a/sound/soc/codecs/rt5682.c +++ b/sound/soc/codecs/rt5682.c @@ -1100,6 +1100,15 @@ void rt5682_jack_detect_handler(struct work_struct *work) return; }
+ if (rt5682->is_sdw) { + if (pm_runtime_status_suspended(rt5682->slave->dev.parent)) { + dev_dbg(&rt5682->slave->dev, + "%s: parent device is pm_runtime_status_suspended, skipping jack detection\n", + __func__); + return; + } + } + dapm = snd_soc_component_get_dapm(rt5682->component);
snd_soc_dapm_mutex_lock(dapm); diff --git a/sound/soc/codecs/rt711.c b/sound/soc/codecs/rt711.c index 6770825d037a..ea25fd58d43a 100644 --- a/sound/soc/codecs/rt711.c +++ b/sound/soc/codecs/rt711.c @@ -245,6 +245,13 @@ static void rt711_jack_detect_handler(struct work_struct *work) if (!rt711->component->card->instantiated) return;
+ if (pm_runtime_status_suspended(rt711->slave->dev.parent)) { + dev_dbg(&rt711->slave->dev, + "%s: parent device is pm_runtime_status_suspended, skipping jack detection\n", + __func__); + return; + } + reg = RT711_VERB_GET_PIN_SENSE | RT711_HP_OUT; ret = regmap_read(rt711->regmap, reg, &jack_status); if (ret < 0)
From: Chao Song chao.song@linux.intel.com
[ Upstream commit 97326be14df7bacc6ba5c62c0556298c27ea0432 ]
The left speaker of max98373 uses spk_r_endpoint, and right speaker uses spk_l_endpoint, this is obviously wrong.
This patch corrects the endpoints for max98373 codec.
Signed-off-by: Chao Song chao.song@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20220406192341.271465-1-pierre-louis.bossart@linux... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/intel/common/soc-acpi-intel-tgl-match.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c index e2658bca6931..3137cea78d48 100644 --- a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c @@ -132,13 +132,13 @@ static const struct snd_soc_acpi_adr_device mx8373_1_adr[] = { { .adr = 0x000123019F837300ull, .num_endpoints = 1, - .endpoints = &spk_l_endpoint, + .endpoints = &spk_r_endpoint, .name_prefix = "Right" }, { .adr = 0x000127019F837300ull, .num_endpoints = 1, - .endpoints = &spk_r_endpoint, + .endpoints = &spk_l_endpoint, .name_prefix = "Left" } };
From: Zheyu Ma zheyuma97@gmail.com
[ Upstream commit 92ccbf17eeacf510cf1eed9c252d9332ca24f02d ]
When the driver fails during probing, the driver should disable the regulator, not just handle it in wm8731_hw_init().
The following log reveals it:
[ 17.812483] WARNING: CPU: 1 PID: 364 at drivers/regulator/core.c:2257 _regulator_put+0x3ec/0x4e0 [ 17.815958] RIP: 0010:_regulator_put+0x3ec/0x4e0 [ 17.824467] Call Trace: [ 17.824774] <TASK> [ 17.825040] regulator_bulk_free+0x82/0xe0 [ 17.825514] devres_release_group+0x319/0x3d0 [ 17.825882] i2c_device_probe+0x766/0x940 [ 17.829198] i2c_register_driver+0xb5/0x130
Signed-off-by: Zheyu Ma zheyuma97@gmail.com Link: https://lore.kernel.org/r/20220405121038.4094051-1-zheyuma97@gmail.com Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/codecs/wm8731.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c index 86b1f6eaa599..518167d90b10 100644 --- a/sound/soc/codecs/wm8731.c +++ b/sound/soc/codecs/wm8731.c @@ -602,7 +602,7 @@ static int wm8731_hw_init(struct device *dev, struct wm8731_priv *wm8731) ret = wm8731_reset(wm8731->regmap); if (ret < 0) { dev_err(dev, "Failed to issue reset: %d\n", ret); - goto err_regulator_enable; + goto err; }
/* Clear POWEROFF, keep everything else disabled */ @@ -619,10 +619,7 @@ static int wm8731_hw_init(struct device *dev, struct wm8731_priv *wm8731)
regcache_mark_dirty(wm8731->regmap);
-err_regulator_enable: - /* Regulators will be enabled by bias management */ - regulator_bulk_disable(ARRAY_SIZE(wm8731->supplies), wm8731->supplies); - +err: return ret; }
@@ -760,21 +757,27 @@ static int wm8731_i2c_probe(struct i2c_client *i2c, ret = PTR_ERR(wm8731->regmap); dev_err(&i2c->dev, "Failed to allocate register map: %d\n", ret); - return ret; + goto err_regulator_enable; }
ret = wm8731_hw_init(&i2c->dev, wm8731); if (ret != 0) - return ret; + goto err_regulator_enable;
ret = devm_snd_soc_register_component(&i2c->dev, &soc_component_dev_wm8731, &wm8731_dai, 1); if (ret != 0) { dev_err(&i2c->dev, "Failed to register CODEC: %d\n", ret); - return ret; + goto err_regulator_enable; }
return 0; + +err_regulator_enable: + /* Regulators will be enabled by bias management */ + regulator_bulk_disable(ARRAY_SIZE(wm8731->supplies), wm8731->supplies); + + return ret; }
static int wm8731_i2c_remove(struct i2c_client *client)
From: Mauro Carvalho Chehab mchehab@kernel.org
[ Upstream commit c7cb4717f641db68e8117635bfcf62a9c27dc8d3 ]
Based on experimental tests, Huawei Matebook D15 actually uses both gpio0 and gpio1: the first one controls the speaker, while the other one controls the headphone.
Also, the headset is mapped as MIC1, instead of MIC2.
So, add a quirk for it.
Signed-off-by: Mauro Carvalho Chehab mchehab@kernel.org Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/d678aef9fc9a07aced611aa7cb8c9b800c649e5a.164935726... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- sound/soc/intel/boards/sof_es8336.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index 28d7670b8f8f..b18951a8f309 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -252,6 +252,15 @@ static const struct dmi_system_id sof_es8336_quirk_table[] = { SOF_ES8336_TGL_GPIO_QUIRK | SOF_ES8336_ENABLE_DMIC) }, + { + .callback = sof_es8336_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"), + DMI_MATCH(DMI_BOARD_NAME, "BOHB-WAX9-PCB-B2"), + }, + .driver_data = (void *)(SOF_ES8336_HEADPHONE_GPIO | + SOC_ES8336_HEADSET_MIC1) + }, {} };
On Tue, Apr 26, 2022 at 03:01:29PM -0400, Sasha Levin wrote:
From: Mauro Carvalho Chehab mchehab@kernel.org
[ Upstream commit c7cb4717f641db68e8117635bfcf62a9c27dc8d3 ]
Based on experimental tests, Huawei Matebook D15 actually uses both gpio0 and gpio1: the first one controls the speaker, while the other one controls the headphone.
Are you sure this doesn't need the rest of the series it came along with?
On Wed, Apr 27, 2022 at 12:28:32PM +0100, Mark Brown wrote:
On Tue, Apr 26, 2022 at 03:01:29PM -0400, Sasha Levin wrote:
From: Mauro Carvalho Chehab mchehab@kernel.org
[ Upstream commit c7cb4717f641db68e8117635bfcf62a9c27dc8d3 ]
Based on experimental tests, Huawei Matebook D15 actually uses both gpio0 and gpio1: the first one controls the speaker, while the other one controls the headphone.
Are you sure this doesn't need the rest of the series it came along with?
I'm not :) Should we queue it too?
On 5/1/22 14:32, Sasha Levin wrote:
On Wed, Apr 27, 2022 at 12:28:32PM +0100, Mark Brown wrote:
On Tue, Apr 26, 2022 at 03:01:29PM -0400, Sasha Levin wrote:
From: Mauro Carvalho Chehab mchehab@kernel.org
[ Upstream commit c7cb4717f641db68e8117635bfcf62a9c27dc8d3 ]
Based on experimental tests, Huawei Matebook D15 actually uses both gpio0 and gpio1: the first one controls the speaker, while the other one controls the headphone.
Are you sure this doesn't need the rest of the series it came along with?
I'm not :) Should we queue it too?
If you add this platform to -stable, you'd need the entire series https://lore.kernel.org/alsa-devel/20220308192610.392950-1-pierre-louis.boss..., and the additions made by Mauro.
My take is that it's not really relevant for -stable, support for this hardware codec is still a works-in-progress and while we'd certainly want to have more distributions use the hardware support it's quite disruptive for -stable maintainers, with the risk of compilation problems and functional issues introduced. it'll make more sense for 5.18+.
From: Gongjun Song gongjun.song@intel.com
[ Upstream commit b07908ab26ceab51165c13714277c19252e62594 ]
Add RaptorLake-P PCI IDs
Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Gongjun Song gongjun.song@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20220421163546.319604-1-pierre-louis.bossart@linux... Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Sasha Levin sashal@kernel.org --- sound/hda/intel-dsp-config.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c index 70fd8b13938e..55deb7447183 100644 --- a/sound/hda/intel-dsp-config.c +++ b/sound/hda/intel-dsp-config.c @@ -410,6 +410,15 @@ static const struct config_entry config_table[] = { .flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC_OR_SOUNDWIRE, .device = 0x54c8, }, + /* RaptorLake-P */ + { + .flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC_OR_SOUNDWIRE, + .device = 0x51ca, + }, + { + .flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC_OR_SOUNDWIRE, + .device = 0x51cb, + }, #endif
};
Hi,
I don't think that this patch needs backporting.
It does not fix anything and could introduce some regression if b7898396f4bb is not also already backported. It could avoid some (unlikely?) allocation failure, but as this case is already handled ("if (!dpcm)"), it shouldn't be an issue if it happened.
Just for my understanding, why has it auto-selected for backport? I thought that a Fixes tag and/or a real reported issue was need for this to happen.
CJ
Le 26/04/2022 à 21:01, Sasha Levin a écrit :
From: Christophe JAILLET christophe.jaillet@wanadoo.fr
[ Upstream commit fb6d679fee95d272c0a94912c4e534146823ee89 ]
At the kzalloc() call in dpcm_be_connect(), there is no spin lock involved. It's merely protected by card->pcm_mutex, instead. The spinlock is applied at the later call with snd_soc_pcm_stream_lock_irq() only for the list manipulations. (See it's *_irq(), not *_irqsave(); that means the context being sleepable at that point.) So, we can use GFP_KERNEL safely there.
This patch revert commit d8a9c6e1f676 ("ASoC: soc-pcm: use GFP_ATOMIC for dpcm structure") which is no longer needed since commit b7898396f4bb ("ASoC: soc-pcm: Fix and cleanup DPCM locking").
Signed-off-by: Christophe JAILLET christophe.jaillet@wanadoo.fr Link: https://lore.kernel.org/r/e740f1930843060e025e3c0f17ec1393cfdafb26.164875796... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org
sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 9a954680d492..11c9853e9e80 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1214,7 +1214,7 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, be_substream->pcm->nonatomic = 1; }
- dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_ATOMIC);
- dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_KERNEL); if (!dpcm) return -ENOMEM;
participants (4)
-
Marion & Christophe JAILLET -
Mark Brown -
Pierre-Louis Bossart -
Sasha Levin