[PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Wei Yongjun]

From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>

In many of the error handling case, the return value 'ret' not set
and 0 will be return from d40_probe() even if error, but we should
return a negative error code instead in those error handling case.
This patch fixed them, and also removed useless variable 'err'.

Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
---
v1 -> v2: rebased on linux-next.git
---
 drivers/dma/ste_dma40.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 7f23d45..a241e25 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3506,7 +3506,6 @@ static int __init d40_probe(struct platform_device *pdev)
 {
 	struct stedma40_platform_data *plat_data = pdev->dev.platform_data;
 	struct device_node *np = pdev->dev.of_node;
-	int err;
 	int ret = -ENOENT;
 	struct d40_base *base = NULL;
 	struct resource *res = NULL;
@@ -3619,6 +3618,7 @@ static int __init d40_probe(struct platform_device *pdev)
 		if (IS_ERR(base->lcpa_regulator)) {
 			d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
 			base->lcpa_regulator = NULL;
+			ret = PTR_ERR(base->lcpa_regulator);
 			goto failure;
 		}
 
@@ -3633,13 +3633,13 @@ static int __init d40_probe(struct platform_device *pdev)
 	}
 
 	base->initialized = true;
-	err = d40_dmaengine_init(base, num_reserved_chans);
-	if (err)
+	ret = d40_dmaengine_init(base, num_reserved_chans);
+	if (ret)
 		goto failure;
 
 	base->dev->dma_parms = &base->dma_parms;
-	err = dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
-	if (err) {
+	ret = dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
+	if (ret) {
 		d40_err(&pdev->dev, "Failed to set dma max seg size\n");
 		goto failure;
 	}
@@ -3647,8 +3647,8 @@ static int __init d40_probe(struct platform_device *pdev)
 	d40_hw_init(base);
 
 	if (np) {
-		err = of_dma_controller_register(np, d40_xlate, NULL);
-		if (err && err != -ENODEV)
+		ret = of_dma_controller_register(np, d40_xlate, NULL);
+		if (ret && ret != -ENODEV)
 			dev_err(&pdev->dev,
 				"could not register of_dma_controller\n");
 	}

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Linus Walleij]

On Thu, May 30, 2013 at 6:32 AM, Wei Yongjun <weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> In many of the error handling case, the return value 'ret' not set
> and 0 will be return from d40_probe() even if error, but we should
> return a negative error code instead in those error handling case.
> This patch fixed them, and also removed useless variable 'err'.
>
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
> ---
> v1 -> v2: rebased on linux-next.git

I've tentatively applied this to my dma40 branch, waiting for Vinod
to ACK it.

Yours,
Linus Walleij

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Vinod Koul]

On Thu, May 30, 2013 at 09:41:38AM +0200, Linus Walleij wrote:
> On Thu, May 30, 2013 at 6:32 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> 
> > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >
> > In many of the error handling case, the return value 'ret' not set
> > and 0 will be return from d40_probe() even if error, but we should
> > return a negative error code instead in those error handling case.
> > This patch fixed them, and also removed useless variable 'err'.
> >
> > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > ---
> > v1 -> v2: rebased on linux-next.git
> 
> I've tentatively applied this to my dma40 branch, waiting for Vinod
> to ACK it.
I though you wanted me to apply this :)

Nevertheless, Acked-by: Vinod Koul <vinod.koul@intel.com>

Can you CC stable too, pls.

--
~Vinod

-- 

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Linus Walleij]

On Thu, May 30, 2013 at 7:33 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Thu, May 30, 2013 at 09:41:38AM +0200, Linus Walleij wrote:
>> On Thu, May 30, 2013 at 6:32 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
>>
>> > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>> >
>> > In many of the error handling case, the return value 'ret' not set
>> > and 0 will be return from d40_probe() even if error, but we should
>> > return a negative error code instead in those error handling case.
>> > This patch fixed them, and also removed useless variable 'err'.
>> >
>> > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>> > ---
>> > v1 -> v2: rebased on linux-next.git
>>
>> I've tentatively applied this to my dma40 branch, waiting for Vinod
>> to ACK it.
> I though you wanted me to apply this :)
>
> Nevertheless, Acked-by: Vinod Koul <vinod.koul@intel.com>
>
> Can you CC stable too, pls.

Hm I'm not sending any DMA40 stuff for stable, sorry ...
if you want it to go into stable you'd better pick this
(the v3 version) into the DMA tree.

Let me know how you want it, I've removed it from my
dma40 branch for the time being.

Yours,
Linus Walleij

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Vinod Koul]

On Tue, Jun 04, 2013 at 11:14:16AM +0200, Linus Walleij wrote:
> On Thu, May 30, 2013 at 7:33 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Thu, May 30, 2013 at 09:41:38AM +0200, Linus Walleij wrote:
> >> On Thu, May 30, 2013 at 6:32 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> >>
> >> > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >> >
> >> > In many of the error handling case, the return value 'ret' not set
> >> > and 0 will be return from d40_probe() even if error, but we should
> >> > return a negative error code instead in those error handling case.
> >> > This patch fixed them, and also removed useless variable 'err'.
> >> >
> >> > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >> > ---
> >> > v1 -> v2: rebased on linux-next.git
> >>
> >> I've tentatively applied this to my dma40 branch, waiting for Vinod
> >> to ACK it.
> > I though you wanted me to apply this :)
> >
> > Nevertheless, Acked-by: Vinod Koul <vinod.koul@intel.com>
> >
> > Can you CC stable too, pls.
> 
> Hm I'm not sending any DMA40 stuff for stable, sorry ...
> if you want it to go into stable you'd better pick this
> (the v3 version) into the DMA tree.
okay
> 
> Let me know how you want it, I've removed it from my
> dma40 branch for the time being.
Have you removed, Also I see a v3 of this, do you want to ack that before I
apply

