gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

PATCH New Vladimir Zapolskiy 2 participants 9 messages Aug 19, 2016 – Sep 8, 2016

Subsystems (4)

Arm i.MX and Layerscape GPIO Pin control Pin controller - Freescale

lore

Share & tools

lore

Vladimir Zapolskiy

author

2016-08-19 22:10 UTC #0 lore raw

cover gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

The change establishes a connection between on-SoC IOMUX controller(s) and GPIO controllers found on some SoC from Freescale/NXP iMX series, if a GPIO controller device node contains common gpio-ranges information.

The change is backward compatible with respect to potentially not updated outdated DTB data without gpio-ranges propery, for such boards the only functional change is lowered initcall priority of GPIO controller driver, which in general anyway is exected to be used only after pinctrl/pinmux controller.

If this change is applied the next interesting applications may be done as a follow-up work, for example switching pad function to GPIO on gpiod request, converting iomux controller driver to strict type and so on.

For actual values of gpio-ranges properties please reference series "ARM: dts: imx: add gpio-ranges properties to some iMX GPIO controllers"
http://archive.arm.linux.org.uk/lurker/message/20160819.220621.86d845d1.en.html

Deepak Das (1):
gpio: mxc: lower level of gpio_mxc_init() initcall

Vladimir Zapolskiy (2):
pinctrl: imx: accept gpio request/free from pinctrl
gpio: mxc: add generic gpio request/free callbacks to pinctrl

 drivers/gpio/gpio-mxc.c                 | 7 ++++++-
 drivers/pinctrl/freescale/pinctrl-imx.c | 4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

Signature · 1 line

2.8.1

Vladimir Zapolskiy

author

2016-08-19 22:10 UTC #1 lore raw

PATCH 1/3 pinctrl: imx: accept gpio request/free from pinctrl

While only Freescale Vybrid SoC has settings of GPIO capabilities done by iomux controller, it is only a matter of GPIO controller driver implementation for the rest of Freescale/NXP SoCs from iMX series.

As a practical example on GPIO request a pad function should be switched to GPIO, but because this requires updates to all particular iMX pinctrl drivers, for simplicity at the moment add only a proper connection between shared pinctrl-imx and pinctrl/pinmux core, namely .gpio_request_enable/.gpio_disable_free/.gpio_set_direction callbacks should return success to a caller. This change allows to progress by adding request/free callbacks into gpio-mxc.c driver.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/pinctrl/freescale/pinctrl-imx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/pinctrl/freescale/pinctrl-imx.c +2 -2

index 56fd88013850..47613201269a 100644
@@ -315,7 +315,7 @@ static int imx_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	/* Currently implementation only for shared mux/conf register */
 	if (!(info->flags & SHARE_MUX_CONF_REG))
-		return -EINVAL;
+		return 0;
 
 	pin_reg = &info->pin_regs[offset];
 	if (pin_reg->mux_reg == -1)

@@ -380,7 +380,7 @@ static int imx_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
 	 * They are part of the shared mux/conf register.
 	 */
 	if (!(info->flags & SHARE_MUX_CONF_REG))
-		return -EINVAL;
+		return 0;
 
 	pin_reg = &info->pin_regs[offset];
 	if (pin_reg->mux_reg == -1)

Signature · 1 line

2.8.1

Vladimir Zapolskiy

author

2016-08-19 22:10 UTC #2 lore raw

PATCH 2/3 gpio: mxc: lower level of gpio_mxc_init() initcall

In reply to Vladimir Zapolskiy

From: Deepak Das <[email protected]>

gpio-mxc probe should be called after imx pinctrl driver to make sure that gpio-ranges are mapped to the corresponding pins of pin controller.

This commit changes the initcall level of gpio-mxc driver from postcore to device level.

Signed-off-by Deepak Das <[email protected]>

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/gpio/gpio-mxc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpio-mxc.c +1 -1

index 1b342a3842c8..1f0deb7d3380 100644
@@ -510,7 +510,7 @@ static int __init gpio_mxc_init(void)
 {
 	return platform_driver_register(&mxc_gpio_driver);
 }
