[PATCH v3 0/6] firmware: ti_sci: Partial-IO support

[PATCH v3 0/6] firmware: ti_sci: Partial-IO support

[Markus Schneider-Pargmann]

Hi,

Series
------
Partial-IO is a poweroff SoC state with a few pingroups active for
wakeup. This state can be entered by sending a TI_SCI PREPARE_SLEEP
message.

The message is sent on poweroff if one of the potential wakeup sources
for this power state are wakeup enabled. The potential wakeup sources
are determined by the power supply name of devices as there is one
always-on power-line that powers all canuart devices that are powered
through Partial-IO as well. The wakeup sources can be individually
enabled/disabled by the user in the running system.

The series is based on v6.12-rc1.

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:

 - can: m_can: Add am62 wakeup support
   https://lore.kernel.org/lkml/20241011-topic-mcan-wakeup-source-v6-12-v3-0-9752c714ad12@baylibre.com
   https://gitlab.baylibre.com/msp8/linux/-/tree/topic/mcan-wakeup-source/v6.12?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/v6.12?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/20240523080225.1288617-1-msp@baylibre.com/
 v2: https://lore.kernel.org/lkml/20240729080101.3859701-1-msp@baylibre.com/

Changes in v3:
 - Remove other modes declared for PREPARE_SLEEP as they probably won't
   ever be used in upstream.
 - Replace the wait loop after sending PREPARE_SLEEP with msleep and do
   an emergency_restart if it exits
 - Remove uarts from DT wakeup sources
 - Split no response handling in ti_sci_do_xfer() into a separate patch
   and use goto instead of if ()
 - Remove DT binding parital-io-wakeup-sources. Instead I am modeling
   the devices that are in the relevant group that are powered during
   Partial-IO with the power supplies that are externally provided to
   the SoC. In this case they are provided through 'vddshv_canuart'. All
   devices using this regulator can be considered a potential wakeup
   source if they are wakeup capable and wakeup enabled.
 - Added devicetree patches adding vcc_3v3_sys regulator and
   vddshv_canuart for am62-lp-sk
 - Add pinctrl entries for am62-lp-sk to add WKUP_EN for mcu_mcan0 and
   mcu_mcan1

Changes in v2:
 - Rebase to v6.11-rc1
 - dt-binding:
    - Update commit message
    - Add more verbose description of the new binding for a better
      explanation.
 - ti_sci driver:
    - Combine ti_sci_do_send() into ti_sci_do_xfer and only wait on a
      response if a flag is set.
    - On failure to enter Partial-IO, do emergency_restart()
    - Add comments
    - Fix small things

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
Markus Schneider-Pargmann (6):
      firmware: ti_sci: Support transfers without response
      firmware: ti_sci: Partial-IO support
      arm64: dts: ti: k3-pinctrl: Add WKUP_EN flag
      arm64: dts: ti: am62-lp-sk: Add vcc_3v3_sys regulator
      arm64: dts: ti: am62-lp-sk: Add vddshv_canuart regulator
      arm64: dts: ti: am62-lp-sk: Add wakeup mcu_mcan0/1 pinctrl

 arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts |  79 +++++++++++++++++-
 arch/arm64/boot/dts/ti/k3-pinctrl.h      |   3 +
 drivers/firmware/ti_sci.c                | 137 ++++++++++++++++++++++++++++++-
 drivers/firmware/ti_sci.h                |   5 ++
 4 files changed, 221 insertions(+), 3 deletions(-)
---
base-commit: 03dc72319cee7d0dfefee9ae7041b67732f6b8cd
change-id: 20241008-topic-am62-partialio-v6-12-b4-c273fbac4447
prerequisite-change-id: 20241006-tisci-syssuspendresume-e6550720a50f:13
prerequisite-patch-id: 945b15416a011cb40007c5d95561786c1776bb98
prerequisite-patch-id: 69a741b9c81d7990937483fc481aafa70e67669d
prerequisite-patch-id: 15e97947da8cb7055745151990c756d81fded804
prerequisite-patch-id: a0efbf22e69d23dba8bb96db4032ca644935709b
prerequisite-patch-id: 2999da190c1ba63aabecc55fae501d442e4e0d7b
prerequisite-change-id: 20241009-topic-mcan-wakeup-source-v6-12-8c1d69931bd8:3
prerequisite-patch-id: a7c60adc118d6b8bf783686a62d03c88b14b853c
prerequisite-patch-id: 90f03012d910402b043334ad5a5a8580e33e3d97
prerequisite-patch-id: 74aa28d4203323744419694568d2f5bbf471e233
prerequisite-patch-id: 56fd0aae20e82eb2dfb48f1b7088d62311a11f05
prerequisite-patch-id: f4febebdfba5f2fbfd1f276b8ee8b72be047dfc8
prerequisite-patch-id: c7f9373ac4b8a623f9cb68788c7d0cb6567e3ed9
prerequisite-patch-id: 33d27d8c3f8d1913ed1ec2abb9f64f4d7cd9f1fb
prerequisite-patch-id: 38ba1a5ffe086d1bc186db125d8610b0cdb191f9
prerequisite-patch-id: 46803aa31b9b00725e58fa27c883be94729941c2

