Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1227753 > unrolled thread

[PATCH 1/3] ASoC: wm0010: disable regulator on error

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] ASoC: wm0010: disable regulator on error Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-18 12:40 +0200
    [PATCH 2/3] ASoC: wm0010: fix memory leak Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-18 12:40 +0200
      Re: [PATCH 2/3] ASoC: wm0010: fix memory leak Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2015-09-18 14:10 +0200
    [PATCH 3/3] ASoC: wm0010: fix error path Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-18 12:40 +0200
      Re: [PATCH 3/3] ASoC: wm0010: fix error path Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2015-09-18 14:10 +0200
    Re: [PATCH 1/3] ASoC: wm0010: disable regulator on error Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2015-09-18 14:00 +0200
      Re: [PATCH 1/3] ASoC: wm0010: disable regulator on error Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-18 14:50 +0200
        Re: [PATCH 1/3] ASoC: wm0010: disable regulator on error Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2015-09-18 15:10 +0200

#1227753 — [PATCH 1/3] ASoC: wm0010: disable regulator on error

We have done regulator_bulk_enable() while booting the DSP but on the
error exit path we have not disbled it.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 sound/soc/codecs/wm0010.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
index 8434d45..79a7cd3 100644
--- a/sound/soc/codecs/wm0010.c
+++ b/sound/soc/codecs/wm0010.c
@@ -739,8 +739,6 @@ static int wm0010_boot(struct snd_soc_codec *codec)
 abort:
 	/* Put the chip back into reset */
 	wm0010_halt(codec);
-	mutex_unlock(&wm0010->lock);
-	return ret;
 
 err_core:
 	mutex_unlock(&wm0010->lock);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1227756 — [PATCH 2/3] ASoC: wm0010: fix memory leak

We have requested for the firmware but we have missed releasing it both
on success and on error path.
While checking the code it turned out that the requested firmware is not
even used. More over the same firmware is being loaded by
wm0010_stage2_load().

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 sound/soc/codecs/wm0010.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
index 79a7cd3..4947be5 100644
--- a/sound/soc/codecs/wm0010.c
+++ b/sound/soc/codecs/wm0010.c
@@ -577,7 +577,6 @@ static int wm0010_boot(struct snd_soc_codec *codec)
 	struct wm0010_priv *wm0010 = snd_soc_codec_get_drvdata(codec);
 	unsigned long flags;
 	int ret;
-	const struct firmware *fw;
 	struct spi_message m;
 	struct spi_transfer t;
 	struct dfw_pllrec pll_rec;
@@ -623,14 +622,6 @@ static int wm0010_boot(struct snd_soc_codec *codec)
 	wm0010->state = WM0010_OUT_OF_RESET;
 	spin_unlock_irqrestore(&wm0010->irq_lock, flags);
 
-	/* First the bootloader */
-	ret = request_firmware(&fw, "wm0010_stage2.bin", codec->dev);
-	if (ret != 0) {
-		dev_err(codec->dev, "Failed to request stage2 loader: %d\n",
-			ret);
-		goto abort;
-	}
-
 	if (!wait_for_completion_timeout(&wm0010->boot_completion,
 					 msecs_to_jiffies(20)))
 		dev_err(codec->dev, "Failed to get interrupt from DSP\n");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227798 — Re: [PATCH 2/3] ASoC: wm0010: fix memory leak

On Fri, Sep 18, 2015 at 04:02:20PM +0530, Sudip Mukherjee wrote:
> We have requested for the firmware but we have missed releasing it both
> on success and on error path.
> While checking the code it turned out that the requested firmware is not
> even used. More over the same firmware is being loaded by
> wm0010_stage2_load().
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---

hmm... odd guess this must have been missed when
wm0010_stage2_load was created.

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227757 — [PATCH 3/3] ASoC: wm0010: fix error path

Fix the error path so that we can free the allocated memory on the error
path instead of releasing them individually on each error.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 sound/soc/codecs/wm0010.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
index 4947be5..3ced902 100644
--- a/sound/soc/codecs/wm0010.c
+++ b/sound/soc/codecs/wm0010.c
@@ -663,10 +663,8 @@ static int wm0010_boot(struct snd_soc_codec *codec)
 		}
 
 		img_swap = kzalloc(len, GFP_KERNEL | GFP_DMA);
-		if (!img_swap) {
-			kfree(out);
-			goto abort;
-		}
+		if (!img_swap)
+			goto abort_out;
 
 		/* We need to re-order for 0010 */
 		byte_swap_64((u64 *)&pll_rec, img_swap, len);
@@ -681,20 +679,16 @@ static int wm0010_boot(struct snd_soc_codec *codec)
 		spi_message_add_tail(&t, &m);
 
 		ret = spi_sync(spi, &m);
