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

[Mike Looijmans <mike.looijmans@topic.nl>]

That looks broken to me.

When reading a GPIO pin, you want to get the actual state of the pin. Even 
though the output state may be driving it high, the actual pin may still be 
low because it's shorted to ground.

M.

On 10-04-2025 10:25, Michal Simek wrote:
> From: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@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.nam@amd.com>
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> Signed-off-by: Michal Simek <michal.simek@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;
>   }
>