Best regards,
-- 
Markus Schneider-Pargmann <msp@baylibre.com>

[PATCH v3 1/6] firmware: ti_sci: Support transfers without response

[Markus Schneider-Pargmann]

Check the header flags if an response is expected or not. If it is not
expected skip the receive part of ti_sci_do_xfer(). This prepares the
driver for one-way messages as prepare_sleep for Partial-IO.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/firmware/ti_sci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 555e41cc08080c78b3991a8d6c06f5030a576d72..9ef86ea27a3c9ac6b9aa4a838a4f5e9fc09a81a9 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -398,10 +398,13 @@ static void ti_sci_put_one_xfer(struct ti_sci_xfers_info *minfo,
 static inline int ti_sci_do_xfer(struct ti_sci_info *info,
 				 struct ti_sci_xfer *xfer)
 {
+	struct ti_sci_msg_hdr *hdr = (struct ti_sci_msg_hdr *)xfer->tx_message.buf;
 	int ret;
 	int timeout;
 	struct device *dev = info->dev;
 	bool done_state = true;
+	bool response_expected = !!(hdr->flags & (TI_SCI_FLAG_REQ_ACK_ON_PROCESSED |
+						  TI_SCI_FLAG_REQ_ACK_ON_RECEIVED));
 
 	ret = mbox_send_message(info->chan_tx, &xfer->tx_message);
 	if (ret < 0)
@@ -409,6 +412,9 @@ static inline int ti_sci_do_xfer(struct ti_sci_info *info,
 
 	ret = 0;
 
+	if (!response_expected)
+		goto no_response;
+
 	if (system_state <= SYSTEM_RUNNING) {
 		/* And we wait for the response. */
 		timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms);
@@ -429,6 +435,7 @@ static inline int ti_sci_do_xfer(struct ti_sci_info *info,
 		dev_err(dev, "Mbox timedout in resp(caller: %pS)\n",
 			(void *)_RET_IP_);
 
+no_response:
 	/*
 	 * NOTE: we might prefer not to need the mailbox ticker to manage the
 	 * transfer queueing since the protocol layer queues things by itself.

-- 
2.45.2

[PATCH v3 2/6] firmware: ti_sci: Partial-IO support

[Markus Schneider-Pargmann]

Add support for Partial-IO poweroff. In Partial-IO pins of a few
hardware units can generate system wakeups while DDR memory is not
powered resulting in a fresh boot of the system. These hardware units in
the SoC are always powered so that some logic can detect pin activity.

If the system supports Partial-IO as described in the fw capabilities, a
sys_off handler is added. This sys_off handler decides if the poweroff
is executed by entering normal poweroff or Partial-IO instead. The
decision is made by checking if wakeup is enabled on all devices that
may wake up the SoC from Partial-IO.

The possible wakeup devices are found by checking which devices are
powered by the regulator supplying the "vddshv_canuart" line. These are
considered possible wakeup sources. Only wakeup sources that are
actually enabled by the user will be considered as a an active wakeup
source. If none of the wakeup sources are enabled the system will do a
normal poweroff. If at least one wakeup source is enabled it will
instead send a TI_SCI_MSG_PREPARE_SLEEP message from the sys_off
handler. Sending this message will result in an immediate shutdown of
the system. No execution is expected after this point. The code will
wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't
entered at that point.

A short documentation about Partial-IO can be found in section 6.2.4.5
of the TRM at
  https://www.ti.com/lit/pdf/spruiv7

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/firmware/ti_sci.c | 130 +++++++++++++++++++++++++++++++++++++++++++++-
 drivers/firmware/ti_sci.h |   5 ++
 2 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 9ef86ea27a3c9ac6b9aa4a838a4f5e9fc09a81a9..fe964e5e2b2a06ba2fb9754537d28661951a6b78 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -3746,6 +3746,115 @@ static const struct dev_pm_ops ti_sci_pm_ops = {
 #endif
 };
 
+/*
+ * Enter Partial-IO, which disables everything including DDR with only a small
+ * logic being active for wakeup.
+ */
+static int tisci_enter_partial_io(struct ti_sci_info *info)
+{
+	struct ti_sci_msg_req_prepare_sleep *req;
+	struct ti_sci_xfer *xfer;
+	struct device *dev = info->dev;
+	int ret = 0;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_PREPARE_SLEEP,
+				   TI_SCI_FLAG_REQ_GENERIC_NORESPONSE,
+				   sizeof(*req), sizeof(struct ti_sci_msg_hdr));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+
+	req = (struct ti_sci_msg_req_prepare_sleep *)xfer->xfer_buf;
+	req->mode = TISCI_MSG_VALUE_SLEEP_MODE_PARTIAL_IO;
+	req->ctx_lo = 0;
+	req->ctx_hi = 0;
+	req->debug_flags = 0;
+
+	dev_info(dev, "Entering Partial-IO because a powered wakeup-enabled device was found.\n");
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
+static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info)
+{
+	static const char canuart_name[] = "vddshv_canuart";
+	struct device_node *wakeup_node = NULL;
+
+	for (wakeup_node = of_find_node_with_property(NULL, "vio-supply");
+	     wakeup_node;
+	     wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) {
+		struct device_node *supply_node;
+		const char *supply_name;
+		struct platform_device *pdev;
+		int ret;
+
+		supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0);
+		if (!supply_node)
+			continue;
+
+		ret = of_property_read_string(supply_node, "regulator-name", &supply_name);
+		of_node_put(supply_node);
+		if (ret) {
+			dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n",
+				 wakeup_node, ret);
+			continue;
+		}
+
+		if (strncmp(canuart_name, supply_name, strlen(canuart_name)))
+			continue;
+
+		pdev = of_find_device_by_node(wakeup_node);
+		if (!pdev)
+			continue;
+
+		if (device_may_wakeup(&pdev->dev)) {
+			dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n",
+				wakeup_node);
+			put_device(&pdev->dev);
+			of_node_put(wakeup_node);
+			return true;
+		}
+		put_device(&pdev->dev);
+	}
+
+	return false;
+}
+
+static int tisci_sys_off_handler(struct sys_off_data *data)
+{
+	struct ti_sci_info *info = data->cb_data;
+	bool enter_partial_io = tisci_canuart_wakeup_enabled(info);
+	int ret;
+
+	if (!enter_partial_io)
+		return NOTIFY_DONE;
+
+	ret = tisci_enter_partial_io(info);
+
+	if (ret) {
+		dev_err(info->dev,
+			"Failed to enter Partial-IO %pe, trying to do an emergency restart\n",
+			ERR_PTR(ret));
+		emergency_restart();
+	}
+
+	mdelay(5000);
+	emergency_restart();
+
+	return NOTIFY_DONE;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3889,6 +3998,19 @@ static int ti_sci_probe(struct platform_device *pdev)
 		goto out;
 	}
 
