[PATCH v2 0/7] can: m_can: Add am62 wakeup support

[PATCH v2 0/7] can: m_can: Add am62 wakeup support

[Markus Schneider-Pargmann]

Hi,

v2
--
In v2 I mostly fixed error handling in this series and rebased to
v6.11-rc1.

I also dropped the omap serial series for Partial-IO as it can't be
tested with Partial-IO at the moment. The code was tested with other low
power modes but they will be upstreamed later on.

Series
------
am62, am62a and am62p support Partial-IO, a poweroff SoC state with a
few pin groups being active for wakeup.

To support mcu_mcan0 and mcu_mcan1 wakeup for the mentioned SoCs, the
series introduces a notion of wake-on-lan for m_can. If the user decides
to enable wake-on-lan for a m_can device, the device is set to wakeup
enabled. A 'wakeup' pinctrl state is selected to enable wakeup flags for
the relevant pins. If wake-on-lan is disabled the default pinctrl is
selected.

It is based on v6.11-rc1 with this series applied:
  https://lore.kernel.org/lkml/20240726195944.2414812-1-msp@baylibre.com/
Also available at
  https://gitlab.baylibre.com/msp8/linux/-/tree/topic/mcan-fixes/v6.11?ref_type=heads

Partial-IO
----------
This series is part of a bigger topic to support Partial-IO on am62,
am62a and am62p. Partial-IO is a poweroff state in which some pins are
able to wakeup the SoC. In detail MCU m_can and two serial port pins can
trigger the wakeup.
A documentation can also be found in section 6.2.4 in the TRM:
  https://www.ti.com/lit/pdf/spruiv7

This other series is relevant for the support of Partial-IO:

 - firmware: ti_sci: Partial-IO support
   https://gitlab.baylibre.com/msp8/linux/-/tree/topic/am62-partialio/v6.11?ref_type=heads

Testing
-------
A test branch is available here that includes all patches required to
test Partial-IO:

https://gitlab.baylibre.com/msp8/linux/-/tree/integration/am62-lp-sk-partialio/v6.11?ref_type=heads

After enabling Wake-on-LAN the system can be powered off and will enter
the Partial-IO state in which it can be woken up by activity on the
specific pins:
    ethtool -s can0 wol p
    ethtool -s can1 wol p
    poweroff

I tested these patches on am62-lp-sk.

Best,
Markus

Previous versions:
 v1: https://lore.kernel.org/lkml/20240523075347.1282395-1-msp@baylibre.com/

Changes in v2:
 - Rebase to v6.11-rc1
 - Squash these two patches for the binding into one:
   dt-bindings: can: m_can: Add wakeup-source property
   dt-bindings: can: m_can: Add wakeup pinctrl state
 - Add error handling to multiple patches of the m_can driver
 - Add error handling in m_can_class_allocate_dev(). This also required
   to add a new patch to return error pointers from
   m_can_class_allocate_dev().

Markus Schneider-Pargmann (6):
  dt-bindings: can: m_can: Add wakeup properties
  can: m_can: Map WoL to device_set_wakeup_enable
  can: m_can: Return ERR_PTR on error in allocation
  can: m_can: Support pinctrl wakeup state
  arm64: dts: ti: k3-am62: Mark mcu_mcan0/1 as wakeup-source
  arm64: dts: ti: k3-am62a-mcu: Mark mcu_mcan0/1 as wakeup-source

Vibhore Vardhan (1):
  arm64: dts: ti: k3-am62p-mcu: Mark mcu_mcan0/1 as wakeup-source

 .../bindings/net/can/bosch,m_can.yaml         |  20 ++++
 arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi       |   2 +
 arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi      |   2 +
 .../dts/ti/k3-am62p-j722s-common-mcu.dtsi     |   2 +
 drivers/net/can/m_can/m_can.c                 | 100 +++++++++++++++++-
 drivers/net/can/m_can/m_can.h                 |   4 +
 drivers/net/can/m_can/m_can_pci.c             |   4 +-
 drivers/net/can/m_can/m_can_platform.c        |   4 +-
 drivers/net/can/m_can/tcan4x5x-core.c         |   4 +-
 9 files changed, 133 insertions(+), 9 deletions(-)

