[PATCH] mmc: sh_mobile_sdhi: fix error return code in sh_mobile_sdhi_probe()

[Guennadi Liakhovetski]

On Wed, 29 May 2013, Wei Yongjun wrote:

> Hi
> 
> On 05/29/2013 02:58 PM, Guennadi Liakhovetski wrote:
> > Hi
> >
> > On Tue, 28 May 2013, Wei Yongjun wrote:
> >
> >> From: Wei Yongjun <yongjun_wei at trendmicro.com.cn>
> >>
> >> Fix to return a negative error code instead of 0 when we cannot get
> >> IRQ source by platform_get_irq(), as done elsewhere in this function.
> >>
> >> Signed-off-by: Wei Yongjun <yongjun_wei at trendmicro.com.cn>
> > Thanks for the patch. Do you think the following version would be even a 
> > bit simpler?
> >
> > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > index fe90853..76661b6 100644
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > @@ -255,18 +255,17 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
> >  	if (multiplexed_isr) {
> >  		while (1) {
> >  			irq = platform_get_irq(pdev, i);
> > -			if (irq < 0)
> > -				break;
> > +			/* There must be at least one IRQ source */
> > +			if (irq < 0) {
> > +				ret = irq;
> > +				goto eirq;
> > +			}
> >  			i++;
> >  			ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_irq, 0,
> >  					  dev_name(&pdev->dev), host);
> >  			if (ret)
> >  				goto eirq;
> >  		}
> > -
> > -		/* There must be at least one IRQ source */
> > -		if (!i)
> > -			goto eirq;
> >  	}
> >  
> >  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
> 
> The orig code used while loop to get many irqs, but with your change,
> we will leave the loop util platform_get_irq() or request_irq() fail
> and nerver success.

Mmmh, ok, right. You mean, the original code wouldn't error out if at 
least 1 IRQ had been detected, whereas my change would, which is wrong. 
Sorry, forget about my version, yours was right.

Acked-by: Guennadi Liakhovetski <g.liakhovetski+renesas at googlemail.com>

Thanks
Guennadi

> Do your means remove the while loop and this device
> need only one irq?
> 
> >
> > If you agree, maybe you could send a v2 of your patch in this form, I'll 
> > ack it then.
> >
> > Thanks
> > Guennadi
> >
> >> ---
> >>  drivers/mmc/host/sh_mobile_sdhi.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> >> index cc4c872..a4316b3 100644
> >> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> >> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> >> @@ -273,8 +273,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
> >>  		}
> >>  
> >>  		/* There must be at least one IRQ source */
> >> -		if (!i)
> >> +		if (!i) {
> >> +			ret = irq;
> >>  			goto eirq;
> >> +		}
> >>  	}
> >>  
> >>  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
> >>

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/