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

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

[Sudip Mukherjee]

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

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

[Sudip Mukherjee]

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

[PATCH 3/3] ASoC: wm0010: fix error path

[Sudip Mukherjee]

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

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

[Charles Keepax]

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

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

[Charles Keepax]

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

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

[Charles Keepax]

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

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

[Charles Keepax]

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

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

[Sudip Mukherjee]

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