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

[Svyatoslav Ryhel]

пн, 9 груд. 2024 р. о 12:11 Jonas Karlman <jonas at 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 at 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;
>
>