Re: [PATCH v2 09/10] gpio: aggregator: cancel deferred probe for devices created via configfs

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Feb 3, 2025 at 4:12 AM Koichiro Den <koichiro.den@xxxxxxxxxxxxx> wrote:
>
> For aggregators initialized via configfs, write 1 to 'live' waits for
> probe completion and returns an error if the probe fails, unlike the
> legacy sysfs interface, which is asynchronous.
>
> Since users control the liveness of the aggregator device and might be
> editting configurations while 'live' is 0, deferred probing is both
> unnatural and unsafe.
>
> Cancel deferred probe for purely configfs-based aggregators when probe
> fails.
>
> Signed-off-by: Koichiro Den <koichiro.den@xxxxxxxxxxxxx>
> ---
>  drivers/gpio/gpio-aggregator.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> index e101b78ad524..174078e02287 100644
> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -1313,7 +1313,6 @@ static struct attribute *gpio_aggregator_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(gpio_aggregator);
>
> -
>  /*
>   *  GPIO Aggregator platform device
>   */
> @@ -1342,8 +1341,22 @@ static int gpio_aggregator_probe(struct platform_device *pdev)
>
>         for (i = 0; i < n; i++) {
>                 descs[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
> -               if (IS_ERR(descs[i]))
> +               if (IS_ERR(descs[i])) {
> +                       /*
> +                        * Deferred probing is not suitable when the aggregator
> +                        * is created by userspace. They should just retry later
> +                        * whenever they like. For device creation via sysfs,
> +                        * error is propagated without overriding for backward
> +                        * compatibility. .prevent_deferred_probe is kept unset
> +                        * for other cases.
> +                        */
> +                       if (!init_via_sysfs && !dev_of_node(dev) &&
> +                           descs[i] == ERR_PTR(-EPROBE_DEFER)) {
> +                               pr_warn("Deferred probe canceled for creation by userspace.\n");
> +                               return -ENODEV;
> +                       }
>                         return PTR_ERR(descs[i]);
> +               }
>         }
>
>         features = (uintptr_t)device_get_match_data(dev);
> --
> 2.45.2
>

Geert, what do you think about making the sysfs interface synchronous
instead? I would argue it's actually more logical as the user will
instinctively expect the aggregator to be ready after write() to
new_device returns.

Bart