+	if (info->fw_caps & MSG_FLAG_CAPS_LPM_PARTIAL_IO) {
+		ret = devm_register_sys_off_handler(dev,
+						    SYS_OFF_MODE_POWER_OFF,
+						    SYS_OFF_PRIO_FIRMWARE,
+						    tisci_sys_off_handler,
+						    info);
+		if (ret) {
+			dev_err(dev, "Failed to register sys_off_handler %pe\n",
+				ERR_PTR(ret));
+			goto out;
+		}
+	}
+
 	dev_info(dev, "ABI: %d.%d (firmware rev 0x%04x '%s')\n",
 		 info->handle.version.abi_major, info->handle.version.abi_minor,
 		 info->handle.version.firmware_revision,
@@ -3898,7 +4020,13 @@ static int ti_sci_probe(struct platform_device *pdev)
 	list_add_tail(&info->node, &ti_sci_list);
 	mutex_unlock(&ti_sci_list_mutex);
 
-	return of_platform_populate(dev->of_node, NULL, NULL, dev);
+	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "platform_populate failed %pe\n", ERR_PTR(ret));
+		goto out;
+	}
+	return 0;
+
 out:
 	if (!IS_ERR(info->chan_tx))
 		mbox_free_channel(info->chan_tx);
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index 053387d7baa064498e6a208daa7f70040ef87281..dec9e20cbe5da8f6d9393d56bb9a1e73cb083a42 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -592,6 +592,11 @@ struct ti_sci_msg_resp_get_clock_freq {
 struct ti_sci_msg_req_prepare_sleep {
 	struct ti_sci_msg_hdr	hdr;
 
+/*
+ * When sending perpare_sleep with MODE_PARTIAL_IO no response will be sent,
+ * no further steps are required.
+ */
+#define TISCI_MSG_VALUE_SLEEP_MODE_PARTIAL_IO				0x03
 #define TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED				0xfd
 	u8			mode;
 	u32			ctx_lo;

-- 
2.45.2

[PATCH v3 3/6] arm64: dts: ti: k3-pinctrl: Add WKUP_EN flag

[Markus Schneider-Pargmann]

WKUP_EN is a flag to enable pin wakeup. Any activity will wakeup the SoC
in that case.

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

diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
index 22b8d73cfd3264735ddf91874e60a0c5fc7ade5b..dd4d53e8420a1d671e04a70d4af8b0ea1b75b2b2 100644
--- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
+++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
@@ -12,6 +12,7 @@
 #define PULLTYPESEL_SHIFT	(17)
 #define RXACTIVE_SHIFT		(18)
 #define DEBOUNCE_SHIFT		(11)
+#define WKUP_EN_SHIFT		(29)
 
 #define PULL_DISABLE		(1 << PULLUDEN_SHIFT)
 #define PULL_ENABLE		(0 << PULLUDEN_SHIFT)
@@ -38,6 +39,8 @@
 #define PIN_DEBOUNCE_CONF5	(5 << DEBOUNCE_SHIFT)
 #define PIN_DEBOUNCE_CONF6	(6 << DEBOUNCE_SHIFT)
 
+#define WKUP_EN			(1 << WKUP_EN_SHIFT)
+
 /* Default mux configuration for gpio-ranges to use with pinctrl */
 #define PIN_GPIO_RANGE_IOPAD	(PIN_INPUT | 7)
 

-- 
2.45.2

[PATCH v3 4/6] arm64: dts: ti: am62-lp-sk: Add vcc_3v3_sys regulator

[Markus Schneider-Pargmann]

vcc_3v3_main is currently modeled slightly wrong. vcc_3v3_main has a
parent called vcc_3v3_sys which is currently not present. Add the
regulator for vcc_3v3_sys to be able to describe other regulators in the
next patch.

Fixes: e6a51ffabfc1 ("arm64: ti: dts: Add support for AM62x LP SK")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
index 8e9fc00a6b3c7459a360f9e1d6bbb60e68c460ab..529360b5e6fe052dd99f04b74c129193922f76ac 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
@@ -34,10 +34,10 @@ vcc_5v0: regulator-1 {
 		regulator-boot-on;
 	};
 
-	vcc_3v3_sys: regulator-2 {
+	vcc_3v3_main: regulator-2 {
 		/* output of LM61460-Q1 */
 		compatible = "regulator-fixed";
-		regulator-name = "vcc_3v3_sys";
+		regulator-name = "vcc_3v3_main";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		vin-supply = <&vmain_pd>;
@@ -70,6 +70,16 @@ vddshv_sdio: regulator-4 {
 		states = <1800000 0x0>,
 			 <3300000 0x1>;
 	};
+
+	vcc_3v3_sys: regulator-5 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_3v3_sys";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc_3v3_main>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
 };
 
 &main_pmx0 {

-- 
2.45.2

[PATCH v3 5/6] arm64: dts: ti: am62-lp-sk: Add vddshv_canuart regulator

[Markus Schneider-Pargmann]

vddshv_canuart on am62-lp-sk is used to power a few units specific units
of the SoC that remain active in a special low power mode. Model this
relationship explicitly.

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

diff --git a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
index 529360b5e6fe052dd99f04b74c129193922f76ac..783ad65e6165ea74010c3240069fc6d99a0cd035 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
@@ -80,6 +80,17 @@ vcc_3v3_sys: regulator-5 {
 		regulator-always-on;
 		regulator-boot-on;
 	};
+
+	vddshv_canuart: regulator-7 {
+		/* TPS22965DSGT */
+		compatible = "regulator-fixed";
+		regulator-name = "vddshv_canuart";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc_3v3_main>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
 };
 
 &main_pmx0 {
@@ -242,3 +253,11 @@ &tlv320aic3106 {
 &gpmc0 {
 	ranges = <0 0 0x00 0x51000000 0x01000000>; /* CS0 space. Min partition = 16MB */
 };
+
+&mcu_mcan0 {
+	vio-supply = <&vddshv_canuart>;
+};
+
+&mcu_mcan1 {
+	vio-supply = <&vddshv_canuart>;
+};

-- 
2.45.2

[PATCH v3 6/6] arm64: dts: ti: am62-lp-sk: Add wakeup mcu_mcan0/1 pinctrl

[Markus Schneider-Pargmann]

Add pincontrol definitions for mcu_mcan0 and mcu_mcan1 for wakeup from
Partial-IO. Add these as wakeup pinctrl entries for both devices.

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

diff --git a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
index 783ad65e6165ea74010c3240069fc6d99a0cd035..de3c9abe39ad35cbab9e480d76e7161bd46af46a 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
@@ -255,9 +255,55 @@ &gpmc0 {
 };
 
 &mcu_mcan0 {
+	pinctrl-names = "default", "wakeup";
+	pinctrl-0 = <&mcu_mcan0_tx_pins_default>, <&mcu_mcan0_rx_pins_default>;
+	pinctrl-1 = <&mcu_mcan0_tx_pins_default>, <&mcu_mcan0_rx_pins_wakeup>;
 	vio-supply = <&vddshv_canuart>;
+	status = "okay";
 };
 
 &mcu_mcan1 {
+	pinctrl-names = "default", "wakeup";
+	pinctrl-0 = <&mcu_mcan1_tx_pins_default>, <&mcu_mcan1_rx_pins_default>;
+	pinctrl-1 = <&mcu_mcan1_tx_pins_default>, <&mcu_mcan1_rx_pins_wakeup>;
 	vio-supply = <&vddshv_canuart>;
+	status = "okay";
+};
+
+&mcu_pmx0 {
+	mcu_mcan0_tx_pins_default: mcu-mcan0-tx-pins-default {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x034, PIN_OUTPUT, 0) /* (D6) MCU_MCAN0_TX */
+		>;
+	};
+
+	mcu_mcan0_rx_pins_default: mcu-mcan0-rx-pins-default {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x038, PIN_INPUT, 0) /* (B3) MCU_MCAN0_RX */
+		>;
+	};
+
+	mcu_mcan0_rx_pins_wakeup: mcu-mcan0-rx-pins-wakeup {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x038, PIN_INPUT | WKUP_EN, 0) /* (B3) MCU_MCAN0_RX */
+		>;
+	};
+
+	mcu_mcan1_tx_pins_default: mcu-mcan1-tx-pins-default {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x03c, PIN_OUTPUT, 0) /* (E5) MCU_MCAN1_TX */
+		>;
+	};
+
+	mcu_mcan1_rx_pins_default: mcu-mcan1-rx-pins-default {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x040, PIN_INPUT, 0) /* (D4) MCU_MCAN1_RX */
+		>;
+	};
+
+	mcu_mcan1_rx_pins_wakeup: mcu-mcan1-rx-pins-wakeup {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x040, PIN_INPUT | WKUP_EN, 0) /* (D4) MCU_MCAN1_RX */
+		>;
+	};
 };

