On Mon, Sep 22, 2026 at 09:40:00AM +0200, Rodolfo Giometti wrote:
> On Sat, Sep 19, 2026 at 05:11:54PM +0000, Eliav Farber wrote:
> > Open question for the maintainers (patch 3): a probe that fails before
> > pps_gpio_get_pins() has run -- devm_kzalloc() or pps_gpio_setup() -- returns
> > without going through err_release_pins, so the pins are left in the
> > core-applied "default" state rather than "inactive". I did not change this
> > in v4 as I would like your guidance on the preferred approach; the trade-offs
> > are laid out at the end of patch 3's changelog. In short:
> >
> >   A. Move pps_gpio_get_pins() to the top of probe and route the
> >      pps_gpio_setup() failure through err_release_pins too, so every path
> >      the driver can act on restores "inactive". (The devm_kzalloc() failure
> >      is inherently before the driver holds any pinctrl handle, so it cannot
> >      be covered by the driver in any option.)
> >   B. Keep v4 as-is and treat "inactive" as a successful-ownership concern:
> >      a probe that never looked up the state never took the pins, so leaving
> >      the core-applied "default" (long-standing behaviour) is acceptable.
> >   C. As A, but keep the release helper's existing NULL guard so it is robust
> >      regardless of ordering (belt and braces).
> >
> > I lean towards B (the restore is meaningful only once the driver has taken
> > ownership), but I am happy to implement A/C if you prefer uniform failure
> > paths.
>
> I would choose option A, keeping the NULL check in the release helper;
> I believe this corresponds to your option C. In fact, to me, it
> represents the symmetrical counterpart to what the core did on the
> driver's behalf.

Done in v5: pps_gpio_get_pins() now runs first in probe(), the
pps_gpio_setup() failure goes through err_release_pins, and the release
helper keeps its NULL guard (option C).

> One thing I would verify on your board before proceeding is this: with
> option A, an -EPROBE_DEFER error returned by pps_gpio_setup() would
> result in the "inactive" state being selected, and the core would
> re-apply "default" on the next attempt. I assume this mux switching is
> harmless, but the hardware is available to you, not me.

Verified on the AL11 K2V6 JRD10: I forced pps_gpio_setup() to return
-EPROBE_DEFER a few times and watched the pin-mux register. Each failed
attempt releases the pins to "inactive", the core re-applies "default"
before the next, and it settles at "default" once probe succeeds -- no
spurious PPS event or warning across the cycles.

Thanks,
Eliav