* [PATCH] gpio: zynq: Read using DATA register when direction is output
@ 2025-04-10 8:25 Michal Simek
2025-04-10 9:26 ` Mike Looijmans
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Michal Simek @ 2025-04-10 8:25 UTC (permalink / raw)
To: u-boot, git
Cc: Venkatesh Yadav Abbarapu, Heiko Schocher, Nam Ian, Peter Robinson,
Tom Rini
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;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: zynq: Read using DATA register when direction is output
2025-04-10 8:25 [PATCH] gpio: zynq: Read using DATA register when direction is output Michal Simek
@ 2025-04-10 9:26 ` Mike Looijmans
2025-04-11 4:08 ` Heiko Schocher
2025-06-02 7:47 ` Michal Simek
2 siblings, 0 replies; 4+ messages in thread
From: Mike Looijmans @ 2025-04-10 9:26 UTC (permalink / raw)
To: u-boot
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;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: zynq: Read using DATA register when direction is output
2025-04-10 8:25 [PATCH] gpio: zynq: Read using DATA register when direction is output Michal Simek
2025-04-10 9:26 ` Mike Looijmans
@ 2025-04-11 4:08 ` Heiko Schocher
2025-06-02 7:47 ` Michal Simek
2 siblings, 0 replies; 4+ messages in thread
From: Heiko Schocher @ 2025-04-11 4:08 UTC (permalink / raw)
To: Michal Simek, u-boot, git
Cc: Venkatesh Yadav Abbarapu, Nam Ian, Peter Robinson, Tom Rini
Hello Michal,
On 10.04.25 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(-)
Reviewed-by: Heiko Schocher <hs@denx.de>
bye,
Heiko
--
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: zynq: Read using DATA register when direction is output
2025-04-10 8:25 [PATCH] gpio: zynq: Read using DATA register when direction is output Michal Simek
2025-04-10 9:26 ` Mike Looijmans
2025-04-11 4:08 ` Heiko Schocher
@ 2025-06-02 7:47 ` Michal Simek
2 siblings, 0 replies; 4+ messages in thread
From: Michal Simek @ 2025-06-02 7:47 UTC (permalink / raw)
To: u-boot, git, Mike Looijmans
Cc: Venkatesh Yadav Abbarapu, Heiko Schocher, Nam Ian, Peter Robinson,
Tom Rini
On 4/10/25 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;
> }
>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-02 7:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10 8:25 [PATCH] gpio: zynq: Read using DATA register when direction is output Michal Simek
2025-04-10 9:26 ` Mike Looijmans
2025-04-11 4:08 ` Heiko Schocher
2025-06-02 7:47 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox