Re: [PATCH v2 resend 1/2] gpio: tps68470: Fix tps68470_gpio_get() reading from the wrong register

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 25, 2023 at 1:32 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> On 1/25/23 12:23, Andy Shevchenko wrote:
> > On Wed, Jan 25, 2023 at 1:05 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> >>
> >> For the regular GPIO pins the value should be read from TPS68470_REG_GPDI,
> >> so that the actual value of the pin is read, rather than the value the pin
> >> would output when put in output mode.
> >
> > It really depends. I think it's a wrong perception and brings nothing
> > to software. If we output, we know what we program, so reading back
> > returns us what we _assume_ should be on the pin under normal
> > circumstances. The difference is OD/OS/OE/OC cases where we output
> > only one possible value.
> >
> >> Fixes: 275b13a65547 ("gpio: Add support for TPS68470 GPIOs")
> >
> > Is it really fixing anything? Can we have more?
> >
> > P.S. Before doing this, I would have a clarification in the
> > documentation. Sorry that I have had no time to respond to my series
> > regarding that. But it seems we have a strong disagreement in the
> > area.
>
> I know disagree on some of the semantics of gpiod_get_value() but 99.9%
> of the consumers of gpiod_get_value() are going to deal with pins
> which are either in input mode, or in some non push-pull (e.g.
> open-collector for i2c) mode. And in those cases reading GPDI
> os the right thing to do.
>
> Calling gpiod_get_value() when the pin is in push-pull mode really
> does not make much sense, so there will be very little if any users
> of that. And this patch fixes the 99.9% other (potential) users
> while not breaking the push-pull case since even then reading GPDI
> should still return the right value.

Some hardware may not even allow what you are doing here.
For example when input and output direction is the same bit in the
register. Another case when the input buffer is simply disabled by the
driver for a reason (power consumption, etc).

That's why we need some flags to be sure we are doing the right thing.

-- 
With Best Regards,
Andy Shevchenko



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux