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

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

[Markus Schneider-Pargmann]

Hi,

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. A list of potential wakeup
sources for the specific SoC is described in the devicetree. The wakeup
sources can be individually enabled/disabled by the user in the running
system.

The series is based on Andrew Davis accepted patches:
  [PATCH 0/4] Unconditionally register TI-SCI reset handler
  https://lore.kernel.org/lkml/20240326223730.54639-1-afd@ti.com/

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.

These two other series are relevant for the support of Partial-IO:

 - serial: 8250: omap: Add am62 wakeup support
 - can: m_can: Add am62 wakeup support

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.9?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

Markus Schneider-Pargmann (5):
  dt-bindings: ti, sci: Add property for partial-io-wakeup-sources
  firmware: ti_sci: Partial-IO support
  arm64: dts: ti: k3-pinctrl: Add WKUP_EN flag
  arm64: dts: ti: k3-am62: Add partial-io wakeup sources
  arm64: dts: ti: k3-am62a: Add partial-io wakeup sources

Vibhore Vardhan (1):
  arm64: dts: ti: k3-am62p: Add partial-io wakeup sources

 .../bindings/arm/keystone/ti,sci.yaml         |   6 +
 arch/arm64/boot/dts/ti/k3-am62.dtsi           |   4 +
 arch/arm64/boot/dts/ti/k3-am62a.dtsi          |   4 +
 arch/arm64/boot/dts/ti/k3-am62p.dtsi          |   4 +
 arch/arm64/boot/dts/ti/k3-pinctrl.h           |   3 +
 drivers/firmware/ti_sci.c                     | 135 +++++++++++++++++-
 drivers/firmware/ti_sci.h                     |  31 ++++
 7 files changed, 186 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/6] dt-bindings: ti, sci: Add property for partial-io-wakeup-sources

[Markus Schneider-Pargmann]

Add a property with an array of phandles to devices that have pins that
are capable to wakeup the SoC from Partial-IO. In Partial-IO everything
is powered off including the DDR. Only pins belonging to a couple of
devices are active and wakeup the system on activity.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index 7f06b1080244..c8ed0dd4fee4 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -61,6 +61,12 @@ properties:
   mboxes:
     minItems: 2
 
+  ti,partial-io-wakeup-sources:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      List of phandles to devicetree nodes that can wakeup the SoC from the
+      Partial IO poweroff mode.
+
   ti,host-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: |
-- 
2.43.0

Re: [PATCH 1/6] dt-bindings: ti, sci: Add property for partial-io-wakeup-sources

[Nishanth Menon]

On 10:02-20240523, Markus Schneider-Pargmann wrote:
> Add a property with an array of phandles to devices that have pins that
> are capable to wakeup the SoC from Partial-IO. In Partial-IO everything
> is powered off including the DDR. Only pins belonging to a couple of
> devices are active and wakeup the system on activity.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index 7f06b1080244..c8ed0dd4fee4 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -61,6 +61,12 @@ properties:
>    mboxes:
>      minItems: 2
>  
> +  ti,partial-io-wakeup-sources:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: |
> +      List of phandles to devicetree nodes that can wakeup the SoC from the
> +      Partial IO poweroff mode.

I think the description needs a bunch of improvement here.

Can I use no board peripherals to be the phandle? say a GPIO expander
irq? This is not clear from the patch how peripherals and pins are
related?

We also need to warn readers that this capability is firmware driven and
not available on all SoC variants.

> +
>    ti,host-id:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: |
> -- 
> 2.43.0
> 

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

Re: [PATCH 1/6] dt-bindings: ti, sci: Add property for partial-io-wakeup-sources

[Krzysztof Kozlowski]

