Re: [PATCH 2/2] pps: clients: gpio: release pins to idle state on remove and shutdown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pps: missing IRQ and timer teardown in pps_gpio_shutdown()
--

commit e71a89236222915be5ef5d5a822ad6e3bcc21b37
Author: Eliav Farber <farbere@amazon.com>

pps: clients: gpio: release pins to idle state on remove and shutdown

Some boards route the PPS input GPIO through a pin controller and mux it
to another function when the driver is not bound. This patch selects the
"idle" pinctrl state in both remove() and shutdown() so that the alternate
mux is restored, for instance before kexec.

> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index 73ec2c7335e51..bdedbed82d82c 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c

[ ... ]

> @@ -216,9 +217,26 @@ static void pps_gpio_remove(struct platform_device *pdev)
>  	timer_delete_sync(&data->echo_timer);
>  	/* reset echo pin in any case */
>  	gpiod_set_value(data->echo_pin, 0);
> +	/*
> +	 * Release the pins to their "idle" state, if the board defines one, so
> +	 * they are handed back to whatever function uses them while pps-gpio is
> +	 * not bound. Boards that do not describe an idle pinctrl state are
> +	 * unaffected. The "default" (active) state is applied automatically by
> +	 * the driver core before probe.
> +	 */
> +	pinctrl_pm_select_idle_state(&pdev->dev);
>  	dev_info(&pdev->dev, "removed IRQ %d as PPS source\n", data->irq);
>  }
>  
> +static void pps_gpio_shutdown(struct platform_device *pdev)
> +{
> +	/*
> +	 * Leave the pins in their "idle" state on shutdown so a subsequent
> +	 * kernel (e.g. after kexec) finds the pin controller in a known state.
> +	 */
> +	pinctrl_pm_select_idle_state(&pdev->dev);

[Severity: High]
Does this code risk causing an interrupt storm during shutdown or kexec?

When pps_gpio_shutdown() alters the physical pin state to "idle" while system
interrupts are still enabled, it could leave the edge-triggered PPS input pin
floating (e.g. if the "idle" state removes pull resistors). Because the IRQ is
never explicitly disabled here and the echo timer is left running, electrical
noise might generate rapid spurious edges.

Since the IRQ handler always returns IRQ_HANDLED, the kernel IRQ core won't
detect and shut down a spurious storm. Could the CPU get stuck in an infinite
interrupt loop, preventing shutdown or kexec from completing? Should the IRQ
and timer be torn down here as they are in pps_gpio_remove()?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916134744.46354-1-farbere@amazon.com?part=2