* [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown
@ 2026-09-16 13:47 Eliav Farber
2026-09-16 13:47 ` [PATCH 1/2] dt-bindings: pps: pps-gpio: document optional idle pinctrl state Eliav Farber
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-16 13:47 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Fabio Estevam, Andrew Morton, Takashi Sakamoto, Eliav Farber,
devicetree, linux-kernel
Some boards route the PPS input GPIO through a pin controller and mux the
pins to a different function when the pps-gpio driver is not active.
The driver core already selects the "default" pinctrl state before probe,
so the pins can be muxed for GPIO/PPS use while the driver is bound without
any driver change. Nothing, however, hands the pins back when the driver
is unbound or the system is shut down (for example before kexec), leaving
them stuck in the GPIO mux for the next kernel.
This series lets pps-gpio select an optional "idle" pinctrl state in
remove() and shutdown(), so a board can describe the alternate mux there
and have it restored. It is a no-op for boards that do not describe an
"idle" state, and depends on CONFIG_PM (which performs the idle-state
lookup).
Patch 1 documents the optional "default"/"idle" pinctrl-names in the
binding; patch 2 implements the driver side.
Tested on an AL11 K2V6 JRD10 board: binding/unbinding each pps-gpio device
toggles the corresponding PBS pin-mux register between the GPIO function
and the alternate ec_ptp_trigger_in function as expected, and re-binding
restores the GPIO function via the core-applied "default" state.
Eliav Farber (2):
dt-bindings: pps: pps-gpio: document optional idle pinctrl state
pps: clients: gpio: release pins to idle state on remove and shutdown
.../devicetree/bindings/pps/pps-gpio.yaml | 15 ++++++++++++++-
drivers/pps/clients/pps-gpio.c | 19 +++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] dt-bindings: pps: pps-gpio: document optional idle pinctrl state
2026-09-16 13:47 [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown Eliav Farber
@ 2026-09-16 13:47 ` Eliav Farber
2026-09-16 13:47 ` [PATCH 2/2] pps: clients: gpio: release pins to idle state on remove and shutdown Eliav Farber
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-16 13:47 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Fabio Estevam, Andrew Morton, Takashi Sakamoto, Eliav Farber,
devicetree, linux-kernel
pps-gpio can now release its pins to an "idle" pinctrl state when the
driver is unbound or the system is shut down, in addition to the "default"
state that the driver core applies before probe.
Document the optional "default" and "idle" pinctrl-names and show both in
the example so boards can describe an alternate pin mux for when PPS is not
active.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
.../devicetree/bindings/pps/pps-gpio.yaml | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.yaml b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
index 383a838744eb..035c780ecd04 100644
--- a/Documentation/devicetree/bindings/pps/pps-gpio.yaml
+++ b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
@@ -28,6 +28,18 @@ properties:
description: Indicates a falling edge assert, when present. Rising edge if absent.
type: boolean
+ pinctrl-names:
+ description:
+ When the PPS input is muxed through a pin controller, name the "default"
+ state to select the PPS/GPIO function while the driver is bound. An
+ optional "idle" state, if present, is selected when the driver is
+ unbound or the system is shut down, handing the pins back to their
+ alternate function.
+ minItems: 1
+ items:
+ - const: default
+ - const: idle
+
required:
- compatible
- gpios
@@ -40,8 +52,9 @@ examples:
pps {
compatible = "pps-gpio";
- pinctrl-names = "default";
+ pinctrl-names = "default", "idle";
pinctrl-0 = <&pinctrl_pps>;
+ pinctrl-1 = <&pinctrl_pps_idle>;
gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
assert-falling-edge;
echo-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] pps: clients: gpio: release pins to idle state on remove and shutdown
2026-09-16 13:47 [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown Eliav Farber
2026-09-16 13:47 ` [PATCH 1/2] dt-bindings: pps: pps-gpio: document optional idle pinctrl state Eliav Farber
@ 2026-09-16 13:47 ` Eliav Farber
2026-09-16 16:48 ` [PATCH 0/2] pps-gpio: restore pin mux on unbind " Rodolfo Giometti
2026-09-16 18:26 ` [PATCH v2 " Eliav Farber
3 siblings, 0 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-16 13:47 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Fabio Estevam, Andrew Morton, Takashi Sakamoto, Eliav Farber,
devicetree, linux-kernel
Some boards route the PPS input GPIO through a pin controller and mux it
to another function when the pps-gpio driver is not bound. The driver core
already applies the "default" pinctrl state before probe, so the pins are
muxed for GPIO/PPS use while the driver is bound without any driver change.
Nothing, however, hands the pins back when the driver is unbound or the
system is shut down. Select the "idle" pinctrl state in both remove() and
shutdown() so a board can describe the alternate mux there and have it
restored, for example before kexec.
pinctrl_pm_select_idle_state() is a no-op for devices that have no pin
controller or no "idle" state, so boards that do not use this are
unaffected. The "idle" state is only looked up and selectable when
CONFIG_PM is enabled; without CONFIG_PM the helper compiles to a no-op and
the pins simply retain their "default" mux.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
drivers/pps/clients/pps-gpio.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 73ec2c7335e5..bdedbed82d82 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/pps_kernel.h>
#include <linux/gpio/consumer.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/list.h>
#include <linux/property.h>
#include <linux/timer.h>
@@ -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);
+}
+
static const struct of_device_id pps_gpio_dt_ids[] = {
{ .compatible = "pps-gpio", },
{ /* sentinel */ }
@@ -228,6 +246,7 @@ MODULE_DEVICE_TABLE(of, pps_gpio_dt_ids);
static struct platform_driver pps_gpio_driver = {
.probe = pps_gpio_probe,
.remove = pps_gpio_remove,
+ .shutdown = pps_gpio_shutdown,
.driver = {
.name = PPS_GPIO_NAME,
.of_match_table = pps_gpio_dt_ids,
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown
2026-09-16 13:47 [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown Eliav Farber
2026-09-16 13:47 ` [PATCH 1/2] dt-bindings: pps: pps-gpio: document optional idle pinctrl state Eliav Farber
2026-09-16 13:47 ` [PATCH 2/2] pps: clients: gpio: release pins to idle state on remove and shutdown Eliav Farber
@ 2026-09-16 16:48 ` Rodolfo Giometti
2026-09-16 18:25 ` Farber, Eliav
2026-09-16 18:26 ` [PATCH v2 " Eliav Farber
3 siblings, 1 reply; 17+ messages in thread
From: Rodolfo Giometti @ 2026-09-16 16:48 UTC (permalink / raw)
To: Eliav Farber, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Fabio Estevam, Andrew Morton, Takashi Sakamoto, devicetree, linux-kernel
On 16/09/2026 15:47, Eliav Farber wrote:
> This series lets pps-gpio select an optional "idle" pinctrl state in
> remove() and shutdown(), so a board can describe the alternate mux there
> and have it restored. It is a no-op for boards that do not describe an
> "idle" state, and depends on CONFIG_PM (which performs the idle-state
> lookup).
Thanks, the problem is real: the pinctrl core never reverts the mux on
unbind, so the pins stay stuck in the GPIO function for whatever comes
next, kexec included.
But "idle" does not mean "not bound", so are you sure this is the
right-thing(TM) to do? pinctrl-state.h documents it as the runtime PM
state and, AFAIK, that is how the rest of the tree uses it. If pps-gpio
ever grows a real runtime PM or a .suspend(), "idle" is already taken
with another meaning -- and 1/2 turns that choice into ABI.
Same question for the CONFIG_PM dependency: why should a CONFIG_PM=n
kernel not get this? A PPS box built without PM is not an odd
configuration, and there the board describes an "idle" state and
nothing happens, silently.
Wouldn't looking the state up in the driver (devm_pinctrl_get() +
pinctrl_lookup_state() + pinctrl_select_state()) avoid both, and leave
you free to pick a name that says what it means?
One thing that does not depend on any of the above: your shutdown()
changes the mux but shuts nothing down. The IRQ is still requested
(request_irq() here is not devm-managed) and the echo timer may still
be armed, so a timer callback can still poke a pin that by then belongs
to somebody else, and the PPS handler stays attached to a line that
other function is now driving. And device_shutdown() is not the end of
the road: the kernel keeps running to load and start the kexec image,
which is the case you are after. Shouldn't it tear down in the same
order remove() does, free_irq() and timer_delete_sync() first and the
mux change last?
Ciao,
Rodolfo
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown
2026-09-16 16:48 ` [PATCH 0/2] pps-gpio: restore pin mux on unbind " Rodolfo Giometti
@ 2026-09-16 18:25 ` Farber, Eliav
0 siblings, 0 replies; 17+ messages in thread
From: Farber, Eliav @ 2026-09-16 18:25 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Fabio Estevam, Andrew Morton, Takashi Sakamoto, devicetree, linux-kernel
> On 16/09/2026 16:48, Rodolfo Giometti wrote:
> But "idle" does not mean "not bound", so are you sure this is the
> right-thing(TM) to do? pinctrl-state.h documents it as the runtime PM
> state and, AFAIK, that is how the rest of the tree uses it. If pps-gpio
> ever grows a real runtime PM or a .suspend(), "idle" is already taken
> with another meaning -- and 1/2 turns that choice into ABI.
Right. Renamed to "inactive" in v2.
> Same question for the CONFIG_PM dependency: why should a CONFIG_PM=n
> kernel not get this? A PPS box built without PM is not an odd
> configuration, and there the board describes an "idle" state and nothing
> happens, silently.
>
> Wouldn't looking the state up in the driver (devm_pinctrl_get() +
> pinctrl_lookup_state() + pinctrl_select_state()) avoid both, and leave
> you free to pick a name that says what it means?
Done. v2 looks the state up in the driver and selects it with
pinctrl_select_state(), dropping the CONFIG_PM dependency.
v2 also requires "default" when "inactive" is present and rejects the
mismatch at probe.
> One thing that does not depend on any of the above: your shutdown()
> changes the mux but shuts nothing down. The IRQ is still requested
> (request_irq() here is not devm-managed) and the echo timer may still be
> armed, so a timer callback can still poke a pin that by then belongs to
> somebody else, and the PPS handler stays attached to a line that other
> function is now driving. And device_shutdown() is not the end of the
> road: the kernel keeps running to load and start the kexec image, which
> is the case you are after. Shouldn't it tear down in the same order
> remove() does, free_irq() and timer_delete_sync() first and the mux
> change last?
Fixed. shutdown() now does free_irq() and timer_delete_sync() first, mux
change last. It does not call pps_unregister_source() -- that is a
remove-time concern, not needed to quiesce the hardware for kexec. Say if
you want it there anyway.
Thanks,
Eliav
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/2] pps-gpio: restore pin mux on unbind and shutdown
2026-09-16 13:47 [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown Eliav Farber
` (2 preceding siblings ...)
2026-09-16 16:48 ` [PATCH 0/2] pps-gpio: restore pin mux on unbind " Rodolfo Giometti
@ 2026-09-16 18:26 ` Eliav Farber
2026-09-16 18:26 ` [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
` (2 more replies)
3 siblings, 3 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-16 18:26 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
Some boards route the PPS input GPIO through a pin controller and mux the
pins to a different function when the pps-gpio driver is not active.
The driver core already selects the "default" pinctrl state before probe,
so the pins can be muxed for GPIO/PPS use while the driver is bound without
any driver change. Nothing, however, hands the pins back when the driver
is unbound or the system is shut down (for example before kexec), leaving
them stuck in the GPIO mux for the next kernel.
This series lets pps-gpio select an optional "inactive" pinctrl state in
remove() and shutdown(), so a board can describe the alternate mux there
and have it restored. The state is looked up and selected by the driver
itself (devm_pinctrl_get() + pinctrl_lookup_state() + pinctrl_select_
state()), so its meaning is unambiguous and it does not depend on
CONFIG_PM. It is a no-op for boards that do not describe an "inactive"
state.
Patch 1 documents the optional "default"/"inactive" pinctrl-names in the
binding; patch 2 implements the driver side.
Tested on an AL11 K2V6 JRD10 board: binding/unbinding each pps-gpio device
toggles the corresponding PBS pin-mux register between the GPIO function
and the alternate ec_ptp_trigger_in function as expected, and re-binding
restores the GPIO function via the core-applied "default" state.
Changes in v2 (all addressing Rodolfo Giometti's review):
- Rename the released state from "idle" to "inactive". "idle" is the
runtime-PM state in pinctrl-state.h; overloading it for "driver not
active" would clash with any future runtime PM or .suspend() and was
being baked into the binding as ABI. "inactive" is not a well-known
state, so no generic PM helper will ever auto-select it -- the driver
drives it explicitly
- Look the state up in the driver (devm_pinctrl_get() +
pinctrl_lookup_state() + pinctrl_select_state()) instead of
pinctrl_pm_select_idle_state(), removing the CONFIG_PM dependency so a
CONFIG_PM=n kernel that describes an "inactive" state now honors it
instead of silently doing nothing
- Fix shutdown(): tear down in the same order as remove() -- free_irq()
and timer_delete_sync() first, the mux change last -- so no IRQ or echo
timer callback can drive a pin after it has been handed back to another
function; shutdown() no longer changes the mux while the hardware is
still live
- Require a "default" state whenever "inactive" is present and reject the
mismatch, rather than releasing pins that were never put into a defined
PPS state
Link: https://lore.kernel.org/all/20260916134744.46354-1-farbere@amazon.com/ [v1]
Eliav Farber (2):
dt-bindings: pps: pps-gpio: document optional pinctrl states
pps: clients: gpio: release pins to an inactive state on remove and
shutdown
.../devicetree/bindings/pps/pps-gpio.yaml | 16 +++-
drivers/pps/clients/pps-gpio.c | 75 +++++++++++++++++++
2 files changed, 90 insertions(+), 1 deletion(-)
base-commit: 9b87fdc9af2fbfcdb5c24a64139685ef80f6573f
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states
2026-09-16 18:26 ` [PATCH v2 " Eliav Farber
@ 2026-09-16 18:26 ` Eliav Farber
2026-09-17 7:13 ` Rodolfo Giometti
2026-09-16 18:26 ` [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
2026-09-17 7:56 ` [PATCH v3 0/3] pps-gpio: restore pin mux on unbind " Eliav Farber
2 siblings, 1 reply; 17+ messages in thread
From: Eliav Farber @ 2026-09-16 18:26 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
When the PPS input GPIO is routed through a pin controller, a board may
need to mux those pins to a different function while pps-gpio is not
driving PPS (for example after the driver is unbound or across a kexec).
Document the optional "default" and "inactive" pinctrl-names and show
both in the example. The "default" state selects the PPS/GPIO function
and is applied by the driver core before probe; the optional "inactive"
state, when present, describes the mux to restore when the driver is
unbound or the system is shut down. As "inactive" is only the state to
restore after "default", it may only appear as the second entry, so a
"default" state is required whenever "inactive" is given.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
Changes in v2:
- Rename the released state from "idle" to "inactive"; document that
"inactive" may only be the second entry, so a "default" state is
required whenever "inactive" is given
.../devicetree/bindings/pps/pps-gpio.yaml | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.yaml b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
index 383a838744eb..7d11ec4656ee 100644
--- a/Documentation/devicetree/bindings/pps/pps-gpio.yaml
+++ b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
@@ -28,6 +28,19 @@ properties:
description: Indicates a falling edge assert, when present. Rising edge if absent.
type: boolean
+ pinctrl-names:
+ description:
+ When the PPS input is muxed through a pin controller, the "default"
+ state selects the PPS/GPIO function and is applied by the driver core
+ before probe. The optional "inactive" state, when present, is selected
+ when the driver is unbound or the system is shut down, handing the pins
+ back to their alternate function. As "inactive" is the state to restore
+ after "default", it may only appear as the second entry.
+ minItems: 1
+ items:
+ - const: default
+ - const: inactive
+
required:
- compatible
- gpios
@@ -40,8 +53,9 @@ examples:
pps {
compatible = "pps-gpio";
- pinctrl-names = "default";
+ pinctrl-names = "default", "inactive";
pinctrl-0 = <&pinctrl_pps>;
+ pinctrl-1 = <&pinctrl_pps_inactive>;
gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
assert-falling-edge;
echo-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown
2026-09-16 18:26 ` [PATCH v2 " Eliav Farber
2026-09-16 18:26 ` [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
@ 2026-09-16 18:26 ` Eliav Farber
2026-09-17 7:14 ` Rodolfo Giometti
2026-09-17 7:56 ` [PATCH v3 0/3] pps-gpio: restore pin mux on unbind " Eliav Farber
2 siblings, 1 reply; 17+ messages in thread
From: Eliav Farber @ 2026-09-16 18:26 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
Some boards route the PPS input GPIO through a pin controller and need to
mux it to another function when pps-gpio is not driving PPS. The driver
core applies the "default" pinctrl state before probe, so the pins are
muxed for GPIO/PPS use while the driver is bound. Nothing, however, hands
the pins back when the driver is unbound or the system is shut down, so
they stay stuck in the GPIO function for whatever runs next, kexec
included.
Look up an optional "inactive" pinctrl state in probe via
devm_pinctrl_get() and pinctrl_lookup_state(), and select it with
pinctrl_select_state() in remove() and shutdown(). The state is looked up
and selected by the driver itself rather than reusing the runtime-PM
"idle"/"sleep" states, so its meaning is unambiguous and it does not
depend on CONFIG_PM. Boards that do not describe an "inactive" state are
unaffected.
Since "inactive" is only meaningful as the mux to restore after the
core-applied "default" state, reject an "inactive" state that is not
paired with a "default" one rather than releasing pins that were never
put into a defined PPS state.
The mux must not change while something can still drive the pins. On
shutdown() the requested IRQ and the echo timer would otherwise outlive
the mux change -- device_shutdown() is not the end of the road, the
kernel keeps running to load and start the kexec image -- so a timer
callback or the PPS handler could poke a line that by then belongs to
another function. Tear down in the same order as remove(): free_irq() and
timer_delete_sync() first, and the mux change last. shutdown() does not
unregister the PPS source, which is a remove-time concern.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
Changes in v2:
- Rename the released state from "idle" to "inactive"
- Look the state up in the driver with devm_pinctrl_get() +
pinctrl_lookup_state() + pinctrl_select_state() instead of
pinctrl_pm_select_idle_state(), removing the CONFIG_PM dependency
- Fix shutdown() to free_irq() and timer_delete_sync() before the mux
change, matching remove(), so no IRQ or timer callback can drive a pin
after it has been handed back
- Require a "default" state whenever "inactive" is present and reject the
mismatch
drivers/pps/clients/pps-gpio.c | 75 ++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 73ec2c7335e5..e619c7bb2f78 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/pps_kernel.h>
#include <linux/gpio/consumer.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/list.h>
#include <linux/property.h>
#include <linux/timer.h>
@@ -30,6 +31,8 @@ struct pps_gpio_device_data {
struct gpio_desc *gpio_pin; /* GPIO port descriptors */
struct gpio_desc *echo_pin;
struct timer_list echo_timer; /* timer to reset echo active state */
+ struct pinctrl *pinctrl; /* pin control handle */
+ struct pinctrl_state *pins_inactive; /* pins released when unbound */
bool assert_falling_edge;
unsigned int echo_active_ms; /* PPS echo active duration */
unsigned long echo_timeout; /* timer timeout value in jiffies */
@@ -96,6 +99,51 @@ static void pps_gpio_echo_timer_callback(struct timer_list *t)
gpiod_set_value(info->echo_pin, 0);
}
+/*
+ * Look up the optional "inactive" pinctrl state: the mux to restore when
+ * pps-gpio is unbound or the system is shut down. It is only meaningful
+ * paired with a "default" state, which the driver core applies before probe
+ * to mux the pins for PPS use. A board that describes neither is unaffected;
+ * one that describes "inactive" without "default" is rejected, since
+ * releasing pins that were never put into a defined PPS state is incoherent.
+ */
+static int pps_gpio_get_pins(struct device *dev)
+{
+ struct pps_gpio_device_data *data = dev_get_drvdata(dev);
+ struct pinctrl_state *pins_default;
+
+ data->pinctrl = devm_pinctrl_get(dev);
+ if (IS_ERR(data->pinctrl))
+ return dev_err_probe(dev, PTR_ERR(data->pinctrl),
+ "failed to get pinctrl\n");
+
+ /* The "inactive" state is optional; without it there is nothing to do. */
+ data->pins_inactive = pinctrl_lookup_state(data->pinctrl, "inactive");
+ if (IS_ERR(data->pins_inactive)) {
+ data->pins_inactive = NULL;
+ return 0;
+ }
+
+ /* "inactive" requires a "default" state to release back from. */
+ pins_default = pinctrl_lookup_state(data->pinctrl, "default");
+ if (IS_ERR(pins_default))
+ return dev_err_probe(dev, PTR_ERR(pins_default),
+ "\"inactive\" pinctrl state requires a \"default\" state\n");
+
+ return 0;
+}
+
+/*
+ * Release the pins to their "inactive" state, if the board describes one, so
+ * they are handed back to whatever function uses them while pps-gpio is not
+ * driving PPS. Boards without an "inactive" state are unaffected.
+ */
+static void pps_gpio_release_pins(struct pps_gpio_device_data *data)
+{
+ if (data->pins_inactive)
+ pinctrl_select_state(data->pinctrl, data->pins_inactive);
+}
+
static int pps_gpio_setup(struct device *dev)
{
struct pps_gpio_device_data *data = dev_get_drvdata(dev);
@@ -161,6 +209,11 @@ static int pps_gpio_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /* pinctrl setup (optional states) */
+ ret = pps_gpio_get_pins(dev);
+ if (ret)
+ return ret;
+
/* IRQ setup */
ret = gpiod_to_irq(data->gpio_pin);
if (ret < 0) {
@@ -216,9 +269,30 @@ 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 last, once nothing can drive them anymore */
+ pps_gpio_release_pins(data);
dev_info(&pdev->dev, "removed IRQ %d as PPS source\n", data->irq);
}
+static void pps_gpio_shutdown(struct platform_device *pdev)
+{
+ struct pps_gpio_device_data *data = platform_get_drvdata(pdev);
+
+ /*
+ * Quiesce the hardware before touching the mux: stop the IRQ and the
+ * echo timer first so nothing can drive the pins, then hand them back
+ * to their "inactive" function. The kernel keeps running after
+ * device_shutdown() (for example to load and start a kexec image), so
+ * the pins must not be released while an IRQ or timer callback can
+ * still reach them. The PPS source is left registered; unregistering
+ * it is a remove-time concern and is unnecessary on shutdown.
+ */
+ free_irq(data->irq, data);
+ timer_delete_sync(&data->echo_timer);
+ gpiod_set_value(data->echo_pin, 0);
+ pps_gpio_release_pins(data);
+}
+
static const struct of_device_id pps_gpio_dt_ids[] = {
{ .compatible = "pps-gpio", },
{ /* sentinel */ }
@@ -228,6 +302,7 @@ MODULE_DEVICE_TABLE(of, pps_gpio_dt_ids);
static struct platform_driver pps_gpio_driver = {
.probe = pps_gpio_probe,
.remove = pps_gpio_remove,
+ .shutdown = pps_gpio_shutdown,
.driver = {
.name = PPS_GPIO_NAME,
.of_match_table = pps_gpio_dt_ids,
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states
2026-09-16 18:26 ` [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
@ 2026-09-17 7:13 ` Rodolfo Giometti
2026-09-17 7:48 ` Farber, Eliav
0 siblings, 1 reply; 17+ messages in thread
From: Rodolfo Giometti @ 2026-09-17 7:13 UTC (permalink / raw)
To: Eliav Farber, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, devicetree, linux-gpio, linux-kernel
On 16/09/2026 20:26, Eliav Farber wrote:
> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.yaml b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
> index 383a838744eb..7d11ec4656ee 100644
> --- a/Documentation/devicetree/bindings/pps/pps-gpio.yaml
> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
> @@ -28,6 +28,19 @@ properties:
> description: Indicates a falling edge assert, when present. Rising edge if absent.
> type: boolean
>
> + pinctrl-names:
> + description:
> + When the PPS input is muxed through a pin controller, the "default"
> + state selects the PPS/GPIO function and is applied by the driver core
> + before probe. The optional "inactive" state, when present, is selected
> + when the driver is unbound or the system is shut down, handing the pins
> + back to their alternate function. As "inactive" is the state to restore
> + after "default", it may only appear as the second entry.
The driver looks the states up by name, not by index, and only
requires that "default" exists. Why you say that?
> + minItems: 1
> + items:
> + - const: default
> + - const: inactive
AFAIK the tuple rules out ["default", "sleep"] and ["init", "default"]
for good, and this is ABI.
> +
> required:
> - compatible
> - gpios
> @@ -40,8 +53,9 @@ examples:
>
> pps {
> compatible = "pps-gpio";
> - pinctrl-names = "default";
> + pinctrl-names = "default", "inactive";
> pinctrl-0 = <&pinctrl_pps>;
> + pinctrl-1 = <&pinctrl_pps_inactive>;
> gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> assert-falling-edge;
> echo-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
Ciao,
Rodolfo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown
2026-09-16 18:26 ` [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
@ 2026-09-17 7:14 ` Rodolfo Giometti
2026-09-17 7:52 ` Farber, Eliav
0 siblings, 1 reply; 17+ messages in thread
From: Rodolfo Giometti @ 2026-09-17 7:14 UTC (permalink / raw)
To: Eliav Farber, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, devicetree, linux-gpio, linux-kernel
On 16/09/2026 20:26, Eliav Farber wrote:
> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index 73ec2c7335e5..e619c7bb2f78 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c
> @@ -17,6 +17,7 @@
> #include <linux/slab.h>
> #include <linux/pps_kernel.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/pinctrl/consumer.h>
> #include <linux/list.h>
> #include <linux/property.h>
> #include <linux/timer.h>
> @@ -30,6 +31,8 @@ struct pps_gpio_device_data {
> struct gpio_desc *gpio_pin; /* GPIO port descriptors */
> struct gpio_desc *echo_pin;
> struct timer_list echo_timer; /* timer to reset echo active state */
> + struct pinctrl *pinctrl; /* pin control handle */
> + struct pinctrl_state *pins_inactive; /* pins released when unbound */
> bool assert_falling_edge;
> unsigned int echo_active_ms; /* PPS echo active duration */
> unsigned long echo_timeout; /* timer timeout value in jiffies */
> @@ -96,6 +99,51 @@ static void pps_gpio_echo_timer_callback(struct timer_list *t)
> gpiod_set_value(info->echo_pin, 0);
> }
>
> +/*
> + * Look up the optional "inactive" pinctrl state: the mux to restore when
> + * pps-gpio is unbound or the system is shut down. It is only meaningful
> + * paired with a "default" state, which the driver core applies before probe
> + * to mux the pins for PPS use. A board that describes neither is unaffected;
> + * one that describes "inactive" without "default" is rejected, since
> + * releasing pins that were never put into a defined PPS state is incoherent.
> + */
> +static int pps_gpio_get_pins(struct device *dev)
> +{
> + struct pps_gpio_device_data *data = dev_get_drvdata(dev);
> + struct pinctrl_state *pins_default;
> +
> + data->pinctrl = devm_pinctrl_get(dev);
> + if (IS_ERR(data->pinctrl))
> + return dev_err_probe(dev, PTR_ERR(data->pinctrl),
> + "failed to get pinctrl\n");
> +
> + /* The "inactive" state is optional; without it there is nothing to do. */
> + data->pins_inactive = pinctrl_lookup_state(data->pinctrl, "inactive");
> + if (IS_ERR(data->pins_inactive)) {
> + data->pins_inactive = NULL;
> + return 0;
> + }
> +
> + /* "inactive" requires a "default" state to release back from. */
> + pins_default = pinctrl_lookup_state(data->pinctrl, "default");
> + if (IS_ERR(pins_default))
> + return dev_err_probe(dev, PTR_ERR(pins_default),
> + "\"inactive\" pinctrl state requires a \"default\" state\n");
> +
> + return 0;
> +}
pinctrl_dt_to_map() returns -ENODEV when the node has no pinctrl-0 and
create_pinctrl() forwards it. So this fails the probe on every pps-gpio
board that describes no pinctrl at all, which is most of them; your
setup has one, so the test does not show it. Shouldn't -ENODEV simply
mean "no pinctrl here, nothing to do"?
> +
> +/*
> + * Release the pins to their "inactive" state, if the board describes one, so
> + * they are handed back to whatever function uses them while pps-gpio is not
> + * driving PPS. Boards without an "inactive" state are unaffected.
> + */
> +static void pps_gpio_release_pins(struct pps_gpio_device_data *data)
> +{
> + if (data->pins_inactive)
> + pinctrl_select_state(data->pinctrl, data->pins_inactive);
> +}
The return value is the only sign that the mux was not restored, which
is what the whole series is for. Why drop it?
> +
> static int pps_gpio_setup(struct device *dev)
> {
> struct pps_gpio_device_data *data = dev_get_drvdata(dev);
> @@ -161,6 +209,11 @@ static int pps_gpio_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + /* pinctrl setup (optional states) */
> + ret = pps_gpio_get_pins(dev);
> + if (ret)
> + return ret;
> +
> /* IRQ setup */
> ret = gpiod_to_irq(data->gpio_pin);
> if (ret < 0) {
> @@ -216,9 +269,30 @@ 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 last, once nothing can drive them anymore */
> + pps_gpio_release_pins(data);
> dev_info(&pdev->dev, "removed IRQ %d as PPS source\n", data->irq);
> }
>
> +static void pps_gpio_shutdown(struct platform_device *pdev)
> +{
> + struct pps_gpio_device_data *data = platform_get_drvdata(pdev);
> +
> + /*
> + * Quiesce the hardware before touching the mux: stop the IRQ and the
> + * echo timer first so nothing can drive the pins, then hand them back
> + * to their "inactive" function. The kernel keeps running after
> + * device_shutdown() (for example to load and start a kexec image), so
> + * the pins must not be released while an IRQ or timer callback can
> + * still reach them. The PPS source is left registered; unregistering
> + * it is a remove-time concern and is unnecessary on shutdown.
> + */
> + free_irq(data->irq, data);
> + timer_delete_sync(&data->echo_timer);
> + gpiod_set_value(data->echo_pin, 0);
> + pps_gpio_release_pins(data);
> +}
> +
> static const struct of_device_id pps_gpio_dt_ids[] = {
> { .compatible = "pps-gpio", },
> { /* sentinel */ }
> @@ -228,6 +302,7 @@ MODULE_DEVICE_TABLE(of, pps_gpio_dt_ids);
> static struct platform_driver pps_gpio_driver = {
> .probe = pps_gpio_probe,
> .remove = pps_gpio_remove,
> + .shutdown = pps_gpio_shutdown,
> .driver = {
> .name = PPS_GPIO_NAME,
> .of_match_table = pps_gpio_dt_ids,
Ciao,
Rodolfo
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states
2026-09-17 7:13 ` Rodolfo Giometti
@ 2026-09-17 7:48 ` Farber, Eliav
0 siblings, 0 replies; 17+ messages in thread
From: Farber, Eliav @ 2026-09-17 7:48 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, devicetree, linux-gpio, linux-kernel
On 16/09/2026, Rodolfo Giometti wrote:
>> + back to their alternate function. As "inactive" is the state to restore
>> + after "default", it may only appear as the second entry.
>
> The driver looks the states up by name, not by index, and only requires
> that "default" exists. Why you say that?
You're right, that was wrong. The driver looks both states up by name
(pinctrl_lookup_state()), so "inactive" can be at any index; it only
requires that a "default" state also exists. I'll drop the "second
entry" wording.
>> + minItems: 1
>> + items:
>> + - const: default
>> + - const: inactive
>
> AFAIK the tuple rules out ["default", "sleep"] and ["init", "default"]
> for good, and this is ABI.
Agreed, the tuple is too strict and would bake that in. In v3 I drop it
and only assert that a "default" state is present:
minItems: 1
contains:
const: default
so ["default", "sleep"], ["init", "default"], etc. all remain valid, and
the description just notes that an optional "inactive" state, if present,
requires "default" and may appear in any position.
Thanks,
Eliav
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown
2026-09-17 7:14 ` Rodolfo Giometti
@ 2026-09-17 7:52 ` Farber, Eliav
0 siblings, 0 replies; 17+ messages in thread
From: Farber, Eliav @ 2026-09-17 7:52 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, devicetree, linux-gpio, linux-kernel
On 17/09/2026 10:14, Rodolfo Giometti wrote:
>> +static int pps_gpio_get_pins(struct device *dev) {
>> + data->pinctrl = devm_pinctrl_get(dev);
>> + if (IS_ERR(data->pinctrl))
>> + return dev_err_probe(dev, PTR_ERR(data->pinctrl),
>> + "failed to get pinctrl\n");
>
> pinctrl_dt_to_map() returns -ENODEV when the node has no pinctrl-0 and
> create_pinctrl() forwards it. So this fails the probe on every pps-gpio
> board that describes no pinctrl at all, which is most of them; your
> setup has one, so the test does not show it. Shouldn't -ENODEV simply
> mean "no pinctrl here, nothing to do"?
Agreed, that's a real regression for boards without pinctrl. In v3
-ENODEV is treated as "no pinctrl described", not an error, and
everything else (including -EPROBE_DEFER) is still propagated:
data->pinctrl = devm_pinctrl_get(dev);
if (IS_ERR(data->pinctrl)) {
/*
* A DT device without "pinctrl-0" yields -ENODEV, which
* is not an error here; propagate anything else.
*/
if (PTR_ERR(data->pinctrl) == -ENODEV) {
data->pinctrl = NULL;
return 0;
}
return dev_err_probe(dev, PTR_ERR(data->pinctrl),
"failed to get pinctrl\n");
}
I've also tested a pps-gpio node with no pinctrl at all to confirm probe,
remove and shutdown behave as before.
>> +static void pps_gpio_release_pins(struct pps_gpio_device_data *data)
>> +{
>> + if (data->pins_inactive)
>> + pinctrl_select_state(data->pinctrl, data->pins_inactive);
>> +}
>
> The return value is the only sign that the mux was not restored, which
> is what the whole series is for. Why drop it?
You're right. In v3 the failure is logged; the helper now takes the
struct device so it can warn (remove()/shutdown() are void and the probe
error path must keep the original error, so it warns rather than
propagates):
static void pps_gpio_release_pins(struct device *dev)
{
struct pps_gpio_device_data *data = dev_get_drvdata(dev);
int ret;
if (!data->pins_inactive)
return;
ret = pinctrl_select_state(data->pinctrl, data->pins_inactive);
if (ret)
dev_warn(dev, "failed to select inactive pinctrl state: %d\n",
ret);
}
Thanks,
Eliav
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 0/3] pps-gpio: restore pin mux on unbind and shutdown
2026-09-16 18:26 ` [PATCH v2 " Eliav Farber
2026-09-16 18:26 ` [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
2026-09-16 18:26 ` [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
@ 2026-09-17 7:56 ` Eliav Farber
2026-09-17 7:56 ` [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes Eliav Farber
` (2 more replies)
2 siblings, 3 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-17 7:56 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
Some boards route the PPS input GPIO through a pin controller and mux the
pins to a different function when the pps-gpio driver is not active.
The driver core already selects the "default" pinctrl state before probe,
so the pins can be muxed for GPIO/PPS use while the driver is bound without
any driver change. Nothing, however, hands the pins back when the driver
is unbound or the system is shut down (for example before kexec), leaving
them stuck in the GPIO mux for the next kernel.
This series lets pps-gpio select an optional "inactive" pinctrl state in
remove() and shutdown(), so a board can describe the alternate mux there
and have it restored. The state is looked up and selected by the driver
itself (devm_pinctrl_get() + pinctrl_lookup_state() + pinctrl_select_
state()), so its meaning is unambiguous and it does not depend on
CONFIG_PM. It is a no-op for boards that do not describe an "inactive"
state.
Patch 1 is a small preparatory fix to propagate the gpiod_to_irq() and
request_threaded_irq() error codes (rather than a hardcoded -EINVAL) on
the probe error paths that patch 3 then converts to gotos. Patch 2
documents the optional "default"/"inactive" pinctrl-names in the binding;
patch 3 implements the driver side.
Tested on an AL11 K2V6 JRD10 board: binding/unbinding each pps-gpio device
toggles the corresponding PBS pin-mux register between the GPIO function
and the alternate ec_ptp_trigger_in function as expected, and re-binding
restores the GPIO function via the core-applied "default" state. Reading
the mux register with the devices left unbound confirms the "inactive"
mux persists. Also tested with a pps-gpio node that describes no pinctrl
at all, where probe, remove and shutdown behave as before.
Changes in v3 (addressing Sashiko's and Rodolfo Giometti's
review):
- New preparatory patch 1: propagate the gpiod_to_irq() and
request_threaded_irq() error codes instead of overwriting them with
-EINVAL, and log the errno on the request_threaded_irq() failure. The
pinctrl patch only converts those returns into gotos, so fixing the
discarded errors separately keeps them out of the pinctrl change
- Do not constrain pinctrl-names to a fixed ["default", "inactive"] tuple
in the binding. The driver looks the states up by name, so "inactive"
may appear in any position and other states may coexist; the binding
now only requires that a "default" state exists
- Treat -ENODEV from devm_pinctrl_get() as "no pinctrl described" rather
than a probe failure. A DT device without a "pinctrl-0" property gets
-ENODEV from the pinctrl core; that is expected, not an error. Real
errors, including -EPROBE_DEFER, are still propagated
- Restore the "inactive" mux on probe failure too. The error paths after
pps_gpio_get_pins() now go through a new err_release_pins label, so a
probe that fails in gpiod_to_irq(), pps_register_source() or
request_threaded_irq() no longer leaves the pins stuck in the
core-applied "default" state
- Warn if applying the "inactive" state fails rather than ignoring the
pinctrl_select_state() return silently
- Trim and de-duplicate the comments added in v2
Changes in v2 (all addressing Rodolfo Giometti's review):
- Rename the released state from "idle" to "inactive". "idle" is the
runtime-PM state in pinctrl-state.h; overloading it for "driver not
active" would clash with any future runtime PM or .suspend() and was
being baked into the binding as ABI. "inactive" is not a well-known
state, so no generic PM helper will ever auto-select it -- the driver
drives it explicitly
- Look the state up in the driver (devm_pinctrl_get() +
pinctrl_lookup_state() + pinctrl_select_state()) instead of
pinctrl_pm_select_idle_state(), removing the CONFIG_PM dependency so a
CONFIG_PM=n kernel that describes an "inactive" state now honors it
instead of silently doing nothing
- Fix shutdown(): tear down in the same order as remove() -- free_irq()
and timer_delete_sync() first, the mux change last -- so no IRQ or echo
timer callback can drive a pin after it has been handed back to another
function; shutdown() no longer changes the mux while the hardware is
still live
- Require a "default" state whenever "inactive" is present and reject the
mismatch, rather than releasing pins that were never put into a defined
PPS state
Link: https://lore.kernel.org/all/20260916134744.46354-1-farbere@amazon.com/ [v1]
Link: https://lore.kernel.org/all/20260916182641.9768-1-farbere@amazon.com/ [v2]
Eliav Farber (3):
pps: clients: gpio: propagate probe error codes
dt-bindings: pps: pps-gpio: document optional pinctrl states
pps: clients: gpio: release pins to an inactive state on remove and
shutdown
.../devicetree/bindings/pps/pps-gpio.yaml | 16 ++-
drivers/pps/clients/pps-gpio.c | 102 +++++++++++++++++-
2 files changed, 113 insertions(+), 5 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes
2026-09-17 7:56 ` [PATCH v3 0/3] pps-gpio: restore pin mux on unbind " Eliav Farber
@ 2026-09-17 7:56 ` Eliav Farber
2026-09-17 9:58 ` Bartosz Golaszewski
2026-09-17 7:56 ` [PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
2026-09-17 7:56 ` [PATCH v3 3/3] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
2 siblings, 1 reply; 17+ messages in thread
From: Eliav Farber @ 2026-09-17 7:56 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
On the two probe error paths that map and request the interrupt, probe
overwrote the error from gpiod_to_irq() and request_threaded_irq() with a
hardcoded -EINVAL, hiding meaningful codes such as -EBUSY, -ENOMEM or
-EPROBE_DEFER from the caller. The request_threaded_irq() failure message
also logged the IRQ number but not the errno.
Return the actual error code from both paths, and add the errno to the
request_threaded_irq() failure message.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
Changes in v3:
- New patch, split out of the pinctrl change: while converting the probe
error paths to a goto, Takashi Sakamoto noted that the hardcoded -EINVAL
discards the real gpiod_to_irq()/request_threaded_irq() error, so fix
that separately first
drivers/pps/clients/pps-gpio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 73ec2c7335e5..038c55c5f7d4 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -165,7 +165,7 @@ static int pps_gpio_probe(struct platform_device *pdev)
ret = gpiod_to_irq(data->gpio_pin);
if (ret < 0) {
dev_err(dev, "failed to map GPIO to IRQ: %d\n", ret);
- return -EINVAL;
+ return ret;
}
data->irq = ret;
@@ -197,8 +197,8 @@ static int pps_gpio_probe(struct platform_device *pdev)
data->info.name, data);
if (ret) {
pps_unregister_source(data->pps);
- dev_err(dev, "failed to acquire IRQ %d\n", data->irq);
- return -EINVAL;
+ dev_err(dev, "failed to acquire IRQ %d: %d\n", data->irq, ret);
+ return ret;
}
dev_dbg(&data->pps->dev, "Registered IRQ %d as PPS source\n",
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states
2026-09-17 7:56 ` [PATCH v3 0/3] pps-gpio: restore pin mux on unbind " Eliav Farber
2026-09-17 7:56 ` [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes Eliav Farber
@ 2026-09-17 7:56 ` Eliav Farber
2026-09-17 7:56 ` [PATCH v3 3/3] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
2 siblings, 0 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-17 7:56 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
When the PPS input GPIO is routed through a pin controller, a board may
need to mux those pins to a different function while pps-gpio is not
driving PPS (for example after the driver is unbound or across a kexec).
Document the optional "default" and "inactive" pinctrl-names and show
both in the example. The "default" state selects the PPS/GPIO function
and is applied by the driver core before probe; the optional "inactive"
state, when present, describes the mux to restore when the driver is
unbound or the system is shut down. The driver looks the states up by
name, so "inactive" may appear in any position; it only requires that a
"default" state also exists.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
Changes in v3:
- Do not constrain pinctrl-names to a fixed ["default", "inactive"]
tuple. The driver looks the states up by name, so "inactive" may
appear in any position and other states may coexist; only require
(via "contains") that a "default" state exists, and reword the
description accordingly
Changes in v2:
- Rename the released state from "idle" to "inactive"
.../devicetree/bindings/pps/pps-gpio.yaml | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.yaml b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
index 383a838744eb..db6ecb17cb54 100644
--- a/Documentation/devicetree/bindings/pps/pps-gpio.yaml
+++ b/Documentation/devicetree/bindings/pps/pps-gpio.yaml
@@ -28,6 +28,19 @@ properties:
description: Indicates a falling edge assert, when present. Rising edge if absent.
type: boolean
+ pinctrl-names:
+ description:
+ When the PPS input is muxed through a pin controller, the standard
+ "default" state selects the PPS/GPIO function and is applied by the
+ driver core before probe. If a state named "inactive" is also present,
+ it is selected when the driver is unbound or the system is shut down,
+ handing the pins back to their alternate function. The "inactive"
+ state, if used, requires a "default" state; its position among the
+ names does not matter.
+ minItems: 1
+ contains:
+ const: default
+
required:
- compatible
- gpios
@@ -40,8 +53,9 @@ examples:
pps {
compatible = "pps-gpio";
- pinctrl-names = "default";
+ pinctrl-names = "default", "inactive";
pinctrl-0 = <&pinctrl_pps>;
+ pinctrl-1 = <&pinctrl_pps_inactive>;
gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
assert-falling-edge;
echo-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 3/3] pps: clients: gpio: release pins to an inactive state on remove and shutdown
2026-09-17 7:56 ` [PATCH v3 0/3] pps-gpio: restore pin mux on unbind " Eliav Farber
2026-09-17 7:56 ` [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes Eliav Farber
2026-09-17 7:56 ` [PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
@ 2026-09-17 7:56 ` Eliav Farber
2 siblings, 0 replies; 17+ messages in thread
From: Eliav Farber @ 2026-09-17 7:56 UTC (permalink / raw)
To: Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, Eliav Farber, devicetree, linux-gpio,
linux-kernel
Some boards route the PPS input GPIO through a pin controller and need to
mux it to another function when pps-gpio is not driving PPS. The driver
core applies the "default" pinctrl state before probe, so the pins are
muxed for GPIO/PPS use while the driver is bound. Nothing, however, hands
the pins back when the driver is unbound or the system is shut down, so
they stay stuck in the GPIO function for whatever runs next, kexec
included.
Look up an optional "inactive" pinctrl state in probe via
devm_pinctrl_get() and pinctrl_lookup_state(), and select it with
pinctrl_select_state() in remove() and shutdown(). The state is looked up
and selected by the driver itself rather than reusing the runtime-PM
"idle"/"sleep" states, so its meaning is unambiguous and it does not
depend on CONFIG_PM. Boards that do not describe an "inactive" state are
unaffected.
Since "inactive" is only meaningful as the mux to restore after the
core-applied "default" state, reject an "inactive" state that is not
paired with a "default" one rather than releasing pins that were never
put into a defined PPS state.
The mux must not change while something can still drive the pins. On
shutdown() the requested IRQ and the echo timer would otherwise outlive
the mux change -- device_shutdown() is not the end of the road, the
kernel keeps running to load and start the kexec image -- so a timer
callback or the PPS handler could poke a line that by then belongs to
another function. Tear down in the same order as remove(): free_irq() and
timer_delete_sync() first, and the mux change last. shutdown() does not
unregister the PPS source, which is a remove-time concern.
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
Changes in v3:
- Treat -ENODEV from devm_pinctrl_get() (a DT device without a
"pinctrl-0" property) as "no pinctrl", not a probe failure; keep
propagating everything else, e.g. -EPROBE_DEFER
- Restore the "inactive" mux on probe failure too: route the error
paths after pps_gpio_get_pins() through a new err_release_pins label
so a failed probe does not leave the pins stuck in the "default" state
- Warn if pinctrl_select_state() fails to apply the "inactive" state
rather than silently ignoring the error (pps_gpio_release_pins() now
takes the struct device to log against)
- Trim and de-duplicate the added comments
Changes in v2:
- Rename the released state from "idle" to "inactive"
- Look the state up in the driver with devm_pinctrl_get() +
pinctrl_lookup_state() + pinctrl_select_state() instead of
pinctrl_pm_select_idle_state(), removing the CONFIG_PM dependency
- Fix shutdown() to free_irq() and timer_delete_sync() before the mux
change, matching remove(), so no IRQ or timer callback can drive a pin
after it has been handed back
- Require a "default" state whenever "inactive" is present and reject the
mismatch
drivers/pps/clients/pps-gpio.c | 100 ++++++++++++++++++++++++++++++++-
1 file changed, 97 insertions(+), 3 deletions(-)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 038c55c5f7d4..2ee8fba8520c 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/pps_kernel.h>
#include <linux/gpio/consumer.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/list.h>
#include <linux/property.h>
#include <linux/timer.h>
@@ -30,6 +31,8 @@ struct pps_gpio_device_data {
struct gpio_desc *gpio_pin; /* GPIO port descriptors */
struct gpio_desc *echo_pin;
struct timer_list echo_timer; /* timer to reset echo active state */
+ struct pinctrl *pinctrl; /* pin control handle */
+ struct pinctrl_state *pins_inactive; /* pins released when unbound */
bool assert_falling_edge;
unsigned int echo_active_ms; /* PPS echo active duration */
unsigned long echo_timeout; /* timer timeout value in jiffies */
@@ -96,6 +99,66 @@ static void pps_gpio_echo_timer_callback(struct timer_list *t)
gpiod_set_value(info->echo_pin, 0);
}
+/*
+ * Look up the optional "inactive" pinctrl state. It requires a "default"
+ * state (applied by the driver core before probe) and is rejected without
+ * one. Absent pinctrl, or an absent "inactive" state, is not an error.
+ */
+static int pps_gpio_get_pins(struct device *dev)
+{
+ struct pps_gpio_device_data *data = dev_get_drvdata(dev);
+ struct pinctrl_state *pins_default;
+
+ data->pinctrl = devm_pinctrl_get(dev);
+ if (IS_ERR(data->pinctrl)) {
+ /*
+ * A DT device without "pinctrl-0" yields -ENODEV, which
+ * is not an error here; propagate anything else.
+ */
+ if (PTR_ERR(data->pinctrl) == -ENODEV) {
+ data->pinctrl = NULL;
+ return 0;
+ }
+ return dev_err_probe(dev, PTR_ERR(data->pinctrl),
+ "failed to get pinctrl\n");
+ }
+
+ /* The "inactive" state is optional. */
+ data->pins_inactive = pinctrl_lookup_state(data->pinctrl, "inactive");
+ if (IS_ERR(data->pins_inactive)) {
+ data->pins_inactive = NULL;
+ return 0;
+ }
+
+ /* "inactive" requires a "default" state to return from. */
+ pins_default = pinctrl_lookup_state(data->pinctrl, "default");
+ if (IS_ERR(pins_default))
+ return dev_err_probe(dev, PTR_ERR(pins_default),
+ "\"inactive\" pinctrl state requires a \"default\" state\n");
+
+ return 0;
+}
+
+/*
+ * Restore the "inactive" pinctrl state, handing the pins back to whatever
+ * function uses them while pps-gpio is not driving PPS. This undoes the
+ * "default" state the driver core applied before probe. A no-op for boards
+ * that describe no "inactive" state.
+ */
+static void pps_gpio_release_pins(struct device *dev)
+{
+ struct pps_gpio_device_data *data = dev_get_drvdata(dev);
+ int ret;
+
+ if (!data->pins_inactive)
+ return;
+
+ ret = pinctrl_select_state(data->pinctrl, data->pins_inactive);
+ if (ret)
+ dev_warn(dev, "failed to select inactive pinctrl state: %d\n",
+ ret);
+}
+
static int pps_gpio_setup(struct device *dev)
{
struct pps_gpio_device_data *data = dev_get_drvdata(dev);
@@ -161,11 +224,16 @@ static int pps_gpio_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /* pinctrl setup (optional states) */
+ ret = pps_gpio_get_pins(dev);
+ if (ret)
+ return ret;
+
/* IRQ setup */
ret = gpiod_to_irq(data->gpio_pin);
if (ret < 0) {
dev_err(dev, "failed to map GPIO to IRQ: %d\n", ret);
- return ret;
+ goto err_release_pins;
}
data->irq = ret;
@@ -187,7 +255,8 @@ static int pps_gpio_probe(struct platform_device *pdev)
if (IS_ERR(data->pps)) {
dev_err(dev, "failed to register IRQ %d as PPS source\n",
data->irq);
- return PTR_ERR(data->pps);
+ ret = PTR_ERR(data->pps);
+ goto err_release_pins;
}
/* register IRQ interrupt handler */
@@ -198,13 +267,18 @@ static int pps_gpio_probe(struct platform_device *pdev)
if (ret) {
pps_unregister_source(data->pps);
dev_err(dev, "failed to acquire IRQ %d: %d\n", data->irq, ret);
- return ret;
+ goto err_release_pins;
}
dev_dbg(&data->pps->dev, "Registered IRQ %d as PPS source\n",
data->irq);
return 0;
+
+err_release_pins:
+ /* Restore the inactive mux on probe failure; safe to do last here. */
+ pps_gpio_release_pins(dev);
+ return ret;
}
static void pps_gpio_remove(struct platform_device *pdev)
@@ -216,9 +290,28 @@ 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 last, once nothing can drive them */
+ pps_gpio_release_pins(&pdev->dev);
dev_info(&pdev->dev, "removed IRQ %d as PPS source\n", data->irq);
}
+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);
+ timer_delete_sync(&data->echo_timer);
+ gpiod_set_value(data->echo_pin, 0);
+ pps_gpio_release_pins(&pdev->dev);
+}
+
static const struct of_device_id pps_gpio_dt_ids[] = {
{ .compatible = "pps-gpio", },
{ /* sentinel */ }
@@ -228,6 +321,7 @@ MODULE_DEVICE_TABLE(of, pps_gpio_dt_ids);
static struct platform_driver pps_gpio_driver = {
.probe = pps_gpio_probe,
.remove = pps_gpio_remove,
+ .shutdown = pps_gpio_shutdown,
.driver = {
.name = PPS_GPIO_NAME,
.of_match_table = pps_gpio_dt_ids,
--
2.47.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes
2026-09-17 7:56 ` [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes Eliav Farber
@ 2026-09-17 9:58 ` Bartosz Golaszewski
0 siblings, 0 replies; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-09-17 9:58 UTC (permalink / raw)
To: Eliav Farber
Cc: Linus Walleij, Bartosz Golaszewski, Fabio Estevam, Andrew Morton,
Takashi Sakamoto, devicetree, linux-gpio, linux-kernel,
Rodolfo Giometti, Rob Herring, Krzysztof Kozlowski, Conor Dooley
On Thu, 17 Sep 2026 09:56:09 +0200, Eliav Farber <farbere@amazon.com> said:
> On the two probe error paths that map and request the interrupt, probe
> overwrote the error from gpiod_to_irq() and request_threaded_irq() with a
> hardcoded -EINVAL, hiding meaningful codes such as -EBUSY, -ENOMEM or
> -EPROBE_DEFER from the caller. The request_threaded_irq() failure message
> also logged the IRQ number but not the errno.
>
> Return the actual error code from both paths, and add the errno to the
> request_threaded_irq() failure message.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-09-17 9:58 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 13:47 [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown Eliav Farber
2026-09-16 13:47 ` [PATCH 1/2] dt-bindings: pps: pps-gpio: document optional idle pinctrl state Eliav Farber
2026-09-16 13:47 ` [PATCH 2/2] pps: clients: gpio: release pins to idle state on remove and shutdown Eliav Farber
2026-09-16 16:48 ` [PATCH 0/2] pps-gpio: restore pin mux on unbind " Rodolfo Giometti
2026-09-16 18:25 ` Farber, Eliav
2026-09-16 18:26 ` [PATCH v2 " Eliav Farber
2026-09-16 18:26 ` [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
2026-09-17 7:13 ` Rodolfo Giometti
2026-09-17 7:48 ` Farber, Eliav
2026-09-16 18:26 ` [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
2026-09-17 7:14 ` Rodolfo Giometti
2026-09-17 7:52 ` Farber, Eliav
2026-09-17 7:56 ` [PATCH v3 0/3] pps-gpio: restore pin mux on unbind " Eliav Farber
2026-09-17 7:56 ` [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes Eliav Farber
2026-09-17 9:58 ` Bartosz Golaszewski
2026-09-17 7:56 ` [PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states Eliav Farber
2026-09-17 7:56 ` [PATCH v3 3/3] pps: clients: gpio: release pins to an inactive state on remove and shutdown Eliav Farber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®