* [PATCH 1/3] ASoC: wcd9335: Fix a double irq free in the remove function
2021-08-16 5:25 [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and remove function Christophe JAILLET
@ 2021-08-16 5:25 ` Christophe JAILLET
2021-08-16 5:25 ` [PATCH 2/3] ASoC: wcd9335: Fix a memory leak in the error handling path of the probe function Christophe JAILLET
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-08-16 5:25 UTC (permalink / raw)
To: srinivas.kandagatla, bgoswami, lgirdwood, broonie, perex, tiwai, vkoul
Cc: alsa-devel, linux-kernel, kernel-janitors, Christophe JAILLET
There is no point in calling 'free_irq()' explicitly for
'WCD9335_IRQ_SLIMBUS' in the remove function.
The irqs are requested in 'wcd9335_setup_irqs()' using a resource managed
function (i.e. 'devm_request_threaded_irq()').
'wcd9335_setup_irqs()' requests all what is defined in the 'wcd9335_irqs'
structure.
This structure has only one entry for 'WCD9335_IRQ_SLIMBUS'.
So 'devm_request...irq()' + explicit 'free_irq()' would lead to a double
free.
Remove the unneeded 'free_irq()' from the remove function.
Fixes: 20aedafdf492 ("ASoC: wcd9335: add support to wcd9335 codec")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
sound/soc/codecs/wcd9335.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index 86c92e03ea5d..933f59e4e56f 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -4869,7 +4869,6 @@ static void wcd9335_codec_remove(struct snd_soc_component *comp)
struct wcd9335_codec *wcd = dev_get_drvdata(comp->dev);
wcd_clsh_ctrl_free(wcd->clsh_ctrl);
- free_irq(regmap_irq_get_virq(wcd->irq_data, WCD9335_IRQ_SLIMBUS), wcd);
}
static int wcd9335_codec_set_sysclk(struct snd_soc_component *comp,
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] ASoC: wcd9335: Fix a memory leak in the error handling path of the probe function
2021-08-16 5:25 [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and remove function Christophe JAILLET
2021-08-16 5:25 ` [PATCH 1/3] ASoC: wcd9335: Fix a double irq free in the " Christophe JAILLET
@ 2021-08-16 5:25 ` Christophe JAILLET
2021-08-16 5:25 ` [PATCH 3/3] ASoC: wcd9335: Disable irq on slave ports in the remove function Christophe JAILLET
2021-08-26 18:30 ` [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and " Mark Brown
3 siblings, 0 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-08-16 5:25 UTC (permalink / raw)
To: srinivas.kandagatla, bgoswami, lgirdwood, broonie, perex, tiwai, vkoul
Cc: alsa-devel, linux-kernel, kernel-janitors, Christophe JAILLET
If 'wcd9335_setup_irqs()' fails, me must release the memory allocated in
'wcd_clsh_ctrl_alloc()', as already done in the remove function.
Add an error handling path and the missing 'wcd_clsh_ctrl_free()' call.
Fixes: 20aedafdf492 ("ASoC: wcd9335: add support to wcd9335 codec")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
sound/soc/codecs/wcd9335.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index 933f59e4e56f..47fe68edea3a 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -4844,6 +4844,7 @@ static void wcd9335_codec_init(struct snd_soc_component *component)
static int wcd9335_codec_probe(struct snd_soc_component *component)
{
struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
+ int ret;
int i;
snd_soc_component_init_regmap(component, wcd->regmap);
@@ -4861,7 +4862,15 @@ static int wcd9335_codec_probe(struct snd_soc_component *component)
for (i = 0; i < NUM_CODEC_DAIS; i++)
INIT_LIST_HEAD(&wcd->dai[i].slim_ch_list);
- return wcd9335_setup_irqs(wcd);
+ ret = wcd9335_setup_irqs(wcd);
+ if (ret)
+ goto free_clsh_ctrl;
+
+ return 0;
+
+free_clsh_ctrl:
+ wcd_clsh_ctrl_free(wcd->clsh_ctrl);
+ return ret;
}
static void wcd9335_codec_remove(struct snd_soc_component *comp)
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] ASoC: wcd9335: Disable irq on slave ports in the remove function
2021-08-16 5:25 [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and remove function Christophe JAILLET
2021-08-16 5:25 ` [PATCH 1/3] ASoC: wcd9335: Fix a double irq free in the " Christophe JAILLET
2021-08-16 5:25 ` [PATCH 2/3] ASoC: wcd9335: Fix a memory leak in the error handling path of the probe function Christophe JAILLET
@ 2021-08-16 5:25 ` Christophe JAILLET
2021-08-26 18:30 ` [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and " Mark Brown
3 siblings, 0 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-08-16 5:25 UTC (permalink / raw)
To: srinivas.kandagatla, bgoswami, lgirdwood, broonie, perex, tiwai, vkoul
Cc: alsa-devel, linux-kernel, kernel-janitors, Christophe JAILLET
The probe calls 'wcd9335_setup_irqs()' to enable interrupts on all slave
ports.
This must be undone in the remove function.
Add a 'wcd9335_teardown_irqs()' function that undoes 'wcd9335_setup_irqs()'
function, and call it from the remove function.
Fixes: 20aedafdf492 ("ASoC: wcd9335: add support to wcd9335 codec")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
As said in the cover letter, this patch is COMPLETELY speculative. It is
untested.
Review with care, because I don't know if it make sense!
---
sound/soc/codecs/wcd9335.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index 47fe68edea3a..d885ced34f60 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -4076,6 +4076,16 @@ static int wcd9335_setup_irqs(struct wcd9335_codec *wcd)
return ret;
}
+static void wcd9335_teardown_irqs(struct wcd9335_codec *wcd)
+{
+ int i;
+
+ /* disable interrupts on all slave ports */
+ for (i = 0; i < WCD9335_SLIM_NUM_PORT_REG; i++)
+ regmap_write(wcd->if_regmap, WCD9335_SLIM_PGD_PORT_INT_EN0 + i,
+ 0x00);
+}
+
static void wcd9335_cdc_sido_ccl_enable(struct wcd9335_codec *wcd,
bool ccl_flag)
{
@@ -4878,6 +4888,7 @@ static void wcd9335_codec_remove(struct snd_soc_component *comp)
struct wcd9335_codec *wcd = dev_get_drvdata(comp->dev);
wcd_clsh_ctrl_free(wcd->clsh_ctrl);
+ wcd9335_teardown_irqs(wcd);
}
static int wcd9335_codec_set_sysclk(struct snd_soc_component *comp,
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and remove function
2021-08-16 5:25 [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and remove function Christophe JAILLET
` (2 preceding siblings ...)
2021-08-16 5:25 ` [PATCH 3/3] ASoC: wcd9335: Disable irq on slave ports in the remove function Christophe JAILLET
@ 2021-08-26 18:30 ` Mark Brown
3 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2021-08-26 18:30 UTC (permalink / raw)
To: bgoswami, perex, Christophe JAILLET, srinivas.kandagatla, tiwai,
vkoul, lgirdwood
Cc: Mark, Brown, broonie, alsa-devel, kernel-janitors, linux-kernel
From: Mark Brown,,, <broonie@kernel.org>
On Mon, 16 Aug 2021 07:25:01 +0200, Christophe JAILLET wrote:
> The first 2 patches are sraightforward and look logical to me.
>
> However, the 3rd one in purely speculative. It is based on the fact that a
> comment states that we enable some irqs on some slave ports. That said, it writes
> 0xFF in some registers.
>
> So, I guess that we should disable these irqs when the driver is removed. That
> said, writing 0x00 at the same place looks logical to me.
>
> [...]
Applied, thanks!
[1/3] ASoC: wcd9335: Fix a double irq free in the remove function
commit: 7a6a723e98aa45f393e6add18f7309dfffa1b0e2
[2/3] ASoC: wcd9335: Fix a memory leak in the error handling path of the probe function
commit: fc6fc81caa63900cef9ebb8b2e365c3ed5a9effb
[3/3] ASoC: wcd9335: Disable irq on slave ports in the remove function
commit: d3efd26af2e044ff2b48d38bb871630282d77e60
Best regards,
--
Mark Brown,,, <broonie@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread