mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v7 0/1] pps: improve PREEMPT_RT performance
@ 2026-06-02  0:44 Calvin Owens
  2026-06-02  0:44 ` [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler Calvin Owens
  0 siblings, 1 reply; 10+ messages in thread
From: Calvin Owens @ 2026-06-02  0:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-rt-devel, Rodolfo Giometti, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt, Greg Kroah-Hartman, Eliav Farber,
	Michael Byczkowski, Ingo Molnar, David Laight, Thomas Gleixner

Hello all,

I'm relaying v7 for Michael.

A quick note, this conflicts with the patch I have out to remove
capture_clear: it's trivial to resolve, but if it saves anybody time let
me know and I can respin one or the other.

Thanks,
Calvin

---
From: Michael Byczkowski <by@by-online.de>

Changes since v6: https://lore.kernel.org/lkml/cover.1779733602.git.calvin@wbinvd.org/
 - patches 2 and 3 are dropped since neither lock is ever taken in
   hardirq context.

Changes since v5: https://lore.kernel.org/lkml/719A31CE-CA58-45C3-A013-1BFE81F724C5@by-online.de/
 - Reordered: the pps_kc_hardpps_lock conversion now precedes the
   pps_device.lock conversion. The previous order would have briefly
   produced a raw_spinlock holding a sleeping spinlock on PREEMPT_RT
   (Sebastian).
 - Patch 1/3: commit message reworded to describe the handler split
   structurally first, then its PREEMPT_RT benefit (Sebastian).
 - Patch 2/3: refactored pps_kc_bind() and pps_kc_remove() to use
   guard(raw_spinlock_irq) for scope-based lock release. Eliminates
   four duplicated unlock call sites in pps_kc_bind() and the
   ambiguous bracket structure that resulted from them (Sebastian).
 - Rodolfo's Acked-by on patch 2/3 is preserved from v5; the guard()
   refactor is purely stylistic and was suggested by Sebastian, but
   please re-ack or NAK if disagreement.

Changes since v4: https://lore.kernel.org/lkml/B24484C5-3117-4C56-9522-1EE9876E64BA@by-online.de/
 - Patch 2/3: added Acked-by: Rodolfo Giometti <giometti@enneenne.com>

Changes since v3: https://lore.kernel.org/lkml/83318241-44C3-48BE-829D-5C5F82A78A74@by-online.de/
 - Patch 2/3: fixed lost indentation on pps_kc_event() call
   (reported by Rodolfo Giometti <giometti@enneenne.com>)

Changes since v2: https://lore.kernel.org/lkml/1BB87C0C-33C1-45C3-B50E-C5F349DA3FDC@by-online.de/
 - Patch 2/3: moved wake_up_interruptible_all() and kill_fasync() out
   of raw_spinlock section to avoid sleeping-in-atomic on PREEMPT_RT
   (reported by Nikolaus Buchwitz <nb@buchwitz.com>)

This patchset addresses three sources of PPS jitter under PREEMPT_RT,
while being fully backward-compatible with non-RT kernels:

1. pps-gpio: The IRQ handler is force-threaded on PREEMPT_RT, so the
   PPS timestamp is captured after scheduling delay rather than at
   interrupt entry. Fix: split into a hardirq primary handler
   (captures timestamp only) and a threaded handler (processes the
   event).

Tested on a Raspberry Pi 5 running 7.0.1 and 7.1-rc PREEMPT_RT kernels.
On non-RT kernels there is zero behavioral change.

Michael Byczkowski (1):
  pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded
    handler

 drivers/pps/clients/pps-gpio.c | 37 +++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 12 deletions(-)

-- 
2.47.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-06-02  0:44 [PATCH v7 0/1] pps: improve PREEMPT_RT performance Calvin Owens
@ 2026-06-02  0:44 ` Calvin Owens
  2026-06-02  6:36   ` Sebastian Andrzej Siewior
  2026-07-05  1:06   ` Andrew Morton
  0 siblings, 2 replies; 10+ messages in thread
From: Calvin Owens @ 2026-06-02  0:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-rt-devel, Rodolfo Giometti, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt, Greg Kroah-Hartman, Eliav Farber,
	Michael Byczkowski, Ingo Molnar, David Laight, Thomas Gleixner

From: Michael Byczkowski <by@by-online.de>

