Re: [PATCH v9 1/4] dt-bindings: can: m_can: Add wakeup properties

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

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

Hi Rob,

On Wed Aug 27, 2025 at 10:04 AM CEST, Markus Schneider-Pargmann wrote:
> Hi Rob,
>
> On Fri Aug 22, 2025 at 4:35 PM CEST, Rob Herring wrote:
>> On Wed, Aug 20, 2025 at 02:42:25PM +0200, Markus Schneider-Pargmann wrote:
>>> The pins associated with m_can have to have a special configuration to
>>> be able to wakeup the SoC from some system states. This configuration is
>>> described in the wakeup pinctrl state while the default state describes
>>> the default configuration. Also add the sleep state which is already in
>>> use by some devicetrees.
>>> 
>>> Also m_can can be a wakeup-source if capable of wakeup.
>>> 
>>> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
>>> ---
>>>  .../devicetree/bindings/net/can/bosch,m_can.yaml   | 25 ++++++++++++++++++++++
>>>  1 file changed, 25 insertions(+)
>>> 
>>> diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
>>> index c4887522e8fe97c3947357b4dbd4ecf20ee8100a..0e00be18a8be681634f25378bb2cdef034dc4e6b 100644
>>> --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
>>> +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
>>> @@ -106,6 +106,26 @@ properties:
>>>          maximum: 32
>>>      minItems: 1
>>>  
>>> +  pinctrl-0:
>>> +    description: Default pinctrl state
>>> +
>>> +  pinctrl-1:
>>> +    description: Can be Sleep or Wakeup pinctrl state
>>> +
>>> +  pinctrl-2:
>>> +    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 allow '"default", "wakeup"' which I think you want.
>>
>> "sleep" and "wakeup" seem mutually exclusive and really are just the 
>> same thing. Both apply to the same mode/state. Whether you can wake from 
>> it is just an additional property (of the state). 
>>
>> So I think you want:
>>
>> items:
>>   - const: default
>>   - enum: [ sleep, wakeup ]
>>
>>
>> Or you should just drop 'wakeup' and just support wakeup with 'sleep' 
>> when 'wakeup-source' is present.
>
> Thanks for your feedback. I see they seem to be mutually exclusive, but
> I think they serve different purposes. The sleep state describes the
> pins when sleeping with wakeup disabled. The wakeup state describes the
> pins when sleeping or off and wakeup is enabled.
>
> Only allowing one of the two states or only using the sleep state will
> enable or disable wakeup statically, there is no way to choose one or
> the other.
>
> For my specific setup, the name of a sleep state is also kind of
> misleading. The SoC is in a poweroff state and sensitive to activity on
> the pins configured for wakeup. It is not just sleeping, it will do a
> fresh boot once woken up. 

Just wanted to ask if this makes sense for you and is OK?

Thanks
Markus

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