RE: [PATCH v4 1/3] pps: clients: gpio: propagate probe error codes

[Farber, Eliav]

On Mon, Sep 22, 2026 at 09:40:00AM +0200, Rodolfo Giometti wrote:
> > @@ -165,7 +165,7 @@ static int pps_gpio_probe(struct platform_device *pdev)
> >       ret = gpiod_to_irq(data->gpio_pin);
> >       if (ret < 0) {
> >               dev_err(dev, "failed to map GPIO to IRQ: %d\n", ret);
> > -             return -EINVAL;
> > +             return ret;
> >       }
> >       data->irq = ret;
>
> Now that the real error survives, -EPROBE_DEFER survives with it -- and
> this dev_err() will then shout once per retry, which is precisely what
> dev_err_probe() exists to avoid.  Wouldn't it be the right thing here?
> pps_gpio_setup() in this same file already uses it, and so does
> pps_gpio_get_pins() in your patch 3, so it would also leave the file
> consistent with itself.

Agreed on silencing the deferral spam. I avoided return dev_err_probe()
because patch 3 turns both returns into goto err_release_pins, and
dev_err_probe()'s idiom is to return the error, not goto.

To get the same suppression without the return, I'd open-code what its
kernel-doc says it replaces, minus the return:

        if (ret != -EPROBE_DEFER)
                dev_err(dev, "failed to map GPIO to IRQ: %d\n", ret);
        else
                dev_dbg(dev, "failed to map GPIO to IRQ: %d\n", ret);
        goto err_release_pins;

Would you like me to change it to this?

> > @@ -197,8 +197,8 @@ static int pps_gpio_probe(struct platform_device *pdev)
> >                         data->info.name, data);
> >       if (ret) {
> >               pps_unregister_source(data->pps);
> > -             dev_err(dev, "failed to acquire IRQ %d\n", data->irq);
> > -             return -EINVAL;
> > +             dev_err(dev, "failed to acquire IRQ %d: %d\n", data->irq, ret);
> > +             return ret;
> >       }
>
> Same question here, although I agree a deferral is far less likely on
> this one.

Same treatment here if you want it.

Thanks,
Eliav