-		if (ret != 0) {
+		if (ret) {
 			dev_err(codec->dev, "First PLL write failed: %d\n", ret);
-			kfree(img_swap);
-			kfree(out);
-			goto abort;
+			goto abort_swap;
 		}
 
 		/* Use a second send of the message to get the return status */
 		ret = spi_sync(spi, &m);
-		if (ret != 0) {
+		if (ret) {
 			dev_err(codec->dev, "Second PLL write failed: %d\n", ret);
-			kfree(img_swap);
-			kfree(out);
-			goto abort;
+			goto abort_swap;
 		}
 
 		p = (u32 *)out;
@@ -727,6 +721,10 @@ static int wm0010_boot(struct snd_soc_codec *codec)
 
 	return 0;
 
+abort_swap:
+	kfree(img_swap);
+abort_out:
+	kfree(out);
 abort:
 	/* Put the chip back into reset */
 	wm0010_halt(codec);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227799 — Re: [PATCH 3/3] ASoC: wm0010: fix error path

On Fri, Sep 18, 2015 at 04:02:21PM +0530, Sudip Mukherjee wrote:
> Fix the error path so that we can free the allocated memory on the error
> path instead of releasing them individually on each error.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227795

On Fri, Sep 18, 2015 at 04:02:19PM +0530, Sudip Mukherjee wrote:
> We have done regulator_bulk_enable() while booting the DSP but on the
> error exit path we have not disbled it.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  sound/soc/codecs/wm0010.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
> index 8434d45..79a7cd3 100644
> --- a/sound/soc/codecs/wm0010.c
> +++ b/sound/soc/codecs/wm0010.c
> @@ -739,8 +739,6 @@ static int wm0010_boot(struct snd_soc_codec *codec)
>  abort:
>  	/* Put the chip back into reset */
>  	wm0010_halt(codec);
> -	mutex_unlock(&wm0010->lock);
> -	return ret;

Does wm0010_halt not disable the regulators?

Thanks,
Charles

>  
>  err_core:
>  	mutex_unlock(&wm0010->lock);
> -- 
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227830

On Fri, Sep 18, 2015 at 12:34:12PM +0100, Charles Keepax wrote:
> On Fri, Sep 18, 2015 at 04:02:19PM +0530, Sudip Mukherjee wrote:
> > We have done regulator_bulk_enable() while booting the DSP but on the
> > error exit path we have not disbled it.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> >  sound/soc/codecs/wm0010.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
> > index 8434d45..79a7cd3 100644
> > --- a/sound/soc/codecs/wm0010.c
> > +++ b/sound/soc/codecs/wm0010.c
> > @@ -739,8 +739,6 @@ static int wm0010_boot(struct snd_soc_codec *codec)
> >  abort:
> >  	/* Put the chip back into reset */
> >  	wm0010_halt(codec);
> > -	mutex_unlock(&wm0010->lock);
> > -	return ret;
> 
> Does wm0010_halt not disable the regulators?
oops, yes, it does. Sorry I should have seen it before posting.
patch 2/3 and 3/3 will still apply even if this 1/3 is not applied.
Should I send a v2 leaving out this patch or is it ok to discard this
patch while applying?

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227848

On Fri, Sep 18, 2015 at 06:12:05PM +0530, Sudip Mukherjee wrote:
> On Fri, Sep 18, 2015 at 12:34:12PM +0100, Charles Keepax wrote:
> > On Fri, Sep 18, 2015 at 04:02:19PM +0530, Sudip Mukherjee wrote:
> > > We have done regulator_bulk_enable() while booting the DSP but on the
> > > error exit path we have not disbled it.
> > > 
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > ---
> > >  sound/soc/codecs/wm0010.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
> > > index 8434d45..79a7cd3 100644
> > > --- a/sound/soc/codecs/wm0010.c
> > > +++ b/sound/soc/codecs/wm0010.c
> > > @@ -739,8 +739,6 @@ static int wm0010_boot(struct snd_soc_codec *codec)
> > >  abort:
> > >  	/* Put the chip back into reset */
> > >  	wm0010_halt(codec);
> > > -	mutex_unlock(&wm0010->lock);
> > > -	return ret;
> > 
> > Does wm0010_halt not disable the regulators?
> oops, yes, it does. Sorry I should have seen it before posting.
> patch 2/3 and 3/3 will still apply even if this 1/3 is not applied.
> Should I send a v2 leaving out this patch or is it ok to discard this
> patch while applying?

As long as they apply cleanly I would think its fine to just
leave them as is.

Thanks,
Charles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web