On 23/05/2024 10:02, Markus Schneider-Pargmann wrote:
> Add a property with an array of phandles to devices that have pins that
> are capable to wakeup the SoC from Partial-IO. In Partial-IO everything
> is powered off including the DDR. Only pins belonging to a couple of
> devices are active and wakeup the system on activity.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index 7f06b1080244..c8ed0dd4fee4 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -61,6 +61,12 @@ properties:
>    mboxes:
>      minItems: 2
>  
> +  ti,partial-io-wakeup-sources:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: |
> +      List of phandles to devicetree nodes that can wakeup the SoC from the
> +      Partial IO poweroff mode.

We have binding for this - wakeup-source in each device - what's wrong
with it?

Best regards,
Krzysztof

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

[Markus Schneider-Pargmann]

Add support for Partial-IO poweroff. In Partial-IO pins of a few modules
can generate system wakeups while DDR memory is not powered resulting in
a fresh boot of the system. The modules that can be wakeup sources are
defined by the devicetree.

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 enter an infinite loop.

The wakeup source device nodes are gathered during probe. But they are
only resolved to the actual devices in the sys_off handler, if they
exist. If they do not exist, they are ignored.

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

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 160968301b1f..04730c4df2de 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -116,6 +116,9 @@ struct ti_sci_info {
 	u8 host_id;
 	/* protected by ti_sci_list_mutex */
 	int users;
+
+	int nr_wakeup_sources;
+	struct device_node **wakeup_source_nodes;
 };
 
 #define cl_to_ti_sci_info(c)	container_of(c, struct ti_sci_info, cl)
@@ -380,6 +383,28 @@ static void ti_sci_put_one_xfer(struct ti_sci_xfers_info *minfo,
 	up(&minfo->sem_xfer_count);
 }
 
+/**
+ * ti_sci_do_send() - Do one send, do not expect a response
+ * @info:	Pointer to SCI entity information
+ * @xfer:	Transfer to initiate
+ *
+ * Return: If send error, return corresponding error, else
+ *	   if all goes well, return 0.
+ */
+static inline int ti_sci_do_send(struct ti_sci_info *info,
+				 struct ti_sci_xfer *xfer)
+{
+	int ret;
+
+	ret = mbox_send_message(info->chan_tx, &xfer->tx_message);
+	if (ret < 0)
+		return ret;
+
+	mbox_client_txdone(info->chan_tx, ret);
+
+	return 0;
+}
+
 /**
  * ti_sci_do_xfer() - Do one transfer
  * @info:	Pointer to SCI entity information
@@ -3262,6 +3287,79 @@ static int tisci_reboot_handler(struct sys_off_data *data)
 	return NOTIFY_BAD;
 }
 
+/* Does not return if successful */
+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;
+
+	ret = ti_sci_do_send(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 int tisci_sys_off_handler(struct sys_off_data *data)
+{
+	struct ti_sci_info *info = data->cb_data;
+	int i;
+	int ret;
+	bool enter_partial_io = false;
+
+	for (i = 0; i != info->nr_wakeup_sources; ++i) {
+		struct platform_device *pdev =
+			of_find_device_by_node(info->wakeup_source_nodes[i]);
+
+		if (!pdev)
+			continue;
+
+		if (device_may_wakeup(&pdev->dev)) {
+			dev_dbg(info->dev, "%pOFp identified as wakeup source\n",
+				info->wakeup_source_nodes[i]);
+			enter_partial_io = true;
+		}
+	}
+
+	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, halting system\n",
+			ERR_PTR(ret));
+
+	/* Halt system/code execution */
+	while (1)
+		;
+
+	return NOTIFY_DONE;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3398,6 +3496,35 @@ static int ti_sci_probe(struct platform_device *pdev)
 		goto out;
 	}
 