Split the pps-gpio interrupt handler into a primary (hardirq) handler
that captures the PPS timestamp at interrupt entry, and a threaded
handler that processes the event. This produces the same two-part
handler structure on both PREEMPT_RT and non-RT kernels.

On non-RT kernels the threaded portion runs immediately after the
primary, with no behavioral change compared to the previous
single-handler implementation.

On PREEMPT_RT, where interrupt handlers are force-threaded by default,
the previous single-handler implementation captured the timestamp
inside the threaded portion, after IRQ-thread scheduling delay. With
the split, the timestamp is captured in true hardirq context as it is
on non-RT kernels, eliminating a significant source of PPS jitter on
RT systems.

Tested-by: Michael Byczkowski <by@by-online.de>
Tested-by: Calvin Owens <calvin@wbinvd.org>
Acked-by: Rodolfo Giometti <giometti@enneenne.com>
Signed-off-by: Michael Byczkowski <by@by-online.de>
Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 drivers/pps/clients/pps-gpio.c | 37 +++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 935da68610c7..ed111621ee5f 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -35,33 +35,44 @@ struct pps_gpio_device_data {
 	bool capture_clear;
 	unsigned int echo_active_ms;	/* PPS echo active duration */
 	unsigned long echo_timeout;	/* timer timeout value in jiffies */
+	struct pps_event_time ts;	/* timestamp captured in hardirq */
 };
 
 /*
  * Report the PPS event
  */
 
-static irqreturn_t pps_gpio_irq_handler(int irq, void *data)
+/*
+ * Primary hardirq handler -- runs in hardirq context even on PREEMPT_RT.
+ * Only captures the timestamp; all other work is deferred to the thread.
+ */
+static irqreturn_t pps_gpio_irq_hardirq(int irq, void *data)
 {
-	const struct pps_gpio_device_data *info;
-	struct pps_event_time ts;
-	int rising_edge;
+	struct pps_gpio_device_data *info = data;
+
+	pps_get_ts(&info->ts);
 
-	/* Get the time stamp first */
-	pps_get_ts(&ts);
+	return IRQ_WAKE_THREAD;
+}
 
