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

[Rodolfo Giometti <giometti@enneenne.com>]

On Sat, Sep 19, 2026 at 05:11:55PM +0000, Eliav Farber wrote:
> Fixes: 161520451dfa ("pps: new client driver using GPIO")
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

You are right and I was wrong on v3.  I went and looked: the hardcoded
-EINVAL on the gpio_to_irq() path is already there in 161520451dfa,
and 4461d65176b4 only turned gpio_to_irq() into gpiod_to_irq() while
leaving those returns alone.  The tag points where it should.

> @@ -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.

> @@ -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.

Either way this is a nit, not an objection: the patch is right as it
stands.

Ciao,

Rodolfo