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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] timers: use of timer_delete_sync() instead of timer_shutdown_sync() in teardown
--

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

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

This commit introduces logic to look up and apply an "inactive" pinctrl state
when the driver is unbound or the system is shut down. This ensures pins are
not left stuck in their default GPIO function for subsequent kernels like
kexec. It introduces a new shutdown callback that tears down the IRQ and
the echo timer before restoring the mux state.

> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index aec534c246af1..203bf465ed8bf 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c

[ ... ]

> @@ -216,9 +298,31 @@ static void pps_gpio_remove(struct platform_device *pdev)

[ ... ]

> +static void pps_gpio_shutdown(struct platform_device *pdev)
> +{
> +	struct pps_gpio_device_data *data = platform_get_drvdata(pdev);
> +
> +	/*
> +	 * The kernel keeps running after device_shutdown() (e.g. to load and
> +	 * start a kexec image), so quiesce the hardware before touching the
> +	 * mux: free the IRQ and stop the echo timer first, then release the
> +	 * pins last, so no callback can drive a pin after it is handed back.
> +	 * The PPS source is left registered; that is a remove-time concern.
> +	 */
> +	free_irq(data->irq, data);
> +	/* reset the echo state, if the board has an echo GPIO */
> +	if (data->echo_pin) {
> +		timer_delete_sync(&data->echo_timer);

[Severity: Low]
Should this teardown sequence in pps_gpio_shutdown() use 
timer_shutdown_sync() instead of timer_delete_sync()?

The modern timer API guidelines mandate using timer_shutdown_sync() in
device teardown paths to robustly prevent the timer from being re-armed.
While free_irq() runs prior and stops the interrupt handler from running
concurrently, timer_shutdown_sync() remains the documented standard for
fully shutting down a timer.

> +		gpiod_set_value(data->echo_pin, 0);
> +	}
> +	pps_gpio_release_pins(&pdev->dev);
> +}

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