-- 
2.45.2

Re: [PATCH v3 0/6] firmware: ti_sci: Partial-IO support

[Rob Herring (Arm)]

On Sat, 12 Oct 2024 16:39:26 +0200, Markus Schneider-Pargmann wrote:
> Hi,
> 
> Series
> ------
> Partial-IO is a poweroff SoC state with a few pingroups active for
> wakeup. This state can be entered by sending a TI_SCI PREPARE_SLEEP
> message.
> 
> The message is sent on poweroff if one of the potential wakeup sources
> for this power state are wakeup enabled. The potential wakeup sources
> are determined by the power supply name of devices as there is one
> always-on power-line that powers all canuart devices that are powered
> through Partial-IO as well. The wakeup sources can be individually
> enabled/disabled by the user in the running system.
> 
> The series is based on v6.12-rc1.
> 
> 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:
> 
>  - can: m_can: Add am62 wakeup support
>    https://lore.kernel.org/lkml/20241011-topic-mcan-wakeup-source-v6-12-v3-0-9752c714ad12@baylibre.com
>    https://gitlab.baylibre.com/msp8/linux/-/tree/topic/mcan-wakeup-source/v6.12?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/v6.12?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/20240523080225.1288617-1-msp@baylibre.com/
>  v2: https://lore.kernel.org/lkml/20240729080101.3859701-1-msp@baylibre.com/
> 
> Changes in v3:
>  - Remove other modes declared for PREPARE_SLEEP as they probably won't
>    ever be used in upstream.
>  - Replace the wait loop after sending PREPARE_SLEEP with msleep and do
>    an emergency_restart if it exits
>  - Remove uarts from DT wakeup sources
>  - Split no response handling in ti_sci_do_xfer() into a separate patch
>    and use goto instead of if ()
>  - Remove DT binding parital-io-wakeup-sources. Instead I am modeling
>    the devices that are in the relevant group that are powered during
>    Partial-IO with the power supplies that are externally provided to
>    the SoC. In this case they are provided through 'vddshv_canuart'. All
>    devices using this regulator can be considered a potential wakeup
>    source if they are wakeup capable and wakeup enabled.
>  - Added devicetree patches adding vcc_3v3_sys regulator and
>    vddshv_canuart for am62-lp-sk
>  - Add pinctrl entries for am62-lp-sk to add WKUP_EN for mcu_mcan0 and
>    mcu_mcan1
> 
> Changes in v2:
>  - Rebase to v6.11-rc1
>  - dt-binding:
>     - Update commit message
>     - Add more verbose description of the new binding for a better
>       explanation.
>  - ti_sci driver:
>     - Combine ti_sci_do_send() into ti_sci_do_xfer and only wait on a
>       response if a flag is set.
>     - On failure to enter Partial-IO, do emergency_restart()
>     - Add comments
>     - Fix small things
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> Markus Schneider-Pargmann (6):
>       firmware: ti_sci: Support transfers without response
>       firmware: ti_sci: Partial-IO support
>       arm64: dts: ti: k3-pinctrl: Add WKUP_EN flag
>       arm64: dts: ti: am62-lp-sk: Add vcc_3v3_sys regulator
>       arm64: dts: ti: am62-lp-sk: Add vddshv_canuart regulator
>       arm64: dts: ti: am62-lp-sk: Add wakeup mcu_mcan0/1 pinctrl
> 
>  arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts |  79 +++++++++++++++++-
>  arch/arm64/boot/dts/ti/k3-pinctrl.h      |   3 +
>  drivers/firmware/ti_sci.c                | 137 ++++++++++++++++++++++++++++++-
>  drivers/firmware/ti_sci.h                |   5 ++
>  4 files changed, 221 insertions(+), 3 deletions(-)
> ---
> base-commit: 03dc72319cee7d0dfefee9ae7041b67732f6b8cd
> change-id: 20241008-topic-am62-partialio-v6-12-b4-c273fbac4447
> prerequisite-change-id: 20241006-tisci-syssuspendresume-e6550720a50f:13
> prerequisite-patch-id: 945b15416a011cb40007c5d95561786c1776bb98
> prerequisite-patch-id: 69a741b9c81d7990937483fc481aafa70e67669d
> prerequisite-patch-id: 15e97947da8cb7055745151990c756d81fded804
> prerequisite-patch-id: a0efbf22e69d23dba8bb96db4032ca644935709b
> prerequisite-patch-id: 2999da190c1ba63aabecc55fae501d442e4e0d7b
> prerequisite-change-id: 20241009-topic-mcan-wakeup-source-v6-12-8c1d69931bd8:3
> prerequisite-patch-id: a7c60adc118d6b8bf783686a62d03c88b14b853c
> prerequisite-patch-id: 90f03012d910402b043334ad5a5a8580e33e3d97
> prerequisite-patch-id: 74aa28d4203323744419694568d2f5bbf471e233
> prerequisite-patch-id: 56fd0aae20e82eb2dfb48f1b7088d62311a11f05
> prerequisite-patch-id: f4febebdfba5f2fbfd1f276b8ee8b72be047dfc8
> prerequisite-patch-id: c7f9373ac4b8a623f9cb68788c7d0cb6567e3ed9
> prerequisite-patch-id: 33d27d8c3f8d1913ed1ec2abb9f64f4d7cd9f1fb
> prerequisite-patch-id: 38ba1a5ffe086d1bc186db125d8610b0cdb191f9
> prerequisite-patch-id: 46803aa31b9b00725e58fa27c883be94729941c2
> 
> Best regards,
> --
> Markus Schneider-Pargmann <msp@baylibre.com>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y ti/k3-am62-lp-sk.dtb' for 20241012-topic-am62-partialio-v6-12-b4-v3-0-f7c6c2739681@baylibre.com:

