1. Patchew
  2. linux
  3. View series

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

Calvin Owens posted 1 patch 1 week, 2 days ago 
drivers/pps/clients/pps-gpio.c | 36 +++-------------------------------
1 file changed, 3 insertions(+), 33 deletions(-)
[PATCH] pps: clients: gpio: Remove dead capture_clear code
Posted by Calvin Owens 1 week, 2 days ago 
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@wbinvd.org>
---
 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",
-- 
2.47.3
Re: [PATCH] pps: clients: gpio: Remove dead capture_clear code
Posted by Rodolfo Giometti 1 week, 1 day ago 
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@wbinvd.org>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

> ---
>   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@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.