April 2022 - Alsa-devel - mailman.alsa-project.org

Fix typo in error in aplay.c
by GitHub pull_request - opened 27 Apr '22

27 Apr '22

alsa-project/alsa-utils pull request #143 was opened from mb720: Hi and thanks a lot for ALSA! I'd like to provide a fix for a typo I just came across while using arecord. Request URL : https://github.com/alsa-project/alsa-utils/pull/143 Patch URL : https://github.com/alsa-project/alsa-utils/pull/143.patch Repository URL: https://github.com/alsa-project/alsa-utils

1 0

[PATCH v2] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload
by Peter Ujfalusi 27 Apr '22

27 Apr '22

It is possible to craft a topology where sof_get_control_data() would do out of bounds access because it expects that it is only called when the payload is bytes type. Confusingly it also handles other types of controls, but the payload parsing implementation is only valid for bytes. Fix the code to count the non bytes controls and instead of storing a pointer to sof_abi_hdr in sof_widget_data (which is only valid for bytes), store the pointer to the data itself and add a new member to save the size of the data. In case of non bytes controls we store the pointer to the chanv itself, which is just an array of values at the end. Reported-by: Sergey Senozhatsky <senozhatsky(a)chromium.org> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> --- Hi, changes since v1: - adjust the payload size for non bytes controls by subtracting the size of the sof_ipc_ctrl_data struct, plus add comment to note this Regards, Peter sound/soc/sof/ipc3-topology.c | 40 ++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index 572bcbfdb356..f9f37e01edfa 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -20,7 +20,8 @@ struct sof_widget_data { int ctrl_type; int ipc_cmd; - struct sof_abi_hdr *pdata; + void *pdata; + size_t pdata_size; struct snd_sof_control *control; }; @@ -784,16 +785,29 @@ static int sof_get_control_data(struct snd_soc_component *scomp, } cdata = wdata[i].control->ipc_control_data; - wdata[i].pdata = cdata->data; - if (!wdata[i].pdata) - return -EINVAL; /* make sure data is valid - data can be updated at runtime */ - if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES && - wdata[i].pdata->magic != SOF_ABI_MAGIC) - return -EINVAL; + if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES) { + if (!cdata->data) + return -EINVAL; + + if (cdata->data->magic != SOF_ABI_MAGIC) + return -EINVAL; + + wdata[i].pdata = cdata->data->data; + wdata[i].pdata_size = cdata->data->size; + } else { + /* points to the control data union */ + wdata[i].pdata = cdata->chanv; + /* + * wdata[i].control->size is calculated with struct_size + * and includes the size of struct sof_ipc_ctrl_data + */ + wdata[i].pdata_size = wdata[i].control->size - + sizeof(struct sof_ipc_ctrl_data); + } - *size += wdata[i].pdata->size; + *size += wdata[i].pdata_size; /* get data type */ switch (cdata->cmd) { @@ -876,10 +890,12 @@ static int sof_process_load(struct snd_soc_component *scomp, */ if (ipc_data_size) { for (i = 0; i < widget->num_kcontrols; i++) { - memcpy(&process->data[offset], - wdata[i].pdata->data, - wdata[i].pdata->size); - offset += wdata[i].pdata->size; + if (!wdata[i].pdata_size) + continue; + + memcpy(&process->data[offset], wdata[i].pdata, + wdata[i].pdata_size); + offset += wdata[i].pdata_size; } } -- 2.36.0

2 7

[PATCH] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload
by Peter Ujfalusi 27 Apr '22

27 Apr '22

It is possible to craft a topology where sof_get_control_data() would do out of bounds access because it expects that it is only called when the payload is bytes type. Confusingly it also handles other types of controls, but the payload parsing implementation is only valid for bytes. Fix the code to count the non bytes controls and instead of storing a pointer to sof_abi_hdr in sof_widget_data (which is only valid for bytes), store the pointer to the data itself and add a new member to save the size of the data. In case of non bytes controls we store the pointer to the chanv itself, which is just an array of values at the end. Reported-by: Sergey Senozhatsky <senozhatsky(a)chromium.org> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> --- Hi, This should fix the out of bounds access, but I'm not sure what would be expected in case of non bytes type of controls, so this patch will consider their size(s) into the counting and copying. Sergey, can you check if it is easy to backport for 5.10.111? There were changes around this (code move to ipc3-topology.c and renames) but the logic remained the same. Regards, Peter sound/soc/sof/ipc3-topology.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index 572bcbfdb356..8a083fd547ba 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -20,7 +20,8 @@ struct sof_widget_data { int ctrl_type; int ipc_cmd; - struct sof_abi_hdr *pdata; + void *pdata; + size_t pdata_size; struct snd_sof_control *control; }; @@ -784,16 +785,23 @@ static int sof_get_control_data(struct snd_soc_component *scomp, } cdata = wdata[i].control->ipc_control_data; - wdata[i].pdata = cdata->data; - if (!wdata[i].pdata) - return -EINVAL; /* make sure data is valid - data can be updated at runtime */ - if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES && - wdata[i].pdata->magic != SOF_ABI_MAGIC) - return -EINVAL; + if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES) { + if (!cdata->data) + return -EINVAL; + + if (cdata->data->magic != SOF_ABI_MAGIC) + return -EINVAL; + + wdata[i].pdata = cdata->data->data; + wdata[i].pdata_size = cdata->data->size; + } else { + wdata[i].pdata = cdata->chanv; /* points to the control data union */ + wdata[i].pdata_size = wdata[i].control->size; + } - *size += wdata[i].pdata->size; + *size += wdata[i].pdata_size; /* get data type */ switch (cdata->cmd) { @@ -876,10 +884,12 @@ static int sof_process_load(struct snd_soc_component *scomp, */ if (ipc_data_size) { for (i = 0; i < widget->num_kcontrols; i++) { - memcpy(&process->data[offset], - wdata[i].pdata->data, - wdata[i].pdata->size); - offset += wdata[i].pdata->size; + if (!wdata[i].pdata_size) + continue; + + memcpy(&process->data[offset], wdata[i].pdata, + wdata[i].pdata_size); + offset += wdata[i].pdata_size; } } -- 2.36.0

2 2

[bug report] ASoC: SOF: Intel: hda: Revisit IMR boot sequence
by Dan Carpenter 27 Apr '22

27 Apr '22

Hello Peter Ujfalusi, The patch 2a68ff846164: "ASoC: SOF: Intel: hda: Revisit IMR boot sequence" from Apr 21, 2022, leads to the following Smatch static checker warning: sound/soc/sof/intel/hda-loader.c:397 hda_dsp_cl_boot_firmware() info: return a literal instead of 'ret' sound/soc/sof/intel/hda-loader.c 381 int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) 382 { 383 struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; 384 struct snd_sof_pdata *plat_data = sdev->pdata; 385 const struct sof_dev_desc *desc = plat_data->desc; 386 const struct sof_intel_dsp_desc *chip_info; 387 struct hdac_ext_stream *hext_stream; 388 struct firmware stripped_firmware; 389 struct snd_dma_buffer dmab; 390 int ret, ret1, i; 391 392 if (hda->imrboot_supported && !sdev->first_boot) { 393 dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n"); 394 hda->boot_iteration = 0; 395 ret = hda_dsp_boot_imr(sdev); 396 if (ret >= 0) --> 397 return ret; The hda_dsp_boot_imr() has some similar stuff where it checks for positive returns. As far as I can see, this code never returns positive values. Normally kernel code returns zero on success and negative error codes on failure. When code returns non-standard things then it really should be documented what the positive returns mean. Nothing complicated, just add a comment at the start of the function. The TLDR back story of this Smatch check is that it's not published but it regularly finds bugs. The issue is that it's more readable, plus it looks more deliberate and intentional to write: if (!ret) return 0; instead of: if (!ret) return ret; With the latter format, the bug is that people intended to write: if (ret) return ret; Obviously this kind of bug would get caught in testing, but testing is often impossible in the kernel because it depends on hardware availability. 398 399 dev_warn(sdev->dev, "IMR restore failed, trying to cold boot\n"); 400 } 401 402 chip_info = desc->chip_info; 403 regards, dan carpenter

2 1

out-of-bounds access in sound/soc/sof/topology.c
by Sergey Senozhatsky 27 Apr '22

27 Apr '22

Hi, I'm running 5.10.111 LTS, so if this has been fixed already then we definitely want to cherry pick the fix for -stable. Anonymous union in this struct is of zero size /* generic control data */ struct sof_ipc_ctrl_data { struct sof_ipc_reply rhdr; uint32_t comp_id; /* control access and data type */ uint32_t type; /**< enum sof_ipc_ctrl_type */ uint32_t cmd; /**< enum sof_ipc_ctrl_cmd */ uint32_t index; /**< control index for comps > 1 control */ /* control data - can either be appended or DMAed from host */ struct sof_ipc_host_buffer buffer; uint32_t num_elems; /**< in array elems or bytes for data type */ uint32_t elems_remaining; /**< elems remaining if sent in parts */ uint32_t msg_index; /**< for large messages sent in parts */ /* reserved for future use */ uint32_t reserved[6]; /* control data - add new types if needed */ union { /* channel values can be used by volume type controls */ struct sof_ipc_ctrl_value_chan chanv[0]; /* component values used by routing controls like mux, mixer */ struct sof_ipc_ctrl_value_comp compv[0]; /* data can be used by binary controls */ struct sof_abi_hdr data[0]; }; } __packed; sof_ipc_ctrl_value_chan and sof_ipc_ctrl_value_comp are of the same size - 8 bytes, while sof_abi_hdr is much larger - _at least_ 32 bytes (`__u32 data[0]` in sof_abi_hdr suggest that there should be more payload after header). But they all contribute 0 to sizeof(sof_ipc_ctrl_data). Now control data allocations looks as follows scontrol->size = struct_size(scontrol->control_data, chanv, le32_to_cpu(mc->num_channels)); scontrol->control_data = kzalloc(scontrol->size, GFP_KERNEL); Which is sizeof(sof_ipc_ctrl_data) + mc->num_channels * sizeof(sof_ipc_ctrl_value_chan) For some reason it uses sizeof(sof_ipc_ctrl_value_chan), which is not the largest member of the union. And this is where the problem is: in order to make control->data.FOO loads and stores legal we need mc->num_channels to be of at least 4. So that sizeof(sof_ipc_ctrl_data) + mc->num_channels * sizeof(sof_ipc_ctrl_value_chan) 92 + 4 * 8 will be the same as sizeof(sof_ipc_ctrl_data) + sizeof(sof_abi_hdr). 92 + 32 Otherwise scontrol->control_data->data.FOO will access nearby/foreign slab object. And there is at least one such memory access. In sof_get_control_data(). wdata[i].pdata = wdata[i].control->control_data->data; *size += wdata[i].pdata->size; pdata->size is at offset 8, but if, say, mc->num_channels == 1 then we allocate only 8 bytes for pdata, so pdata->size is 4 bytes outside of allocated slab object. Thoughts?

4 8

[PATCH] ASoC: SOF: mediatek: Fix allyesconfig build error
by Tinghan Shen 27 Apr '22

27 Apr '22

ld: sound/soc/sof/mediatek/mt8186/mt8186-clk.o:(.opd+0x18): multiple definition of `adsp_clock_on'; sound/soc/sof/mediatek/mt8195/mt8195-clk.o:(.opd+0x60): first defined here ld: sound/soc/sof/mediatek/mt8186/mt8186-clk.o: in function `.adsp_clock_on': ld: sound/soc/sof/mediatek/mt8186/mt8186-clk.o:(.opd+0x30): multiple definition of `adsp_clock_off'; sound/soc/sof/mediatek/mt8195/mt8195-clk.o:(.opd+0x78): first defined here ld: sound/soc/sof/mediatek/mt8186/mt8186-clk.o: in function `.adsp_clock_off': ld: sound/soc/sof/mediatek/mt8186/mt8186-loader.o:(.opd+0x0): multiple definition of `sof_hifixdsp_boot_sequence'; sound/soc/sof/mediatek/mt8195/mt8195-loader.o:(.opd+0x0): first defined here ld: sound/soc/sof/mediatek/mt8186/mt8186-loader.o: in function `.sof_hifixdsp_boot_sequence': ld: sound/soc/sof/mediatek/mt8186/mt8186-loader.o:(.opd+0x18): multiple definition of `sof_hifixdsp_shutdown'; sound/soc/sof/mediatek/mt8195/mt8195-loader.o:(.opd+0x18): first defined here ld: sound/soc/sof/mediatek/mt8186/mt8186-loader.o: in function `.sof_hifixdsp_shutdown': Fixes: 91316c3dbe48 ("ASoC: SOF: mediatek: Add mt8186 sof fw loader and dsp ops") Fixes: 83e1b65ad2ac ("ASoC: SOF: mediatek: Add mt8186 dsp clock support") Signed-off-by: Tinghan Shen <tinghan.shen(a)mediatek.com> --- sound/soc/sof/mediatek/mt8186/mt8186-clk.c | 4 ++-- sound/soc/sof/mediatek/mt8186/mt8186-clk.h | 4 ++-- sound/soc/sof/mediatek/mt8186/mt8186-loader.c | 4 ++-- sound/soc/sof/mediatek/mt8186/mt8186.c | 18 +++++++++--------- sound/soc/sof/mediatek/mt8186/mt8186.h | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/sound/soc/sof/mediatek/mt8186/mt8186-clk.c b/sound/soc/sof/mediatek/mt8186/mt8186-clk.c index 5f805981b8e6..22220fd50b62 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186-clk.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186-clk.c @@ -74,7 +74,7 @@ static void adsp_disable_all_clock(struct snd_sof_dev *sdev) clk_disable_unprepare(priv->clk[CLK_TOP_AUDIODSP]); } -int adsp_clock_on(struct snd_sof_dev *sdev) +int mt8186_adsp_clock_on(struct snd_sof_dev *sdev) { struct device *dev = sdev->dev; int ret; @@ -92,7 +92,7 @@ int adsp_clock_on(struct snd_sof_dev *sdev) return 0; } -void adsp_clock_off(struct snd_sof_dev *sdev) +void mt8186_adsp_clock_off(struct snd_sof_dev *sdev) { snd_sof_dsp_write(sdev, DSP_REG_BAR, ADSP_CK_EN, 0); snd_sof_dsp_write(sdev, DSP_REG_BAR, ADSP_UART_CTRL, 0); diff --git a/sound/soc/sof/mediatek/mt8186/mt8186-clk.h b/sound/soc/sof/mediatek/mt8186/mt8186-clk.h index fa174dfceff0..89c23caf0fee 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186-clk.h +++ b/sound/soc/sof/mediatek/mt8186/mt8186-clk.h @@ -19,6 +19,6 @@ enum adsp_clk_id { }; int mt8186_adsp_init_clock(struct snd_sof_dev *sdev); -int adsp_clock_on(struct snd_sof_dev *sdev); -void adsp_clock_off(struct snd_sof_dev *sdev); +int mt8186_adsp_clock_on(struct snd_sof_dev *sdev); +void mt8186_adsp_clock_off(struct snd_sof_dev *sdev); #endif diff --git a/sound/soc/sof/mediatek/mt8186/mt8186-loader.c b/sound/soc/sof/mediatek/mt8186/mt8186-loader.c index 6ab4921b1010..548b12c33d8a 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186-loader.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186-loader.c @@ -11,7 +11,7 @@ #include "mt8186.h" #include "../../ops.h" -void sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr) +void mt8186_sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr) { /* set RUNSTALL to stop core */ snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_HIFI_IO_CONFIG, @@ -39,7 +39,7 @@ void sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr) RUNSTALL, 0); } -void sof_hifixdsp_shutdown(struct snd_sof_dev *sdev) +void mt8186_sof_hifixdsp_shutdown(struct snd_sof_dev *sdev) { /* set RUNSTALL to stop core */ snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_HIFI_IO_CONFIG, diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c index c8faa63497c6..6d574fd4492e 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c @@ -211,7 +211,7 @@ static int mt8186_run(struct snd_sof_dev *sdev) adsp_bootup_addr = SRAM_PHYS_BASE_FROM_DSP_VIEW; dev_dbg(sdev->dev, "HIFIxDSP boot from base : 0x%08X\n", adsp_bootup_addr); - sof_hifixdsp_boot_sequence(sdev, adsp_bootup_addr); + mt8186_sof_hifixdsp_boot_sequence(sdev, adsp_bootup_addr); return 0; } @@ -284,9 +284,9 @@ static int mt8186_dsp_probe(struct snd_sof_dev *sdev) return ret; } - ret = adsp_clock_on(sdev); + ret = mt8186_adsp_clock_on(sdev); if (ret) { - dev_err(sdev->dev, "adsp_clock_on fail!\n"); + dev_err(sdev->dev, "mt8186_adsp_clock_on fail!\n"); return ret; } @@ -297,18 +297,18 @@ static int mt8186_dsp_probe(struct snd_sof_dev *sdev) static int mt8186_dsp_remove(struct snd_sof_dev *sdev) { - sof_hifixdsp_shutdown(sdev); + mt8186_sof_hifixdsp_shutdown(sdev); adsp_sram_power_off(sdev); - adsp_clock_off(sdev); + mt8186_adsp_clock_off(sdev); return 0; } static int mt8186_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state) { - sof_hifixdsp_shutdown(sdev); + mt8186_sof_hifixdsp_shutdown(sdev); adsp_sram_power_off(sdev); - adsp_clock_off(sdev); + mt8186_adsp_clock_off(sdev); return 0; } @@ -317,9 +317,9 @@ static int mt8186_dsp_resume(struct snd_sof_dev *sdev) { int ret; - ret = adsp_clock_on(sdev); + ret = mt8186_adsp_clock_on(sdev); if (ret) { - dev_err(sdev->dev, "adsp_clock_on fail!\n"); + dev_err(sdev->dev, "mt8186_adsp_clock_on fail!\n"); return ret; } diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.h b/sound/soc/sof/mediatek/mt8186/mt8186.h index df52ae9659e4..98b2965e5ba6 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.h +++ b/sound/soc/sof/mediatek/mt8186/mt8186.h @@ -75,6 +75,6 @@ struct snd_sof_dev; #define SIZE_SHARED_DRAM_UL 0x40000 /*Shared buffer for Uplink*/ #define TOTAL_SIZE_SHARED_DRAM_FROM_TAIL (SIZE_SHARED_DRAM_DL + SIZE_SHARED_DRAM_UL) -void sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr); -void sof_hifixdsp_shutdown(struct snd_sof_dev *sdev); +void mt8186_sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr); +void mt8186_sof_hifixdsp_shutdown(struct snd_sof_dev *sdev); #endif -- 2.18.0

1 0

[PATCH] ASoC: Intel: avs: Depend on CONFIG_ACPI
by Amadeusz Sławiński 26 Apr '22

26 Apr '22

Apparently the assumption that driver can build without ACPI was too optimistic as avs selects SND_SOC_ACPI which requires working ACPI implementation. So depend on ACPI without COMPILE_TEST alternative. Fixes: 47a1886a610a ("ASoC: Intel: avs: Enable AVS driver only on x86 platforms") Reported-by: kernel test robot <lkp(a)intel.com> Reviewed-by: Cezary Rojewski <cezary.rojewski(a)intel.com> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski(a)linux.intel.com> --- sound/soc/intel/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index be42c4eff165..9fdd6e32951f 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -211,8 +211,8 @@ config SND_SOC_INTEL_KEEMBAY config SND_SOC_INTEL_AVS tristate "Intel AVS driver" - depends on (X86 && ACPI) || COMPILE_TEST - depends on PCI + depends on X86 || COMPILE_TEST + depends on PCI && ACPI depends on COMMON_CLK select SND_SOC_ACPI select SND_SOC_TOPOLOGY -- 2.25.1

4 9

[PATCH AUTOSEL 4.9 1/3] ASoC: wm8731: Disable the regulator when probing fails
by Sasha Levin 26 Apr '22

26 Apr '22

From: Zheyu Ma <zheyuma97(a)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(a)gmail.com> Link: https://lore.kernel.org/r/20220405121038.4094051-1-zheyuma97@gmail.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)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 4f9a1eb28120..abe5e77ba171 100644 --- a/sound/soc/codecs/wm8731.c +++ b/sound/soc/codecs/wm8731.c @@ -604,7 +604,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 */ @@ -621,10 +621,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; } @@ -768,21 +765,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 = snd_soc_register_codec(&i2c->dev, &soc_codec_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) -- 2.35.1

1 0

[PATCH AUTOSEL 4.14 1/5] ASoC: wm8731: Disable the regulator when probing fails
by Sasha Levin 26 Apr '22

26 Apr '22

From: Zheyu Ma <zheyuma97(a)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(a)gmail.com> Link: https://lore.kernel.org/r/20220405121038.4094051-1-zheyuma97@gmail.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)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 4f9a1eb28120..abe5e77ba171 100644 --- a/sound/soc/codecs/wm8731.c +++ b/sound/soc/codecs/wm8731.c @@ -604,7 +604,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 */ @@ -621,10 +621,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; } @@ -768,21 +765,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 = snd_soc_register_codec(&i2c->dev, &soc_codec_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) -- 2.35.1

1 0

[PATCH AUTOSEL 4.19 1/6] ASoC: wm8731: Disable the regulator when probing fails
by Sasha Levin 26 Apr '22

26 Apr '22

From: Zheyu Ma <zheyuma97(a)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(a)gmail.com> Link: https://lore.kernel.org/r/20220405121038.4094051-1-zheyuma97@gmail.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)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 7c8fad865d6b..3c5c02b034a9 100644 --- a/sound/soc/codecs/wm8731.c +++ b/sound/soc/codecs/wm8731.c @@ -604,7 +604,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 */ @@ -621,10 +621,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; } @@ -768,21 +765,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) -- 2.35.1

1 0