arch/arm64/boot/dts/ti/k3-am62-lp-sk.dtb: pinctrl@4084000: 'mcu-mcan0-rx-pins-default', 'mcu-mcan0-rx-pins-wakeup', 'mcu-mcan0-tx-pins-default', 'mcu-mcan1-rx-pins-default', 'mcu-mcan1-rx-pins-wakeup', 'mcu-mcan1-tx-pins-default' do not match any of the regexes: '-pins(-[0-9]+)?$|-pin$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/pinctrl/pinctrl-single.yaml#

Re: [PATCH v3 2/6] firmware: ti_sci: Partial-IO support

[Nishanth Menon]

On 16:39-20241012, Markus Schneider-Pargmann wrote:
[...]
> 
> The possible wakeup devices are found by checking which devices are
> powered by the regulator supplying the "vddshv_canuart" line. These are
> considered possible wakeup sources. Only wakeup sources that are
> actually enabled by the user will be considered as a an active wakeup
> source. If none of the wakeup sources are enabled the system will do a
> normal poweroff. If at least one wakeup source is enabled it will
> instead send a TI_SCI_MSG_PREPARE_SLEEP message from the sys_off
> handler. Sending this message will result in an immediate shutdown of
> the system. No execution is expected after this point. The code will
> wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't
> entered at that point.
> 
[...]