-- 
2.45.2

[PATCH v2 1/7] dt-bindings: can: m_can: Add wakeup properties

[Markus Schneider-Pargmann]

m_can can be a wakeup source on some devices. Especially on some of the
am62* SoCs pins, connected to m_can in the mcu, can be used to wakeup
the SoC.

The wakeup-source property defines on which devices m_can can be used
for wakeup.

The pins associated with m_can have to have a special configuration to
be able to wakeup the SoC. This configuration is described in the wakeup
pinctrl state while the default state describes the default
configuration.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 .../bindings/net/can/bosch,m_can.yaml         | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
index c4887522e8fe..ef63f6b8455d 100644
--- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
+++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
@@ -106,6 +106,22 @@ properties:
         maximum: 32
     minItems: 1
 
+  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
+
   power-domains:
     description:
       Power domain provider node and an args specifier containing
@@ -122,6 +138,10 @@ properties:
     minItems: 1
     maxItems: 2
 
+  wakeup-source:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: This device is capable to wakeup the SoC.
+
 required:
   - compatible
   - reg
-- 
2.45.2

[PATCH v2 2/7] can: m_can: Map WoL to device_set_wakeup_enable

[Markus Schneider-Pargmann]

In some devices the pins of the m_can module can act as a wakeup source.
This patch helps do that by connecting the PHY_WAKE WoL option to
device_set_wakeup_enable. By marking this device as being wakeup
enabled, this setting can be used by platform code to decide which
sleep or poweroff mode to use.

Also this prepares the driver for the next patch in which the pinctrl
settings are changed depending on the desired wakeup source.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 81e05746d7d4..2e4ccf05e162 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2182,6 +2182,36 @@ static int m_can_set_coalesce(struct net_device *dev,
 	return 0;
 }
 
+static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct m_can_classdev *cdev = netdev_priv(dev);
+
+	wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0;
+	wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0;
+}
+
+static int m_can_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct m_can_classdev *cdev = netdev_priv(dev);
+	bool wol_enable = !!wol->wolopts & WAKE_PHY;
+	int ret;
+
+	if ((wol->wolopts & WAKE_PHY) != wol->wolopts)
+		return -EINVAL;
+
+	if (wol_enable == device_may_wakeup(cdev->dev))
+		return 0;
+
+	ret = device_set_wakeup_enable(cdev->dev, wol_enable);
+	if (ret) {
+		netdev_err(cdev->net, "Failed to set wakeup enable %pE\n",
+			   ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops m_can_ethtool_ops_coalescing = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ |
 		ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ |
@@ -2191,10 +2221,14 @@ static const struct ethtool_ops m_can_ethtool_ops_coalescing = {
 	.get_ts_info = ethtool_op_get_ts_info,
 	.get_coalesce = m_can_get_coalesce,
 	.set_coalesce = m_can_set_coalesce,
+	.get_wol = m_can_get_wol,
+	.set_wol = m_can_set_wol,
 };
 
 static const struct ethtool_ops m_can_ethtool_ops = {
 	.get_ts_info = ethtool_op_get_ts_info,
+	.get_wol = m_can_get_wol,
+	.set_wol = m_can_set_wol,
 };
 
 static int register_m_can_dev(struct m_can_classdev *cdev)
@@ -2321,6 +2355,9 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
 		goto out;
 	}
 
+	if (dev->of_node && of_property_read_bool(dev->of_node, "wakeup-source"))
+		device_set_wakeup_capable(dev, true);
+
 	/* Get TX FIFO size
 	 * Defines the total amount of echo buffers for loopback
 	 */
-- 
2.45.2

[PATCH v2 3/7] can: m_can: Return ERR_PTR on error in allocation

[Markus Schneider-Pargmann]

We have more detailed error values available, return them in the core
driver and the calling drivers to return proper errors to callers.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/net/can/m_can/m_can.c          | 6 +++---
 drivers/net/can/m_can/m_can_pci.c      | 4 ++--
 drivers/net/can/m_can/m_can_platform.c | 4 ++--
 drivers/net/can/m_can/tcan4x5x-core.c  | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 2e4ccf05e162..5b80a7d1f9a1 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2352,7 +2352,7 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
 					     sizeof(mram_config_vals) / 4);
 	if (ret) {
 		dev_err(dev, "Could not get Message RAM configuration.");
-		goto out;
+		return ERR_PTR(ret);
 	}
 
 	if (dev->of_node && of_property_read_bool(dev->of_node, "wakeup-source"))
