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

[Stefan Herbrechtsmeier]

Am 30.07.2018 um 15:42 schrieb Michal Simek:
> 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.

Okay. In this case:

Reviewed-by: Stefan Herbrechtsmeier <stefan at herbrechtsmeier.net>