> +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info)
> +{
> +	static const char canuart_name[] = "vddshv_canuart";
> +	struct device_node *wakeup_node = NULL;
> +
> +	for (wakeup_node = of_find_node_with_property(NULL, "vio-supply");
> +	     wakeup_node;
> +	     wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) {
> +		struct device_node *supply_node;
> +		const char *supply_name;
> +		struct platform_device *pdev;
> +		int ret;
> +
> +		supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0);
> +		if (!supply_node)
> +			continue;
> +
> +		ret = of_property_read_string(supply_node, "regulator-name", &supply_name);
> +		of_node_put(supply_node);
> +		if (ret) {
> +			dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n",
> +				 wakeup_node, ret);
> +			continue;
> +		}
> +
> +		if (strncmp(canuart_name, supply_name, strlen(canuart_name)))
> +			continue;
> +
> +		pdev = of_find_device_by_node(wakeup_node);
> +		if (!pdev)
> +			continue;
> +
> +		if (device_may_wakeup(&pdev->dev)) {
> +			dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n",
> +				wakeup_node);
> +			put_device(&pdev->dev);
> +			of_node_put(wakeup_node);
> +			return true;
> +		}
> +		put_device(&pdev->dev);
> +	}
> +
> +	return false;
> +}
> +

What is the binding that supports this? I just do not think that
scanning the entire tree for vio-supply implies you will get thr right
property here.