-	info = data;
+/*
+ * Threaded handler -- processes the PPS event using the timestamp
+ * captured in hardirq context above.
+ */
+static irqreturn_t pps_gpio_irq_thread(int irq, void *data)
+{
+	struct pps_gpio_device_data *info = data;
+	int rising_edge;
 
-	/* 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);
+		pps_event(info->pps, &info->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);
+		pps_event(info->pps, &info->ts, PPS_CAPTURECLEAR, data);
 	else
 		dev_warn_ratelimited(&info->pps->dev, "IRQ did not trigger any PPS event\n");
 
@@ -210,8 +221,10 @@ static int pps_gpio_probe(struct platform_device *pdev)
 	}
 
 	/* register IRQ interrupt handler */
-	ret = request_irq(data->irq, pps_gpio_irq_handler,
-			  get_irqf_trigger_flags(data), data->info.name, data);
+	ret = request_threaded_irq(data->irq,
+			  pps_gpio_irq_hardirq, pps_gpio_irq_thread,
+			  get_irqf_trigger_flags(data) | IRQF_ONESHOT,
+			  data->info.name, data);
 	if (ret) {
 		pps_unregister_source(data->pps);
 		dev_err(dev, "failed to acquire IRQ %d\n", data->irq);
-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-06-02  0:44 ` [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler Calvin Owens
@ 2026-06-02  6:36   ` Sebastian Andrzej Siewior
  2026-06-03 17:29     ` Michael Byczkowski
  2026-07-05  1:06   ` Andrew Morton
  1 sibling, 1 reply; 10+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-06-02  6:36 UTC (permalink / raw)
  To: Calvin Owens
  Cc: linux-kernel, linux-rt-devel, Rodolfo Giometti, Clark Williams,
	Steven Rostedt, Greg Kroah-Hartman, Eliav Farber,
	Michael Byczkowski, Ingo Molnar, David Laight, Thomas Gleixner

On 2026-06-01 17:44:09 [-0700], Calvin Owens wrote:
> From: Michael Byczkowski <by@by-online.de>
> 
> Split the pps-gpio interrupt handler into a primary (hardirq) handler
> that captures the PPS timestamp at interrupt entry, and a threaded
> handler that processes the event. This produces the same two-part
> handler structure on both PREEMPT_RT and non-RT kernels.
> 
> On non-RT kernels the threaded portion runs immediately after the
> primary, with no behavioral change compared to the previous
> single-handler implementation.
> 
> On PREEMPT_RT, where interrupt handlers are force-threaded by default,
> the previous single-handler implementation captured the timestamp
> inside the threaded portion, after IRQ-thread scheduling delay. With
> the split, the timestamp is captured in true hardirq context as it is
> on non-RT kernels, eliminating a significant source of PPS jitter on
> RT systems.
> 
> Tested-by: Michael Byczkowski <by@by-online.de>
> Tested-by: Calvin Owens <calvin@wbinvd.org>
> Acked-by: Rodolfo Giometti <giometti@enneenne.com>
> Signed-off-by: Michael Byczkowski <by@by-online.de>
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> ---

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-06-02  6:36   ` Sebastian Andrzej Siewior
@ 2026-06-03 17:29     ` Michael Byczkowski
  2026-07-03 18:20       ` Michael Byczkowski
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Byczkowski @ 2026-06-03 17:29 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Calvin Owens, linux-kernel, linux-rt-devel, Rodolfo Giometti,
	Clark Williams, Steven Rostedt, Greg Kroah-Hartman, Eliav Farber,
	Ingo Molnar, David Laight, Thomas Gleixner

Thank you Sebastian, and thank you Calvin for relaying. I appreciate the
thorough review that got this to the right shape.

Best regards,
	Michael


> On 2. Jun 2026, at 08:36, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
> On 2026-06-01 17:44:09 [-0700], Calvin Owens wrote:
>> From: Michael Byczkowski <by@by-online.de>
>> 
>> Split the pps-gpio interrupt handler into a primary (hardirq) handler
>> that captures the PPS timestamp at interrupt entry, and a threaded
>> handler that processes the event. This produces the same two-part
>> handler structure on both PREEMPT_RT and non-RT kernels.
>> 
>> On non-RT kernels the threaded portion runs immediately after the
>> primary, with no behavioral change compared to the previous
>> single-handler implementation.
>> 
>> On PREEMPT_RT, where interrupt handlers are force-threaded by default,
>> the previous single-handler implementation captured the timestamp
>> inside the threaded portion, after IRQ-thread scheduling delay. With
>> the split, the timestamp is captured in true hardirq context as it is
>> on non-RT kernels, eliminating a significant source of PPS jitter on
>> RT systems.
>> 
>> Tested-by: Michael Byczkowski <by@by-online.de>
>> Tested-by: Calvin Owens <calvin@wbinvd.org>
>> Acked-by: Rodolfo Giometti <giometti@enneenne.com>
>> Signed-off-by: Michael Byczkowski <by@by-online.de>
>> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
>> ---
> 
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Sebastian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-06-03 17:29     ` Michael Byczkowski
@ 2026-07-03 18:20       ` Michael Byczkowski
  2026-07-04  5:50         ` Rodolfo Giometti
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Byczkowski @ 2026-07-03 18:20 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, Calvin Owens, linux-kernel,
	linux-rt-devel, Rodolfo Giometti, Clark Williams, Steven Rostedt,
	Greg Kroah-Hartman, Eliav Farber, Ingo Molnar, David Laight,
	Thomas Gleixner

Dear All,

Now that 7.2-rc1 is out and this didn't make the merge window, could someone please confirm which tree will carry this for 7.3?
So far, the patch has been:
    • Reviewed-by: Sebastian Andrzej Siewior (Jun 2)
    • Acked-by: Rodolfo Giometti (PPS maintainer, from earlier revisions)
    • Tested-by from Calvin Owens and myself

I'm not sure whether it should go via drivers/pps (Rodolfo), the rt tree, or -mm. Happy to help route it wherever makes sense.
lore: https://lore.kernel.org/lkml/79BC1B96-FD2C-4191-8766-6C46BF8A1089@by-online.de/

Thank you very much and best regards,
Michael



> On 3. Jun 2026, at 19:29, Michael Byczkowski <by@by-online.de> wrote:
> 
> Thank you Sebastian, and thank you Calvin for relaying. I appreciate the
> thorough review that got this to the right shape.
> 
> Best regards,
> Michael
> 
> 
>> On 2. Jun 2026, at 08:36, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>> 
>> On 2026-06-01 17:44:09 [-0700], Calvin Owens wrote:
>>> From: Michael Byczkowski <by@by-online.de>
>>> 
>>> Split the pps-gpio interrupt handler into a primary (hardirq) handler
>>> that captures the PPS timestamp at interrupt entry, and a threaded
>>> handler that processes the event. This produces the same two-part
>>> handler structure on both PREEMPT_RT and non-RT kernels.
>>> 
>>> On non-RT kernels the threaded portion runs immediately after the
>>> primary, with no behavioral change compared to the previous
>>> single-handler implementation.
>>> 
>>> On PREEMPT_RT, where interrupt handlers are force-threaded by default,
>>> the previous single-handler implementation captured the timestamp
>>> inside the threaded portion, after IRQ-thread scheduling delay. With
>>> the split, the timestamp is captured in true hardirq context as it is
>>> on non-RT kernels, eliminating a significant source of PPS jitter on
>>> RT systems.
>>> 
>>> Tested-by: Michael Byczkowski <by@by-online.de>
>>> Tested-by: Calvin Owens <calvin@wbinvd.org>
>>> Acked-by: Rodolfo Giometti <giometti@enneenne.com>
>>> Signed-off-by: Michael Byczkowski <by@by-online.de>
>>> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
>>> ---
>> 
>> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> 
>> Sebastian
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-07-03 18:20       ` Michael Byczkowski
@ 2026-07-04  5:50         ` Rodolfo Giometti
  0 siblings, 0 replies; 10+ messages in thread
From: Rodolfo Giometti @ 2026-07-04  5:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michael Byczkowski, Sebastian Andrzej Siewior, Calvin Owens,
	linux-kernel, linux-rt-devel, Clark Williams, Steven Rostedt,
	Greg Kroah-Hartman, Eliav Farber, Ingo Molnar, David Laight,
	Thomas Gleixner

Hi Andrew,

Could you please pick this patch up for the 7.3 merge window via the -mm tree?

Since the PPS subsystem doesn't have its own dedicated git tree, routing it 
through -mm is the standard path. The patch already has my Acked-by, along with 
all the necessary reviews and test tags.

Thanks,
Rodolfo

On 03/07/2026 20:20, Michael Byczkowski wrote:
> Dear All,
> 
> Now that 7.2-rc1 is out and this didn't make the merge window, could someone please confirm which tree will carry this for 7.3?
> So far, the patch has been:
>      • Reviewed-by: Sebastian Andrzej Siewior (Jun 2)
>      • Acked-by: Rodolfo Giometti (PPS maintainer, from earlier revisions)
>      • Tested-by from Calvin Owens and myself
> 
> I'm not sure whether it should go via drivers/pps (Rodolfo), the rt tree, or -mm. Happy to help route it wherever makes sense.
> lore: https://lore.kernel.org/lkml/79BC1B96-FD2C-4191-8766-6C46BF8A1089@by-online.de/
> 
> Thank you very much and best regards,
> Michael
> 
> 
> 
>> On 3. Jun 2026, at 19:29, Michael Byczkowski <by@by-online.de> wrote:
>>
>> Thank you Sebastian, and thank you Calvin for relaying. I appreciate the
>> thorough review that got this to the right shape.
>>
>> Best regards,
>> Michael
>>
>>
>>> On 2. Jun 2026, at 08:36, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>>>
>>> On 2026-06-01 17:44:09 [-0700], Calvin Owens wrote:
>>>> From: Michael Byczkowski <by@by-online.de>
>>>>
>>>> Split the pps-gpio interrupt handler into a primary (hardirq) handler
>>>> that captures the PPS timestamp at interrupt entry, and a threaded
>>>> handler that processes the event. This produces the same two-part
>>>> handler structure on both PREEMPT_RT and non-RT kernels.
>>>>
>>>> On non-RT kernels the threaded portion runs immediately after the
>>>> primary, with no behavioral change compared to the previous
>>>> single-handler implementation.
>>>>
>>>> On PREEMPT_RT, where interrupt handlers are force-threaded by default,
>>>> the previous single-handler implementation captured the timestamp
>>>> inside the threaded portion, after IRQ-thread scheduling delay. With
>>>> the split, the timestamp is captured in true hardirq context as it is
>>>> on non-RT kernels, eliminating a significant source of PPS jitter on
>>>> RT systems.
>>>>
>>>> Tested-by: Michael Byczkowski <by@by-online.de>
>>>> Tested-by: Calvin Owens <calvin@wbinvd.org>
>>>> Acked-by: Rodolfo Giometti <giometti@enneenne.com>
>>>> Signed-off-by: Michael Byczkowski <by@by-online.de>
>>>> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
>>>> ---
>>>
>>> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>
>>> Sebastian
>>
> 


-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-06-02  0:44 ` [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler Calvin Owens
  2026-06-02  6:36   ` Sebastian Andrzej Siewior
@ 2026-07-05  1:06   ` Andrew Morton
  2026-07-05  1:35     ` Calvin Owens
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2026-07-05  1:06 UTC (permalink / raw)
  To: Calvin Owens
  Cc: linux-kernel, linux-rt-devel, Rodolfo Giometti,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Greg Kroah-Hartman, Eliav Farber, Michael Byczkowski,
	Ingo Molnar, David Laight, Thomas Gleixner

On Mon,  1 Jun 2026 17:44:09 -0700 Calvin Owens <calvin@wbinvd.org> wrote:

> Split the pps-gpio interrupt handler into a primary (hardirq) handler
> that captures the PPS timestamp at interrupt entry, and a threaded
> handler that processes the event. This produces the same two-part
> handler structure on both PREEMPT_RT and non-RT kernels.
> 
> On non-RT kernels the threaded portion runs immediately after the
> primary, with no behavioral change compared to the previous
> single-handler implementation.
> 
> On PREEMPT_RT, where interrupt handlers are force-threaded by default,
> the previous single-handler implementation captured the timestamp
> inside the threaded portion, after IRQ-thread scheduling delay. With
> the split, the timestamp is captured in true hardirq context as it is
> on non-RT kernels, eliminating a significant source of PPS jitter on
> RT systems.

Thanks, I'll add this to mm.git's mm-nonmm-unstable branch.  This means
it will get linux-next exposure immediately.

All being well, I'll later move this into the non-rebasing
mm-nonmm-stable branch with the intent to upstream it in the next merge
window.

Sashiko AI review might have found some issues, one of them
pre-existing.  Please check it out:

	https://sashiko.dev/#/patchset/2e32729029fbf6977ecf04665eb00f2efd3e2c17.1780359378.git.calvin@wbinvd.org


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-07-05  1:06   ` Andrew Morton
@ 2026-07-05  1:35     ` Calvin Owens
  2026-07-05 20:18       ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Calvin Owens @ 2026-07-05  1:35 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-rt-devel, Rodolfo Giometti,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Greg Kroah-Hartman, Eliav Farber, Michael Byczkowski,
	Ingo Molnar, David Laight, Thomas Gleixner

On Saturday 07/04 at 18:06 -0700, Andrew Morton wrote:
> Thanks, I'll add this to mm.git's mm-nonmm-unstable branch.  This means
> it will get linux-next exposure immediately.
> 
> All being well, I'll later move this into the non-rebasing
> mm-nonmm-stable branch with the intent to upstream it in the next merge
> window.
> 
> Sashiko AI review might have found some issues, one of them
> pre-existing.  Please check it out:
> 
> 	https://sashiko.dev/#/patchset/2e32729029fbf6977ecf04665eb00f2efd3e2c17.1780359378.git.calvin@wbinvd.org

Hi Andrew,

I've sent three patches which address all the sashiko issues:

    https://lore.kernel.org/lkml/c5c97c3b3c9d66010382094fd538e59a38f4aacf.1781289959.git.calvin@wbinvd.org/
    https://lore.kernel.org/lkml/672778c177ac9b6fdcb445e35c97ac4ca7d1149f.1780506611.git.calvin@wbinvd.org/
    https://lore.kernel.org/lkml/7a71b8e74bce9582d3286d31e6c85ac77465e421.1780108620.git.calvin@wbinvd.org/

I can resend those as a series if it saves any time, let me know (the
last one has a trivial conflict with the patch you just merged).

Thanks,
Calvin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler
  2026-07-05  1:35     ` Calvin Owens
@ 2026-07-05 20:18       ` Andrew Morton
  2026-07-06 17:19         ` [PATCH v2 next] pps-gpio: Remove dead capture_clear code Calvin Owens
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2026-07-05 20:18 UTC (permalink / raw)
  To: Calvin Owens
  Cc: linux-kernel, linux-rt-devel, Rodolfo Giometti,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Greg Kroah-Hartman, Eliav Farber, Michael Byczkowski,
	Ingo Molnar, David Laight, Thomas Gleixner

On Sat, 4 Jul 2026 18:35:53 -0700 Calvin Owens <calvin@wbinvd.org> wrote:

> On Saturday 07/04 at 18:06 -0700, Andrew Morton wrote:
> > Thanks, I'll add this to mm.git's mm-nonmm-unstable branch.  This means
> > it will get linux-next exposure immediately.
> > 
> > All being well, I'll later move this into the non-rebasing
> > mm-nonmm-stable branch with the intent to upstream it in the next merge
> > window.
> > 
> > Sashiko AI review might have found some issues, one of them
> > pre-existing.  Please check it out:
> > 
> > 	https://sashiko.dev/#/patchset/2e32729029fbf6977ecf04665eb00f2efd3e2c17.1780359378.git.calvin@wbinvd.org
> 
> Hi Andrew,
> 
> I've sent three patches which address all the sashiko issues:
> 
>     https://lore.kernel.org/lkml/c5c97c3b3c9d66010382094fd538e59a38f4aacf.1781289959.git.calvin@wbinvd.org/
>     https://lore.kernel.org/lkml/672778c177ac9b6fdcb445e35c97ac4ca7d1149f.1780506611.git.calvin@wbinvd.org/

OK, thanks, grabbed.

>     https://lore.kernel.org/lkml/7a71b8e74bce9582d3286d31e6c85ac77465e421.1780108620.git.calvin@wbinvd.org/
> 
> I can resend those as a series if it saves any time, let me know (the
> last one has a trivial conflict with the patch you just merged).

The last patch ("pps: pps-gpio: split IRQ handler into hardirq
timestamper + threaded handler") appears to be altering code which
isn't there any more.  So yes please, resend.  Please cc myself.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 next] pps-gpio: Remove dead capture_clear code
  2026-07-05 20:18       ` Andrew Morton
@ 2026-07-06 17:19         ` Calvin Owens
  0 siblings, 0 replies; 10+ messages in thread
From: Calvin Owens @ 2026-07-06 17:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rodolfo Giometti, Andrew Morton

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
Acked-by: Rodolfo Giometti <giometti@enneenne.com>
Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
Changes in v2:
* Rebased onto next-20260706 for Andrew

v1: https://lore.kernel.org/lkml/7a71b8e74bce9582d3286d31e6c85ac77465e421.1780108620.git.calvin@wbinvd.org/

 drivers/pps/clients/pps-gpio.c | 37 ++++------------------------------
 1 file changed, 4 insertions(+), 33 deletions(-)

diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 7d87481280b21..73ec2c7335e51 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -31,7 +31,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 */
 	struct pps_event_time ts;	/* timestamp captured in hardirq */
@@ -61,19 +60,8 @@ static irqreturn_t pps_gpio_irq_hardirq(int irq, void *data)
 static irqreturn_t pps_gpio_irq_thread(int irq, void *data)
 {
 	struct pps_gpio_device_data *info = data;
-	int rising_edge;
-
-	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, &info->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, &info->ts, PPS_CAPTURECLEAR, data);
-	else
-		dev_warn_ratelimited(&info->pps->dev, "IRQ did not trigger any PPS event\n");
+
+	pps_event(info->pps, &info->ts, PPS_CAPTUREASSERT, data);
 
 	return IRQ_HANDLED;
 }
@@ -89,11 +77,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 */
@@ -155,15 +138,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)
@@ -196,9 +172,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);
@@ -210,8 +183,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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-06 17:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-02  0:44 [PATCH v7 0/1] pps: improve PREEMPT_RT performance Calvin Owens
2026-06-02  0:44 ` [PATCH v7 1/1] pps: pps-gpio: split IRQ handler into hardirq timestamper + threaded handler Calvin Owens
2026-06-02  6:36   ` Sebastian Andrzej Siewior
2026-06-03 17:29     ` Michael Byczkowski
2026-07-03 18:20       ` Michael Byczkowski
2026-07-04  5:50         ` Rodolfo Giometti
2026-07-05  1:06   ` Andrew Morton
2026-07-05  1:35     ` Calvin Owens
2026-07-05 20:18       ` Andrew Morton
2026-07-06 17:19         ` [PATCH v2 next] pps-gpio: Remove dead capture_clear code Calvin Owens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox