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