| Message ID | CAPgLHd-gjJ6ju3HyhiQi98YFGojvU3aYSTn7SOe4NhQPY1r++w@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. Yours, Linus Walleij
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
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
On 05/31/2013 02:29 AM, Andy Shevchenko wrote: > 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. 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
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
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
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
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@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. 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
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
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"); }