On 23.7.2018 20:42, Stefan Herbrechtsmeier wrote:
> Hi Michal,
>
> 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 <ste...@herbrechtsmeier.net>
>> Signed-off-by: Michal Simek <michal.si...@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
>> @@ -358,6 +358,11 @@ struct xilinx_gpio_platdata {
>> int bank_max[XILINX_GPIO_MAX_BANK];
>> int bank_input[XILINX_GPIO_MAX_BANK];
>> int bank_output[XILINX_GPIO_MAX_BANK];
>> + u32 dout_default[XILINX_GPIO_MAX_BANK];
>> +};
>> +
>> +struct xilinx_gpio_privdata {
>> + u32 output_val[XILINX_GPIO_MAX_BANK];
>> };
>> static int xilinx_gpio_get_bank_pin(unsigned offset, u32 *bank_num,
>> @@ -387,6 +392,7 @@ static int xilinx_gpio_set_value(struct udevice
>> *dev, unsigned offset,
>> int value)
>> {
>> struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>> + struct xilinx_gpio_privdata *priv = dev_get_priv(dev);
>> int val, ret;
>> u32 bank, pin;
>> @@ -394,19 +400,21 @@ static int xilinx_gpio_set_value(struct
>> udevice *dev, unsigned offset,
>> if (ret)
>> return ret;
>> - debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x\n",
>> - __func__, (ulong)platdata->regs, value, offset, bank, pin);
>> + val = priv->output_val[bank];
>> +
>> + debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x, out
>> %x\n",
>> + __func__, (ulong)platdata->regs, value, offset, bank, pin,
>> val);
>> if (value) {
>> - val = readl(&platdata->regs->gpiodata + bank * 2);
>> val = val | (1 << pin);
>> writel(val, &platdata->regs->gpiodata + bank * 2);
>> } else {
>> - val = readl(&platdata->regs->gpiodata + bank * 2);
>> val = val & ~(1 << pin);
>> writel(val, &platdata->regs->gpiodata + bank * 2);
>> }
>
> You could replace the two writel function calls by one.
>
Will update this in v2.
>> + 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.
Thanks,
Michal
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
