Re: [PATCH] pps: clients: gpio: Remove dead capture_clear code

[Rodolfo Giometti]

On 30/05/2026 04:44, Calvin Owens wrote:
> The capture_clear field is never set, and all code conditional on it
> being set has been unreachable since the platform data logic was removed
> from pps-gpio in ee89646619ba ("pps: clients: gpio: Get rid of legacy
> platform data").
>
> I think the only logical thing to do here is to remove it all, since no
> in-tree code ever actually used it in the first place, and it has been
> completely dead code for over five years (since v5.13).
>
> Sashiko asked some questions about the gpiod_get_value() call which
> caused me to look deeper and figure this out, but it did not actually
> notice capture_clear is never set.
>
> Fixes: ee89646619ba ("pps: clients: gpio: Get rid of legacy platform data")
> Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=1
> Signed-off-by: Calvin Owens <calvin@xxxxxxxxxx>

Acked-by: Rodolfo Giometti <giometti@xxxxxxxxxxxx>

> ---
>   drivers/pps/clients/pps-gpio.c | 36 +++-------------------------------
>   1 file changed, 3 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index 935da68610c7..b48db73e7ba6 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c
> @@ -32,7 +32,6 @@ struct pps_gpio_device_data {
>   	struct gpio_desc *echo_pin;
>   	struct timer_list echo_timer;	/* timer to reset echo active state */
>   	bool assert_falling_edge;
> -	bool capture_clear;
>   	unsigned int echo_active_ms;	/* PPS echo active duration */
>   	unsigned long echo_timeout;	/* timer timeout value in jiffies */
>   };
> @@ -45,25 +44,13 @@ static irqreturn_t pps_gpio_irq_handler(int irq, void *data)
>   {
>   	const struct pps_gpio_device_data *info;
>   	struct pps_event_time ts;
> -	int rising_edge;
>   
>   	/* Get the time stamp first */
>   	pps_get_ts(&ts);
>   
>   	info = data;
>   
> -	/* Small trick to bypass the check on edge's direction when capture_clear is unset */
> -	rising_edge = info->capture_clear ?
> -		      gpiod_get_value(info->gpio_pin) : !info->assert_falling_edge;
> -	if ((rising_edge && !info->assert_falling_edge) ||
> -			(!rising_edge && info->assert_falling_edge))
> -		pps_event(info->pps, &ts, PPS_CAPTUREASSERT, data);
> -	else if (info->capture_clear &&
> -			((rising_edge && info->assert_falling_edge) ||
> -			(!rising_edge && !info->assert_falling_edge)))
> -		pps_event(info->pps, &ts, PPS_CAPTURECLEAR, data);
> -	else
> -		dev_warn_ratelimited(&info->pps->dev, "IRQ did not trigger any PPS event\n");
> +	pps_event(info->pps, &ts, PPS_CAPTUREASSERT, data);
>   
>   	return IRQ_HANDLED;
>   }
> @@ -79,11 +66,6 @@ static void pps_gpio_echo(struct pps_device *pps, int event, void *data)
>   		if (pps->params.mode & PPS_ECHOASSERT)
>   			gpiod_set_value(info->echo_pin, 1);
>   		break;
> -
> -	case PPS_CAPTURECLEAR:
> -		if (pps->params.mode & PPS_ECHOCLEAR)
> -			gpiod_set_value(info->echo_pin, 1);
> -		break;
>   	}
>   
>   	/* fire the timer */
> @@ -145,15 +127,8 @@ static int pps_gpio_setup(struct device *dev)
>   static unsigned long
>   get_irqf_trigger_flags(const struct pps_gpio_device_data *data)
>   {
> -	unsigned long flags = data->assert_falling_edge ?
> -		IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
> -
> -	if (data->capture_clear) {
> -		flags |= ((flags & IRQF_TRIGGER_RISING) ?
> -				IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING);
> -	}
> -
> -	return flags;
> +	return data->assert_falling_edge ? IRQF_TRIGGER_FALLING :
> +					   IRQF_TRIGGER_RISING;
>   }
>   
>   static int pps_gpio_probe(struct platform_device *pdev)
> @@ -186,9 +161,6 @@ static int pps_gpio_probe(struct platform_device *pdev)
>   	/* initialize PPS specific parts of the bookkeeping data structure. */
>   	data->info.mode = PPS_CAPTUREASSERT | PPS_OFFSETASSERT |
>   		PPS_ECHOASSERT | PPS_CANWAIT | PPS_TSFMT_TSPEC;
> -	if (data->capture_clear)
> -		data->info.mode |= PPS_CAPTURECLEAR | PPS_OFFSETCLEAR |
> -			PPS_ECHOCLEAR;
>   	data->info.owner = THIS_MODULE;
>   	snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
>   		 pdev->name, pdev->id);
> @@ -200,8 +172,6 @@ static int pps_gpio_probe(struct platform_device *pdev)
>   
>   	/* register PPS source */
>   	pps_default_params = PPS_CAPTUREASSERT | PPS_OFFSETASSERT;
> -	if (data->capture_clear)
> -		pps_default_params |= PPS_CAPTURECLEAR | PPS_OFFSETCLEAR;
>   	data->pps = pps_register_source(&data->info, pps_default_params);
>   	if (IS_ERR(data->pps)) {
>   		dev_err(dev, "failed to register IRQ %d as PPS source\n",


--
GNU/Linux Solutions                  e-mail: giometti@xxxxxxxxxxxx
Linux Device Driver                          giometti@xxxxxxxx
Embedded Systems                     phone:  +39 349 2432127
UNIX programming