pinctrl: sh-pfc: Let gpio_chip.to_irq() return -ENXIO on error

Message ID CACRpkdZLSRQfUfc4d=uO0CtH3svN+a17HLbEQYtdy8kMAS_sUA@mail.gmail.com
State New
Headers show

Commit Message

Linus Walleij May 2, 2016, 9:06 a.m.
On Mon, May 2, 2016 at 11:04 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

> [silly response deleted]

>

> Scrap it.


:D

> The only annoying thing is that 0 cannot easily be propagated upstream as

> an error code, so it has to be tested for explicitly.


Well with the Big Penguin's clear opinion on the matter there is not
much we can do.

What about this?

                }
                gpios->irq[i] = ret;

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

Comments

Linus Walleij May 2, 2016, 9:25 a.m. | #1
On Mon, May 2, 2016 at 11:10 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, May 2, 2016 at 11:06 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> On Mon, May 2, 2016 at 11:04 AM, Geert Uytterhoeven

>> <geert@linux-m68k.org> wrote:

>>

>>> [silly response deleted]

>>>

>>> Scrap it.

>>

>> :D

>>

>>> The only annoying thing is that 0 cannot easily be propagated upstream as

>>> an error code, so it has to be tested for explicitly.

>>

>> Well with the Big Penguin's clear opinion on the matter there is not

>> much we can do.

>>

>> What about this?

>>

>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c

>> b/drivers/tty/serial/serial_mctrl_gpio.c

>> index 02147361eaa9..2297ec781681 100644

>> --- a/drivers/tty/serial/serial_mctrl_gpio.c

>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c

>> @@ -172,6 +172,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct

>> uart_port *port, unsigned int idx)

>>                         dev_err(port->dev,

>>                                 "failed to find corresponding irq for

>> %s (idx=%d, err=%d)\n",

>>                                 mctrl_gpios_desc[i].name, idx, ret);

>> +                       /*

>> +                        * Satisfy the error code semantics for a missing IRQ,

>> +                        * 0 means NO_IRQ, but the framework needs to return

>> +                        * a negative to deal with the error.

>> +                        */

>> +                       if (!ret)

>> +                               ret = -ENOSYS;

>

> No, not -ENOSYS, as that triggers my initial problem again (please read the

> deleted silly response ;-)

> Checkpatch says: "ENOSYS means 'invalid syscall nr' and nothing else".

>

> -EINVAL?


Sure, whatever works with your semantics.

Will you test & send a patch?

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
Linus Walleij May 2, 2016, 11:16 a.m. | #2
On Mon, May 2, 2016 at 12:05 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

> gpiod_to_irq() is documented to return a negative error code on failure,

> cfr. Documentation/gpio/consumer.txt and drivers/gpio/gpiolib.c.

> In fact the check in mctrl_gpio_init() is the only caller that

> considers zero as an

> error.

>

> Perhaps gpiod_to_irq() should handle .to_irq() returning zero instead?


OK good point, I sent a patch like so, let's see what people say.

I'm a bit torn: part of me feel that since the function has *_to_irq()
in it it should use zero for NO_IRQ if there is no translation.

But this works too, whatever, rough consensus and running code.

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

Patch hide | download patch | download mbox

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
b/drivers/tty/serial/serial_mctrl_gpio.c
index 02147361eaa9..2297ec781681 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -172,6 +172,13 @@  struct mctrl_gpios *mctrl_gpio_init(struct
uart_port *port, unsigned int idx)
                        dev_err(port->dev,
                                "failed to find corresponding irq for
%s (idx=%d, err=%d)\n",
                                mctrl_gpios_desc[i].name, idx, ret);
+                       /*
+                        * Satisfy the error code semantics for a missing IRQ,
+                        * 0 means NO_IRQ, but the framework needs to return
+                        * a negative to deal with the error.
+                        */
+                       if (!ret)
+                               ret = -ENOSYS;
                        return ERR_PTR(ret);