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@xxxxxxxxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxx>
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/
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
