Re: BGPIOF_READ_OUTPUT_REG_SET problem

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Wed, Dec 16, 2015 at 7:58 PM, Vladimir Zapolskiy
<vladimir_zapolskiy@xxxxxxxxxx> wrote:

> Later on Bjorn's commit e20538b82 ("gpio: Propagate errors from
> chip->get()") (dated Aug 28, 2015) changed the return type from bool to int,
> so you get the error now.

Right.

> I believe the right decision will be to fix all currently broken gpiochip
> .get() functions, most probably returning a negative error was done on
> purpose, Linus, please confirm.

On (A): yes the right thing to do is to fix all broken .get() functions.
Help appreciated.

Yes, that is described in a response in the mail chain: we once believed
that get/set would not return negative so I was about to actually make
them bool. I was thinking it's just a register write or something.
gpiolib was once invented for that simple usecase.

But then we realized that e.g. a GPIO expander can fail an I2C
transaction for whatever reason (hardware failure for example) and
we need to handle that, so Björn's fix is the right one and we need
to fix the drivers.

I'm sorry for not realizing the scope of the problem :(

>         chip = desc->chip;
>         offset = gpio_chip_hwgpio(desc);
> -       value = chip->get ? chip->get(chip, offset) : false;
> +       value = chip->get ? chip->get(chip, offset) : -EIO;
> +       value = value < 0 ? value : !!value;
>
> ^^^ this !! is a naive attempt to imitate backward compatible behaviour,
> however it is broken, because if some !0 value is returned, then it might
> happen that (value < 0) branch is selected.

Hm. But we still need to propagate errors.

Maybe we should patch this back to not propagate errors for the
v4.4-rcS and then revert that and do the real fix, proofreading all
vulnerable drivers for v4.5, what do you say?

(I'll send a patch.)

> Linus, I missed any relative discussion on the mailing list, what is the
> final decision, can _gpiod_get_raw_value() return errors or not?

What do you think about my proposal to revert to the old behaviour
for v4.4 and we'll go over all drivers and provide a deeper fix in v4.5?

Yours,
Linus Walleij
--
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