-postcore_initcall(gpio_mxc_init);
+device_initcall(gpio_mxc_init);
 
 MODULE_AUTHOR("Freescale Semiconductor, "
 	      "Daniel Mack <danielncaiaq.de>, "

Signature · 1 line

2.8.1

Vladimir Zapolskiy

author

2016-08-19 22:10 UTC #3 lore raw

PATCH 3/3 gpio: mxc: add generic gpio request/free callbacks to pinctrl

In reply to Vladimir Zapolskiy

If a GPIO controller description in board DTB contains information about mappings between GPIOs and pads under IOMUX control use it to request and free GPIOs with respect to pinctrl/pinmux subsystems.

One of immediate positive functional changes is inability to request non-existing GPIOs, i.e. if there is no pad such. Also pinctrl/pinmux may now properly account pads occupied by requested GPIOs.

The change has no effect, if "gpio-ranges" property is not found including the case if a board has no DTB firmware.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/gpio/gpio-mxc.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/gpio/gpio-mxc.c +5 -0

index 1f0deb7d3380..342f34f90bbd 100644
@@ -458,6 +458,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	if (err)
 		goto out_bgio;
 
+	if (of_property_read_bool(np, "gpio-ranges")) {
+		port->gc.request = gpiochip_generic_request;
+		port->gc.free = gpiochip_generic_free;
+	}
+
 	port->gc.to_irq = mxc_gpio_to_irq;
 	port->gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 :
 					     pdev->id * 32;

Signature · 1 line

2.8.1

Vladimir Zapolskiy

author

2016-09-02 13:11 UTC #4 lore raw

cover gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

In reply to Vladimir Zapolskiy

Hi Shawn, Fabio,

Quoted 21 lines · Vladimir Zapolskiy

The change establishes a connection between on-SoC IOMUX controller(s) and GPIO controllers found on some SoC from Freescale/NXP iMX series, if a GPIO controller device node contains common gpio-ranges information.
The change is backward compatible with respect to potentially not updated outdated DTB data without gpio-ranges propery, for such boards the only functional change is lowered initcall priority of GPIO controller driver, which in general anyway is exected to be used only after pinctrl/pinmux controller.
If this change is applied the next interesting applications may be done as a follow-up work, for example switching pad function to GPIO on gpiod request, converting iomux controller driver to strict type and so on.
For actual values of gpio-ranges properties please reference series "ARM: dts: imx: add gpio-ranges properties to some iMX GPIO controllers"
http://archive.arm.linux.org.uk/lurker/message/20160819.220621.86d845d1.en.html
Deepak Das (1):
gpio: mxc: lower level of gpio_mxc_init() initcall

Quoted 8 lines

Vladimir Zapolskiy (2):
pinctrl: imx: accept gpio request/free from pinctrl
gpio: mxc: add generic gpio request/free callbacks to pinctrl
 drivers/gpio/gpio-mxc.c                 | 7 ++++++-
 drivers/pinctrl/freescale/pinctrl-imx.c | 4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

no comments so far, please could you express your concerns about this change? IMHO it would be nice to have this feature enabled in v4.9.

I assume that the most worrisome commit is the change of GPIO controller driver init level, but I belive it is safe enough, no?

Thank you.

--
With best wishes,
Vladimir

Shawn Guo

2016-09-05 02:12 UTC #5 lore raw

cover gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

Quoted 33 lines · Vladimir Zapolskiy

Hi Shawn, Fabio,
Quoted 21 lines · Vladimir Zapolskiy
The change establishes a connection between on-SoC IOMUX controller(s) and GPIO controllers found on some SoC from Freescale/NXP iMX series, if a GPIO controller device node contains common gpio-ranges information.
The change is backward compatible with respect to potentially not updated outdated DTB data without gpio-ranges propery, for such boards the only functional change is lowered initcall priority of GPIO controller driver, which in general anyway is exected to be used only after pinctrl/pinmux controller.
If this change is applied the next interesting applications may be done as a follow-up work, for example switching pad function to GPIO on gpiod request, converting iomux controller driver to strict type and so on.
For actual values of gpio-ranges properties please reference series "ARM: dts: imx: add gpio-ranges properties to some iMX GPIO controllers"
http://archive.arm.linux.org.uk/lurker/message/20160819.220621.86d845d1.en.html
Deepak Das (1):
gpio: mxc: lower level of gpio_mxc_init() initcall
Quoted 8 lines
Vladimir Zapolskiy (2):
pinctrl: imx: accept gpio request/free from pinctrl
gpio: mxc: add generic gpio request/free callbacks to pinctrl
drivers/gpio/gpio-mxc.c | 7 ++++++-
drivers/pinctrl/freescale/pinctrl-imx.c | 4 ++--
2 files changed, 8 insertions(+), 3 deletions(-)

Quoted 6 lines

no comments so far, please could you express your concerns about this change? IMHO it would be nice to have this feature enabled in v4.9.
I assume that the most worrisome commit is the change of GPIO controller driver init level, but I belive it is safe enough, no?

If I understand it correctly, with the change, most of GPIO client device drivers use the same init level as GPIO driver. So we are not sure if GPIO driver is ready when client drivers try to request GPIO. It shouldn't be a problem if every client driver handle the failure with deferred probing, but I'm not sure that's the case now.

Shawn

Vladimir Zapolskiy

author

2016-09-05 11:49 UTC #6 lore raw

cover gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

Hi Shawn,

Quoted 38 lines · Shawn Guo

Quoted 37 lines · Vladimir Zapolskiy
Hi Shawn, Fabio,
Quoted 21 lines · Vladimir Zapolskiy
The change establishes a connection between on-SoC IOMUX controller(s) and GPIO controllers found on some SoC from Freescale/NXP iMX series, if a GPIO controller device node contains common gpio-ranges information.
The change is backward compatible with respect to potentially not updated outdated DTB data without gpio-ranges propery, for such boards the only functional change is lowered initcall priority of GPIO controller driver, which in general anyway is exected to be used only after pinctrl/pinmux controller.
If this change is applied the next interesting applications may be done as a follow-up work, for example switching pad function to GPIO on gpiod request, converting iomux controller driver to strict type and so on.
For actual values of gpio-ranges properties please reference series "ARM: dts: imx: add gpio-ranges properties to some iMX GPIO controllers"
http://archive.arm.linux.org.uk/lurker/message/20160819.220621.86d845d1.en.html
Deepak Das (1):
gpio: mxc: lower level of gpio_mxc_init() initcall
Quoted 8 lines
Vladimir Zapolskiy (2):
pinctrl: imx: accept gpio request/free from pinctrl
gpio: mxc: add generic gpio request/free callbacks to pinctrl
drivers/gpio/gpio-mxc.c | 7 ++++++-
drivers/pinctrl/freescale/pinctrl-imx.c | 4 ++--
2 files changed, 8 insertions(+), 3 deletions(-)
no comments so far, please could you express your concerns about this change? IMHO it would be nice to have this feature enabled in v4.9.

Quoted 6 lines

Quoted 2 lines
I assume that the most worrisome commit is the change of GPIO controller driver init level, but I belive it is safe enough, no?
If I understand it correctly, with the change, most of GPIO client device drivers use the same init level as GPIO driver. So we are not sure if GPIO driver is ready when client drivers try to request GPIO.

please give it a test run (with and without DTS gpio-ranges changes, or just 2/3 from the series), I've tested on iMX6Q SabreLite, SabreAuto, SabreSD and I don't see any regressions or noticeable increase of -EPROBE_DEFER hits due to not ready GPIOs.

Quoted 1 line

most of GPIO client device drivers use the same init level as GPIO driver.

IMHO that will be the case, if the change 2/3 from the series is applied.

Statisticaly most of GPIO consumers should settle on device_initcall or module_init level, the latter one is translated to device_initcall if a driver is built-in, so the proposed downgrading of GPIO controller driver init call level looks to be appropriate.

However pinctrl/pinmux driver should definitely have higher init level priority in comparison to GPIO controller driver, every GPIO consumer is a pinctrl/pinmux consumer as well, most of iMX pinctrl/pinmux drivers are initialized at arch_initcall level, so there is a plenty of options to satisfy (GPIO driver initcall) < (pinmux/pinctrl driver initcall) equation:
* change pinmux/pinctrl driver initcall level to postcore_initcall and GPIO driver initcall level to arch_initcall or lower one,
* keep pinmux/pinctrl driver@arch_initcall level, set GPIO driver initcall level to any lower than arch_initcall (e.g. device_initcall)
* keep everything as is, drop 2/3 and rely on -EPROBE_DEFER from GPIO driver probe due to not yet initialized pinctrl driver.

Quoted 3 lines

It shouldn't be a problem if every client driver handle the failure with deferred probing, but I'm not sure that's the case now.

From what I see it is the case for the most critical drivers, for the
rest I believe it is a bug, which can be revealed by shifting GPIO driver initcall level and fixed, see also

https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2016-August/003419.html

--
With best wishes,
Vladimir

Shawn Guo

2016-09-08 01:41 UTC #7 lore raw

cover gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

Quoted 35 lines · Vladimir Zapolskiy

Quoted 3 lines
If I understand it correctly, with the change, most of GPIO client device drivers use the same init level as GPIO driver. So we are not sure if GPIO driver is ready when client drivers try to request GPIO.
please give it a test run (with and without DTS gpio-ranges changes, or just 2/3 from the series), I've tested on iMX6Q SabreLite, SabreAuto, SabreSD and I don't see any regressions or noticeable increase of -EPROBE_DEFER hits due to not ready GPIOs.
Quoted 1 line
most of GPIO client device drivers use the same init level as GPIO driver.
IMHO that will be the case, if the change 2/3 from the series is applied.
Statisticaly most of GPIO consumers should settle on device_initcall or module_init level, the latter one is translated to device_initcall if a driver is built-in, so the proposed downgrading of GPIO controller driver init call level looks to be appropriate.
However pinctrl/pinmux driver should definitely have higher init level priority in comparison to GPIO controller driver, every GPIO consumer is a pinctrl/pinmux consumer as well, most of iMX pinctrl/pinmux drivers are initialized at arch_initcall level, so there is a plenty of options to satisfy (GPIO driver initcall) < (pinmux/pinctrl driver initcall) equation:
* change pinmux/pinctrl driver initcall level to postcore_initcall and GPIO driver initcall level to arch_initcall or lower one,
* keep pinmux/pinctrl driver at arch_initcall level, set GPIO driver initcall level to any lower than arch_initcall (e.g. device_initcall)
* keep everything as is, drop 2/3 and rely on -EPROBE_DEFER from GPIO driver probe due to not yet initialized pinctrl driver.
Quoted 3 lines
It shouldn't be a problem if every client driver handle the failure with deferred probing, but I'm not sure that's the case now.

Quoted 5 lines

From what I see it is the case for the most critical drivers, for the rest I believe it is a bug, which can be revealed by shifting GPIO driver initcall level and fixed, see also
https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2016-August/003419.html

Okay, fair enough.

For the series,

Acked-by Shawn Guo <[email protected]>

Vladimir Zapolskiy

author

2016-09-08 01:52 UTC #8 lore raw

cover gpio/pinctrl: imx: let IOMUX controller know about on-SoC GPIOs

Hi Shawn,

Quoted 40 lines · Shawn Guo

Quoted 39 lines · Vladimir Zapolskiy
Quoted 3 lines
If I understand it correctly, with the change, most of GPIO client device drivers use the same init level as GPIO driver. So we are not sure if GPIO driver is ready when client drivers try to request GPIO.
please give it a test run (with and without DTS gpio-ranges changes, or just 2/3 from the series), I've tested on iMX6Q SabreLite, SabreAuto, SabreSD and I don't see any regressions or noticeable increase of -EPROBE_DEFER hits due to not ready GPIOs.
Quoted 1 line
most of GPIO client device drivers use the same init level as GPIO driver.
IMHO that will be the case, if the change 2/3 from the series is applied.
Statisticaly most of GPIO consumers should settle on device_initcall or module_init level, the latter one is translated to device_initcall if a driver is built-in, so the proposed downgrading of GPIO controller driver init call level looks to be appropriate.
However pinctrl/pinmux driver should definitely have higher init level priority in comparison to GPIO controller driver, every GPIO consumer is a pinctrl/pinmux consumer as well, most of iMX pinctrl/pinmux drivers are initialized at arch_initcall level, so there is a plenty of options to satisfy (GPIO driver initcall) < (pinmux/pinctrl driver initcall) equation:
* change pinmux/pinctrl driver initcall level to postcore_initcall and GPIO driver initcall level to arch_initcall or lower one,
* keep pinmux/pinctrl driver at arch_initcall level, set GPIO driver initcall level to any lower than arch_initcall (e.g. device_initcall)
* keep everything as is, drop 2/3 and rely on -EPROBE_DEFER from GPIO driver probe due to not yet initialized pinctrl driver.
Quoted 3 lines
It shouldn't be a problem if every client driver handle the failure with deferred probing, but I'm not sure that's the case now.
From what I see it is the case for the most critical drivers, for the rest I believe it is a bug, which can be revealed by shifting GPIO driver initcall level and fixed, see also

Quoted 8 lines

Quoted 1 line
https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2016-August/003419.html
Okay, fair enough.
For the series,
Acked-by Shawn Guo <[email protected]>

thank you for review/ack, just in parallel I've sent v2 with a shift to subsys_initcall, which, I agree, might be more preferrable.

If you more like v2, please ack that series.

Thank you in advance.

--
With best wishes,
Vladimir