@@ -2367,7 +2367,7 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
 	net_dev = alloc_candev(sizeof_priv, tx_fifo_size);
 	if (!net_dev) {
 		dev_err(dev, "Failed to allocate CAN device");
-		goto out;
+		return ERR_PTR(-ENOMEM);
 	}
 
 	class_dev = netdev_priv(net_dev);
@@ -2376,7 +2376,7 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
 	SET_NETDEV_DEV(net_dev, dev);
 
 	m_can_of_parse_mram(class_dev, mram_config_vals);
-out:
+
 	return class_dev;
 }
 EXPORT_SYMBOL_GPL(m_can_class_allocate_dev);
diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c
index d72fe771dfc7..05a01dfdbfbf 100644
--- a/drivers/net/can/m_can/m_can_pci.c
+++ b/drivers/net/can/m_can/m_can_pci.c
@@ -111,8 +111,8 @@ static int m_can_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
 
 	mcan_class = m_can_class_allocate_dev(&pci->dev,
 					      sizeof(struct m_can_pci_priv));
-	if (!mcan_class)
-		return -ENOMEM;
+	if (IS_ERR(mcan_class))
+		return PTR_ERR(mcan_class);
 
 	priv = cdev_to_priv(mcan_class);
 
diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index 983ab80260dd..a5024e2cff8d 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -87,8 +87,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
 
 	mcan_class = m_can_class_allocate_dev(&pdev->dev,
 					      sizeof(struct m_can_plat_priv));
-	if (!mcan_class)
-		return -ENOMEM;
+	if (IS_ERR(mcan_class))
+		return PTR_ERR(mcan_class);
 
 	priv = cdev_to_priv(mcan_class);
 
diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 2f73bf3abad8..4c40b4447275 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -375,8 +375,8 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
 
 	mcan_class = m_can_class_allocate_dev(&spi->dev,
 					      sizeof(struct tcan4x5x_priv));
-	if (!mcan_class)
-		return -ENOMEM;
+	if (IS_ERR(mcan_class))
+		return PTR_ERR(mcan_class);
 
 	ret = m_can_check_mram_cfg(mcan_class, TCAN4X5X_MRAM_SIZE);
 	if (ret)
-- 
2.45.2

[PATCH v2 4/7] can: m_can: Support pinctrl wakeup state

[Markus Schneider-Pargmann]

am62 requires a wakeup flag being set in pinctrl when mcan pins acts as
a wakeup source. Add support to select the wakeup state if WOL is
enabled.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/net/can/m_can/m_can.c | 57 +++++++++++++++++++++++++++++++++++
 drivers/net/can/m_can/m_can.h |  4 +++
 2 files changed, 61 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 5b80a7d1f9a1..b71e7f8e9727 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2193,6 +2193,7 @@ static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 static int m_can_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct m_can_classdev *cdev = netdev_priv(dev);
+	struct pinctrl_state *new_pinctrl_state = NULL;
 	bool wol_enable = !!wol->wolopts & WAKE_PHY;
 	int ret;
 
@@ -2209,7 +2210,27 @@ static int m_can_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 		return ret;
 	}
 
+	if (wol_enable)
+		new_pinctrl_state = cdev->pinctrl_state_wakeup;
+	else
+		new_pinctrl_state = cdev->pinctrl_state_default;
+
+	if (!IS_ERR_OR_NULL(new_pinctrl_state)) {
+		ret = pinctrl_select_state(cdev->pinctrl, new_pinctrl_state);
+		if (ret) {
+			netdev_err(cdev->net, "Failed to select pinctrl state %pE\n",
+				   ERR_PTR(ret));
+			goto err_wakeup_enable;
+		}
+	}
+
 	return 0;
