From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
Date: Tue, 24 Jul 2018 21:56:23 +0200
Subject: [U-Boot] [PATCH 3/4] gpio: xilinx: Not read output values via
	regs
In-Reply-To: <36e1b314-74a5-04bc-8563-629c5ebeda22@xilinx.com>
References: <61f5486be81d3a2e9170aa7c383de68b78b944cb.1532346215.git.michal.simek@xilinx.com>
 <c0bbc06f6f2033844a73a78bc994c14fa965048c.1532346215.git.michal.simek@xilinx.com>
 <f02731e4-7e94-09fa-ef55-1779446930d2@herbrechtsmeier.net>
 <36e1b314-74a5-04bc-8563-629c5ebeda22@xilinx.com>
Message-ID: <76009864-c697-e216-a7ad-2ad8f728c0c1@herbrechtsmeier.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de

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