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

[Dmitry Torokhov]

On Sun, Sep 20, 2026 at 12:24:32PM +0200, Krzysztof Kozlowski wrote:
> On 16/09/2026 17:36, Markus Schneider-Pargmann wrote:
> > 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@xxxxxx>
> >>>>>>>> ---
> >>>>>>>>  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@xxxxxxxxxxxx
> > 
> > And then reviewed/applied in v10:
> >   https://lore.kernel.org/r/175993598226.3512549.5295923279078928995.robh@xxxxxxxxxx
> 
> So we seems to be discussing same issue again, because commit msg lacks
> proper explanations.
> 
> The problem and difference here is that there is no sleep state.

Isn't there? The wakeup can be controlled from userspace and can't we
possible have 2 distinct pin control states, one for when a device is a
potential wakeup source, and another one when the device is in "off"
state while system is asleep?

Thanks.

-- 
Dmitry