+
+err_wakeup_enable:
+	/* Revert wakeup enable */
+	device_set_wakeup_enable(cdev->dev, !wol_enable);
+
+	return ret;
 }
 
 static const struct ethtool_ops m_can_ethtool_ops_coalescing = {
@@ -2377,7 +2398,43 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
 
 	m_can_of_parse_mram(class_dev, mram_config_vals);
 
+	class_dev->pinctrl = devm_pinctrl_get(dev);
+	if (IS_ERR(class_dev->pinctrl)) {
+		ret = PTR_ERR(class_dev->pinctrl);
+
+		if (ret != -ENODEV) {
+			dev_err_probe(dev, ret, "Failed to get pinctrl\n");
+			goto err_free_candev;
+		}
+
+		class_dev->pinctrl = NULL;
+	} else {
+		class_dev->pinctrl_state_wakeup = pinctrl_lookup_state(class_dev->pinctrl, "wakeup");
+		if (IS_ERR(class_dev->pinctrl_state_wakeup)) {
+			ret = PTR_ERR(class_dev->pinctrl_state_wakeup);
+			ret = -EIO;
+
+			if (ret != -ENODEV) {
+				dev_err_probe(dev, ret, "Failed to lookup pinctrl wakeup state\n");
+				goto err_free_candev;
+			}
+
+			class_dev->pinctrl_state_wakeup = NULL;
+		} else {
+			class_dev->pinctrl_state_default = pinctrl_lookup_state(class_dev->pinctrl, "default");
+			if (IS_ERR(class_dev->pinctrl_state_default)) {
+				ret = PTR_ERR(class_dev->pinctrl_state_default);
+				dev_err_probe(dev, ret, "Failed to lookup pinctrl default state\n");
+				goto err_free_candev;
+			}
+		}
+	}
+
 	return class_dev;
+
+err_free_candev:
+	free_candev(net_dev);
+	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(m_can_class_allocate_dev);
 
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 92b2bd8628e6..b75b0dd6ccc9 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -126,6 +126,10 @@ struct m_can_classdev {
 	struct mram_cfg mcfg[MRAM_CFG_NUM];
 
 	struct hrtimer hrtimer;
+
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pinctrl_state_default;
+	struct pinctrl_state *pinctrl_state_wakeup;
 };
 
 struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, int sizeof_priv);
-- 
2.45.2

[PATCH v2 5/7] arm64: dts: ti: k3-am62: Mark mcu_mcan0/1 as wakeup-source

[Markus Schneider-Pargmann]

mcu_mcan0 and mcu_mcan1 can be wakeup sources for the SoC. Mark them
accordingly in the devicetree.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
index e66d486ef1f2..56a40b641667 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
@@ -159,6 +159,7 @@ mcu_mcan0: can@4e08000 {
 		clocks = <&k3_clks 188 6>, <&k3_clks 188 1>;
 		clock-names = "hclk", "cclk";
 		bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+		wakeup-source;
 		status = "disabled";
 	};
 
@@ -171,6 +172,7 @@ mcu_mcan1: can@4e18000 {
 		clocks = <&k3_clks 189 6>, <&k3_clks 189 1>;
 		clock-names = "hclk", "cclk";
 		bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+		wakeup-source;
 		status = "disabled";
 	};
 };
-- 
2.45.2

[PATCH v2 6/7] arm64: dts: ti: k3-am62a-mcu: Mark mcu_mcan0/1 as wakeup-source

[Markus Schneider-Pargmann]

mcu_mcan0 and mcu_mcan1 can be wakeup sources for the SoC. Mark them
accordingly in the devicetree.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi
index 8c36e56f4138..f0f6b7650233 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi
@@ -153,6 +153,7 @@ mcu_mcan0: can@4e08000 {
 		clocks = <&k3_clks 188 6>, <&k3_clks 188 1>;
 		clock-names = "hclk", "cclk";
 		bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+		wakeup-source;
 		status = "disabled";
 	};
 
