ASoC: max98373: Fix calls to free_gpio()
diff mbox series

Message ID 1572628130-16955-1-git-send-email-yong.zhi@intel.com
State New
Headers show
Series
  • ASoC: max98373: Fix calls to free_gpio()
Related show

Commit Message

Yong Zhi Nov. 1, 2019, 5:08 p.m. UTC
Don't Call free_gpio() when request_gpio() fails, call it
at error paths to avoid resource leak.

Signed-off-by: Yong Zhi <yong.zhi@intel.com>
---
 sound/soc/codecs/max98373.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Kai Vehmanen Nov. 4, 2019, 10:27 a.m. UTC | #1
Hi,

On Fri, 1 Nov 2019, Yong Zhi wrote:

> Don't Call free_gpio() when request_gpio() fails, call it
> at error paths to avoid resource leak.

patch looks good to me:

Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

PS This is good as a clear bugfix. For future, use of
   devm_gpiod_get_*() and friends is recommended.
Daniel Baluta Nov. 4, 2019, 1:03 p.m. UTC | #2
Hi Yong,

On Fri, 2019-11-01 at 12:08 -0500, Yong Zhi wrote:
> Don't Call free_gpio() when request_gpio() fails, call it
> at error paths to avoid resource leak.
> 

Uosing devm_gpio_request will make the change cleaner.
What do you think?

thanks,
Daniel.
Mark Brown Nov. 4, 2019, 1:17 p.m. UTC | #3
On Mon, Nov 04, 2019 at 01:03:38PM +0000, Daniel Baluta wrote:
> On Fri, 2019-11-01 at 12:08 -0500, Yong Zhi wrote:
> > Don't Call free_gpio() when request_gpio() fails, call it
> > at error paths to avoid resource leak.

> Uosing devm_gpio_request will make the change cleaner.
> What do you think?

Yes, that's even better.
Yong Zhi Nov. 4, 2019, 2:46 p.m. UTC | #4
Hi, Kai, Daniel and Mark, 

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Monday, November 4, 2019 7:18 AM
> To: Daniel Baluta <daniel.baluta@nxp.com>
> Cc: pierre-louis.bossart@linux.intel.com; Zhi, Yong <yong.zhi@intel.com>;
> Nujella, Sathyanarayana <sathyanarayana.nujella@intel.com>;
> ryans.lee@maximintegrated.com; alsa-devel@alsa-project.org
> Subject: Re: [alsa-devel] [PATCH] ASoC: max98373: Fix calls to free_gpio()
> 
> On Mon, Nov 04, 2019 at 01:03:38PM +0000, Daniel Baluta wrote:
> > On Fri, 2019-11-01 at 12:08 -0500, Yong Zhi wrote:
> > > Don't Call free_gpio() when request_gpio() fails, call it at error
> > > paths to avoid resource leak.
> 
> > Uosing devm_gpio_request will make the change cleaner.
> > What do you think?
> 
> Yes, that's even better.

Acked, will use devm version which is more elegant, thanks for the code review. -Yong

Patch
diff mbox series

diff --git a/sound/soc/codecs/max98373.c b/sound/soc/codecs/max98373.c
index eb709d528259..83984b5244c5 100644
--- a/sound/soc/codecs/max98373.c
+++ b/sound/soc/codecs/max98373.c
@@ -964,7 +964,6 @@  static int max98373_i2c_probe(struct i2c_client *i2c,
 		if (ret) {
 			dev_err(&i2c->dev, "%s: Failed to request gpio %d\n",
 				__func__, max98373->reset_gpio);
-			gpio_free(max98373->reset_gpio);
 			return -EINVAL;
 		}
 		gpio_direction_output(max98373->reset_gpio, 0);
@@ -979,16 +978,23 @@  static int max98373_i2c_probe(struct i2c_client *i2c,
 	if (ret < 0) {
 		dev_err(&i2c->dev,
 			"Failed to read: 0x%02X\n", MAX98373_R21FF_REV_ID);
-		return ret;
+		goto err_gpio_free;
 	}
 	dev_info(&i2c->dev, "MAX98373 revisionID: 0x%02X\n", reg);
 
 	/* codec registeration */
 	ret = devm_snd_soc_register_component(&i2c->dev, &soc_codec_dev_max98373,
 		max98373_dai, ARRAY_SIZE(max98373_dai));
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to register codec: %d\n", ret);
+		goto err_gpio_free;
+	}
+
+	return 0;
 
+err_gpio_free:
+	if (gpio_is_valid(max98373->reset_gpio))
+		gpio_free(max98373->reset_gpio);
 	return ret;
 }