Re: [PATCH 2/3] dt-bindings: serial: 8250_omap: Add wakeup pinctrl state

[Kendall Willis <k-willis@ti.com>]

Hi Rob,
On 9/5/25 15:41, Rob Herring wrote:
> On Thu, Sep 04, 2025 at 04:24:54PM -0500, Kendall Willis wrote:
>> From: Markus Schneider-Pargmann <msp@baylibre.com>
>>
>> Pins associated with the 8250 omap unit can be the source of a wakeup in
>> deep sleep states. To be able to wakeup, these pins have to be
>> configured in a special way. To support this configuration add the
>> default and wakeup pinctrl states. Add support for the sleep state as
>> well which is in use by some devicetrees.
>>
>> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
>> Signed-off-by: Kendall Willis <k-willis@ti.com>
>> ---
>>   .../devicetree/bindings/serial/8250_omap.yaml   | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml
>> index 851a5291b4be4..1c4040a9f9d0b 100644
>> --- a/Documentation/devicetree/bindings/serial/8250_omap.yaml
>> +++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml
>> @@ -77,6 +77,23 @@ properties:
>>           description:
>>             List of phandles to system idle states in which UARTs can wakeup the system.
>>   
>> +  pinctrl-0:
>> +    description: Default pinctrl state
>> +
>> +  pinctrl-1:
>> +    description: Can be "sleep" or "wakeup" pinctrl state
>> +
>> +  pinctrl-names:
>> +    description:
>> +      When present should contain at least "default" describing the default pin
>> +      states. Other states are "sleep" which describes the pinstate when
>> +      sleeping and "wakeup" describing the pins if wakeup is enabled.
>> +    minItems: 1
>> +    items:
>> +      - const: default
>> +      - const: sleep
>> +      - const: wakeup
> 
> This doesn't match what 'pinctrl-1' says. Perhaps you want?:
> 
> items:
>    - const: default
>    - enum: [ sleep, wakeup ]

That is my mistake, I should have added another pinctrl-2 which could be 
either "sleep" or "wakeup" state. This would mean that all three states 
could be present at the same time so it should stay as separate items. 
The thought process for the availability of all three states is that if 
the UART is not wakeup capable, it could use the sleep state when 
suspending.

However, the serial 8250 omap driver does not use the sleep pinctrl 
state as of now, so for the next version I think I will remove the sleep 
state since it is not necessary/used as of this moment.

Thanks for reviewing!

Best,
Kendall

> 
>> +
>>   required:
>>     - compatible
>>     - reg
>> -- 
>> 2.34.1
>>