@@ -165,6 +166,7 @@ mcu_mcan1: can@4e18000 {
 		clocks = <&k3_clks 189 6>, <&k3_clks 189 1>;
 		clock-names = "hclk", "cclk";
 		bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+		wakeup-source;
 		status = "disabled";
 	};
 };
-- 
2.45.2

[PATCH v2 7/7] arm64: dts: ti: k3-am62p-mcu: Mark mcu_mcan0/1 as wakeup-source

[Markus Schneider-Pargmann]

From: Vibhore Vardhan <vibhore@ti.com>

mcu_mcan0 and mcu_mcan1 can be wakeup sources for the SoC. Mark them
accordingly in the devicetree. Based on the patch for AM62a.

Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
index e65db6ce02bf..d6eccaea7a46 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
@@ -171,6 +171,7 @@ mcu_mcan0: can@4e08000 {
 		interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-names = "int0", "int1";
+		wakeup-source;
 		status = "disabled";
 	};
 
@@ -186,6 +187,7 @@ mcu_mcan1: can@4e18000 {
 		interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-names = "int0", "int1";
+		wakeup-source;
 		status = "disabled";
 	};
 
-- 
2.45.2

Re: [PATCH v2 4/7] can: m_can: Support pinctrl wakeup state

[Martin Hundebøll]

Hi Markus,

