Re: [PATCH] gpio: zynq: Read using DATA register when direction is output

[Michal Simek]

On 4/10/25 10:25, Michal Simek wrote:
From: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>

Gpio status command reads the DATA_RO register rather than
DATA registers even when the direction is "output", fix this
by reading from DATA register when direction is "output".

Signed-off-by: Nam Ian <ian....@amd.com>
Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>
Signed-off-by: Michal Simek <michal.si...@amd.com>
---

  drivers/gpio/zynq_gpio.c | 15 +++++++++++----
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/zynq_gpio.c b/drivers/gpio/zynq_gpio.c
index 7db58c70663e..4fdce39d91b6 100644
--- a/drivers/gpio/zynq_gpio.c
+++ b/drivers/gpio/zynq_gpio.c
@@ -64,6 +64,7 @@
  /* MSW Mask & Data -WO */
  #define ZYNQ_GPIO_DATA_MSW_OFFSET(BANK)       (0x004 + (8 * BANK))
  /* Data Register-RW */
+#define ZYNQ_GPIO_DATA_OFFSET(BANK)    (0x040 + (4 * BANK))
  #define ZYNQ_GPIO_DATA_RO_OFFSET(BANK)        (0x060 + (4 * BANK))
  /* Direction mode reg-RW */
  #define ZYNQ_GPIO_DIRM_OFFSET(BANK)   (0x204 + (0x40 * BANK))
@@ -230,7 +231,7 @@ static int check_gpio(unsigned gpio, struct udevice *dev)
  
  static int zynq_gpio_get_value(struct udevice *dev, unsigned gpio)
  {
-       u32 data;
+       u32 data, reg;
        unsigned int bank_num, bank_pin_num;
        struct zynq_gpio_plat *plat = dev_get_plat(dev);
  
@@ -239,9 +240,15 @@ static int zynq_gpio_get_value(struct udevice *dev, unsigned gpio)
  
  	zynq_gpio_get_bank_pin(gpio, &bank_num, &bank_pin_num, dev);
  
-	data = readl(plat->base +
-                            ZYNQ_GPIO_DATA_RO_OFFSET(bank_num));
-
+       reg = readl(plat->base + ZYNQ_GPIO_DIRM_OFFSET(bank_num));
+       reg &= BIT(bank_pin_num);
+       if (reg != GPIOF_INPUT) {
+               data = readl(plat->base +
+                                       ZYNQ_GPIO_DATA_OFFSET(bank_num));
+       } else {
+               data = readl(plat->base +
+                                       ZYNQ_GPIO_DATA_RO_OFFSET(bank_num));
+       }
        return (data >> bank_pin_num) & 1;
  }
  

I have looked at Linux kernel and there is different logic. It should be syncup.
Second. I agree with Mike that what we need is to read actual value not really 
value which has been written.

Thanks,
Michal