[PATCH] gpio: mxc_gpio: Fix i.MX8M GPIO output status read
Currently the driver gets value from PSR register, but this register is only for input mode. For output mode, it always returns 0, not the value we set for output.
This patch changes to use DR register, which returns the DR value for output mode, and PSR value for input mode.
This patch is based on code from Ye Li ye.li@nxp.com
Signed-off-by: Robert Krikke robert.krikke@nedap.com Signed-off-by: Harm Berntsen harm.berntsen@nedap.com CC: Ye Li ye.li@nxp.com CC: Stefano Babic sbabic@denx.de ---
drivers/gpio/mxc_gpio.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 06e6b2279f..c30246399c 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -36,6 +36,17 @@ struct mxc_bank_info { struct gpio_regs *regs; };
+static int gpio_bank_get_value(struct gpio_regs *regs, int offset) +{ + u32 gpio_val; + + if (IS_ENABLED(CONFIG_ARCH_IMX8M)) + gpio_val = readl(®s->gpio_dr); + else + gpio_val = readl(®s->gpio_psr); + return (gpio_val >> offset) & 0x01; +} + #if !CONFIG_IS_ENABLED(DM_GPIO) #define GPIO_TO_PORT(n) ((n) / 32)
@@ -125,18 +136,13 @@ int gpio_get_value(unsigned gpio) { unsigned int port = GPIO_TO_PORT(gpio); struct gpio_regs *regs; - u32 val; - if (port >= ARRAY_SIZE(gpio_ports)) return -1;
gpio &= 0x1f;
regs = (struct gpio_regs *)gpio_ports[port]; - - val = (readl(®s->gpio_psr) >> gpio) & 0x01; - - return val; + return gpio_bank_get_value(regs, gpio); }
int gpio_request(unsigned gpio, const char *label) @@ -211,7 +217,7 @@ 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(®s->gpio_psr) >> offset) & 0x01; + return gpio_bank_get_value(regs, offset); }
/* set GPIO pin 'gpio' as an input */
Hi Harm and Ye Li,
On Fri, Aug 13, 2021 at 11:35 AM Harm Berntsen harm.berntsen@nedap.com wrote:
Currently the driver gets value from PSR register, but this register is only for input mode. For output mode, it always returns 0, not the value we set for output.
This patch changes to use DR register, which returns the DR value for output mode, and PSR value for input mode.
This patch is based on code from Ye Li ye.li@nxp.com
Could this issue be fixed by setting the SION bit mode?
Hi Fabio Estevam and Ye Li,
On Sat, 2022-01-15 at 10:43 -0300, Fabio Estevam wrote:
Hi Harm and Ye Li,
On Fri, Aug 13, 2021 at 11:35 AM Harm Berntsen harm.berntsen@nedap.com wrote:
Currently the driver gets value from PSR register, but this register is only for input mode. For output mode, it always returns 0, not the value we set for output.
This patch changes to use DR register, which returns the DR value for output mode, and PSR value for input mode.
This patch is based on code from Ye Li ye.li@nxp.com
Could this issue be fixed by setting the SION bit mode?
SION works! I did not know about it :). I see the imxrt1020-evk, imxrt1050-evk and imx53-kp use it in their device tree. To make this feature available, I had to ensure the IMX_PAD_SION was available in my dts by adding a define:
#define IMX_PAD_SION 0x40000000
I could then configure the pinmux in my dts file. It would be nice to make this define available by default.
To conclude, I would now need to enable SION in order to remove the 'Warning: value of pin is still 0' text in my serial output. I'm fine with that.
Hi Harm,
Adding Angus, who submitted a similar patch: https://patchwork.ozlabs.org/project/uboot/patch/20211128145143.1433262-1-an...
On Mon, Jan 17, 2022 at 12:55 PM Harm Berntsen harm.berntsen@nedap.com wrote:
Could this issue be fixed by setting the SION bit mode?
SION works! I did not know about it :). I see the imxrt1020-evk,
Ok, great!
imxrt1050-evk and imx53-kp use it in their device tree. To make this feature available, I had to ensure the IMX_PAD_SION was available in my dts by adding a define:
#define IMX_PAD_SION 0x40000000
I could then configure the pinmux in my dts file. It would be nice to make this define available by default.
To conclude, I would now need to enable SION in order to remove the 'Warning: value of pin is still 0' text in my serial output. I'm fine with that.
Actually, there are several i.MX dts files that set bit 30 (SION bit):
For example, the I2C pins in imx8mq-evk.dts look like this:
MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL 0x4000007f
Does it work if you just set bit 30 in your device tree? Which SoC are you using?
Hi,
On Mon, 2022-01-17 at 13:57 -0300, Fabio Estevam wrote:
Hi Harm,
Adding Angus, who submitted a similar patch: https://patchwork.ozlabs.org/project/uboot/patch/20211128145143.1433262-1-an...
On Mon, Jan 17, 2022 at 12:55 PM Harm Berntsen harm.berntsen@nedap.com wrote:
Could this issue be fixed by setting the SION bit mode?
SION works! I did not know about it :). I see the imxrt1020-evk,
Ok, great!
imxrt1050-evk and imx53-kp use it in their device tree. To make this feature available, I had to ensure the IMX_PAD_SION was available in my dts by adding a define:
#define IMX_PAD_SION 0x40000000
I could then configure the pinmux in my dts file. It would be nice to make this define available by default.
To conclude, I would now need to enable SION in order to remove the 'Warning: value of pin is still 0' text in my serial output. I'm fine with that.
Actually, there are several i.MX dts files that set bit 30 (SION bit):
For example, the I2C pins in imx8mq-evk.dts look like this:
MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL 0x4000007f
Does it work if you just set bit 30 in your device tree? Which SoC are you using?
I did not check the manual assignment of bit 30 in the dts, I now see that it is used more often than I thought.
I'm using an imx8mn on U-Boot 2021.10. It was as simple as setting the pinctrl as you described and in the same way as Angus has done. I don't see why it does not work for him. He has an imx8mq so maybe that chip behaves differently?
Just for completeness, some snippets from my device tree:
#define IMX_PAD_SION 0x40000000
&gpio3 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_misc_gpio3>; };
pinctrl_misc_gpio3: misc-gpio3 { fsl,pins = < MX8MN_IOMUXC_SAI5_RXD2_GPIO3_IO23 IMX_PAD_SION MX8MN_IOMUXC_SAI5_RXD3_GPIO3_IO24 IMX_PAD_SION >; };
On Tue, Jan 18, 2022 at 1:13 PM Harm Berntsen harm.berntsen@nedap.com wrote:
I did not check the manual assignment of bit 30 in the dts, I now see that it is used more often than I thought.
I'm using an imx8mn on U-Boot 2021.10. It was as simple as setting the pinctrl as you described and in the same way as Angus has done. I don't see why it does not work for him. He has an imx8mq so maybe that chip behaves differently?
After reading Angus' thread again, maybe there is an issue with his dts, where he uses pinctrl_hog1 and pinctrl_hog3.
I suspect that these pinctrl groups are not getting initialized at all.
To confirm that I would suggest Angus look at the IOMUXC_SW_MUX_CTL_PAD registers contents and verify whether the SION bit is set or not.
Another suggestion is to group all hog pins into pinctrl_hog instead.
Just for completeness, some snippets from my device tree:
#define IMX_PAD_SION 0x40000000
&gpio3 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_misc_gpio3>; };
pinctrl_misc_gpio3: misc-gpio3 { fsl,pins = < MX8MN_IOMUXC_SAI5_RXD2_GPIO3_IO23 IMX_PAD_SION MX8MN_IOMUXC_SAI5_RXD3_GPIO3_IO24 IMX_PAD_SION >; };
participants (2)
-
Fabio Estevam -
Harm Berntsen