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

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:

quoted

On Wed, Aug 20, 2025 at 02:42:25PM +0200, Markus Schneider-Pargmann wrote:

quoted

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