Re: [PATCH 20/21] usb: use devm_irq_of_parse_and_map() where appropriate

[Andreas Larsson]

On 2014-06-16 11:44, Nikita Yushchenko wrote:
> > > diff --git a/drivers/usb/host/ehci-grlib.c
> > > b/drivers/usb/host/ehci-grlib.c
> > > index 495b6fb..666c03e 100644
> > > --- a/drivers/usb/host/ehci-grlib.c
> > > +++ b/drivers/usb/host/ehci-grlib.c
> > > @@ -111,11 +111,11 @@ static int ehci_hcd_grlib_probe(struct
> > > platform_device *op)
> > >        hcd->rsrc_start = res.start;
> > >        hcd->rsrc_len = resource_size(&res);
> > >
> > > -    irq = irq_of_parse_and_map(dn, 0);
> > > -    if (irq == NO_IRQ) {
> > > -        dev_err(&op->dev, "%s: irq_of_parse_and_map failed\n",
> > > +    irq = devm_irq_of_parse_and_map(&op->dev, dn, 0);
> > > +    if (irq <= 0) {
> > > +        dev_err(&op->dev, "%s: devm_irq_of_parse_and_map failed\n",
> > >                __FILE__);
> > > -        rv = -EBUSY;
> > > +        rv = irq ? irq : -EINVAL;
> >
> > Here and in more places below you change the return value from -EBUSY to
> > -EINVAL when irq == 0. These changes and the reason for them is not
> > something that is commented upon in the commit message. Maybe these
> > changes were not intended or should be in a separate patch?
>
> Although errno codes are quite unspecific, I can't think a valid reason
> to return -EBUSY on [devm_]irq_of_parse_and_map() failure. It could be
> -EINVAL or -ENODEV, but not -EBUSY ...
>
> Since changing line that sets error code anyway, I decided to change
> -EBUST to -ENODEV.
>
> But I agree that this is not the topic of the patch.
>
> IS it better to remove this change from changeset alltogether, or to
> mention it in commit's log message?

Given that there are a lot of -EBUSY being returned in drivers/usb/host 
- not just in these error cases, maybe it is better to not touch this in 
this patch set.

Best regards,
Andreas Larsson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/