Re: [PATCH v6 4/4] pps: clients: gpio: release pins to an inactive state on remove and shutdown

[Linus Walleij]

On Wed, Sep 23, 2026 at 8:23 PM Eliav Farber <farbere@xxxxxxxxxx> wrote:

> Some boards route the PPS input GPIO through a pin controller and need to
> mux it to another function when pps-gpio is not driving PPS. The driver
> core applies the "default" pinctrl state before probe, so the pins are
> muxed for GPIO/PPS use while the driver is bound. Nothing, however, hands
> the pins back when the driver is unbound or the system is shut down, so
> they stay stuck in the GPIO function for whatever runs next, kexec
> included.
>
> Look up an optional "inactive" pinctrl state in probe via
> devm_pinctrl_get() and pinctrl_lookup_state(), and select it with
> pinctrl_select_state() in remove() and shutdown(). The state is looked up
> and selected by the driver itself rather than reusing the runtime-PM
> "idle"/"sleep" states, so its meaning is unambiguous and it does not
> depend on CONFIG_PM. Boards that do not describe an "inactive" state are
> unaffected.
>
> Since "inactive" is only meaningful as the mux to restore after the
> core-applied "default" state, reject an "inactive" state that is not
> paired with a "default" one rather than releasing pins that were never
> put into a defined PPS state.
>
> Look up the pinctrl states first in probe(), before pps_gpio_setup(), and
> route every subsequent failure through a common err_release_pins label.
> The driver core applies the "default" mux before probe(), so a probe that
> fails after this point would otherwise leave the pins stuck in "default";
> releasing them to "inactive" on the error path is the symmetrical
> counterpart to what the core did on the driver's behalf. A failure in
> pps_gpio_get_pins() itself returns directly, as no state was taken yet;
> pps_gpio_release_pins() is a no-op when no "inactive" state was found.
>
> Convert the pps_register_source() failure path to dev_err_probe() too,
> so all three probe error paths that share err_release_pins log the same
> way. This path already returned PTR_ERR(data->pps), so this is a logging
> change, not a fix.
>
> The mux must not change while something can still drive the pins. On
> shutdown() the requested IRQ and the echo timer would otherwise outlive
> the mux change -- device_shutdown() is not the end of the road, the
> kernel keeps running to load and start the kexec image -- so a timer
> callback or the PPS handler could poke a line that by then belongs to
> another function. Tear down in the same order as remove(): free_irq()
> first, then the echo timer (only when the board has an echo GPIO, as in
> remove()), and the mux change last. shutdown() does not unregister the
> PPS source, which is a remove-time concern.
>
> Signed-off-by: Eliav Farber <farbere@xxxxxxxxxx>

v6 looks reasonable to me!
Reviewed-by: Linus Walleij <linusw@xxxxxxxxxx>

Yours,
Linus Walleij