[U-Boot] [PATCH 3/4] gpio: xilinx: Not read output values via regs

[Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>]

Am 24.07.2018 um 12:31 schrieb Michal Simek:
> On 23.7.2018 20:42, Stefan Herbrechtsmeier wrote:
>> Am 23.07.2018 um 13:43 schrieb Michal Simek:
>>> Reading registers for finding out output value is not working because
>>> input value is read instead in case of tristate.
>>>
>>> Reported-by: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>    drivers/gpio/xilinx_gpio.c | 38 +++++++++++++++++++++++++++++++++-----
>>>    1 file changed, 33 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
>>> index 4da9ae114d87..9d3e9379d0e5 100644
>>> --- a/drivers/gpio/xilinx_gpio.c
>>> +++ b/drivers/gpio/xilinx_gpio.c

[snip]

>>>    +    priv->output_val[bank] = val;
>>> +
>>>        return val;
>>>    };
>>>    @@ -441,6 +449,7 @@ static int xilinx_gpio_get_function(struct
>>> udevice *dev, unsigned offset)
>>>    static int xilinx_gpio_get_value(struct udevice *dev, unsigned offset)
>>>    {
>>>        struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>>> +    struct xilinx_gpio_privdata *priv = dev_get_priv(dev);
>>>        int val, ret;
>>>        u32 bank, pin;
>>>    @@ -451,7 +460,14 @@ static int xilinx_gpio_get_value(struct udevice
>>> *dev, unsigned offset)
>>>        debug("%s: regs: %lx, gpio: %x, bank %x, pin %x\n", __func__,
>>>              (ulong)platdata->regs, offset, bank, pin);
>>>    -    val = readl(&platdata->regs->gpiodata + bank * 2);
>>> +    if (xilinx_gpio_get_function(dev, offset) == GPIOF_INPUT) {
>>> +        debug("%s: Read input value from reg\n", __func__);
>>> +        val = readl(&platdata->regs->gpiodata + bank * 2);
>>> +    } else {
>>> +        debug("%s: Read saved output value\n", __func__);
>>> +        val = priv->output_val[bank];
>>> +    }
>> Why you don't always read the data register? This doesn't work for three
>> state outputs.
> In three state register every bit/pin is 0 - output, 1 input.
> It means else part is output and I read saved value in priv->output_val.
> If pin is setup as INPUT then I need read data reg to find out input value.
> Maybe you are commenting something else but please let me know if there
> is any other bug.

What happen if I have an open drain output. Even if the gpio output is 1 
the input could read a 0. You driver will always return the output value 
and not the real input value. According to the picture in documentation 
and my tests a data register write writes the output registers and a 
data register read reads the input registers.

Why should the driver return the desired state (output register) and not 
the real state (input register)?

Best regards
   Stefan