pinctrl: sirf: atlas7: fix of_irq_get() error check
diff mbox

Message ID 20170730193855.099325987@cogentembedded.com
State New
Headers show

Commit Message

Sergei Shtylyov July 30, 2017, 7:38 p.m. UTC
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(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Linus Walleij Aug. 3, 2017, 9:31 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Aug. 4, 2017, 4:58 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Aug. 7, 2017, 9:07 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Aug. 7, 2017, 9:53 a.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Aug. 7, 2017, 1:30 p.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Aug. 7, 2017, 4:20 p.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

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;