Re: [PATCH 1/2] dt-bindings: input: gpio-keys: add pinctrl states

["Markus Schneider-Pargmann" <msp@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 4324 bytes --]

On Wed Sep 16, 2026 at 9:17 AM CEST, Krzysztof Kozlowski wrote:
> On Tue, Sep 15, 2026 at 12:41:03PM -0500, Kendall Willis wrote:
>> On 19:07-20260915, Krzysztof Kozlowski wrote:
>> > On 15/09/2026 19:05, Krzysztof Kozlowski wrote:
>> > > On 15/09/2026 18:48, Kendall Willis wrote:
>> > >> On 12:13-20260914, Krzysztof Kozlowski wrote:
>> > >>> On Sat, Sep 12, 2026 at 04:33:53PM -0500, Kendall Willis wrote:
>> > >>>> Document pinctrl properties on the gpio-keys device node. By using the
>> > >>>> wakeup pinctrl state, the pins are able to wakeup the system from a
>> > >>>> low-power state. The default pinctrl state describes the default pin
>> > >>>> configuration.
>> > >>>>
>> > >>>> Signed-off-by: Kendall Willis <k-willis@ti.com>
>> > >>>> ---
>> > >>>>  Documentation/devicetree/bindings/input/gpio-keys.yaml | 16 ++++++++++++++++
>> > >>>>  1 file changed, 16 insertions(+)
>> > >>>>
>> > >>>> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
>> > >>>> index cc78c2152921308fe0cad3e29ca78a5fad08f066..b554933e93412d8b6c2ec401dc1e1eeff57d4190 100644
>> > >>>> --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
>> > >>>> +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
>> > >>>> @@ -22,6 +22,22 @@ properties:
>> > >>>>  
>> > >>>>    poll-interval: true
>> > >>>>  
>> > >>>> +  pinctrl-0:
>> > >>>> +    description: Default pinctrl state
>> > >>>> +
>> > >>>> +  pinctrl-1:
>> > >>>> +    description: Wakeup pinctrl state
>> > >>>> +
>> > >>>> +  pinctrl-names:
>> > >>>> +    description:
>> > >>>> +      When present should contain at least "default" describing the default pin
>> > >>>> +      states. The second state called "wakeup" describes the pins in their
>> > >>>> +      wakeup configuration required to exit sleep states.
>> > >>>> +    minItems: 1
>> > >>>> +    items:
>> > >>>> +      - const: default
>> > >>>> +      - const: wakeup
>> > >>>
>> > >>> This will introduce new warnings, which should be being fixed in this
>> > >>> patchset (e.g. at91-kizbox3-hs.dts).
>> > >>>
>> > >>
>> > >> Will fix the binding to work with current device trees.
>> > >>
>> > >>> But nevertheless, isn't second state the sleep state? How can you
>> > >>> configure pins for the wakeup state - like being in the wakeup? You
>> > >>> configure the pins for given state, which will be a system suspend, so
>> > >>> sleep?
>> > >>>
>> > >>
>> > >> The sleep state usually refers to putting the pins in a state to save
>> > >> power. The wakeup pinctrl state is for putting the pins in a state to
>> > >> allow wakeup from suspend for that device. Both 'sleep' and 'wakeup'
>> > >> are for system suspend, but they fill different functions. The CAN
>> > >> subsystem also uses the 'wakeup' state in addition to the 'sleep' state
>> > >> [1].
>> > > 
>> > > So you mean sleep would be a separate state? But then aren't both
>> > > exactly the same states? IOW, if device is wakeup-source, it will have
>> > > for "sleep" state pin configuration allowing to wakeup.
>> 
>> The 'sleep' and 'wakeup' pin states are both used for suspend. They
>> would just be used separately since only one could be used at a time.
>> The reason 'wakeup' is separated out is because it is more specific for
>> allowing wakeup from the device.
>
> If they cannot be used together, then it is the same state.

If I understand this patch correctly, 'sleep' is not relevant here.
But 'sleep' and 'wakeup' serve different purposes and can be
used/defined together in a single device in the devicetree:
 - 'sleep' describes the pin state during suspend if wakeup is not
   enabled by the user or there is no 'wakeup' pin state defined.
 - 'wakeup' describes the pin state during suspend if wakeup is enabled
   by the user.
If the user enables wakeup, 'wakeup' is the relevant state and otherwise
'sleep'. Which one applieas is a runtime decision of the the user.

I introduced these two states for m_can for that reason. This was
discussed here:
  https://lore.kernel.org/r/DCD1YPX4T779.ADK4JCGW1MU7@baylibre.com

And then reviewed/applied in v10:
  https://lore.kernel.org/r/175993598226.3512549.5295923279078928995.robh@kernel.org

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]