> On 16/09/2026 16:48, Rodolfo Giometti wrote:
> But "idle" does not mean "not bound", so are you sure this is the
> right-thing(TM) to do? pinctrl-state.h documents it as the runtime PM
> state and, AFAIK, that is how the rest of the tree uses it. If pps-gpio
> ever grows a real runtime PM or a .suspend(), "idle" is already taken
> with another meaning -- and 1/2 turns that choice into ABI.

Right. Renamed to "inactive" in v2.

> Same question for the CONFIG_PM dependency: why should a CONFIG_PM=n
> kernel not get this? A PPS box built without PM is not an odd
> configuration, and there the board describes an "idle" state and nothing
> happens, silently.
>
> Wouldn't looking the state up in the driver (devm_pinctrl_get() +
> pinctrl_lookup_state() + pinctrl_select_state()) avoid both, and leave
> you free to pick a name that says what it means?

Done. v2 looks the state up in the driver and selects it with
pinctrl_select_state(), dropping the CONFIG_PM dependency.

v2 also requires "default" when "inactive" is present and rejects the
mismatch at probe.

> One thing that does not depend on any of the above: your shutdown()
> changes the mux but shuts nothing down. The IRQ is still requested
> (request_irq() here is not devm-managed) and the echo timer may still be
> armed, so a timer callback can still poke a pin that by then belongs to
> somebody else, and the PPS handler stays attached to a line that other
> function is now driving. And device_shutdown() is not the end of the
> road: the kernel keeps running to load and start the kexec image, which
> is the case you are after. Shouldn't it tear down in the same order
> remove() does, free_irq() and timer_delete_sync() first and the mux
> change last?

Fixed. shutdown() now does free_irq() and timer_delete_sync() first, mux
change last. It does not call pps_unregister_source() -- that is a
remove-time concern, not needed to quiesce the hardware for kexec. Say if
you want it there anyway.

Thanks,
Eliav