[PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown

[PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown

[Eliav Farber]

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

[PATCH 1/2] dt-bindings: pps: pps-gpio: document optional idle pinctrl state

[Eliav Farber]

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

[PATCH 2/2] pps: clients: gpio: release pins to idle state on remove and shutdown

[Eliav Farber]

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

Re: [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown

[Rodolfo Giometti]

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

RE: [PATCH 0/2] pps-gpio: restore pin mux on unbind and shutdown

[Farber, Eliav]

> 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

[PATCH v2 0/2] pps-gpio: restore pin mux on unbind and shutdown

[Eliav Farber]

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

[PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states

[Eliav Farber]

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

Re: [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states

[Rodolfo Giometti]

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

RE: [PATCH v2 1/2] dt-bindings: pps: pps-gpio: document optional pinctrl states

[Farber, Eliav]

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

[PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown

[Eliav Farber]

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

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

[Rodolfo Giometti]

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

RE: [PATCH v2 2/2] pps: clients: gpio: release pins to an inactive state on remove and shutdown

[Farber, Eliav]

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

[PATCH v3 0/3] pps-gpio: restore pin mux on unbind and shutdown

[Eliav Farber]

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

[PATCH v3 1/3] pps: clients: gpio: propagate probe error codes

[Eliav Farber]

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

Re: [PATCH v3 1/3] pps: clients: gpio: propagate probe error codes

[Bartosz Golaszewski]

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>

[PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states

[Eliav Farber]

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

Re: [PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states

[Rob Herring (Arm)]

On Thu, 17 Sep 2026 07:56:10 +0000, Eliav Farber wrote:
> 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(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pps/pps-gpio.example.dtb: pps (pps-gpio): pinctrl-names: ['default', 'inactive'] is too long
	from schema $id: http://devicetree.org/schemas/pps/pps-gpio.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260917075611.47881-3-farbere@amazon.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 2/3] dt-bindings: pps: pps-gpio: document optional pinctrl states

[Rob Herring]

On Thu, Sep 17, 2026 at 07:56:10AM +0000, Eliav Farber wrote:
> 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.

Don't express in prose what can be defined in schema.

> +    minItems: 1
> +    contains:
> +      const: default

minItems: 1
items:
  - const: default
  - const: inactive

And yes, position does matter. pinctrl-0 was already implicitly defined 
as 'default'. You can't change it to pinctrl-1 now.

Rob

[PATCH v3 3/3] pps: clients: gpio: release pins to an inactive state on remove and shutdown

[Eliav Farber]

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