On Mon, 2024-07-29 at 09:41 +0200, Markus Schneider-Pargmann wrote:
> am62 requires a wakeup flag being set in pinctrl when mcan pins acts
> as
> a wakeup source. Add support to select the wakeup state if WOL is
> enabled.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/net/can/m_can/m_can.c | 57
> +++++++++++++++++++++++++++++++++++
>  drivers/net/can/m_can/m_can.h |  4 +++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/m_can.c
> b/drivers/net/can/m_can/m_can.c
> index 5b80a7d1f9a1..b71e7f8e9727 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -2193,6 +2193,7 @@ static void m_can_get_wol(struct net_device
> *dev, struct ethtool_wolinfo *wol)
>  static int m_can_set_wol(struct net_device *dev, struct
> ethtool_wolinfo *wol)
>  {
>  	struct m_can_classdev *cdev = netdev_priv(dev);
> +	struct pinctrl_state *new_pinctrl_state = NULL;
>  	bool wol_enable = !!wol->wolopts & WAKE_PHY;
>  	int ret;
>  
> @@ -2209,7 +2210,27 @@ static int m_can_set_wol(struct net_device
> *dev, struct ethtool_wolinfo *wol)
>  		return ret;
>  	}
>  
> +	if (wol_enable)
> +		new_pinctrl_state = cdev->pinctrl_state_wakeup;
> +	else
> +		new_pinctrl_state = cdev->pinctrl_state_default;
> +
> +	if (!IS_ERR_OR_NULL(new_pinctrl_state)) {

Why not do

	if (IS_ERR_OR_NULL(new_pinctrl_state))
		return 0;

?

// Martin

> +		ret = pinctrl_select_state(cdev->pinctrl,
> new_pinctrl_state);
> +		if (ret) {
> +			netdev_err(cdev->net, "Failed to select
> pinctrl state %pE\n",
> +				   ERR_PTR(ret));
> +			goto err_wakeup_enable;
> +		}
> +	}
> +
>  	return 0;
> +
> +err_wakeup_enable:
> +	/* Revert wakeup enable */
> +	device_set_wakeup_enable(cdev->dev, !wol_enable);
> +
> +	return ret;
>  }
>  
>  static const struct ethtool_ops m_can_ethtool_ops_coalescing = {
> @@ -2377,7 +2398,43 @@ struct m_can_classdev
> *m_can_class_allocate_dev(struct device *dev,
>  
>  	m_can_of_parse_mram(class_dev, mram_config_vals);
>  
> +	class_dev->pinctrl = devm_pinctrl_get(dev);
> +	if (IS_ERR(class_dev->pinctrl)) {
> +		ret = PTR_ERR(class_dev->pinctrl);
> +
> +		if (ret != -ENODEV) {
> +			dev_err_probe(dev, ret, "Failed to get
> pinctrl\n");
> +			goto err_free_candev;
> +		}
> +
> +		class_dev->pinctrl = NULL;
> +	} else {
> +		class_dev->pinctrl_state_wakeup =
> pinctrl_lookup_state(class_dev->pinctrl, "wakeup");
> +		if (IS_ERR(class_dev->pinctrl_state_wakeup)) {
> +			ret = PTR_ERR(class_dev-
> >pinctrl_state_wakeup);
> +			ret = -EIO;
> +
> +			if (ret != -ENODEV) {
> +				dev_err_probe(dev, ret, "Failed to
> lookup pinctrl wakeup state\n");
> +				goto err_free_candev;
> +			}
> +
> +			class_dev->pinctrl_state_wakeup = NULL;
> +		} else {
> +			class_dev->pinctrl_state_default =
> pinctrl_lookup_state(class_dev->pinctrl, "default");
> +			if (IS_ERR(class_dev-
> >pinctrl_state_default)) {
> +				ret = PTR_ERR(class_dev-
> >pinctrl_state_default);
> +				dev_err_probe(dev, ret, "Failed to
> lookup pinctrl default state\n");
> +				goto err_free_candev;
> +			}
> +		}
> +	}
> +
>  	return class_dev;
> +
> +err_free_candev:
> +	free_candev(net_dev);
> +	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(m_can_class_allocate_dev);
>  
> diff --git a/drivers/net/can/m_can/m_can.h
> b/drivers/net/can/m_can/m_can.h
> index 92b2bd8628e6..b75b0dd6ccc9 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -126,6 +126,10 @@ struct m_can_classdev {
>  	struct mram_cfg mcfg[MRAM_CFG_NUM];
>  
>  	struct hrtimer hrtimer;
> +
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *pinctrl_state_default;
> +	struct pinctrl_state *pinctrl_state_wakeup;
>  };
>  
>  struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
> int sizeof_priv);

Re: [PATCH v2 2/7] can: m_can: Map WoL to device_set_wakeup_enable

[Andrew Lunn]

On Mon, Jul 29, 2024 at 09:41:30AM +0200, Markus Schneider-Pargmann wrote:
> In some devices the pins of the m_can module can act as a wakeup source.
> This patch helps do that by connecting the PHY_WAKE WoL option to
> device_set_wakeup_enable. By marking this device as being wakeup
> enabled, this setting can be used by platform code to decide which
> sleep or poweroff mode to use.
> 
> Also this prepares the driver for the next patch in which the pinctrl
> settings are changed depending on the desired wakeup source.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 81e05746d7d4..2e4ccf05e162 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -2182,6 +2182,36 @@ static int m_can_set_coalesce(struct net_device *dev,
>  	return 0;
>  }
>  
> +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> +{
> +	struct m_can_classdev *cdev = netdev_priv(dev);
> +
> +	wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0;
> +	wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0;
> +}

It is nice to see Ethernet WoL mapped to CAN :-)

So will any activity on the CAN BUS wake the device? Or does it need
to be addresses to this device?

	Andrew

Re: [PATCH v2 2/7] can: m_can: Map WoL to device_set_wakeup_enable

[Marc Kleine-Budde]

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

On 29.07.2024 21:27:04, Andrew Lunn wrote:
> On Mon, Jul 29, 2024 at 09:41:30AM +0200, Markus Schneider-Pargmann wrote:
> > In some devices the pins of the m_can module can act as a wakeup source.
> > This patch helps do that by connecting the PHY_WAKE WoL option to
> > device_set_wakeup_enable. By marking this device as being wakeup
> > enabled, this setting can be used by platform code to decide which
> > sleep or poweroff mode to use.
> > 
> > Also this prepares the driver for the next patch in which the pinctrl
> > settings are changed depending on the desired wakeup source.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >  drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index 81e05746d7d4..2e4ccf05e162 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -2182,6 +2182,36 @@ static int m_can_set_coalesce(struct net_device *dev,
> >  	return 0;
> >  }
> >  
> > +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> > +{
> > +	struct m_can_classdev *cdev = netdev_priv(dev);
> > +
> > +	wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0;
> > +	wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0;
> > +}
> 
> It is nice to see Ethernet WoL mapped to CAN :-)
> 
> So will any activity on the CAN BUS wake the device? Or does it need
> to be addresses to this device?

Unless you have a special filtering transceiver, which is the CAN
equivalent of a PHY, CAN interfaces usually wake up on the first
message on the bus. That message is usually lost.

Note: The details of the m_can IP core might be different.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

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

Re: [PATCH v2 2/7] can: m_can: Map WoL to device_set_wakeup_enable

[Andrew Lunn]

> > > +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> > > +{
> > > +	struct m_can_classdev *cdev = netdev_priv(dev);
> > > +
> > > +	wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0;
> > > +	wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0;
> > > +}
> > 
> > It is nice to see Ethernet WoL mapped to CAN :-)
> > 
> > So will any activity on the CAN BUS wake the device? Or does it need
> > to be addresses to this device?
> 
> Unless you have a special filtering transceiver, which is the CAN
> equivalent of a PHY, CAN interfaces usually wake up on the first
> message on the bus. That message is usually lost.

Thanks for the info. WAKE_PHY does seem the most appropriate then.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 2/7] can: m_can: Map WoL to device_set_wakeup_enable

