[PATCH v2 0/2] GPIO: improve and fix pmic gpios behavior

First patch makes support of PMIC GPIO children with no dedicated GPIO node simpler (just add select PMIC_GPIO to PMIC GPIO driver)
Second fixes getting device if correct one was found among PMIC children.
--- Changes from v1 - dropped the prompt text - changed the help to star with "Select this option ..." ---
Svyatoslav Ryhel (2): gpio: add PMIC_GPIO Kconfig option gpio: fix request of PMIC GPIO child
drivers/gpio/Kconfig | 8 ++++++++ drivers/gpio/gpio-uclass.c | 10 +++++++++- 2 files changed, 17 insertions(+), 1 deletion(-)

Add more generic Kconfig option to be enabled by the PMIC drivers which do not have dedicated GPIO node and use same phandle to refer to both core driver and GPIO child.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com --- drivers/gpio/Kconfig | 8 ++++++++ drivers/gpio/gpio-uclass.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 92a8597420a..13b7e9a53dd 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -255,6 +255,7 @@ config MAX7320_GPIO config MAX77663_GPIO bool "MAX77663 GPIO cell of PMIC driver" depends on DM_GPIO && DM_PMIC_MAX77663 + select PMIC_GPIO help GPIO driver for MAX77663 PMIC from Maxim Semiconductor. MAX77663 PMIC has 8 pins that can be configured as GPIOs @@ -474,6 +475,13 @@ config PIC32_GPIO help Say yes here to support Microchip PIC32 GPIOs.
+config PMIC_GPIO + bool + depends on DM_GPIO + help + Select this option if your PMIC does not have dedicated GPIO node + and phandle of the PMIC itself is used as a GPIO phandle. + config OCTEON_GPIO bool "Octeon II/III/TX/TX2 GPIO driver" depends on DM_GPIO && PCI && (ARCH_OCTEON || ARCH_OCTEONTX || ARCH_OCTEONTX2) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0213271e3a6..e776906fe73 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1142,7 +1142,7 @@ static int gpio_request_tail(int ret, const char *nodename, ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node, &desc->dev); if (ret) { -#if CONFIG_IS_ENABLED(MAX77663_GPIO) || CONFIG_IS_ENABLED(PALMAS_GPIO) +#if CONFIG_IS_ENABLED(PMIC_GPIO) struct udevice *pmic; ret = uclass_get_device_by_ofnode(UCLASS_PMIC, args->node, &pmic);

If correct PMIC child was found it should be requested as well.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com --- drivers/gpio/gpio-uclass.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index e776906fe73..e6c00c48722 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1160,6 +1160,14 @@ static int gpio_request_tail(int ret, const char *nodename, /* if loop exits without GPIO device return error */ if (device_get_uclass_id(desc->dev) != UCLASS_GPIO) goto err; + + ret = uclass_get_device_by_name(UCLASS_GPIO, desc->dev->name, + &desc->dev); + if (ret) { + log_debug("%s: getting GPIO device failed %d\n", + __func__, ret); + goto err; + } #else debug("%s: uclass_get_device_by_ofnode failed\n", __func__);

Hi Svyatoslav,
On 2024-12-09 07:44, Svyatoslav Ryhel wrote:
If correct PMIC child was found it should be requested as well.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com
drivers/gpio/gpio-uclass.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index e776906fe73..e6c00c48722 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1160,6 +1160,14 @@ static int gpio_request_tail(int ret, const char *nodename, /* if loop exits without GPIO device return error */ if (device_get_uclass_id(desc->dev) != UCLASS_GPIO) goto err;
ret = uclass_get_device_by_name(UCLASS_GPIO, desc->dev->name,
&desc->dev);
if (ret) {
log_debug("%s: getting GPIO device failed %d\n",
__func__, ret);
goto err;
}
#else debug("%s: uclass_get_device_by_ofnode failed\n", __func__);
Instead of extending this PMIC/GPIO workaround maybe it is better to fix the issue at bind time? Look like the max77663 pmic driver use device_bind_driver() instead of device_bind_driver_to_node().
Also palmas gpio driver should not need to be bind, the DTs I can find in tree all have a gpio node with compatible = ti,palmas-gpio. Do not really understand why the palmas driver should need this workaround the gpios props seem to bind to &palmas_gpio node in the DTs I checked.
Maybe something like following diff could work for your boards? I.e. revert c03cd98d1a16 ("drivers: gpio-uclass: support PMIC GPIO children") and bind gpio dirver to the pmic node.
Binding to pmic node should probably only happen if the pmic node has the gpio-controller prop.
Regards, Jonas
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0213271e3a69..67fc776cada2 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1142,29 +1142,9 @@ static int gpio_request_tail(int ret, const char *nodename, ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node, &desc->dev); if (ret) { -#if CONFIG_IS_ENABLED(MAX77663_GPIO) || CONFIG_IS_ENABLED(PALMAS_GPIO) - struct udevice *pmic; - ret = uclass_get_device_by_ofnode(UCLASS_PMIC, args->node, - &pmic); - if (ret) { - log_debug("%s: PMIC device get failed, err %d\n", - __func__, ret); - goto err; - } - - device_foreach_child(desc->dev, pmic) { - if (device_get_uclass_id(desc->dev) == UCLASS_GPIO) - break; - } - - /* if loop exits without GPIO device return error */ - if (device_get_uclass_id(desc->dev) != UCLASS_GPIO) - goto err; -#else debug("%s: uclass_get_device_by_ofnode failed\n", __func__); goto err; -#endif } } ret = gpio_find_and_xlate(desc, args); diff --git a/drivers/power/pmic/max77663.c b/drivers/power/pmic/max77663.c index cf08b6a7e1df..838eae50740d 100644 --- a/drivers/power/pmic/max77663.c +++ b/drivers/power/pmic/max77663.c @@ -56,8 +56,8 @@ static int max77663_bind(struct udevice *dev) }
if (IS_ENABLED(CONFIG_MAX77663_GPIO)) { - ret = device_bind_driver(dev, MAX77663_GPIO_DRIVER, - "gpio", NULL); + ret = device_bind_driver_to_node(dev, MAX77663_GPIO_DRIVER, + "gpio", dev_ofnode(dev), NULL); if (ret) { log_err("cannot bind GPIOs (ret = %d)\n", ret); return ret; diff --git a/drivers/power/pmic/palmas.c b/drivers/power/pmic/palmas.c index f676bf641694..faa876f4e73d 100644 --- a/drivers/power/pmic/palmas.c +++ b/drivers/power/pmic/palmas.c @@ -45,7 +45,6 @@ static int palmas_read(struct udevice *dev, uint reg, uint8_t *buff, int len) static int palmas_bind(struct udevice *dev) { ofnode pmic_node = ofnode_null(), regulators_node; - ofnode subnode, gpio_node; int children, ret;
if (IS_ENABLED(CONFIG_SYSRESET_PALMAS)) { @@ -57,14 +56,6 @@ static int palmas_bind(struct udevice *dev) } }
- gpio_node = ofnode_find_subnode(dev_ofnode(dev), "gpio"); - if (ofnode_valid(gpio_node)) { - ret = device_bind_driver_to_node(dev, PALMAS_GPIO_DRIVER, - "gpio", gpio_node, NULL); - if (ret) - log_err("cannot bind GPIOs (ret = %d)\n", ret); - } - dev_for_each_subnode(subnode, dev) { const char *name; char *temp;

пн, 9 груд. 2024 р. о 12:11 Jonas Karlman jonas@kwiboo.se пише:
Hi Svyatoslav,
On 2024-12-09 07:44, Svyatoslav Ryhel wrote:
If correct PMIC child was found it should be requested as well.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com
drivers/gpio/gpio-uclass.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index e776906fe73..e6c00c48722 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1160,6 +1160,14 @@ static int gpio_request_tail(int ret, const char *nodename, /* if loop exits without GPIO device return error */ if (device_get_uclass_id(desc->dev) != UCLASS_GPIO) goto err;
ret = uclass_get_device_by_name(UCLASS_GPIO, desc->dev->name,
&desc->dev);
if (ret) {
log_debug("%s: getting GPIO device failed %d\n",
__func__, ret);
goto err;
}
#else debug("%s: uclass_get_device_by_ofnode failed\n", __func__);
Instead of extending this PMIC/GPIO workaround maybe it is better to fix the issue at bind time? Look like the max77663 pmic driver use device_bind_driver() instead of device_bind_driver_to_node().
Actually this is a valid point. Thank you for this suggestion.
Also palmas gpio driver should not need to be bind, the DTs I can find in tree all have a gpio node with compatible = ti,palmas-gpio. Do not really understand why the palmas driver should need this workaround the gpios props seem to bind to &palmas_gpio node in the DTs I checked.
This, on the other hand, will not work since pmic drivers do not automatically bind all child nodes, I have checked and gpio driver was not bound.
Maybe something like following diff could work for your boards? I.e. revert c03cd98d1a16 ("drivers: gpio-uclass: support PMIC GPIO children") and bind gpio dirver to the pmic node.
Binding to pmic node should probably only happen if the pmic node has the gpio-controller prop.
Regards, Jonas
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0213271e3a69..67fc776cada2 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1142,29 +1142,9 @@ static int gpio_request_tail(int ret, const char *nodename, ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node, &desc->dev); if (ret) { -#if CONFIG_IS_ENABLED(MAX77663_GPIO) || CONFIG_IS_ENABLED(PALMAS_GPIO)
struct udevice *pmic;
ret = uclass_get_device_by_ofnode(UCLASS_PMIC, args->node,
&pmic);
if (ret) {
log_debug("%s: PMIC device get failed, err %d\n",
__func__, ret);
goto err;
}
device_foreach_child(desc->dev, pmic) {
if (device_get_uclass_id(desc->dev) == UCLASS_GPIO)
break;
}
/* if loop exits without GPIO device return error */
if (device_get_uclass_id(desc->dev) != UCLASS_GPIO)
goto err;
-#else debug("%s: uclass_get_device_by_ofnode failed\n", __func__); goto err; -#endif } } ret = gpio_find_and_xlate(desc, args); diff --git a/drivers/power/pmic/max77663.c b/drivers/power/pmic/max77663.c index cf08b6a7e1df..838eae50740d 100644 --- a/drivers/power/pmic/max77663.c +++ b/drivers/power/pmic/max77663.c @@ -56,8 +56,8 @@ static int max77663_bind(struct udevice *dev) }
if (IS_ENABLED(CONFIG_MAX77663_GPIO)) {
ret = device_bind_driver(dev, MAX77663_GPIO_DRIVER,
"gpio", NULL);
ret = device_bind_driver_to_node(dev, MAX77663_GPIO_DRIVER,
"gpio", dev_ofnode(dev), NULL); if (ret) { log_err("cannot bind GPIOs (ret = %d)\n", ret); return ret;
diff --git a/drivers/power/pmic/palmas.c b/drivers/power/pmic/palmas.c index f676bf641694..faa876f4e73d 100644 --- a/drivers/power/pmic/palmas.c +++ b/drivers/power/pmic/palmas.c @@ -45,7 +45,6 @@ static int palmas_read(struct udevice *dev, uint reg, uint8_t *buff, int len) static int palmas_bind(struct udevice *dev) { ofnode pmic_node = ofnode_null(), regulators_node;
ofnode subnode, gpio_node; int children, ret; if (IS_ENABLED(CONFIG_SYSRESET_PALMAS)) {
@@ -57,14 +56,6 @@ static int palmas_bind(struct udevice *dev) } }
gpio_node = ofnode_find_subnode(dev_ofnode(dev), "gpio");
if (ofnode_valid(gpio_node)) {
ret = device_bind_driver_to_node(dev, PALMAS_GPIO_DRIVER,
"gpio", gpio_node, NULL);
if (ret)
log_err("cannot bind GPIOs (ret = %d)\n", ret);
}
dev_for_each_subnode(subnode, dev) { const char *name; char *temp;
participants (2)
-
Jonas Karlman
-
Svyatoslav Ryhel