[PATCH] mxc_gpio: on imx8m read the DR instead of the PSR
On imx8m it is more correct to read the data register than the pad status register.
In output mode the pad status register does not contain the value being output so we must read the DR for the value.
In input mode the DR will contain the same value as the PSR when the pad mode is GPIO.
If the pad mode is not GPIO and the pin is an input then 0 is always returned.
Signed-off-by: Angus Ainslie angus@akkea.ca --- drivers/gpio/mxc_gpio.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 03471db9e80..0d9f6f44042 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -211,6 +211,9 @@ 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) { +if (IS_ENABLED(CONFIG_IMX8M)) + return (readl(®s->gpio_dr) >> offset) & 0x01; +else return (readl(®s->gpio_psr) >> offset) & 0x01; }
Hi Angus,
On Sun, Nov 28, 2021 at 11:52 AM Angus Ainslie angus@akkea.ca wrote:
On imx8m it is more correct to read the data register than the pad status register.
In output mode the pad status register does not contain the value being output so we must read the DR for the value.
In input mode the DR will contain the same value as the PSR when the pad mode is GPIO.
If the pad mode is not GPIO and the pin is an input then 0 is always returned.
Have you tried setting the SION bit of the pad?
Hi Fabio,
On 2021-11-28 7:15 a.m., Fabio Estevam wrote:
Hi Angus,
On Sun, Nov 28, 2021 at 11:52 AM Angus Ainslie angus@akkea.ca wrote:
On imx8m it is more correct to read the data register than the pad status register.
In output mode the pad status register does not contain the value being output so we must read the DR for the value.
In input mode the DR will contain the same value as the PSR when the pad mode is GPIO.
If the pad mode is not GPIO and the pin is an input then 0 is always returned.
Have you tried setting the SION bit of the pad?
I have tried it with the SION bit set but I thought it would be better if the dts would match the linux dts and return the same result.
Angus
Hi Angus,
On Sun, Nov 28, 2021 at 12:18 PM Angus Ainslie angus.ainslie@puri.sm wrote:
I have tried it with the SION bit set but I thought it would be better if the dts would match the linux dts and return the same result.
Just to confirm: with the SION bit set, you read the correct values, right?
If so, the fix is just to set the SION bit in both Linux and U-Boot.
There were several attempts to change the mxc gpio driver like you proposed, and the response have been always to set the SION bit, which works well in Linux and U-Boot.
Hi Fabio,
On 2021-11-28 07:21, Fabio Estevam wrote:
Hi Angus,
On Sun, Nov 28, 2021 at 12:18 PM Angus Ainslie angus.ainslie@puri.sm wrote:
I have tried it with the SION bit set but I thought it would be better if the dts would match the linux dts and return the same result.
Just to confirm: with the SION bit set, you read the correct values, right?
I just tried it again and setting the SION does not read back the correct values. I believe the changes below [1] should be the correct ones for my dts.
u-boot=> gpio status Bank GPIO1_: GPIO1_0: output: 0 [x] csi_en.gpio-hog GPIO1_3: output: 1 [x] sdcard_pwr.gpio-hog GPIO1_4: output: 1 [x] audio_pwr.gpio-hog GPIO1_12: output: 0 [x] hub_nreset.gpio-hog GPIO1_14: output: 1 [x] hub_pwr.gpio-hog
Bank GPIO3_: GPIO3_17: output: 0 [x] chg_en.gpio-hog GPIO3_24: output: 0 [x] tps_reset.gpio-hog u-boot=> gpio set GPIO1_12 gpio: pin GPIO1_12 (gpio 12) value is 1 Warning: value of pin is still 0
The gpio-hog should also have set hub_nreset.gpio-hog to 1.
[1] diff --git a/arch/arm/dts/imx8mq-librem5.dts b/arch/arm/dts/imx8mq-librem5.dts index b28420654f..b60a8538af 100644 --- a/arch/arm/dts/imx8mq-librem5.dts +++ b/arch/arm/dts/imx8mq-librem5.dts @@ -56,29 +56,29 @@ &iomuxc { pinctrl_hog1: hoggrp1 { fsl,pins = < - MX8MQ_IOMUXC_GPIO1_IO00_GPIO1_IO0 0x83 /* CAMERA_PWR_EN_3V3 */ - MX8MQ_IOMUXC_GPIO1_IO03_GPIO1_IO3 0x83 /* TF_PWR_3V3_EN */ - MX8MQ_IOMUXC_GPIO1_IO04_GPIO1_IO4 0x83 /* AUDIO_POWER_EN_3V3 */ - MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5 0x83 /* DSI_EN_3V3 */ - MX8MQ_IOMUXC_GPIO1_IO11_GPIO1_IO11 0x3f - MX8MQ_IOMUXC_GPIO1_IO12_GPIO1_IO12 0x3f - MX8MQ_IOMUXC_GPIO1_IO14_GPIO1_IO14 0x83 /* HUB_PWR_3V3_EN */ - MX8MQ_IOMUXC_ENET_MDC_GPIO1_IO16 0xC0 - MX8MQ_IOMUXC_ENET_MDIO_GPIO1_IO17 0xC0 - MX8MQ_IOMUXC_ENET_TD1_GPIO1_IO20 0x83 /* DSI_BIAS_EN */ - MX8MQ_IOMUXC_ENET_TXC_GPIO1_IO23 0x83 /* FLASH_EN */ + MX8MQ_IOMUXC_GPIO1_IO00_GPIO1_IO0 0x40000083 /* CAMERA_PWR_EN_3V3 */ + MX8MQ_IOMUXC_GPIO1_IO03_GPIO1_IO3 0x40000083 /* TF_PWR_3V3_EN */ + MX8MQ_IOMUXC_GPIO1_IO04_GPIO1_IO4 0x40000083 /* AUDIO_POWER_EN_3V3 */ + MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5 0x40000083 /* DSI_EN_3V3 */ + MX8MQ_IOMUXC_GPIO1_IO11_GPIO1_IO11 0x4000003f + MX8MQ_IOMUXC_GPIO1_IO12_GPIO1_IO12 0x4000003f + MX8MQ_IOMUXC_GPIO1_IO14_GPIO1_IO14 0x40000083 /* HUB_PWR_3V3_EN */ + MX8MQ_IOMUXC_ENET_MDC_GPIO1_IO16 0x400000C0 + MX8MQ_IOMUXC_ENET_MDIO_GPIO1_IO17 0x400000C0 + MX8MQ_IOMUXC_ENET_TD1_GPIO1_IO20 0x40000083 /* DSI_BIAS_EN */ + MX8MQ_IOMUXC_ENET_TXC_GPIO1_IO23 0x40000083 /* FLASH_EN */ >; };
pinctrl_hog3: hoggrp3 { fsl,pins = < - MX8MQ_IOMUXC_NAND_CE1_B_GPIO3_IO2 0x3 /* CHG_EN */ - MX8MQ_IOMUXC_NAND_CE3_B_GPIO3_IO4 0x3 /* CHG_OTG_OUT_EN */ - MX8MQ_IOMUXC_NAND_DATA04_GPIO3_IO10 0x83 /* WIFI3V3_EN */ - MX8MQ_IOMUXC_NAND_DATA06_GPIO3_IO12 0x83 /* GPS3V3_EN */ - MX8MQ_IOMUXC_NAND_DQS_GPIO3_IO14 0x83 /* BACKLINGE_EN */ - MX8MQ_IOMUXC_NAND_WP_B_GPIO3_IO18 0x83 /* 4G_PWR_EN */ - MX8MQ_IOMUXC_SAI5_RXD3_GPIO3_IO24 0x83 /* TPS_RESET */ + MX8MQ_IOMUXC_NAND_CE1_B_GPIO3_IO2 0x40000003 /* CHG_EN */ + MX8MQ_IOMUXC_NAND_CE3_B_GPIO3_IO4 0x40000003 /* CHG_OTG_OUT_EN */ + MX8MQ_IOMUXC_NAND_DATA04_GPIO3_IO10 0x40000083 /* WIFI3V3_EN */ + MX8MQ_IOMUXC_NAND_DATA06_GPIO3_IO12 0x40000083 /* GPS3V3_EN */ + MX8MQ_IOMUXC_NAND_DQS_GPIO3_IO14 0x40000083 /* BACKLINGE_EN */ + MX8MQ_IOMUXC_NAND_WP_B_GPIO3_IO18 0x40000083 /* 4G_PWR_EN */
If so, the fix is just to set the SION bit in both Linux and U-Boot.
There were several attempts to change the mxc gpio driver like you proposed, and the response have been always to set the SION bit, which works well in Linux and U-Boot.
Hi Angus,
On Sun, Nov 28, 2021 at 12:42 PM Angus Ainslie angus@akkea.ca wrote:
[1] diff --git a/arch/arm/dts/imx8mq-librem5.dts b/arch/arm/dts/imx8mq-librem5.dts index b28420654f..b60a8538af 100644 --- a/arch/arm/dts/imx8mq-librem5.dts +++ b/arch/arm/dts/imx8mq-librem5.dts @@ -56,29 +56,29 @@ &iomuxc { pinctrl_hog1: hoggrp1 {
I suspect the hog groups are not getting initialized.
Please try:
&iomuxc { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_hog>;
pinctrl_hog: hoggrp { fsl,pins = < ...... >; };
and then group all the hog pins under the same group.
Then check IOMUXC_SW_MUX_CTL_PAD to make sure the SION bit is set.
Hi Fabio,
On 2022-01-18 08:59, Fabio Estevam wrote:
Hi Angus,
On Sun, Nov 28, 2021 at 12:42 PM Angus Ainslie angus@akkea.ca wrote:
[1] diff --git a/arch/arm/dts/imx8mq-librem5.dts b/arch/arm/dts/imx8mq-librem5.dts index b28420654f..b60a8538af 100644 --- a/arch/arm/dts/imx8mq-librem5.dts +++ b/arch/arm/dts/imx8mq-librem5.dts @@ -56,29 +56,29 @@ &iomuxc { pinctrl_hog1: hoggrp1 {
I suspect the hog groups are not getting initialized.
Please try:
&iomuxc { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_hog>;
pinctrl_hog: hoggrp { fsl,pins = < ...... >;};
and then group all the hog pins under the same group.
Then check IOMUXC_SW_MUX_CTL_PAD to make sure the SION bit is set.
I decided to bypass the devicetree to test it on the imx8mq
static const iomux_v3_cfg_t configure_pads[] = { IMX8MQ_PAD_GPIO1_IO03__GPIO1_IO3 | MUX_PAD_CTRL(PAD_CTL_DSE6) | MUX_MODE_SION, IMX8MQ_PAD_GPIO1_IO14__GPIO1_IO14 | MUX_PAD_CTRL(PAD_CTL_DSE6) | MUX_MODE_SION, IMX8MQ_PAD_ENET_MDC__GPIO1_IO16 | MUX_PAD_CTRL(PAD_CTL_PUE) | MUX_MODE_SION, IMX8MQ_PAD_ENET_MDIO__GPIO1_IO17 | MUX_PAD_CTRL(PAD_CTL_PUE) | MUX_MODE_SION, };
static inline void init_pinmux(void) { imx_iomux_v3_setup_multiple_pads(configure_pads, ARRAY_SIZE(configure_pads)); }
And this works so I need to figure out what is wrong with my devicetree configuration.
Thanks Angus
Hi Angus,
On Thu, Jan 20, 2022 at 10:56 AM Angus Ainslie angus@akkea.ca wrote:
I decided to bypass the devicetree to test it on the imx8mq
static const iomux_v3_cfg_t configure_pads[] = { IMX8MQ_PAD_GPIO1_IO03__GPIO1_IO3 | MUX_PAD_CTRL(PAD_CTL_DSE6) | MUX_MODE_SION, IMX8MQ_PAD_GPIO1_IO14__GPIO1_IO14 | MUX_PAD_CTRL(PAD_CTL_DSE6) | MUX_MODE_SION, IMX8MQ_PAD_ENET_MDC__GPIO1_IO16 | MUX_PAD_CTRL(PAD_CTL_PUE) | MUX_MODE_SION, IMX8MQ_PAD_ENET_MDIO__GPIO1_IO17 | MUX_PAD_CTRL(PAD_CTL_PUE) | MUX_MODE_SION, };
static inline void init_pinmux(void) { imx_iomux_v3_setup_multiple_pads(configure_pads, ARRAY_SIZE(configure_pads)); }
And this works so I need to figure out what is wrong with my devicetree configuration.
Ok, great :-)
The issue with your dts is that the hog pinctrl group is not referenced anywhere.
The example I sent in my last email should fix the problem:
pinctrl-names = "default"; pinctrl-0 = <&pinctrl_hog>;
Hi Fabio,
On 2021-11-28 07:15, Fabio Estevam wrote:
Hi Angus,
On Sun, Nov 28, 2021 at 11:52 AM Angus Ainslie angus@akkea.ca wrote:
On imx8m it is more correct to read the data register than the pad status register.
In output mode the pad status register does not contain the value being output so we must read the DR for the value.
In input mode the DR will contain the same value as the PSR when the pad mode is GPIO.
If the pad mode is not GPIO and the pin is an input then 0 is always returned.
Have you tried setting the SION bit of the pad?
I have tried it with the SION bit set but I thought it would be better if the dts would match the linux dts and return the same result.
Angus
participants (3)
-
Angus Ainslie -
Angus Ainslie -
Fabio Estevam