--
~Vinod

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Linus Walleij]

On Wed, Jun 12, 2013 at 11:54 AM, Vinod Koul <vinod.koul@intel.com> wrote:

>> Let me know how you want it, I've removed it from my
>> dma40 branch for the time being.
>
> Have you removed, Also I see a v3 of this, do you want to ack that before I
> apply

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I haven't sent this patch to ARM SoC so it will not appear from any other source
in the merge window or cause any conflicts.

Yours,
Linus Walleij

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Vinod Koul]

On Thu, Jun 13, 2013 at 09:49:04AM +0200, Linus Walleij wrote:
> On Wed, Jun 12, 2013 at 11:54 AM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> 
> >> Let me know how you want it, I've removed it from my
> >> dma40 branch for the time being.
> >
> > Have you removed, Also I see a v3 of this, do you want to ack that before I
> > apply
> 
> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> I haven't sent this patch to ARM SoC so it will not appear from any other source
> in the merge window or cause any conflicts.
Doesnt apply on my next or -rc6. I think it has some ste dependecies. So either
we wait and I send this for next -rc2 or you apply with my Ack

--
~Vinod

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Linus Walleij]

On Mon, Jun 17, 2013 at 3:50 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Thu, Jun 13, 2013 at 09:49:04AM +0200, Linus Walleij wrote:

>> I haven't sent this patch to ARM SoC so it will not appear from any other source
>> in the merge window or cause any conflicts.
>
> Doesnt apply on my next or -rc6. I think it has some ste dependecies. So either
> we wait and I send this for next -rc2 or you apply with my Ack

I have it queued in my tree so it won't be forgotten.

That said: ARM SoC folks: can you apply this fix directly
to wherever in the ARM SoC tree the DMA40 patches have
ended up? (Looks like next/drivers)

Yours,
Linus Walleij

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Andy Shevchenko]

On Thu, May 30, 2013 at 7:32 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> In many of the error handling case, the return value 'ret' not set
> and 0 will be return from d40_probe() even if error, but we should
> return a negative error code instead in those error handling case.
> This patch fixed them, and also removed useless variable 'err'.

Hold on, please.

> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c

> @@ -3619,6 +3618,7 @@ static int __init d40_probe(struct platform_device *pdev)
>                 if (IS_ERR(base->lcpa_regulator)) {
>                         d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
>                         base->lcpa_regulator = NULL;
> +                       ret = PTR_ERR(base->lcpa_regulator);

Is it really what we want?

I thixh you may remove that NULL assignment.


>                         goto failure;
>                 }
>

> @@ -3647,8 +3647,8 @@ static int __init d40_probe(struct platform_device *pdev)
>         d40_hw_init(base);
>
>         if (np) {
> -               err = of_dma_controller_register(np, d40_xlate, NULL);
> -               if (err && err != -ENODEV)
> +               ret = of_dma_controller_register(np, d40_xlate, NULL);
> +               if (ret && ret != -ENODEV)

>From the discussion of dw_dmac I remember we decide that ENODEV check
is redundant.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH -next v2] dmaengine: ste_dma40: fix error return code in d40_probe()

[Wei Yongjun]

On 05/31/2013 02:29 AM, Andy Shevchenko wrote:
> On Thu, May 30, 2013 at 7:32 AM, Wei Yongjun <weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> In many of the error handling case, the return value 'ret' not set
>> and 0 will be return from d40_probe() even if error, but we should
>> return a negative error code instead in those error handling case.
>> This patch fixed them, and also removed useless variable 'err'.
> Hold on, please.
>
>> --- a/drivers/dma/ste_dma40.c
>> +++ b/drivers/dma/ste_dma40.c
>> @@ -3619,6 +3618,7 @@ static int __init d40_probe(struct platform_device *pdev)
>>                 if (IS_ERR(base->lcpa_regulator)) {
>>                         d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
>>                         base->lcpa_regulator = NULL;
>> +                       ret = PTR_ERR(base->lcpa_regulator);
> Is it really what we want?
>
> I thixh you may remove that NULL assignment.

Ohh, I will move the ret = PTR_ERR(base->lcpa_regulator) above the NULL
assignment, the failure path test for base->lcpa_regulator to release regulator.

        if (base->lcpa_regulator) {
            regulator_disable(base->lcpa_regulator);
            regulator_put(base->lcpa_regulator);
        }

>
>
>>                         goto failure;
>>                 }
>>
>> @@ -3647,8 +3647,8 @@ static int __init d40_probe(struct platform_device *pdev)
>>         d40_hw_init(base);
>>
>>         if (np) {
>> -               err = of_dma_controller_register(np, d40_xlate, NULL);
>> -               if (err && err != -ENODEV)
>> +               ret = of_dma_controller_register(np, d40_xlate, NULL);
>> +               if (ret && ret != -ENODEV)
> >From the discussion of dw_dmac I remember we decide that ENODEV check
> is redundant.

Get it, I will remove the ENODEV check.

Thanks,
Yongjun Wei