[Markus Schneider-Pargmann]

Hi,

On Mon, Jul 29, 2024 at 09:37:56PM GMT, Andrew Lunn wrote:
> > > > +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> > > > +{
> > > > +	struct m_can_classdev *cdev = netdev_priv(dev);
> > > > +
> > > > +	wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0;
> > > > +	wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0;
> > > > +}
> > > 
> > > It is nice to see Ethernet WoL mapped to CAN :-)
> > > 
> > > So will any activity on the CAN BUS wake the device? Or does it need
> > > to be addresses to this device?
> > 
> > Unless you have a special filtering transceiver, which is the CAN
> > equivalent of a PHY, CAN interfaces usually wake up on the first
> > message on the bus. That message is usually lost.
> 
> Thanks for the info. WAKE_PHY does seem the most appropriate then.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Thank you.

Just to extend on Marc's explanation specifically for m_can:

For this very low power mode 'Partial-IO' the m_can IP is not active.
The m_can pins will trigger a wakeup for any activity. Also as the SoC
needs to do a normal boot, I would guess there are more messages lost
when waking up from Partial-IO. Other low power modes that will be
upstreamed in the future will not need as much time to be able to
receive CAN messages again.

Best
Markus

Re: [PATCH v2 1/7] dt-bindings: can: m_can: Add wakeup properties

[Rob Herring]

On Mon, Jul 29, 2024 at 09:41:29AM +0200, Markus Schneider-Pargmann wrote:
> m_can can be a wakeup source on some devices. Especially on some of the
> am62* SoCs pins, connected to m_can in the mcu, can be used to wakeup
> the SoC.
> 
> The wakeup-source property defines on which devices m_can can be used
> for wakeup.
> 
> The pins associated with m_can have to have a special configuration to
> be able to wakeup the SoC. This configuration is described in the wakeup
> pinctrl state while the default state describes the default
> configuration.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  .../bindings/net/can/bosch,m_can.yaml         | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> index c4887522e8fe..ef63f6b8455d 100644
> --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> @@ -106,6 +106,22 @@ properties:
>          maximum: 32
>      minItems: 1
>  
> +  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
> +
>    power-domains:
>      description:
>        Power domain provider node and an args specifier containing
> @@ -122,6 +138,10 @@ properties:
>      minItems: 1
>      maxItems: 2
>  
> +  wakeup-source:
> +    $ref: /schemas/types.yaml#/definitions/flag

I thought we had a common schema defining the type, but we don't. I'm 
going to add it now. So just need:

wakeup-source: true

> +    description: This device is capable to wakeup the SoC.
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.45.2
>