Just giving an example to illustrate this point:
Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt says it
needs vio-supply -> so i have a node with the wireless supply as
vio-supply -> Since we are scanning from NULL for vio-supply, we hit
that, that is a bad choice for enabling io-retention.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v3 2/6] firmware: ti_sci: Partial-IO support

[Markus Schneider-Pargmann]

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

Hi Nishanth,

On Fri, Oct 25, 2024 at 12:42:04PM GMT, Nishanth Menon wrote:
> On 16:39-20241012, Markus Schneider-Pargmann wrote:
> [...]
> > 
> > The possible wakeup devices are found by checking which devices are
> > powered by the regulator supplying the "vddshv_canuart" line. These are
> > considered possible wakeup sources. Only wakeup sources that are
> > actually enabled by the user will be considered as a an active wakeup
> > source. If none of the wakeup sources are enabled the system will do a
> > normal poweroff. If at least one wakeup source is enabled it will
> > instead send a TI_SCI_MSG_PREPARE_SLEEP message from the sys_off
> > handler. Sending this message will result in an immediate shutdown of
> > the system. No execution is expected after this point. The code will
> > wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't
> > entered at that point.
> > 
> [...]
> 
> > +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info)
> > +{
> > +	static const char canuart_name[] = "vddshv_canuart";
> > +	struct device_node *wakeup_node = NULL;
> > +
> > +	for (wakeup_node = of_find_node_with_property(NULL, "vio-supply");
> > +	     wakeup_node;
> > +	     wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) {
> > +		struct device_node *supply_node;
> > +		const char *supply_name;
> > +		struct platform_device *pdev;
> > +		int ret;
> > +
> > +		supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0);
> > +		if (!supply_node)
> > +			continue;
> > +
> > +		ret = of_property_read_string(supply_node, "regulator-name", &supply_name);
> > +		of_node_put(supply_node);
> > +		if (ret) {
> > +			dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n",
> > +				 wakeup_node, ret);
> > +			continue;
> > +		}
> > +
> > +		if (strncmp(canuart_name, supply_name, strlen(canuart_name)))
> > +			continue;
> > +
> > +		pdev = of_find_device_by_node(wakeup_node);
> > +		if (!pdev)
> > +			continue;
> > +
> > +		if (device_may_wakeup(&pdev->dev)) {
> > +			dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n",
> > +				wakeup_node);
> > +			put_device(&pdev->dev);
> > +			of_node_put(wakeup_node);
> > +			return true;
> > +		}
> > +		put_device(&pdev->dev);
> > +	}
> > +
> > +	return false;
> > +}
> > +
> 
> What is the binding that supports this? I just do not think that
> scanning the entire tree for vio-supply implies you will get thr right
> property here.
> 
> Just giving an example to illustrate this point:
> Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt says it
> needs vio-supply -> so i have a node with the wireless supply as
> vio-supply -> Since we are scanning from NULL for vio-supply, we hit
> that, that is a bad choice for enabling io-retention.

