kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ASoC: wcd9335: Firx some resources leak in the probe and remove function
@ 2021-08-16  5:25 Christophe JAILLET
  2021-08-16  5:25 ` [PATCH 1/3] ASoC: wcd9335: Fix a double irq free in the " Christophe JAILLET
                   ` (3 more replies)
  0 siblings, 4 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 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.

This cis untested and NOT based on any documentation. Just a blind fix.
Review with care.
You'll be warned :)


Christophe JAILLET (3):
  ASoC: wcd9335: Fix a double irq free in the remove function
  ASoC: wcd9335: Fix a memory leak in the error handling path of the
    probe function
  ASoC: wcd9335: Disable irq on slave ports in the remove function

 sound/soc/codecs/wcd9335.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [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	[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	[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	[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

end of thread, other threads:[~2021-08-26 19:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).