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

From: Krzysztof Kozlowski

Date: Tue Sep 22 2026 - 04:51:48 EST


On 21/09/2026 21:35, Dmitry Torokhov wrote:
> 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?

Isn't. Quoting patch:

+ items:
+ - const: default
+ - const: wakeup

No sleep. :( So both the commit msg, rationale and could improve so we
won't be asking the same questions at v2.

> 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.
>


Best regards,
Krzysztof