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

Michal Simek michal.simek at xilinx.com
Tue Jul 24 10:31:55 UTC 2018


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 <stefan at herbrechtsmeier.net>
>> Signed-off-by: Michal Simek <michal.simek at 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


More information about the U-Boot mailing list