diff mbox series

[v4] gpio: mxc_gpio: fix reading state of GPIO pins in output mode

Message ID py.Zbyp.4iSBCzZ4lgW.1cpo6S@seznam.cz
State Accepted
Delegated to: Fabio Estevam
Headers show
Series [v4] gpio: mxc_gpio: fix reading state of GPIO pins in output mode | expand

Commit Message

Tomas Paukrt Aug. 28, 2024, 1:09 p.m. UTC
The PSR register works correctly for GPIO pins in input mode,
but always returns 0 for GPIO pins in output mode unless the SION
bit is set.

The DR register should be used for GPIO pins in output mode
to allow correct getting of previously set output value.

Please note that the Linux gpio-mxc driver and the NXP U-Boot mxc_gpio
driver already use the DR register for all GPIO pins in output mode:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=442b2494b17d1a4f0a14721580271eb23ebffd42
https://github.com/nxp-imx/uboot-imx/commit/4afc3f90943c6b117f79b66d2cd04e64f437b0c2

Signed-off-by: Tomas Paukrt <tomaspaukrt@email.cz>
---
 drivers/gpio/mxc_gpio.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Marek Vasut Aug. 28, 2024, 1:24 p.m. UTC | #1
On 8/28/24 3:09 PM, Tomas Paukrt wrote:
> The PSR register works correctly for GPIO pins in input mode,
> but always returns 0 for GPIO pins in output mode unless the SION
> bit is set.
> 
> The DR register should be used for GPIO pins in output mode
> to allow correct getting of previously set output value.
> 
> Please note that the Linux gpio-mxc driver and the NXP U-Boot mxc_gpio
> driver already use the DR register for all GPIO pins in output mode:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=442b2494b17d1a4f0a14721580271eb23ebffd42
> https://github.com/nxp-imx/uboot-imx/commit/4afc3f90943c6b117f79b66d2cd04e64f437b0c2
> 
> Signed-off-by: Tomas Paukrt <tomaspaukrt@email.cz>

Changelog is missing, what changed since V3 of this patch ?
Tomas Paukrt Aug. 28, 2024, 2:03 p.m. UTC | #2
> Changelog is missing, what changed since V3 of this patch ?

There was a typo in V3 that caused compilation failure without CONFIG_DM_GPIO.
Marek Vasut Aug. 28, 2024, 7:14 p.m. UTC | #3
On 8/28/24 3:09 PM, Tomas Paukrt wrote:
> The PSR register works correctly for GPIO pins in input mode,
> but always returns 0 for GPIO pins in output mode unless the SION
> bit is set.
> 
> The DR register should be used for GPIO pins in output mode
> to allow correct getting of previously set output value.
> 
> Please note that the Linux gpio-mxc driver and the NXP U-Boot mxc_gpio
> driver already use the DR register for all GPIO pins in output mode:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=442b2494b17d1a4f0a14721580271eb23ebffd42
> https://github.com/nxp-imx/uboot-imx/commit/4afc3f90943c6b117f79b66d2cd04e64f437b0c2
> 
> Signed-off-by: Tomas Paukrt <tomaspaukrt@email.cz>

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks
Fabio Estevam Aug. 28, 2024, 7:39 p.m. UTC | #4
Hi Tomas,

On Wed, Aug 28, 2024 at 10:09 AM Tomas Paukrt <tomaspaukrt@email.cz> wrote:
>
> The PSR register works correctly for GPIO pins in input mode,
> but always returns 0 for GPIO pins in output mode unless the SION
> bit is set.
>
> The DR register should be used for GPIO pins in output mode
> to allow correct getting of previously set output value.
>
> Please note that the Linux gpio-mxc driver and the NXP U-Boot mxc_gpio
> driver already use the DR register for all GPIO pins in output mode:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=442b2494b17d1a4f0a14721580271eb23ebffd42

Yes, it makes sense to align the i.MX GPIO behavior with the Linux
driver, thanks:

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Tested-by: Fabio Estevam <festevam@gmail.com>
Fabio Estevam Sept. 2, 2024, 5:46 p.m. UTC | #5
On Wed, Aug 28, 2024 at 10:09 AM Tomas Paukrt <tomaspaukrt@email.cz> wrote:
>
> The PSR register works correctly for GPIO pins in input mode,
> but always returns 0 for GPIO pins in output mode unless the SION
> bit is set.
>
> The DR register should be used for GPIO pins in output mode
> to allow correct getting of previously set output value.
>
> Please note that the Linux gpio-mxc driver and the NXP U-Boot mxc_gpio
> driver already use the DR register for all GPIO pins in output mode:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=442b2494b17d1a4f0a14721580271eb23ebffd42
> https://github.com/nxp-imx/uboot-imx/commit/4afc3f90943c6b117f79b66d2cd04e64f437b0c2
>
> Signed-off-by: Tomas Paukrt <tomaspaukrt@email.cz>

Applied, thanks.
diff mbox series

Patch

diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
index cac6b32..28176e1 100644
--- a/drivers/gpio/mxc_gpio.c
+++ b/drivers/gpio/mxc_gpio.c
@@ -133,7 +133,10 @@  int gpio_get_value(unsigned gpio)
 
 	regs = (struct gpio_regs *)gpio_ports[port];
 
-	val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
+	if ((readl(&regs->gpio_dir) >> gpio) & 0x01)
+		val = (readl(&regs->gpio_dr) >> gpio) & 0x01;
+	else
+		val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
 
 	return val;
 }
@@ -210,7 +213,10 @@  static void mxc_gpio_bank_set_value(struct gpio_regs *regs, int offset,
 
 static int mxc_gpio_bank_get_value(struct gpio_regs *regs, int offset)
 {
-	return (readl(&regs->gpio_psr) >> offset) & 0x01;
+	if ((readl(&regs->gpio_dir) >> offset) & 0x01)
+		return (readl(&regs->gpio_dr) >> offset) & 0x01;
+	else
+		return (readl(&regs->gpio_psr) >> offset) & 0x01;
 }
 
 /* set GPIO pin 'gpio' as an input */