| Message ID | 20170730193855.099325987@cogentembedded.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jul 30, 2017 at 9:38 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > of_irq_get() may return any negative error number as well as 0 on failure, > while the driver only checks for -EPROBE_DEFER, blithely continuing with > the call to gpiochip_set_chained_irqchip() -- that function expects the > parent IRQ as *unsigned int*, so would probably do nothing when a large > IRQ number resulting from a conversion of a negative error number is passed > to it, however passing 0 would probably work but the driver won't receive > valid GPIO bank interrupts. > > Check for 'ret <= 0' instead and return -ENXIO from the driver's probe iff > of_irq_get() returned 0. > > Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio support") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Patch applied! Yours, Linus Walleij
On 08/04/2017 12:31 AM, Linus Walleij wrote: >> of_irq_get() may return any negative error number as well as 0 on failure, >> while the driver only checks for -EPROBE_DEFER, blithely continuing with >> the call to gpiochip_set_chained_irqchip() -- that function expects the >> parent IRQ as *unsigned int*, so would probably do nothing when a large >> IRQ number resulting from a conversion of a negative error number is passed >> to it, however passing 0 would probably work but the driver won't receive >> valid GPIO bank interrupts. >> >> Check for 'ret <= 0' instead and return -ENXIO from the driver's probe iff >> of_irq_get() returned 0. >> >> Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio support") >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > Patch applied! Thanks! But to which branch? I'm not seeing it in linux-pinctrl.git... > Yours, > Linus Walleij MBR, Sergei
On Fri, Aug 4, 2017 at 6:58 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 08/04/2017 12:31 AM, Linus Walleij wrote: > >>> of_irq_get() may return any negative error number as well as 0 on >>> failure, >>> while the driver only checks for -EPROBE_DEFER, blithely continuing with >>> the call to gpiochip_set_chained_irqchip() -- that function expects the >>> parent IRQ as *unsigned int*, so would probably do nothing when a large >>> IRQ number resulting from a conversion of a negative error number is >>> passed >>> to it, however passing 0 would probably work but the driver won't receive >>> valid GPIO bank interrupts. >>> >>> Check for 'ret <= 0' instead and return -ENXIO from the driver's probe >>> iff >>> of_irq_get() returned 0. >>> >>> Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio >>> support") >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> >> >> Patch applied! > > > Thanks! But to which branch? I'm not seeing it in linux-pinctrl.git... devel. It seems the SiRF people are not using upstream (see other mails) so I do not see we need to expediate it to fixes. Yours, Linus Walleij
Hello! On 8/7/2017 12:07 PM, Linus Walleij wrote: >>>> of_irq_get() may return any negative error number as well as 0 on >>>> failure, >>>> while the driver only checks for -EPROBE_DEFER, blithely continuing with >>>> the call to gpiochip_set_chained_irqchip() -- that function expects the >>>> parent IRQ as *unsigned int*, so would probably do nothing when a large >>>> IRQ number resulting from a conversion of a negative error number is >>>> passed >>>> to it, however passing 0 would probably work but the driver won't receive >>>> valid GPIO bank interrupts. >>>> >>>> Check for 'ret <= 0' instead and return -ENXIO from the driver's probe >>>> iff >>>> of_irq_get() returned 0. >>>> >>>> Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio >>>> support") >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> >>> >>> Patch applied! >> >> >> Thanks! But to which branch? I'm not seeing it in linux-pinctrl.git... > > devel. Still not seeing the patch, perhaps you forgot to push? > It seems the SiRF people are not using upstream (see other mails) so I do > not see we need to expediate it to fixes. Up to you. > Yours, > Linus Walleij MBR, Sergei
On Mon, Aug 7, 2017 at 11:53 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 8/7/2017 12:07 PM, Linus Walleij wrote: > >>>>> of_irq_get() may return any negative error number as well as 0 on >>>>> failure, >>>>> while the driver only checks for -EPROBE_DEFER, blithely continuing >>>>> with >>>>> the call to gpiochip_set_chained_irqchip() -- that function expects the >>>>> parent IRQ as *unsigned int*, so would probably do nothing when a >>>>> large >>>>> IRQ number resulting from a conversion of a negative error number is >>>>> passed >>>>> to it, however passing 0 would probably work but the driver won't >>>>> receive >>>>> valid GPIO bank interrupts. >>>>> >>>>> Check for 'ret <= 0' instead and return -ENXIO from the driver's probe >>>>> iff >>>>> of_irq_get() returned 0. >>>>> >>>>> Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio >>>>> support") >>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>>> >>>> >>>> >>>> Patch applied! >>> >>> >>> >>> Thanks! But to which branch? I'm not seeing it in linux-pinctrl.git... >> >> >> devel. > > > Still not seeing the patch, perhaps you forgot to push? Nah just haven't had time to push yet... My mailbox is filling up with new patches at the same pace that I apply them it seems. Linus
On 08/07/2017 04:30 PM, Linus Walleij wrote: >>>>>> of_irq_get() may return any negative error number as well as 0 on >>>>>> failure, >>>>>> while the driver only checks for -EPROBE_DEFER, blithely continuing >>>>>> with >>>>>> the call to gpiochip_set_chained_irqchip() -- that function expects the >>>>>> parent IRQ as *unsigned int*, so would probably do nothing when a >>>>>> large >>>>>> IRQ number resulting from a conversion of a negative error number is >>>>>> passed >>>>>> to it, however passing 0 would probably work but the driver won't >>>>>> receive >>>>>> valid GPIO bank interrupts. >>>>>> >>>>>> Check for 'ret <= 0' instead and return -ENXIO from the driver's probe >>>>>> iff >>>>>> of_irq_get() returned 0. >>>>>> >>>>>> Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio >>>>>> support") >>>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>>>> >>>>> Patch applied! >>>> >>>> Thanks! But to which branch? I'm not seeing it in linux-pinctrl.git... >>> >>> devel. >> >> Still not seeing the patch, perhaps you forgot to push? > > Nah just haven't had time to push yet... Ah... seeing the patches now, thanks! > My mailbox is filling up with new patches at the same pace that I > apply them it seems. The joys of being a maintainer. :-) > Linus MBR, Sergei
Index: linux-pinctrl/drivers/pinctrl/sirf/pinctrl-atlas7.c =================================================================== --- linux-pinctrl.orig/drivers/pinctrl/sirf/pinctrl-atlas7.c +++ linux-pinctrl/drivers/pinctrl/sirf/pinctrl-atlas7.c @@ -6081,9 +6081,11 @@ static int atlas7_gpio_probe(struct plat /* Get interrupt number from DTS */ ret = of_irq_get(np, idx); - if (ret == -EPROBE_DEFER) { + if (ret <= 0) { dev_err(&pdev->dev, "Unable to find IRQ number. ret=%d\n", ret); + if (!ret) + ret = -ENXIO; goto failed; } bank->irq = ret;
of_irq_get() may return any negative error number as well as 0 on failure, while the driver only checks for -EPROBE_DEFER, blithely continuing with the call to gpiochip_set_chained_irqchip() -- that function expects the parent IRQ as *unsigned int*, so would probably do nothing when a large IRQ number resulting from a conversion of a negative error number is passed to it, however passing 0 would probably work but the driver won't receive valid GPIO bank interrupts. Check for 'ret <= 0' instead and return -ENXIO from the driver's probe iff of_irq_get() returned 0. Fixes: f9367793293d ("pinctrl: sirf: add sirf atlas7 pinctrl and gpio support") Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- The patch is against the 'fixes' branch of Linus W.'s 'linux-pinctrl.git' repo. drivers/pinctrl/sirf/pinctrl-atlas7.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)