Re: [PATCH v2 2/2] gpio: fix request of PMIC GPIO child

пн, 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;