There is no bining that specifically supports this as I think it is not
needed. The devices that are capable to wakeup the system from
Partial-IO are all powered through one supply line that is always-on. It
is called 'vddshv_canuart' and the name of this supply is checked
in the above code as well. Yes I am using 'vio-supply', but only to
search for the potential consumers of this supply.
So wl1251 will be skipped in above code at

  if (strncmp(canuart_name, supply_name, strlen(canuart_name)))

Best
Markus

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

Re: [PATCH v3 2/6] firmware: ti_sci: Partial-IO support

[Nishanth Menon]

On 15:49-20241028, Markus Schneider-Pargmann wrote:
> Hi Nishanth,
> 
> On Fri, Oct 25, 2024 at 12:42:04PM GMT, Nishanth Menon wrote:
> > On 16:39-20241012, Markus Schneider-Pargmann wrote:
> > [...]
> > > 
> > > The possible wakeup devices are found by checking which devices are
> > > powered by the regulator supplying the "vddshv_canuart" line. These are
> > > considered possible wakeup sources. Only wakeup sources that are
> > > actually enabled by the user will be considered as a an active wakeup
> > > source. If none of the wakeup sources are enabled the system will do a
> > > normal poweroff. If at least one wakeup source is enabled it will
> > > instead send a TI_SCI_MSG_PREPARE_SLEEP message from the sys_off
> > > handler. Sending this message will result in an immediate shutdown of
> > > the system. No execution is expected after this point. The code will
> > > wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't
> > > entered at that point.
> > > 
> > [...]
> > 
> > > +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info)
> > > +{
> > > +	static const char canuart_name[] = "vddshv_canuart";
> > > +	struct device_node *wakeup_node = NULL;
> > > +
> > > +	for (wakeup_node = of_find_node_with_property(NULL, "vio-supply");
> > > +	     wakeup_node;
> > > +	     wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) {
> > > +		struct device_node *supply_node;
> > > +		const char *supply_name;
> > > +		struct platform_device *pdev;
> > > +		int ret;
> > > +
> > > +		supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0);
> > > +		if (!supply_node)
> > > +			continue;
> > > +
> > > +		ret = of_property_read_string(supply_node, "regulator-name", &supply_name);
> > > +		of_node_put(supply_node);
> > > +		if (ret) {
> > > +			dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n",
> > > +				 wakeup_node, ret);
> > > +			continue;
> > > +		}
> > > +
> > > +		if (strncmp(canuart_name, supply_name, strlen(canuart_name)))
> > > +			continue;
> > > +
> > > +		pdev = of_find_device_by_node(wakeup_node);
> > > +		if (!pdev)
> > > +			continue;
> > > +
> > > +		if (device_may_wakeup(&pdev->dev)) {
> > > +			dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n",
> > > +				wakeup_node);
> > > +			put_device(&pdev->dev);
> > > +			of_node_put(wakeup_node);
> > > +			return true;
> > > +		}
> > > +		put_device(&pdev->dev);
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > > +
> > 
> > What is the binding that supports this? I just do not think that
> > scanning the entire tree for vio-supply implies you will get thr right
> > property here.
> > 
> > Just giving an example to illustrate this point:
> > Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt says it
> > needs vio-supply -> so i have a node with the wireless supply as
> > vio-supply -> Since we are scanning from NULL for vio-supply, we hit
> > that, that is a bad choice for enabling io-retention.
> 
> There is no bining that specifically supports this as I think it is not
> needed. The devices that are capable to wakeup the system from
> Partial-IO are all powered through one supply line that is always-on. It
> is called 'vddshv_canuart' and the name of this supply is checked
> in the above code as well. Yes I am using 'vio-supply', but only to

In effect, you are looking for nodes that have vio-supply pointing to
a regulator called vddshv_canuart. Not only is it too specific to a
device, but a board as well. Without documentation in binding, users
who don't have sufficient information is bound to mess this up. Further,
vddshv_canuart may not even be a regulator - there will need to be
checks for that on top of just a strncmp.


> search for the potential consumers of this supply.
> So wl1251 will be skipped in above code at
> 
>   if (strncmp(canuart_name, supply_name, strlen(canuart_name)))

Aah, thanks, but sorry, but I would prefer the drivers handle the
specifics. If a new peripheral comes on to the list for a different
device, or a different regulator name appears for can, we would be
dealing with name mapping etc.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D