Re: [PATCH] gpio: nomadik: Back out some managed resources

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Tue, Mar 5, 2024 at 10:26 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Several commits introduce managed resources (devm_*) into the
> nmk_gpio_populate_chip() function.
>
> This isn't always working because when called from the Nomadik pin
> control driver we just want to populate some states for the device as
> the same states are used by the pin control driver.
>
> Some managed resources such as devm_kzalloc() etc will work, as the
> passed in platform device will be used for lifecycle management,
> but in some cases where we used the looked-up platform device
> for the GPIO block, this will cause problems for the combined
> pin control and GPIO driver, because it adds managed resources
> to the GPIO device before it is probed, which is something that
> the device core will not accept, and all of the GPIO blocks will
> refuse to probe:
>
> platform 8012e000.gpio: Resources present before probing
> platform 8012e080.gpio: Resources present before probing
> (...)
>
> Fix this by not tying any managed resources to the looked-up
> gpio_pdev/gpio_dev device, let's just live with the fact that
> these need imperative resource management for now.

...

> -       clk = devm_clk_get_optional(gpio_dev, NULL);
> +       /* NOTE: do not use devm_ here! */
> +       clk = clk_get_optional(gpio_dev, NULL);
>         if (IS_ERR(clk)) {
>                 platform_device_put(gpio_pdev);
>                 return (void *)clk;

> -       reset = devm_reset_control_get_optional_shared(gpio_dev, NULL);
> +       /* NOTE: do not use devm_ here! */
> +       reset = reset_control_get_optional_shared(gpio_dev, NULL);
>         if (IS_ERR(reset)) {
> -               dev_err(&pdev->dev, "failed getting reset control: %ld\n",
> +               dev_err(dev, "failed getting reset control: %ld\n",
>                         PTR_ERR(reset));
>                 return ERR_CAST(reset);
>         }
> @@ -588,7 +594,7 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct fwnode_handle *fwnode,
>          */
>         ret = reset_control_deassert(reset);
>         if (ret) {
> -               dev_err(&pdev->dev, "failed reset deassert: %d\n", ret);
> +               dev_err(dev, "failed reset deassert: %d\n", ret);
>                 return ERR_PTR(ret);
>         }

Yeah, but you forgot to update the error path as it needs to unroll
the changes, no?

-- 
With Best Regards,
Andy Shevchenko