+	if (of_property_read_bool(dev->of_node, "ti,partial-io-wakeup-sources")) {
+		info->nr_wakeup_sources =
+			of_count_phandle_with_args(dev->of_node,
+						   "ti,partial-io-wakeup-sources",
+						   NULL);
+		info->wakeup_source_nodes =
+			devm_kzalloc(dev, sizeof(*info->wakeup_source_nodes),
+				     GFP_KERNEL);
+
+		for (i = 0; i != info->nr_wakeup_sources; ++i) {
+			struct device_node *devnode =
+				of_parse_phandle(dev->of_node,
+						 "ti,partial-io-wakeup-sources",
+						 i);
+			info->wakeup_source_nodes[i] = devnode;
+		}
+
+		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,
@@ -3407,7 +3534,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 ef3a8214d002..6d8b12341f68 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -35,6 +35,9 @@
 #define TI_SCI_MSG_QUERY_CLOCK_FREQ	0x010d
 #define TI_SCI_MSG_GET_CLOCK_FREQ	0x010e
 
+/* Low Power Mode Requests */
+#define TI_SCI_MSG_PREPARE_SLEEP	0x0300
+
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
@@ -545,6 +548,34 @@ struct ti_sci_msg_resp_get_clock_freq {
 	u64 freq_hz;
 } __packed;
 
+#define TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP				0x0
+#define TISCI_MSG_VALUE_SLEEP_MODE_MCU_ONLY				0x1
+#define TISCI_MSG_VALUE_SLEEP_MODE_STANDBY				0x2
+#define TISCI_MSG_VALUE_SLEEP_MODE_PARTIAL_IO				0x3
+
+/**
+ * struct tisci_msg_prepare_sleep_req - Request for TISCI_MSG_PREPARE_SLEEP.
+ *
+ * @hdr				TISCI header to provide ACK/NAK flags to the host.
+ * @mode			Low power mode to enter.
+ * @ctx_lo			Low 32-bits of physical pointer to address to use for context save.
+ * @ctx_hi			High 32-bits of physical pointer to address to use for context save.
+ * @debug_flags			Flags that can be set to halt the sequence during suspend or
+ *				resume to allow JTAG connection and debug.
+ *
+ * This message is used as the first step of entering a low power mode. It
+ * allows configurable information, including which state to enter to be
+ * easily shared from the application, as this is a non-secure message and
+ * therefore can be sent by anyone.
+ */
+struct ti_sci_msg_req_prepare_sleep {
+	struct ti_sci_msg_hdr	hdr;
+	u8			mode;
+	u32			ctx_lo;
+	u32			ctx_hi;
+	u32			debug_flags;
+} __packed;
+
 #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
 
 /**
-- 
2.43.0

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

[Nishanth Menon]

On 10:02-20240523, Markus Schneider-Pargmann wrote:
> Add support for Partial-IO poweroff. In Partial-IO pins of a few modules
> can generate system wakeups while DDR memory is not powered resulting in
> a fresh boot of the system. The modules that can be wakeup sources are
> defined by the devicetree.
> 
> 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 enter an infinite loop.
> 
> The wakeup source device nodes are gathered during probe. But they are
> only resolved to the actual devices in the sys_off handler, if they
> exist. If they do not exist, they are ignored.

Would have helped to provide link to relevant documentation here.

> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/firmware/ti_sci.c | 135 +++++++++++++++++++++++++++++++++++++-
>  drivers/firmware/ti_sci.h |  31 +++++++++
>  2 files changed, 165 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 160968301b1f..04730c4df2de 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -116,6 +116,9 @@ struct ti_sci_info {
>  	u8 host_id;
>  	/* protected by ti_sci_list_mutex */
>  	int users;
> +
> +	int nr_wakeup_sources;
> +	struct device_node **wakeup_source_nodes;

Documentation please.
>  };
>  
>  #define cl_to_ti_sci_info(c)	container_of(c, struct ti_sci_info, cl)
> @@ -380,6 +383,28 @@ static void ti_sci_put_one_xfer(struct ti_sci_xfers_info *minfo,
>  	up(&minfo->sem_xfer_count);
>  }
>  
> +/**
> + * ti_sci_do_send() - Do one send, do not expect a response

is ti_sci_send_no_response() a better name?

I have a basic question about an API at kernel level that does'nt
return back.. but I will ask in the context of tisci_enter_partial_io
below.


> + * @info:	Pointer to SCI entity information
> + * @xfer:	Transfer to initiate
> + *
> + * Return: If send error, return corresponding error, else
> + *	   if all goes well, return 0.
> + */
> +static inline int ti_sci_do_send(struct ti_sci_info *info,
> +				 struct ti_sci_xfer *xfer)
> +{
> +	int ret;

should'nt we make sure TI_SCI_FLAG_REQ_ACK_ON_PROCESSED is not set?
> +
> +	ret = mbox_send_message(info->chan_tx, &xfer->tx_message);
> +	if (ret < 0)
> +		return ret;
> +
> +	mbox_client_txdone(info->chan_tx, ret);
> +
> +	return 0;
> +}
> +

