Re: [PATCH] pinctrl: sh-pfc: Let gpio_chip.to_irq() return -ENXIO on error

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Linus,

On Mon, May 2, 2016 at 10:30 AM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Mon, May 2, 2016 at 10:03 AM, Geert Uytterhoeven
> <geert@xxxxxxxxxxxxxx> wrote:
>> On Sun, May 1, 2016 at 10:48 AM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>>>>   - Drivers that call irq_find_mapping(), irq_create_mapping(), or
>>>>     irq_create_fwspec_mapping() return zero!  This also applies to the
>>>>     core helper gpiochip_to_irq().
>>>
>>> Zero means NO_IRQ.
>>>
>>> Reminder:
>>> http://lwn.net/Articles/470820/
>>>
>>> What we should do is patch all drivers to return 0 on failure, and
>>> patch any consumers like mctrl_gpio_init() to handle that correctly.
>>
>> That's the Long Term Plan. There are still too many non-zero NO_IRQ
>> definitions in use...
>>
>> Is -ENXIO acceptable for the short term?
>
> I don't understand. You say you have a problem  with
> mctrl_gpio_init() which looks like this:
>
>         ret = gpiod_to_irq(gpios->gpio[i]);
>         if (ret <= 0) {
>             dev_err(port->dev, (...)
>
> This function is already *correctly* handling zero as NO_IRQ
> i.e. an error.
>
> Just make your driver return 0/NO_IRQ and it is fixed.
>
> Or are there other problems that you're not telling about?

"mctrl_gpio_init() returns -ENOSYS if GPIOLIB is not enabled, which is
 intended to be ignored by its callers. However, ignoring -ENOSYS when it
 was caused by a gpiod_to_irq() failure will lead to a crash later:"

Please see the !CONFIG_GPIOLIB stubs in drivers/tty/serial/serial_mctrl_gpio.h.

Hence -ENOSYS is to be ignored by the driver, and it's safe to call
any of the mctrl_gpio_*() helpers if !CONFIG_GPIOLIB. That was done so
drivers don't have to check for !CONFIG_GPIOLIB theirselves.

However, if CONFIG_GPIOLIB=y, and -ENOSYS was a real error, calling
any of the mctrl_gpio_*() helpers will cause a crash. Without testing for
CONFIG_GPIOLIB, there's no way the driver can know -ENOSYS was a
real error.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html