Hi Vladimir,
On Sunday 27 December 2015 02:35:36 Vladimir Zapolskiy wrote:
> On 27.12.2015 00:35, Laurent Pinchart wrote:
> > On Tuesday 22 December 2015 15:20:09 Linus Walleij wrote:
> >> As we want gpio_chip .get() calls to be able to return negative
> >> error codes and propagate to drivers, we need to go over all
> >> drivers and make sure their return values are clamped to [0,1].
> >> We do this by using the ret = !!(val) design pattern.
> >
> > The patch itself looks good to me, but wouldn't it be easier to patch the
> > caller to clamp positive values to [0,1] and propagate negative values
> > untouched ?
>
> this has been done in v4.3 e20538b82f1f ("gpio: Propagate errors from
> chip->get()"), but the change causes problems with GPIO line id 31 and
> the change is temporarily reverted by 45ad7db90b ("gpio: revert get() to
> non-errorprogating behaviour").
>
> See also a recent discussion related to this problem
> http://www.spinics.net/lists/linux-gpio/msg10677.html
>
> >> Also start returning the error code if something fails, as the
> >> end of the series augment the core to support this.
> >>
> >> Cc: Grygorii Strashko <[email protected]>
> >> Cc: George Cherian <[email protected]>
> >> Cc: Laurent Pinchart <[email protected]>
> >> Signed-off-by: Linus Walleij <[email protected]>
> >> ---
> >>
> >> drivers/gpio/gpio-pcf857x.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> >> index bf511c0efa48..f64380a7d004 100644
> >> --- a/drivers/gpio/gpio-pcf857x.c
> >> +++ b/drivers/gpio/gpio-pcf857x.c
> >> @@ -154,7 +154,7 @@ static int pcf857x_get(struct gpio_chip *chip,
> >> unsigned
> >> offset) int value;
> >>
> >> value = gpio->read(gpio->client);
> >>
> >> - return (value < 0) ? 0 : (value & (1 << offset));
> >> + return (value < 0) ? value : !!(value & (1 << offset));
> >>
> >> }
>
> Back to your question, assume here in unmodified version the case of
> (offset == 31) [1], on upper level the returned value will be
> misapprehended as a negative number.
>
> [1] (offset == 31) may be an invalid GPIO line id in this particular
> driver, but some other gpiochip drivers should support line id 31.
Would something like the following make sense ?
value = chip->get ? chip->get(chip, offset) : -EIO;
value = IS_ERR_VALUE(value) ? value : !!value;
Granted, GPIO drivers would still need to make sure that the value they return
from register reads don't get considered as an error code, but any val & (1 <<
offset) would be fine, which is the most common case.
If you still think that patching all GPIO drivers is better I won't oppose to
that.
> >> static int pcf857x_output(struct gpio_chip *chip, unsigned offset, int
> >>
> >> value)
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
