All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	Richard Fitzgerald <rf@opensource.cirrus.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] ASoC: cs42l42: free_irq() before powering-down on probe() fail
Date: Tue, 26 Oct 2021 13:57:22 +0100	[thread overview]
Message-ID: <20211026125722.10220-3-rf@opensource.cirrus.com> (raw)
In-Reply-To: <20211026125722.10220-1-rf@opensource.cirrus.com>

Relying on devm to free the irq handler on probe failure leaves a
small window of opportunity for an interrupt to become pending and
then the handler to run after the chip has been reset and powered
off.

For safety cs42l42_probe() should free the irq in the error path.
As the irq is now disabled by the driver in probe() and remove()
there is no point allocating it as a devres-managed item, so
convert to plain non-devres.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l42.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c
index dc12842ee6e1..1029f6b3eb48 100644
--- a/sound/soc/codecs/cs42l42.c
+++ b/sound/soc/codecs/cs42l42.c
@@ -2078,17 +2078,16 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client,
 
 	/* Request IRQ if one was specified */
 	if (i2c_client->irq) {
-		ret = devm_request_threaded_irq(&i2c_client->dev,
-						i2c_client->irq,
-						NULL, cs42l42_irq_thread,
-						IRQF_ONESHOT | IRQF_TRIGGER_LOW,
-						"cs42l42", cs42l42);
+		ret = request_threaded_irq(i2c_client->irq,
+					   NULL, cs42l42_irq_thread,
+					   IRQF_ONESHOT | IRQF_TRIGGER_LOW,
+					   "cs42l42", cs42l42);
 		if (ret == -EPROBE_DEFER) {
-			goto err_disable;
+			goto err_disable_noirq;
 		} else if (ret != 0) {
 			dev_err(&i2c_client->dev,
 				"Failed to request IRQ: %d\n", ret);
-			goto err_disable;
+			goto err_disable_noirq;
 		}
 	}
 
@@ -2158,6 +2157,10 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client,
 	regmap_write(cs42l42->regmap, CS42L42_PWR_CTL1, 0xff);
 
 err_disable:
+	if (i2c_client->irq)
+		free_irq(i2c_client->irq, cs42l42);
+
+err_disable_noirq:
 	gpiod_set_value_cansleep(cs42l42->reset_gpio, 0);
 err_disable_noreset:
 	regulator_bulk_disable(ARRAY_SIZE(cs42l42->supplies),
@@ -2170,7 +2173,7 @@ static int cs42l42_i2c_remove(struct i2c_client *i2c_client)
 	struct cs42l42_private *cs42l42 = i2c_get_clientdata(i2c_client);
 
 	if (i2c_client->irq)
-		devm_free_irq(&i2c_client->dev, i2c_client->irq, cs42l42);
+		free_irq(i2c_client->irq, cs42l42);
 
 	/*
 	 * The driver might not have control of reset and power supplies,
-- 
2.11.0


WARNING: multiple messages have this Message-ID (diff)
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
	<patches@opensource.cirrus.com>,
	Richard Fitzgerald <rf@opensource.cirrus.com>
Subject: [PATCH 2/2] ASoC: cs42l42: free_irq() before powering-down on probe() fail
Date: Tue, 26 Oct 2021 13:57:22 +0100	[thread overview]
Message-ID: <20211026125722.10220-3-rf@opensource.cirrus.com> (raw)
In-Reply-To: <20211026125722.10220-1-rf@opensource.cirrus.com>

Relying on devm to free the irq handler on probe failure leaves a
small window of opportunity for an interrupt to become pending and
then the handler to run after the chip has been reset and powered
off.

For safety cs42l42_probe() should free the irq in the error path.
As the irq is now disabled by the driver in probe() and remove()
there is no point allocating it as a devres-managed item, so
convert to plain non-devres.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l42.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c
index dc12842ee6e1..1029f6b3eb48 100644
--- a/sound/soc/codecs/cs42l42.c
+++ b/sound/soc/codecs/cs42l42.c
@@ -2078,17 +2078,16 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client,
 
 	/* Request IRQ if one was specified */
 	if (i2c_client->irq) {
-		ret = devm_request_threaded_irq(&i2c_client->dev,
-						i2c_client->irq,
-						NULL, cs42l42_irq_thread,
-						IRQF_ONESHOT | IRQF_TRIGGER_LOW,
-						"cs42l42", cs42l42);
+		ret = request_threaded_irq(i2c_client->irq,
+					   NULL, cs42l42_irq_thread,
+					   IRQF_ONESHOT | IRQF_TRIGGER_LOW,
+					   "cs42l42", cs42l42);
 		if (ret == -EPROBE_DEFER) {
-			goto err_disable;
+			goto err_disable_noirq;
 		} else if (ret != 0) {
 			dev_err(&i2c_client->dev,
 				"Failed to request IRQ: %d\n", ret);
-			goto err_disable;
+			goto err_disable_noirq;
 		}
 	}
 
@@ -2158,6 +2157,10 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client,
 	regmap_write(cs42l42->regmap, CS42L42_PWR_CTL1, 0xff);
 
 err_disable:
+	if (i2c_client->irq)
+		free_irq(i2c_client->irq, cs42l42);
+
+err_disable_noirq:
 	gpiod_set_value_cansleep(cs42l42->reset_gpio, 0);
 err_disable_noreset:
 	regulator_bulk_disable(ARRAY_SIZE(cs42l42->supplies),
@@ -2170,7 +2173,7 @@ static int cs42l42_i2c_remove(struct i2c_client *i2c_client)
 	struct cs42l42_private *cs42l42 = i2c_get_clientdata(i2c_client);
 
 	if (i2c_client->irq)
-		devm_free_irq(&i2c_client->dev, i2c_client->irq, cs42l42);
+		free_irq(i2c_client->irq, cs42l42);
 
 	/*
 	 * The driver might not have control of reset and power supplies,
-- 
2.11.0


  parent reply	other threads:[~2021-10-26 12:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-26 12:57 [PATCH 0/2] ASoC: cs42l42: Fixes to power-down Richard Fitzgerald
2021-10-26 12:57 ` Richard Fitzgerald
2021-10-26 12:57 ` [PATCH 1/2] ASoC: cs42l42: Reset and power-down on remove() and failed probe() Richard Fitzgerald
2021-10-26 12:57   ` Richard Fitzgerald
2021-10-26 12:57 ` Richard Fitzgerald [this message]
2021-10-26 12:57   ` [PATCH 2/2] ASoC: cs42l42: free_irq() before powering-down on probe() fail Richard Fitzgerald
2021-10-26 19:06 ` [PATCH 0/2] ASoC: cs42l42: Fixes to power-down Mark Brown
2021-10-26 19:06   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211026125722.10220-3-rf@opensource.cirrus.com \
    --to=rf@opensource.cirrus.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.