[U-Boot] [PATCH 4/4] gpio: xilinx: Simplify logic in xilinx_gpio_set_value

Michal Simek michal.simek at xilinx.com
Mon Jul 30 13:42:10 UTC 2018


On 30.7.2018 15:31, Stefan Herbrechtsmeier wrote:
> Hi Michal,
> 
> Am 30.07.2018 um 14:34 schrieb Michal Simek:
>> There is no reason to do read/write for if/else separately.
>>
>> Reported-by: Stefan Herbrechtsmeier <stefan at herbrechtsmeier.net>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>>   drivers/gpio/xilinx_gpio.c | 13 ++++++-------
>>   1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
>> index 1e5f3da8d7e8..cccfa7561739 100644
>> --- a/drivers/gpio/xilinx_gpio.c
>> +++ b/drivers/gpio/xilinx_gpio.c
>> @@ -61,18 +61,17 @@ static int xilinx_gpio_set_value(struct udevice
>> *dev, unsigned offset,
>>       if (ret)
>>           return ret;
>>   +    val = readl(&platdata->regs->gpiodata + bank * 2);
>> +
>>       debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x\n",
>>             __func__, (ulong)platdata->regs, value, offset, bank, pin);
>>   -    if (value) {
>> -        val = readl(&platdata->regs->gpiodata + bank * 2);
>> +    if (value)
>>           val = val | (1 << pin);
>> -        writel(val, &platdata->regs->gpiodata + bank * 2);
>> -    } else {
>> -        val = readl(&platdata->regs->gpiodata + bank * 2);
>> +    else
>>           val = val & ~(1 << pin);
>> -        writel(val, &platdata->regs->gpiodata + bank * 2);
>> -    }
>> +
>> +    writel(val, &platdata->regs->gpiodata + bank * 2);
>>         return val;
>>   };
> 
> This doesn't work because you read the general purpose input ports
> (register) and write to the general purpose output ports (register).
> This leads to problems if an output only pin inside an bidirectional
> port has no connection between the two registers or the input and output
> registers has different values.
> 
> The value of the output register must be saved inside the driver.

This is only simplifying logic not doing changes which we are talking
about in separate thread.
Your comment is how to get that initial value.

Thanks,
Michal



More information about the U-Boot mailing list