I am not sure I like two functions sending mbox_send_message. what do
you think of the following?

Use xfer-> hdr.flags and check against TI_SCI_FLAG_REQ_ACK_ON_PROCESSED
 itself to decide if ti_sci_do_xfer should expect a response or not?

>  /**
>   * ti_sci_do_xfer() - Do one transfer
>   * @info:	Pointer to SCI entity information
> @@ -3262,6 +3287,79 @@ static int tisci_reboot_handler(struct sys_off_data *data)
>  	return NOTIFY_BAD;
>  }
>  
> +/* Does not return if successful */

It wasn't clear from the commit message the strategy used. You are
triggering system off path here - do we loose the contents of
DDR in this flow? The power state needs to clearly described and the
rationale of using a variant of "off" path documented as well.

Looking further in to the code, I see we are unconditionally registering
the sys_off_handler based on ti,partial-io-wakeup-sources property being
present - how do we differentiate between actual PMIC power off desire
of user Vs just a few IO down power state for the user.

> +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;
> +
> +	ret = ti_sci_do_send(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 int tisci_sys_off_handler(struct sys_off_data *data)
> +{
> +	struct ti_sci_info *info = data->cb_data;
> +	int i;
> +	int ret;
> +	bool enter_partial_io = false;
> +
> +	for (i = 0; i != info->nr_wakeup_sources; ++i) {
> +		struct platform_device *pdev =
> +			of_find_device_by_node(info->wakeup_source_nodes[i]);
> +
> +		if (!pdev)
> +			continue;
> +
> +		if (device_may_wakeup(&pdev->dev)) {
> +			dev_dbg(info->dev, "%pOFp identified as wakeup source\n",
> +				info->wakeup_source_nodes[i]);
> +			enter_partial_io = true;
> +		}
> +	}
> +
> +	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, halting system\n",
> +			ERR_PTR(ret));

Is there no other diagnostics we can provide here?

> +
> +	/* Halt system/code execution */
> +	while (1)
> +		;
Why halt (1) -> spinning CPU in a while loop is not a power save mode
(at least idle?) :D

Why not fall through and loose power state context and allow the PMIC or
some other shutdown handler to attempt to power off?
> +
> +	return NOTIFY_DONE;
> +}
> +
>  /* Description for K2G */
>  static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>  	.default_host_id = 2,
> @@ -3398,6 +3496,35 @@ static int ti_sci_probe(struct platform_device *pdev)
>  		goto out;
>  	}
>  
> +	if (of_property_read_bool(dev->of_node, "ti,partial-io-wakeup-sources")) {
> +		info->nr_wakeup_sources =
> +			of_count_phandle_with_args(dev->of_node,
> +						   "ti,partial-io-wakeup-sources",
> +						   NULL);
> +		info->wakeup_source_nodes =
> +			devm_kzalloc(dev, sizeof(*info->wakeup_source_nodes),
> +				     GFP_KERNEL);
> +
> +		for (i = 0; i != info->nr_wakeup_sources; ++i) {
> +			struct device_node *devnode =
> +				of_parse_phandle(dev->of_node,
> +						 "ti,partial-io-wakeup-sources",
> +						 i);
> +			info->wakeup_source_nodes[i] = devnode;
> +		}
> +
> +		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,
> @@ -3407,7 +3534,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;

Unrelated change - please separate into different patch. Sounds like a
fix?

> +
>  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 ef3a8214d002..6d8b12341f68 100644
> --- a/drivers/firmware/ti_sci.h
> +++ b/drivers/firmware/ti_sci.h
> @@ -35,6 +35,9 @@
>  #define TI_SCI_MSG_QUERY_CLOCK_FREQ	0x010d
>  #define TI_SCI_MSG_GET_CLOCK_FREQ	0x010e
>  
> +/* Low Power Mode Requests */
> +#define TI_SCI_MSG_PREPARE_SLEEP	0x0300

Looking at https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#device-configuration-and-control-apis

Don't you need TISCI_MSG_SET_IO_ISOLATION  support?

Also, reading https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-enter-sleep

"This message is to be sent after TISCI_MSG_PREPARE_SLEEP and actually
triggers entry into the specified low power mode."

our call sequence seems to be just prepare_sleep and expect it to power
off the SoC? does'nt the PMIC need to be powered off?

> +
>  /* Resource Management Requests */
>  #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
>  
> @@ -545,6 +548,34 @@ struct ti_sci_msg_resp_get_clock_freq {
>  	u64 freq_hz;
>  } __packed;
>  
> +#define TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP				0x0
> +#define TISCI_MSG_VALUE_SLEEP_MODE_MCU_ONLY				0x1
> +#define TISCI_MSG_VALUE_SLEEP_MODE_STANDBY				0x2
> +#define TISCI_MSG_VALUE_SLEEP_MODE_PARTIAL_IO				0x3

Where are these values coming from?
https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-prepare-sleep

Does not seem to have these?

I think we are picking from:
https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#supported-low-power-modes

? The documentation could use a little cleanup there :(

> +
> +/**
> + * struct tisci_msg_prepare_sleep_req - Request for TISCI_MSG_PREPARE_SLEEP.

s/tisci_msg_prepare_sleep_req/ti_sci_msg_req_prepare_sleep ?

> + *
> + * @hdr				TISCI header to provide ACK/NAK flags to the host.
> + * @mode			Low power mode to enter.
> + * @ctx_lo			Low 32-bits of physical pointer to address to use for context save.
> + * @ctx_hi			High 32-bits of physical pointer to address to use for context save.
> + * @debug_flags			Flags that can be set to halt the sequence during suspend or
> + *				resume to allow JTAG connection and debug.

There are no schemes to enable there? Are we going to manually modify
the driver for every step of the debug?

> + *
> + * This message is used as the first step of entering a low power mode. It
> + * allows configurable information, including which state to enter to be
> + * easily shared from the application, as this is a non-secure message and
> + * therefore can be sent by anyone.
> + */
> +struct ti_sci_msg_req_prepare_sleep {
> +	struct ti_sci_msg_hdr	hdr;
> +	u8			mode;
> +	u32			ctx_lo;
> +	u32			ctx_hi;
> +	u32			debug_flags;
> +} __packed;

Also are we supposed to use this header for other low power sequences?
From the definitions of TISCI_MSG_VALUE_SLEEP_MODE_* it looks like there
are additional usage? Just trying to understand if follow on patches
will not have to refactor things here.

> +
>  #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
>  
>  /**
> -- 
> 2.43.0
> 

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

[PATCH 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 4cd2df467d0b..e36bce881f44 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)
+
 #define AM62AX_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
 #define AM62AX_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
 
-- 
2.43.0

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

[Nishanth Menon]

On 10:02-20240523, Markus Schneider-Pargmann wrote:
> 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 4cd2df467d0b..e36bce881f44 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)
> +
>  #define AM62AX_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>  #define AM62AX_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))

Could we do the due diligence in ensuring that there are no SoCs (like
tda devices) that may vary from the above definition? Please provide
that information in commit message.

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

[PATCH 4/6] arm64: dts: ti: k3-am62: Add partial-io wakeup sources

[Markus Schneider-Pargmann]

In Partial-IO mode there are a number of possible wakeup sources. Add
the list of phandles to these wakeup sources.

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

diff --git a/arch/arm64/boot/dts/ti/k3-am62.dtsi b/arch/arm64/boot/dts/ti/k3-am62.dtsi
index f0781f2bea29..9ffa6652492e 100644
--- a/arch/arm64/boot/dts/ti/k3-am62.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62.dtsi
@@ -120,3 +120,7 @@ dss_vp1_clk: clock-divider-oldi {
 #include "k3-am62-main.dtsi"
 #include "k3-am62-mcu.dtsi"
 #include "k3-am62-wakeup.dtsi"
+
+&dmsc {
+	ti,partial-io-wakeup-sources = <&mcu_mcan0>, <&mcu_mcan1>, <&mcu_uart0>, <&wkup_uart0>;
+};
-- 
2.43.0

[PATCH 5/6] arm64: dts: ti: k3-am62a: Add partial-io wakeup sources

[Markus Schneider-Pargmann]

In Partial-IO mode there are a number of possible wakeup sources. Add
the list of phandles to these wakeup sources.

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

diff --git a/arch/arm64/boot/dts/ti/k3-am62a.dtsi b/arch/arm64/boot/dts/ti/k3-am62a.dtsi
index b1b884600293..5c13851c29ec 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a.dtsi
@@ -123,3 +123,7 @@ cbass_wakeup: bus@b00000 {
 #include "k3-am62a-main.dtsi"
 #include "k3-am62a-mcu.dtsi"
 #include "k3-am62a-wakeup.dtsi"
+
+&dmsc {
+	ti,partial-io-wakeup-sources = <&mcu_mcan0>, <&mcu_mcan1>, <&mcu_uart0>, <&wkup_uart0>;
+};
-- 
2.43.0

[PATCH 6/6] arm64: dts: ti: k3-am62p: Add partial-io wakeup sources

[Markus Schneider-Pargmann]

From: Vibhore Vardhan <vibhore@ti.com>

In Partial-IO mode there are a number of possible wakeup sources. Add
the list of phandles to these wakeup sources. 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.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62p.dtsi b/arch/arm64/boot/dts/ti/k3-am62p.dtsi
index 94babc412575..4d43cc972056 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p.dtsi
@@ -123,3 +123,7 @@ cbass_wakeup: bus@b00000 {
 #include "k3-am62p-main.dtsi"
 #include "k3-am62p-mcu.dtsi"
 #include "k3-am62p-wakeup.dtsi"
+
+&dmsc {
+	ti,partial-io-wakeup-sources = <&mcu_mcan0>, <&mcu_mcan1>, <&mcu_uart0>, <&wkup_uart0>;
+};
-- 
2.43.0

Re: [PATCH 6/6] arm64: dts: ti: k3-am62p: Add partial-io wakeup sources

[Nishanth Menon]

On 10:02-20240523, Markus Schneider-Pargmann wrote:
> From: Vibhore Vardhan <vibhore@ti.com>
> 
> In Partial-IO mode there are a number of possible wakeup sources. Add
> the list of phandles to these wakeup sources. 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.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p.dtsi b/arch/arm64/boot/dts/ti/k3-am62p.dtsi
> index 94babc412575..4d43cc972056 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62p.dtsi
> @@ -123,3 +123,7 @@ cbass_wakeup: bus@b00000 {
>  #include "k3-am62p-main.dtsi"
>  #include "k3-am62p-mcu.dtsi"
>  #include "k3-am62p-wakeup.dtsi"
> +
> +&dmsc {
> +	ti,partial-io-wakeup-sources = <&mcu_mcan0>, <&mcu_mcan1>, <&mcu_uart0>, <&wkup_uart0>;
> +};

NOTE: https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#supported-low-power-modes
table of SoCs supported do not provide J722s or AM62p in
the list. I suspect it is an oversight, but seems to be a mismatch atm.

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

Re: [PATCH 6/6] arm64: dts: ti: k3-am62p: Add partial-io wakeup sources

[Dhruva Gole]

Hi Nishanth,

On May 24, 2024 at 00:54:58 -0500, Nishanth Menon wrote:
> On 10:02-20240523, Markus Schneider-Pargmann wrote:
> > From: Vibhore Vardhan <vibhore@ti.com>
> > 
> > In Partial-IO mode there are a number of possible wakeup sources. Add
> > the list of phandles to these wakeup sources. 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.dtsi | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-am62p.dtsi b/arch/arm64/boot/dts/ti/k3-am62p.dtsi
> > index 94babc412575..4d43cc972056 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am62p.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am62p.dtsi
> > @@ -123,3 +123,7 @@ cbass_wakeup: bus@b00000 {
> >  #include "k3-am62p-main.dtsi"
> >  #include "k3-am62p-mcu.dtsi"
> >  #include "k3-am62p-wakeup.dtsi"
> > +
> > +&dmsc {
> > +	ti,partial-io-wakeup-sources = <&mcu_mcan0>, <&mcu_mcan1>, <&mcu_uart0>, <&wkup_uart0>;
> > +};
> 
> NOTE: https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#supported-low-power-modes
> table of SoCs supported do not provide J722s or AM62p in
> the list. I suspect it is an oversight, but seems to be a mismatch atm.

Yes, you are right. We will fix this in the documentation soon.
AM62P will be added to that list. J722s does not have lpm support.

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

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

[Nishanth Menon]

On 10:02-20240523, Markus Schneider-Pargmann wrote:
> Hi,
> 
> 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. A list of potential wakeup
> sources for the specific SoC is described in the devicetree. The wakeup
> sources can be individually enabled/disabled by the user in the running
> system.
> 
> The series is based on Andrew Davis accepted patches:
>   [PATCH 0/4] Unconditionally register TI-SCI reset handler
>   https://lore.kernel.org/lkml/20240326223730.54639-1-afd@ti.com/
> 
> 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.
> 
> These two other series are relevant for the support of Partial-IO:
> 
>  - serial: 8250: omap: Add am62 wakeup support
>  - can: m_can: Add am62 wakeup support

In future, please provide the lore url to the dependency patches (I
understand you were probably posting them back to back, but if you wait some
10-15 mins after the dependent series are posted, they will appear in
the lore.kernel.org and you could provide those links). That would save
us maintainers having to lookup based off subject line.

> 
> 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.9?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
> 
> Markus Schneider-Pargmann (5):
>   dt-bindings: ti, sci: Add property for partial-io-wakeup-sources
>   firmware: ti_sci: Partial-IO support
>   arm64: dts: ti: k3-pinctrl: Add WKUP_EN flag
>   arm64: dts: ti: k3-am62: Add partial-io wakeup sources
>   arm64: dts: ti: k3-am62a: Add partial-io wakeup sources
> 
> Vibhore Vardhan (1):
>   arm64: dts: ti: k3-am62p: Add partial-io wakeup sources
> 
>  .../bindings/arm/keystone/ti,sci.yaml         |   6 +
>  arch/arm64/boot/dts/ti/k3-am62.dtsi           |   4 +
>  arch/arm64/boot/dts/ti/k3-am62a.dtsi          |   4 +
>  arch/arm64/boot/dts/ti/k3-am62p.dtsi          |   4 +
>  arch/arm64/boot/dts/ti/k3-pinctrl.h           |   3 +
>  drivers/firmware/ti_sci.c                     | 135 +++++++++++++++++-
>  drivers/firmware/ti_sci.h                     |  31 ++++
>  7 files changed, 186 insertions(+), 1 deletion(-)
> 
> -- 
> 2.43.0
> 

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