[PATCH v10 0/4] firmware: ti_sci: Introduce system suspend support

[PATCH v10 0/4] firmware: ti_sci: Introduce system suspend support

[Kevin Hilman]

Abstract
********

This series introduces necessary ti_sci driver functionality to support
various Suspend-to-RAM modes on TI AM62 family of devices. These Low
Power Modes include Deep Sleep and MCU Only as described in section
"6.2.4 Power Modes" of the AM62P Technical Reference Manual [0].

Summary
*******

This series is a restructuring and rebase of the patch series by
Dave Gerlach [1] and Dhruva Gole [2]. It applies on top of Linux
6.11-rc1.

The kernel triggers entry to Low Power Mode through the mem suspend
transition with the following:

* At the  bootloader stage, one is  expected to package the  TIFS stub
  which then gets  pulled into the Tightly coupled memory  of the Device
  Mgr  (DM) R5  when it  starts up.  If using  U-Boot, then  it requires
  tispl.bin to contain the TIFS stub. Refer to documentation in upstream
  u-boot[3] for further details. The  supported firmware version is from
  TI Processor SDK >= 10.00 ie. tag 10.00.04 from ti-linux-firmware [4].

* Use a TF-A binary that supports PSCI_SYSTEM_SUSPEND call. This causes
  system to use PSCI system suspend as last step of mem sleep.

* We add support for the TISCI_MSG_QUERY_FW_CAPS message, used to retrieve
  the firmware capabilities of the currently running system firmware [6].
  Sysfw version >= 10.00.04 support LPM_DM_MANAGED capability, where
  Device Mgr firmware now manages which low power mode is chosen. Going
  forward, this is the default configuration supported for TI AM62 family
  of devices. The state chosen by the DM can be influenced by sending
  constraints using the new LPM constraint APIs. (Patch 1)

* The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP
  message in order to provide details about suspend. The ti_sci driver
  must send this message to firmware with the above information
  included, which it does during the driver suspend handler when
  PM_MEM_SUSPEND is the determined state being entered. The mode being
  sent depends on whether firmware capabilities have support for
  LPM_DM_MANAGED feature. Legacy firmware or those supporting other
  modes can extend the mode selection logic as needed. (Patch 2)

* We also add the remaining TISCI Low Power Mode messages required for
  inquiring wake reason and managing LPM constraints as part of a new PM
  ops. These messages are part of the TISCI PM Low Power Mode API [5].
  (Patch 3)

* Finally if any CPUs have PM QoS resume latency constraints set, we
  aggregate these and set the TISCI system-wide latency constraint.
  (Patch 4)

Testing
*******

This series can for example be tested with a am62a-lp-sk board.

For am62a-lp-sk all usb nodes have to be disabled at the moment (usbss0,
usb0, usbss1 and usb1). There is currently an issue with USB Link Power
Management and turning off the USB device which is being worked on.

Once booted suspend/resume can be tested with rtcwake:
  $ rtcwake -m mem -s 10 -d /dev/rtc0

Make sure /dev/rtc0 corresponds to rtc-ti-k3:
  $ dmesg | grep rtc-ti-k3
  rtc-ti-k3 2b1f0000.rtc: registered as rtc0

Base commit:
************

v6.11-rc1

Changelog:
**********
v10:
- add "wake reason" handling which is also supported by 10.x
  firmware update, but was mistakenly left out of v9
- fix debug print of which CPU caused max CPU latency
- update TRM pointer[0] to point to AM62P TRM which has better
  description of low-power modes shared across AM62P family
- update u-boot documentaion pointer to point to upstream u-boot
  commit.
  
v9:
- Include Kevin's patch to add CPU latency constraint management into
  this series. Posted here in v3:
  https://lore.kernel.org/lkml/20240802214220.3472221-1-khilman@baylibre.com/
- reorder patches to avoid any build warnings
- ti_sci_cmd_prepare_sleep was moved into the patch that adds suspend
  and resume calls
- pmops wake_reason is now only set if the capabilities exist
- Use pmops pointer instead of full path

v8:
- Restructuring of code to include all TISCI PM LPM ops
- Removing malloc related to TIFS Stub as it is managed by DM
- Dropping has_lpm check as suggested by Nishanth
- Using LPM_DM_MANAGED capability for mode selection
- Updating the suspend and resume callback handlers

v7:
- Address Andrew's concerns on SYSFW fw_caps API
- Remove all the unused functions and variables including
  set_io_isolation and wake_reason calls
- use dma_free_attrs
- remove IO isolation related code from linux side,

v6:
- link to v6 [5]
- Loading of FS Stub from linux no longer needed, hence drop that patch,
- Drop 1/6 and 5/6 from the previous series [4].
- Add system suspend resume callbacks which were removed in
commit 9225bcdedf16297a346082e7d23b0e8434aa98ed ("firmware: ti_sci: Use
system_state to determine polling")
- Use IO isolation while putting the system in suspend to RAM

v5:
- Add support (patch 3) for detecting the low power modes (LPM) of the
  FW/SoC with a recently introduced core TISCI_MSG_QUERY_FW_CAPS message.
- Use TISCI_MSG_QUERY_FW_CAPS instead of misusing the
  TISCI_MSG_PREPARE_SLEEP to detect the FW/SoC low power caps (patch 4).
- Take into account the supported LPMs in ti_sci_prepare_system_suspend()
  and handle the case when CONFIG_SUSPEND is not enabled (patch 6) that
  was reported by Roger Quadros and LKP.
- Pick up Rob Herring's "Reviewed-by" tag for the binding patch.

v4:
- Fix checkpacth warnings in patches 2 and 3.
- Drop the links with anchors in patch 2.

v3:
- Fix the compile warnings on 32-bit platforms reported by the kernel
  test robot in patches (3,5).
- Pick up Roger's "Tested-by" tags.

v2:
- Addressed comments received for v1 series [1].
- Updated v1 patch 5 to use pm notifier to avoid firmware loading
  issues.
- Dropped the reserved region requirement and allocate DMA memory
  instead. The reserved region binding patch is also removed.
- Introduce two more TISCI LPM messages that are supported in SysFW.
- Fixes in error handling.

References:
***********

[0] https://www.ti.com/lit/pdf/spruiv7
[1] https://lore.kernel.org/lkml/20220421203659.27853-1-d-gerlach@ti.com
[2] https://lore.kernel.org/lkml/20230804115037.754994-1-d-gole@ti.com
[3] https://source.denx.de/u-boot/u-boot/-/commit/962f60abca82bb11501bc0c627abacda15bed076
[4] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/?h=10.00.06&id=193f7d7570583a41ddc50a221e37c32be6be583e
[5] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
[6] https://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html#tisci-msg-query-fw-caps

Dave Gerlach (1):
  firmware: ti_sci: Introduce Power Management Ops

Georgi Vlaev (1):
  firmware: ti_sci: Add support for querying the firmware caps

Kevin Hilman (1):
  firmware: ti_sci: add CPU latency constraint management

Vibhore Vardhan (1):
  firmware: ti_sci: Add system suspend and resume call

 drivers/firmware/ti_sci.c              | 452 ++++++++++++++++++++++++-
 drivers/firmware/ti_sci.h              | 137 +++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  46 +++
 3 files changed, 633 insertions(+), 2 deletions(-)

--
2.45.2

---
Dave Gerlach (1):
      firmware: ti_sci: Introduce Power Management Ops

Georgi Vlaev (1):
      firmware: ti_sci: Add support for querying the firmware caps

Kevin Hilman (1):
      firmware: ti_sci: add CPU latency constraint management

Vibhore Vardhan (1):
      firmware: ti_sci: Add system suspend and resume call

 drivers/firmware/ti_sci.c              | 468 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/firmware/ti_sci.h              | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  46 +++++++++++++++++
 3 files changed, 655 insertions(+), 2 deletions(-)
---
base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
change-id: 20240813-lpm-constraints-firmware-msp-1c0bd092a137

Best regards,
-- 
Kevin Hilman <khilman@baylibre.com>

[PATCH v10 1/4] firmware: ti_sci: Add support for querying the firmware caps

[Kevin Hilman]

From: Georgi Vlaev <g-vlaev@ti.com>

Add support for the TISCI_MSG_QUERY_FW_CAPS message, used to retrieve
the firmware capabilities of the currently running system firmware. The
message belongs to the TISCI general core message API [1] and is
available in SysFW version 08.04.03 and above. Currently, the message is
supported on devices with split architecture of the system firmware (DM
+ TIFS) like AM62x. Old revisions or not yet supported platforms will
NACK this request.

We're using this message locally in ti_sci.c to get the low power
featutes of the FW/SoC. As there's no other kernel consumers yet, this
is not added to struct ti_sci_core_ops.

Sysfw version >= 10.00.04 support LPM_DM_MANAGED capability [2], where
Device Mgr firmware now manages which low power mode is chosen. Going
forward, this is the default configuration supported for TI AM62 family
of devices. The state chosen by the DM can be influenced by sending
constraints using the new LPM constraint APIs.

[1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html
[2] https://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html#tisci-msg-query-fw-caps

Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
[vibhore@ti.com: Support for LPM_DM_MANAGED mode]
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/firmware/ti_sci.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/firmware/ti_sci.h | 22 ++++++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 160968301b1f..f77e13577eb8 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2,7 +2,7 @@
 /*
  * Texas Instruments System Control Interface Protocol Driver
  *
- * Copyright (C) 2015-2022 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C) 2015-2024 Texas Instruments Incorporated - https://www.ti.com/
  *	Nishanth Menon
  */
 
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/soc/ti/ti-msgmgr.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/sys_soc.h>
 #include <linux/reboot.h>
 
 #include "ti_sci.h"
@@ -98,6 +99,7 @@ struct ti_sci_desc {
  * @minfo:	Message info
  * @node:	list head
  * @host_id:	Host ID
+ * @fw_caps:	FW/SoC low power capabilities
  * @users:	Number of users of this instance
  */
 struct ti_sci_info {
@@ -114,6 +116,7 @@ struct ti_sci_info {
 	struct ti_sci_xfers_info minfo;
 	struct list_head node;
 	u8 host_id;
+	u64 fw_caps;
 	/* protected by ti_sci_list_mutex */
 	int users;
 };
@@ -1651,6 +1654,62 @@ static int ti_sci_cmd_clk_get_freq(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_msg_cmd_query_fw_caps() - Get the FW/SoC capabilities
+ * @handle:		Pointer to TI SCI handle
+ * @fw_caps:		Each bit in fw_caps indicating one FW/SOC capability
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_msg_cmd_query_fw_caps(const struct ti_sci_handle *handle,
+					u64 *fw_caps)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_xfer *xfer;
+	struct ti_sci_msg_resp_query_fw_caps *resp;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_QUERY_FW_CAPS,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(struct ti_sci_msg_hdr),
+				   sizeof(*resp));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_resp_query_fw_caps *)xfer->xfer_buf;
+
+	if (!ti_sci_is_response_ack(resp)) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	if (fw_caps)
+		*fw_caps = resp->fw_caps;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
 static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
 {
 	struct ti_sci_info *info;
@@ -3390,6 +3449,14 @@ static int ti_sci_probe(struct platform_device *pdev)
 		goto out;
 	}
 
+	/*
+	 * Check if the firmware supports any optional low power modes.
+	 * Old revisions of TIFS (< 08.04) will NACK the request.
+	 */
+	ret = ti_sci_msg_cmd_query_fw_caps(&info->handle, &info->fw_caps);
+	if (ret || !(info->fw_caps & MSG_FLAG_CAPS_GENERIC))
+		pr_debug("Unable to detect LPM capabilities in fw_caps\n");
+
 	ti_sci_setup_ops(info);
 
 	ret = devm_register_restart_handler(dev, tisci_reboot_handler, info);
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index 5846c60220f5..73ca9503606b 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -19,6 +19,7 @@
 #define TI_SCI_MSG_WAKE_REASON	0x0003
 #define TI_SCI_MSG_GOODBYE	0x0004
 #define TI_SCI_MSG_SYS_RESET	0x0005
+#define TI_SCI_MSG_QUERY_FW_CAPS	0x0022
 
 /* Device requests */
 #define TI_SCI_MSG_SET_DEVICE_STATE	0x0200
@@ -132,6 +133,27 @@ struct ti_sci_msg_req_reboot {
 	struct ti_sci_msg_hdr hdr;
 } __packed;
 
+/**
+ * struct ti_sci_msg_resp_query_fw_caps - Response for query firmware caps
+ * @hdr:	Generic header
+ * @fw_caps:	Each bit in fw_caps indicating one FW/SOC capability
+ *		MSG_FLAG_CAPS_GENERIC: Generic capability (LPM not supported)
+ *		MSG_FLAG_CAPS_LPM_PARTIAL_IO: Partial IO in LPM
+ *		MSG_FLAG_CAPS_LPM_DM_MANAGED: LPM can be managed by DM
+ *
+ * Response to a generic message with message type TI_SCI_MSG_QUERY_FW_CAPS
+ * providing currently available SOC/firmware capabilities. SoC that don't
+ * support low power modes return only MSG_FLAG_CAPS_GENERIC capability.
+ */
+struct ti_sci_msg_resp_query_fw_caps {
+	struct ti_sci_msg_hdr hdr;
+#define MSG_FLAG_CAPS_GENERIC		TI_SCI_MSG_FLAG(0)
+#define MSG_FLAG_CAPS_LPM_PARTIAL_IO	TI_SCI_MSG_FLAG(4)
+#define MSG_FLAG_CAPS_LPM_DM_MANAGED	TI_SCI_MSG_FLAG(5)
+#define MSG_MASK_CAPS_LPM		GENMASK_ULL(4, 1)
+	u64 fw_caps;
+} __packed;
+
 /**
  * struct ti_sci_msg_req_set_device_state - Set the desired state of the device
  * @hdr:		Generic header

-- 
2.46.0

[PATCH v10 2/4] firmware: ti_sci: Add system suspend and resume call

[Kevin Hilman]

From: Vibhore Vardhan <vibhore@ti.com>

Introduce system suspend call that enables the ti_sci driver to support
low power mode when the user space issues a suspend to mem.

The following power management operations defined in the TISCI
Low Power Mode API [1] are implemented to support suspend and resume:

1) TISCI_MSG_PREPARE_SLEEP
Prepare the SOC for entering into a low power mode and
provide details to firmware about the state being entered.

2) TISCI_MSG_SET_IO_ISOLATION
Control the IO isolation for Low Power Mode.

Also, write a ti_sci_prepare_system_suspend call to be used in the driver
suspend handler to allow the system to identify the low power mode being
entered and if necessary, send TISCI_MSG_PREPARE_SLEEP with information
about the mode being entered.

Sysfw version >= 10.00.04 support LPM_DM_MANAGED capability [2], where
Device Mgr firmware now manages which low power mode is chosen. Going
forward, this is the default configuration supported for TI AM62 family
of devices. The state chosen by the DM can be influenced by sending
constraints using the new LPM constraint APIs.

In case the firmware does not support LPM_DM_MANAGED mode, the mode
selection logic can be extended as needed. If no suspend-to-RAM modes
are supported, return without taking any action.

We're using "pm_suspend_target_state" to map the kernel's target suspend
state to SysFW low power mode. Make sure this is available only when
CONFIG_SUSPEND is enabled.

Suspend has to be split into two parts, ti_sci_suspend() will send
the prepare sleep message to prepare suspend. ti_sci_suspend_noirq()
sets IO isolation which needs to be done as late as possible to avoid
any issues. On resume this has to be done as early as possible.

[1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html

Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Signed-off-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/firmware/ti_sci.c              | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  45 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/soc/ti/ti_sci_protocol.h |   4 ++++
 3 files changed, 236 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index f77e13577eb8..808149dcc635 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/soc/ti/ti-msgmgr.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/suspend.h>
 #include <linux/sys_soc.h>
 #include <linux/reboot.h>
 
@@ -1654,6 +1655,65 @@ static int ti_sci_cmd_clk_get_freq(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_cmd_prepare_sleep() - Prepare system for system suspend
+ * @handle:		pointer to TI SCI handle
+ * @mode:		Device identifier
+ * @ctx_lo:		Low part of address for context save
+ * @ctx_hi:		High part of address for context save
+ * @debug_flags:	Debug flags to pass to firmware
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
+				    u32 ctx_lo, u32 ctx_hi, u32 debug_flags)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_msg_req_prepare_sleep *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_PREPARE_SLEEP,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	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 = mode;
+	req->ctx_lo = ctx_lo;
+	req->ctx_hi = ctx_hi;
+	req->debug_flags = debug_flags;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
 /**
  * ti_sci_msg_cmd_query_fw_caps() - Get the FW/SoC capabilities
  * @handle:		Pointer to TI SCI handle
@@ -1710,6 +1770,58 @@ static int ti_sci_msg_cmd_query_fw_caps(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_cmd_set_io_isolation() - Enable IO isolation in LPM
+ * @handle:		Pointer to TI SCI handle
+ * @state:		The desired state of the IO isolation
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_cmd_set_io_isolation(const struct ti_sci_handle *handle,
+				       u8 state)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_msg_req_set_io_isolation *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_SET_IO_ISOLATION,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	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_set_io_isolation *)xfer->xfer_buf;
+	req->state = state;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
 static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
 {
 	struct ti_sci_info *info;
@@ -3321,6 +3433,79 @@ static int tisci_reboot_handler(struct sys_off_data *data)
 	return NOTIFY_BAD;
 }
 
+#ifdef CONFIG_SUSPEND
+static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
+{
+	u8 mode;
+
+	/*
+	 * Map and validate the target Linux suspend state to TISCI LPM.
+	 * Default is to let Device Manager select the low power mode.
+	 */
+	switch (pm_suspend_target_state) {
+	case PM_SUSPEND_MEM:
+		if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED)
+			mode = TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED;
+		else
+			/* DM Managed is not supported by the firmware. */
+			return -EOPNOTSUPP;
+		break;
+	default:
+		/*
+		 * Do not fail if we don't have action to take for a
+		 * specific suspend mode.
+		 */
+		return 0;
+	}
+
+	return ti_sci_cmd_prepare_sleep(&info->handle, mode, 0, 0, 0);
+}
+
+static int ti_sci_suspend(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ti_sci_prepare_system_suspend(info);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ti_sci_suspend_noirq(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret = 0;
+
+	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
+	if (ret)
+		return ret;
+	dev_dbg(dev, "%s: set isolation: %d\n", __func__, ret);
+
+	return 0;
+}
+
+static int ti_sci_resume_noirq(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret = 0;
+
+	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
+	if (ret)
+		return ret;
+	dev_dbg(dev, "%s: disable isolation: %d\n", __func__, ret);
+
+	return 0;
+}
+
+static const struct dev_pm_ops ti_sci_pm_ops = {
+	.suspend = ti_sci_suspend,
+	.suspend_noirq = ti_sci_suspend_noirq,
+	.resume_noirq = ti_sci_resume_noirq,
+};
+#endif  /* CONFIG_SUSPEND */
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3490,6 +3675,9 @@ static struct platform_driver ti_sci_driver = {
 		   .name = "ti-sci",
 		   .of_match_table = of_match_ptr(ti_sci_of_match),
 		   .suppress_bind_attrs = true,
+#ifdef CONFIG_SUSPEND
+		   .pm = &ti_sci_pm_ops,
+#endif
 	},
 };
 module_platform_driver(ti_sci_driver);
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index 73ca9503606b..8efe4d0e61fb 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -6,7 +6,7 @@
  * The system works in a message response protocol
  * See: https://software-dl.ti.com/tisci/esd/latest/index.html for details
  *
- * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C)  2015-2024 Texas Instruments Incorporated - https://www.ti.com/
  */
 
 #ifndef __TI_SCI_H
@@ -36,6 +36,10 @@
 #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
+#define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
+
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
@@ -567,6 +571,45 @@ struct ti_sci_msg_resp_get_clock_freq {
 	u64 freq_hz;
 } __packed;
 
+/**
+ * struct tisci_msg_req_prepare_sleep - 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;
+
+#define TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED				0xfd
+	u8			mode;
+	u32			ctx_lo;
+	u32			ctx_hi;
+	u32			debug_flags;
+} __packed;
+
+/**
+ * struct tisci_msg_set_io_isolation_req - Request for TI_SCI_MSG_SET_IO_ISOLATION.
+ *
+ * @hdr:	Generic header
+ * @state:	The deseared state of the IO isolation.
+ *
+ * This message is used to enable/disable IO isolation for low power modes.
+ * Response is generic ACK / NACK message.
+ */
+struct ti_sci_msg_req_set_io_isolation {
+	struct ti_sci_msg_hdr hdr;
+	u8 state;
+} __packed;
+
 #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
 
 /**
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index bd0d11af76c5..1f1871e23f76 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -195,6 +195,10 @@ struct ti_sci_clk_ops {
 			u64 *current_freq);
 };
 
+/* TISCI LPM IO isolation control values */
+#define TISCI_MSG_VALUE_IO_ENABLE			1
+#define TISCI_MSG_VALUE_IO_DISABLE			0
+
 /**
  * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
  * @start:	Start index of the first resource range.

-- 
2.46.0

[PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Kevin Hilman]

From: Dave Gerlach <d-gerlach@ti.com>

Introduce power management ops supported by the TISCI
Low Power Mode API [1].

1) TISCI_MSG_LPM_WAKE_REASON
Get which wake up source woke the SoC from Low Power Mode.
The wake up source IDs will be common for all K3 platforms.

2) TISCI_MSG_LPM_SET_DEVICE_CONSTRAINT
Set LPM constraint on behalf of a device. By setting a constraint, the
device ensures that it will not be powered off or reset in the selected
mode.

3) TISCI_MSG_LPM_SET_LATENCY_CONSTRAINT
Set LPM resume latency constraint. By setting a constraint, the host
ensures that the resume time from selected mode will be less than the
constraint value.

[1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
[g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support]
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
[a-kaur@ti.com: SET_DEVICE_CONSTRAINT support]
Signed-off-by: Akashdeep Kaur <a-kaur@ti.com>
[vibhore@ti.com: SET_LATENCY_CONSTRAINT support]
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/firmware/ti_sci.c              | 189 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/ti/ti_sci_protocol.h |  42 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 808149dcc635..107494406fa5 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1822,6 +1822,179 @@ static int ti_sci_cmd_set_io_isolation(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM
+ * @handle:		Pointer to TI SCI handle
+ * @source:		The wakeup source that woke the SoC from LPM
+ * @timestamp:		Timestamp of the wakeup event
+ * @pin:		The pin that has triggered wake up
+ * @mode:		The last entered low power mode
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_msg_cmd_lpm_wake_reason(const struct ti_sci_handle *handle,
+					  u32 *source, u64 *timestamp, u8 *pin, u8 *mode)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_xfer *xfer;
+	struct ti_sci_msg_resp_lpm_wake_reason *resp;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_WAKE_REASON,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(struct ti_sci_msg_hdr),
+				   sizeof(*resp));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_resp_lpm_wake_reason *)xfer->xfer_buf;
+
+	if (!ti_sci_is_response_ack(resp)) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	if (source)
+		*source = resp->wake_source;
+	if (timestamp)
+		*timestamp = resp->wake_timestamp;
+	if (pin)
+		*pin = resp->wake_pin;
+	if (mode)
+		*mode = resp->mode;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
+/**
+ * ti_sci_cmd_set_device_constraint() - Set LPM constraint on behalf of a device
+ * @handle:	pointer to TI SCI handle
+ * @id:	Device identifier
+ * @state:	The desired state of device constraint: set or clear
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_cmd_set_device_constraint(const struct ti_sci_handle *handle,
+					    u32 id, u8 state)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_msg_req_lpm_set_device_constraint *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	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_lpm_set_device_constraint *)xfer->xfer_buf;
+	req->id = id;
+	req->state = state;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
+/**
+ * ti_sci_cmd_set_latency_constraint() - Set LPM resume latency constraint
+ * @handle:	pointer to TI SCI handle
+ * @latency:	maximum acceptable latency (in ms) to wake up from LPM
+ * @state:	The desired state of latency constraint: set or clear
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_cmd_set_latency_constraint(const struct ti_sci_handle *handle,
+					     u16 latency, u8 state)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_msg_req_lpm_set_latency_constraint *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	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_lpm_set_latency_constraint *)xfer->xfer_buf;
+	req->latency = latency;
+	req->state = state;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
 static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
 {
 	struct ti_sci_info *info;
@@ -2964,6 +3137,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	struct ti_sci_core_ops *core_ops = &ops->core_ops;
 	struct ti_sci_dev_ops *dops = &ops->dev_ops;
 	struct ti_sci_clk_ops *cops = &ops->clk_ops;
+	struct ti_sci_pm_ops *pmops = &ops->pm_ops;
 	struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops;
 	struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops;
 	struct ti_sci_rm_ringacc_ops *rops = &ops->rm_ring_ops;
@@ -3003,6 +3177,13 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	cops->set_freq = ti_sci_cmd_clk_set_freq;
 	cops->get_freq = ti_sci_cmd_clk_get_freq;
 
+	if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) {
+		pr_debug("detected DM managed LPM in fw_caps\n");
+		pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason;
+		pmops->set_device_constraint = ti_sci_cmd_set_device_constraint;
+		pmops->set_latency_constraint = ti_sci_cmd_set_latency_constraint;
+	}
+
 	rm_core_ops->get_range = ti_sci_cmd_get_resource_range;
 	rm_core_ops->get_range_from_shost =
 				ti_sci_cmd_get_resource_range_from_shost;
@@ -3490,12 +3671,20 @@ static int ti_sci_resume_noirq(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
 	int ret = 0;
+	u32 source;
+	u64 time;
+	u8 pin;
+	u8 mode;
 
 	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
 	if (ret)
 		return ret;
 	dev_dbg(dev, "%s: disable isolation: %d\n", __func__, ret);
 
+	ti_sci_msg_cmd_lpm_wake_reason(&info->handle, &source, &time, &pin, &mode);
+	dev_info(dev, "ti_sci: wakeup source:0x%x, pin:0x%x, mode:0x%x\n",
+		 source, pin, mode);
+
 	return 0;
 }
 
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index 8efe4d0e61fb..053387d7baa0 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -38,7 +38,10 @@
 
 /* Low Power Mode Requests */
 #define TI_SCI_MSG_PREPARE_SLEEP	0x0300
+#define TI_SCI_MSG_LPM_WAKE_REASON	0x0306
 #define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
+#define TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT	0x0309
+#define TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT	0x030A
 
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
@@ -610,6 +613,79 @@ struct ti_sci_msg_req_set_io_isolation {
 	u8 state;
 } __packed;
 
+/**
+ * struct ti_sci_msg_resp_lpm_wake_reason - Response for TI_SCI_MSG_LPM_WAKE_REASON.
+ *
+ * @hdr:		Generic header.
+ * @wake_source:	The wake up source that woke soc from LPM.
+ * @wake_timestamp:	Timestamp at which soc woke.
+ * @wake_pin: The pin that has triggered wake up.
+ * @mode: The last entered low power mode.
+ * @rsvd:	Reserved for future use.
+ *
+ * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON,
+ * used to query the wake up source, pin and entered low power mode.
+ */
+struct ti_sci_msg_resp_lpm_wake_reason {
+	struct ti_sci_msg_hdr hdr;
+	u32 wake_source;
+	u64 wake_timestamp;
+	u8 wake_pin;
+	u8 mode;
+	u32 rsvd[2];
+} __packed;
+
+/**
+ * struct ti_sci_msg_req_lpm_set_device_constraint - Request for
+ * TISCI_MSG_LPM_SET_DEVICE_CONSTRAINT.
+ *
+ * @hdr:	TISCI header to provide ACK/NAK flags to the host.
+ * @id:	Device ID of device whose constraint has to be modified.
+ * @state:	The desired state of device constraint: set or clear.
+ * @rsvd:	Reserved for future use.
+ *
+ * This message is used by host to set constraint on the device. This can be
+ * sent anytime after boot before prepare sleep message. Any device can set a
+ * constraint on the low power mode that the SoC can enter. It allows
+ * configurable information to be easily shared from the application, as this
+ * is a non-secure message and therefore can be sent by anyone. By setting a
+ * constraint, the device ensures that it will not be powered off or reset in
+ * the selected mode. Note: Access Restriction: Exclusivity flag of Device will
+ * be honored. If some other host already has constraint on this device ID,
+ * NACK will be returned.
+ */
+struct ti_sci_msg_req_lpm_set_device_constraint {
+	struct ti_sci_msg_hdr hdr;
+	u32 id;
+	u8 state;
+	u32 rsvd[2];
+} __packed;
+
+/**
+ * struct ti_sci_msg_req_lpm_set_latency_constraint - Request for
+ * TISCI_MSG_LPM_SET_LATENCY_CONSTRAINT.
+ *
+ * @hdr:	TISCI header to provide ACK/NAK flags to the host.
+ * @wkup_latency:	The maximum acceptable latency to wake up from low power mode
+ *			in milliseconds. The deeper the state, the higher the latency.
+ * @state:	The desired state of wakeup latency constraint: set or clear.
+ * @rsvd:	Reserved for future use.
+ *
+ * This message is used by host to set wakeup latency from low power mode. This can
+ * be sent anytime after boot before prepare sleep message, and can be sent after
+ * current low power mode is exited. Any device can set a constraint on the low power
+ * mode that the SoC can enter. It allows configurable information to be easily shared
+ * from the application, as this is a non-secure message and therefore can be sent by
+ * anyone. By setting a wakeup latency constraint, the host ensures that the resume time
+ * from selected low power mode will be less than the constraint value.
+ */
+struct ti_sci_msg_req_lpm_set_latency_constraint {
+	struct ti_sci_msg_hdr hdr;
+	u16 latency;
+	u8 state;
+	u32 rsvd;
+} __packed;
+
 #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
 
 /**
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index 1f1871e23f76..4ba9e520a28f 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -199,6 +199,47 @@ struct ti_sci_clk_ops {
 #define TISCI_MSG_VALUE_IO_ENABLE			1
 #define TISCI_MSG_VALUE_IO_DISABLE			0
 
+/* TISCI LPM wake up sources */
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_I2C0	0x00
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_UART0	0x10
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_GPIO0	0x20
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_ICEMELTER0	0x30
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER0	0x40
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER1	0x41
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_RTC0	0x50
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_RESET		0x60
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB0		0x70
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB1		0x71
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO		0x80
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_IO		0x81
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO		0x82
+#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID		0xFF
+
+/* TISCI LPM constraint state values */
+#define TISCI_MSG_CONSTRAINT_SET			1
+#define TISCI_MSG_CONSTRAINT_CLR			0
+
+/**
+ * struct ti_sci_pm_ops - Low Power Mode (LPM) control operations
+ * @lpm_wake_reason: Get the wake up source that woke the SoC from LPM
+ *		- source: The wake up source that woke soc from LPM.
+ *		- timestamp: Timestamp at which soc woke.
+ * @set_device_constraint: Set LPM constraint on behalf of a device
+ *		- id: Device Identifier
+ *		- state: The desired state of device constraint: set or clear.
+ * @set_latency_constraint: Set LPM resume latency constraint
+ *		- latency: maximum acceptable latency to wake up from low power mode
+ *		- state: The desired state of latency constraint: set or clear.
+ */
+struct ti_sci_pm_ops {
+	int (*lpm_wake_reason)(const struct ti_sci_handle *handle,
+			       u32 *source, u64 *timestamp, u8 *pin, u8 *mode);
+	int (*set_device_constraint)(const struct ti_sci_handle *handle,
+				     u32 id, u8 state);
+	int (*set_latency_constraint)(const struct ti_sci_handle *handle,
+				      u16 latency, u8 state);
+};
+
 /**
  * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
  * @start:	Start index of the first resource range.
@@ -543,6 +584,7 @@ struct ti_sci_ops {
 	struct ti_sci_core_ops core_ops;
 	struct ti_sci_dev_ops dev_ops;
 	struct ti_sci_clk_ops clk_ops;
+	struct ti_sci_pm_ops pm_ops;
 	struct ti_sci_rm_core_ops rm_core_ops;
 	struct ti_sci_rm_irq_ops rm_irq_ops;
 	struct ti_sci_rm_ringacc_ops rm_ring_ops;

-- 
2.46.0

[PATCH v10 4/4] firmware: ti_sci: add CPU latency constraint management

[Kevin Hilman]

During system-wide suspend, check if any of the CPUs have PM QoS
resume latency constraints set.  If so, set TI SCI constraint.

TI SCI has a single system-wide latency constraint, so use the max of
any of the CPU latencies as the system-wide value.

Note: DM firmware clears all constraints at resume time, so
constraints need to be checked/updated/sent at each system suspend.

Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/firmware/ti_sci.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 107494406fa5..e0b11ca30af5 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -9,6 +9,7 @@
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
 #include <linux/bitmap.h>
+#include <linux/cpu.h>
 #include <linux/debugfs.h>
 #include <linux/export.h>
 #include <linux/io.h>
@@ -19,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/pm_qos.h>
 #include <linux/property.h>
 #include <linux/semaphore.h>
 #include <linux/slab.h>
@@ -3645,7 +3647,27 @@ static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
 static int ti_sci_suspend(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
-	int ret;
+	struct device *cpu_dev, *cpu_dev_max = NULL;
+	s32 val, cpu_lat = 0;
+	int i, ret;
+
+	if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) {
+		for_each_possible_cpu(i) {
+			cpu_dev = get_cpu_device(i);
+			val = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_RESUME_LATENCY);
+			if (val != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) {
+				cpu_lat = max(cpu_lat, val);
+				cpu_dev_max = cpu_dev;
+			}
+		}
+		if (cpu_dev_max) {
+			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u\n", __func__, cpu_lat);
+			ret = ti_sci_cmd_set_latency_constraint(&info->handle,
+								cpu_lat, TISCI_MSG_CONSTRAINT_SET);
+			if (ret)
+				return ret;
+		}
+	}
 
 	ret = ti_sci_prepare_system_suspend(info);
 	if (ret)

-- 
2.46.0

Re: [PATCH v10 0/4] firmware: ti_sci: Introduce system suspend support

[Dhruva Gole]

Hello all,

On Aug 14, 2024 at 08:39:27 -0700, Kevin Hilman wrote:
> Abstract
> ********
> 
> This series introduces necessary ti_sci driver functionality to support
> various Suspend-to-RAM modes on TI AM62 family of devices. These Low
> Power Modes include Deep Sleep and MCU Only as described in section
> "6.2.4 Power Modes" of the AM62P Technical Reference Manual [0].
> 
> Summary
> *******
> 
> This series is a restructuring and rebase of the patch series by
> Dave Gerlach [1] and Dhruva Gole [2]. It applies on top of Linux
> 6.11-rc1.
> 
> The kernel triggers entry to Low Power Mode through the mem suspend
> transition with the following:
> 
> * At the  bootloader stage, one is  expected to package the  TIFS stub
>   which then gets  pulled into the Tightly coupled memory  of the Device
>   Mgr  (DM) R5  when it  starts up.  If using  U-Boot, then  it requires
>   tispl.bin to contain the TIFS stub. Refer to documentation in upstream
>   u-boot[3] for further details. The  supported firmware version is from
>   TI Processor SDK >= 10.00 ie. tag 10.00.04 from ti-linux-firmware [4].
> 
> * Use a TF-A binary that supports PSCI_SYSTEM_SUSPEND call. This causes
>   system to use PSCI system suspend as last step of mem sleep.
> 
> * We add support for the TISCI_MSG_QUERY_FW_CAPS message, used to retrieve
>   the firmware capabilities of the currently running system firmware [6].
>   Sysfw version >= 10.00.04 support LPM_DM_MANAGED capability, where
>   Device Mgr firmware now manages which low power mode is chosen. Going
>   forward, this is the default configuration supported for TI AM62 family
>   of devices. The state chosen by the DM can be influenced by sending
>   constraints using the new LPM constraint APIs. (Patch 1)
> 
> * The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP
>   message in order to provide details about suspend. The ti_sci driver
>   must send this message to firmware with the above information
>   included, which it does during the driver suspend handler when
>   PM_MEM_SUSPEND is the determined state being entered. The mode being
>   sent depends on whether firmware capabilities have support for
>   LPM_DM_MANAGED feature. Legacy firmware or those supporting other
>   modes can extend the mode selection logic as needed. (Patch 2)
> 
> * We also add the remaining TISCI Low Power Mode messages required for
>   inquiring wake reason and managing LPM constraints as part of a new PM
>   ops. These messages are part of the TISCI PM Low Power Mode API [5].
>   (Patch 3)
> 
> * Finally if any CPUs have PM QoS resume latency constraints set, we
>   aggregate these and set the TISCI system-wide latency constraint.
>   (Patch 4)
> 
> Testing
> *******
> 
> This series can for example be tested with a am62a-lp-sk board.
> 
> For am62a-lp-sk all usb nodes have to be disabled at the moment (usbss0,
> usb0, usbss1 and usb1). There is currently an issue with USB Link Power
> Management and turning off the USB device which is being worked on.
> 
> Once booted suspend/resume can be tested with rtcwake:
>   $ rtcwake -m mem -s 10 -d /dev/rtc0
> 
> Make sure /dev/rtc0 corresponds to rtc-ti-k3:
>   $ dmesg | grep rtc-ti-k3
>   rtc-ti-k3 2b1f0000.rtc: registered as rtc0

Additionally, one can refer to this guide for more details:
https://software-dl.ti.com/processor-sdk-linux/esd/AM62X/latest/exports/docs/linux/Foundational_Components/Power_Management/pm_low_power_modes.html

For the series,
Tested-by: Dhruva Gole <d-gole@ti.com>

Logs: https://gist.github.com/DhruvaG2000/658d0d4683b13ab41025df19a8eafc2f

Tree with all the patches applied:
https://github.com/DhruvaG2000/v-linux/tree/lpm-k3-next


-- 
Best regards,
Dhruva

Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Akashdeep Kaur]

On 14/08/24 21:09, Kevin Hilman wrote:
> From: Dave Gerlach <d-gerlach@ ti. com> Introduce power management ops 
> supported by the TISCI Low Power Mode API [1]. 1) 
> TISCI_MSG_LPM_WAKE_REASON Get which wake up source woke the SoC from Low 
> Power Mode. The wake up source IDs will be
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!tFdqXRfP9mwbCoXPtNggdrCuJ3bSsNDHabWG8s9g6Hh_v3tSab2OpTcpCeIxxvksAU_Fa5Zl41XmoQoqJiZdQtEck4kHNZot31hxXq-ZtaRCwdotcPeN9ST2tD8$>
> ZjQcmQRYFpfptBannerEnd
> 
> From: Dave Gerlach <d-gerlach@ti.com>
> 
> Introduce power management ops supported by the TISCI
> Low Power Mode API [1].
> 
> 1) TISCI_MSG_LPM_WAKE_REASON
> Get which wake up source woke the SoC from Low Power Mode.
> The wake up source IDs will be common for all K3 platforms.
> 
> 2) TISCI_MSG_LPM_SET_DEVICE_CONSTRAINT
> Set LPM constraint on behalf of a device. By setting a constraint, the
> device ensures that it will not be powered off or reset in the selected
> mode.
> 
> 3) TISCI_MSG_LPM_SET_LATENCY_CONSTRAINT
> Set LPM resume latency constraint. By setting a constraint, the host
> ensures that the resume time from selected mode will be less than the
> constraint value.
> 
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> [g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support]
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> [a-kaur@ti.com: SET_DEVICE_CONSTRAINT support]
> Signed-off-by: Akashdeep Kaur <a-kaur@ti.com>
> [vibhore@ti.com: SET_LATENCY_CONSTRAINT support]
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
[...]

My concerns are now addressed.

Reviewed-by: Akashdeep Kaur <a-kaur@ti.com>

Re: [PATCH v10 1/4] firmware: ti_sci: Add support for querying the firmware caps

[Nishanth Menon]

On 08:39-20240814, Kevin Hilman wrote:
> From: Georgi Vlaev <g-vlaev@ti.com>
> 

Mostly minor nits below:

> Add support for the TISCI_MSG_QUERY_FW_CAPS message, used to retrieve
> the firmware capabilities of the currently running system firmware. The
> message belongs to the TISCI general core message API [1] and is
> available in SysFW version 08.04.03 and above. Currently, the message is
> supported on devices with split architecture of the system firmware (DM
> + TIFS) like AM62x. Old revisions or not yet supported platforms will
> NACK this request.
> 
> We're using this message locally in ti_sci.c to get the low power
> featutes of the FW/SoC. As there's no other kernel consumers yet, this
s/featutes/features/ ?

> is not added to struct ti_sci_core_ops.
> 
> Sysfw version >= 10.00.04 support LPM_DM_MANAGED capability [2], where
> Device Mgr firmware now manages which low power mode is chosen. Going
> forward, this is the default configuration supported for TI AM62 family
> of devices. The state chosen by the DM can be influenced by sending
> constraints using the new LPM constraint APIs.
> 
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html
> [2] https://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html#tisci-msg-query-fw-caps

While both the links are valid,
      https://software-dl.ti.com/tisci/esd/latest/index.html has been
used in documentation, so we should stay consistent on the domain name
here.

> 
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> [vibhore@ti.com: Support for LPM_DM_MANAGED mode]
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/firmware/ti_sci.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/firmware/ti_sci.h | 22 ++++++++++++++++++++++

[...]

>  
> +/**
> + * ti_sci_msg_cmd_query_fw_caps() - Get the FW/SoC capabilities
> + * @handle:		Pointer to TI SCI handle
> + * @fw_caps:		Each bit in fw_caps indicating one FW/SOC capability
> + *
> + * Return: 0 if all went well, else returns appropriate error value.
> + */
> +static int ti_sci_msg_cmd_query_fw_caps(const struct ti_sci_handle *handle,
> +					u64 *fw_caps)
> +{
> +	struct ti_sci_info *info;
> +	struct ti_sci_xfer *xfer;
> +	struct ti_sci_msg_resp_query_fw_caps *resp;
> +	struct device *dev;
> +	int ret = 0;
> +
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	info = handle_to_ti_sci_info(handle);
> +	dev = info->dev;
> +
> +	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_QUERY_FW_CAPS,
> +				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
> +				   sizeof(struct ti_sci_msg_hdr),
> +				   sizeof(*resp));
> +	if (IS_ERR(xfer)) {
> +		ret = PTR_ERR(xfer);
> +		dev_err(dev, "Message alloc failed(%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = ti_sci_do_xfer(info, xfer);
> +	if (ret) {
> +		dev_err(dev, "Mbox send fail %d\n", ret);
> +		goto fail;
> +	}
> +
> +	resp = (struct ti_sci_msg_resp_query_fw_caps *)xfer->xfer_buf;
> +
> +	if (!ti_sci_is_response_ack(resp)) {

Add a dev_err here and indicating failure to detect caps?

> +		ret = -ENODEV;
> +		goto fail;
> +	}
> +
> +	if (fw_caps)
> +		*fw_caps = resp->fw_caps;
> +
> +fail:

> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	return ret;
> +}
> +
>  static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
>  {
>  	struct ti_sci_info *info;
> @@ -3390,6 +3449,14 @@ static int ti_sci_probe(struct platform_device *pdev)
>  		goto out;
>  	}
>  
> +	/*
> +	 * Check if the firmware supports any optional low power modes.
> +	 * Old revisions of TIFS (< 08.04) will NACK the request.

Move this comment as part of the function documentation

> +	 */
> +	ret = ti_sci_msg_cmd_query_fw_caps(&info->handle, &info->fw_caps);
> +	if (ret || !(info->fw_caps & MSG_FLAG_CAPS_GENERIC))

ret can be set for various reasons including older firmware that may not
support fw_caps - just checking against fw_caps might suffice for the
dev_debug ?

> +		pr_debug("Unable to detect LPM capabilities in fw_caps\n");

Please use dev_dbg

Just 2 cents: adding a dev_dbg with details on what matches we got
might be helpful for products in the field?

[...]

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

Re: [PATCH v10 2/4] firmware: ti_sci: Add system suspend and resume call

[Nishanth Menon]

On 08:39-20240814, Kevin Hilman wrote:
> From: Vibhore Vardhan <vibhore@ti.com>
> 
[...]

> + *
> + * Return: 0 if all went well, else returns appropriate error value.
> + */
> +static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
> +				    u32 ctx_lo, u32 ctx_hi, u32 debug_flags)
> +{
> +	struct ti_sci_info *info;
> +	struct ti_sci_msg_req_prepare_sleep *req;
> +	struct ti_sci_msg_hdr *resp;
> +	struct ti_sci_xfer *xfer;
> +	struct device *dev;
> +	int ret = 0;
> +
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	info = handle_to_ti_sci_info(handle);
> +	dev = info->dev;
> +
> +	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_PREPARE_SLEEP,
> +				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
> +				   sizeof(*req), sizeof(*resp));
> +	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 = mode;
> +	req->ctx_lo = ctx_lo;
> +	req->ctx_hi = ctx_hi;
> +	req->debug_flags = debug_flags;
> +
> +	ret = ti_sci_do_xfer(info, xfer);
> +	if (ret) {
> +		dev_err(dev, "Mbox send fail %d\n", ret);
> +		goto fail;
> +	}
> +
> +	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
> +
> +	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;

Here and other functions:

I think adding a dev_err for a NAKed message is probably useful for debug.

> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	return ret;
> +}
> +
>  /**
>   * ti_sci_msg_cmd_query_fw_caps() - Get the FW/SoC capabilities
>   * @handle:		Pointer to TI SCI handle
> @@ -1710,6 +1770,58 @@ static int ti_sci_msg_cmd_query_fw_caps(const struct ti_sci_handle *handle,
>  	return ret;
>  }
>  
> +/**
> + * ti_sci_cmd_set_io_isolation() - Enable IO isolation in LPM
> + * @handle:		Pointer to TI SCI handle
> + * @state:		The desired state of the IO isolation
> + *
> + * Return: 0 if all went well, else returns appropriate error value.
> + */
> +static int ti_sci_cmd_set_io_isolation(const struct ti_sci_handle *handle,
> +				       u8 state)

I probably might have preferred to have set_io_isolation as a
different patch, but OK, it is not too hard to see the hookup
commonality.

> +{
> +	struct ti_sci_info *info;
> +	struct ti_sci_msg_req_set_io_isolation *req;
> +	struct ti_sci_msg_hdr *resp;
> +	struct ti_sci_xfer *xfer;
> +	struct device *dev;
> +	int ret = 0;
> +
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	info = handle_to_ti_sci_info(handle);
> +	dev = info->dev;
> +
> +	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_SET_IO_ISOLATION,
> +				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
> +				   sizeof(*req), sizeof(*resp));
> +	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_set_io_isolation *)xfer->xfer_buf;
> +	req->state = state;
> +
> +	ret = ti_sci_do_xfer(info, xfer);
> +	if (ret) {
> +		dev_err(dev, "Mbox send fail %d\n", ret);
> +		goto fail;
> +	}
> +
> +	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
> +
> +	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;

here as well.

> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	return ret;
> +}
> +
>  static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
>  {
>  	struct ti_sci_info *info;
> @@ -3321,6 +3433,79 @@ static int tisci_reboot_handler(struct sys_off_data *data)
>  	return NOTIFY_BAD;
>  }
>  
> +#ifdef CONFIG_SUSPEND
> +static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
> +{
> +	u8 mode;
> +
> +	/*
> +	 * Map and validate the target Linux suspend state to TISCI LPM.
> +	 * Default is to let Device Manager select the low power mode.
> +	 */
> +	switch (pm_suspend_target_state) {
> +	case PM_SUSPEND_MEM:
> +		if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED)
> +			mode = TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED;
> +		else
> +			/* DM Managed is not supported by the firmware. */

dev_err print is useful for debug?

> +			return -EOPNOTSUPP;
> +		break;
> +	default:
> +		/*
> +		 * Do not fail if we don't have action to take for a
> +		 * specific suspend mode.
> +		 */
> +		return 0;
> +	}
> +
> +	return ti_sci_cmd_prepare_sleep(&info->handle, mode, 0, 0, 0);

Umm.. context address is 0x0?

> +}
> +
> +static int ti_sci_suspend(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = ti_sci_prepare_system_suspend(info);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ti_sci_suspend_noirq(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> +	if (ret)
> +		return ret;

Does this add any value here?

> +	dev_dbg(dev, "%s: set isolation: %d\n", __func__, ret);
> +
> +	return 0;
> +}
> +
> +static int ti_sci_resume_noirq(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
> +	if (ret)
> +		return ret;

Same..
> +	dev_dbg(dev, "%s: disable isolation: %d\n", __func__, ret);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops ti_sci_pm_ops = {
> +	.suspend = ti_sci_suspend,
> +	.suspend_noirq = ti_sci_suspend_noirq,
> +	.resume_noirq = ti_sci_resume_noirq,
> +};
> +#endif  /* CONFIG_SUSPEND */

Mostly, I am seeing usage with CONFIG_PM_SLEEP instead of
CONFIG_SUSPEND. is that a better option? and further,  usage tends to be
as follows:

static int __maybe_unused ti_sci_resume_noirq(struct device *dev)

static const struct dev_pm_ops ti_sci_pm_ops = {
#ifdef CONFIG_PM_SLEEP
	.suspend = ti_sci_suspend,
	.suspend_noirq = ti_sci_suspend_noirq,
	.resume_noirq = ti_sci_resume_noirq,
#endif
};

Keeps the #ifdeffery just localized to one place.

Thoughts?

> +
>  /* Description for K2G */
>  static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>  	.default_host_id = 2,
> @@ -3490,6 +3675,9 @@ static struct platform_driver ti_sci_driver = {
>  		   .name = "ti-sci",
>  		   .of_match_table = of_match_ptr(ti_sci_of_match),
>  		   .suppress_bind_attrs = true,
> +#ifdef CONFIG_SUSPEND
> +		   .pm = &ti_sci_pm_ops,
> +#endif
>  	},
>  };
>  module_platform_driver(ti_sci_driver);

[...]

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

Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Nishanth Menon]

On 08:39-20240814, Kevin Hilman wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
> 
[...]
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 808149dcc635..107494406fa5 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -1822,6 +1822,179 @@ static int ti_sci_cmd_set_io_isolation(const struct ti_sci_handle *handle,
>  	return ret;
>  }
>  
> +/**
> + * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM
> + * @handle:		Pointer to TI SCI handle
> + * @source:		The wakeup source that woke the SoC from LPM
> + * @timestamp:		Timestamp of the wakeup event
> + * @pin:		The pin that has triggered wake up
> + * @mode:		The last entered low power mode
> + *
> + * Return: 0 if all went well, else returns appropriate error value.
> + */
> +static int ti_sci_msg_cmd_lpm_wake_reason(const struct ti_sci_handle *handle,
> +					  u32 *source, u64 *timestamp, u8 *pin, u8 *mode)
> +{
> +	struct ti_sci_info *info;
> +	struct ti_sci_xfer *xfer;
> +	struct ti_sci_msg_resp_lpm_wake_reason *resp;
> +	struct device *dev;
> +	int ret = 0;
> +
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	info = handle_to_ti_sci_info(handle);
> +	dev = info->dev;
> +
> +	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_WAKE_REASON,
> +				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
> +				   sizeof(struct ti_sci_msg_hdr),
> +				   sizeof(*resp));
> +	if (IS_ERR(xfer)) {
> +		ret = PTR_ERR(xfer);
> +		dev_err(dev, "Message alloc failed(%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = ti_sci_do_xfer(info, xfer);
> +	if (ret) {
> +		dev_err(dev, "Mbox send fail %d\n", ret);
> +		goto fail;
> +	}
> +
> +	resp = (struct ti_sci_msg_resp_lpm_wake_reason *)xfer->xfer_buf;
> +
> +	if (!ti_sci_is_response_ack(resp)) {

A dev_err might be worth here.

> +		ret = -ENODEV;
> +		goto fail;
> +	}
> +
> +	if (source)
> +		*source = resp->wake_source;
> +	if (timestamp)
> +		*timestamp = resp->wake_timestamp;
> +	if (pin)
> +		*pin = resp->wake_pin;
> +	if (mode)
> +		*mode = resp->mode;
> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	return ret;
> +}
> +
> +/**
> + * ti_sci_cmd_set_device_constraint() - Set LPM constraint on behalf of a device
> + * @handle:	pointer to TI SCI handle
> + * @id:	Device identifier
> + * @state:	The desired state of device constraint: set or clear
> + *
> + * Return: 0 if all went well, else returns appropriate error value.
> + */
> +static int ti_sci_cmd_set_device_constraint(const struct ti_sci_handle *handle,
> +					    u32 id, u8 state)
> +{
> +	struct ti_sci_info *info;
> +	struct ti_sci_msg_req_lpm_set_device_constraint *req;
> +	struct ti_sci_msg_hdr *resp;
> +	struct ti_sci_xfer *xfer;
> +	struct device *dev;
> +	int ret = 0;
> +
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	info = handle_to_ti_sci_info(handle);
> +	dev = info->dev;
> +
> +	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT,
> +				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
> +				   sizeof(*req), sizeof(*resp));
> +	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_lpm_set_device_constraint *)xfer->xfer_buf;
> +	req->id = id;
> +	req->state = state;
> +
> +	ret = ti_sci_do_xfer(info, xfer);
> +	if (ret) {
> +		dev_err(dev, "Mbox send fail %d\n", ret);
> +		goto fail;
> +	}
> +
> +	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
> +
> +	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;

A dev_err might be worth here. - same elsewhere. I see a different style
of if (ret) vs ? 0: -ENODEV usage - might be good to be
consistent through out.

> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	return ret;
> +}
> +
> +/**
> + * ti_sci_cmd_set_latency_constraint() - Set LPM resume latency constraint
> + * @handle:	pointer to TI SCI handle
> + * @latency:	maximum acceptable latency (in ms) to wake up from LPM
> + * @state:	The desired state of latency constraint: set or clear
> + *
> + * Return: 0 if all went well, else returns appropriate error value.
> + */
> +static int ti_sci_cmd_set_latency_constraint(const struct ti_sci_handle *handle,
> +					     u16 latency, u8 state)
> +{
> +	struct ti_sci_info *info;
> +	struct ti_sci_msg_req_lpm_set_latency_constraint *req;
> +	struct ti_sci_msg_hdr *resp;
> +	struct ti_sci_xfer *xfer;
> +	struct device *dev;
> +	int ret = 0;
> +
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	info = handle_to_ti_sci_info(handle);
> +	dev = info->dev;
> +
> +	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT,
> +				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
> +				   sizeof(*req), sizeof(*resp));
> +	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_lpm_set_latency_constraint *)xfer->xfer_buf;
> +	req->latency = latency;
> +	req->state = state;
> +
> +	ret = ti_sci_do_xfer(info, xfer);
> +	if (ret) {
> +		dev_err(dev, "Mbox send fail %d\n", ret);
> +		goto fail;
> +	}
> +
> +	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
> +
> +	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;

Same

> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	return ret;
> +}
> +
>  static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
>  {
>  	struct ti_sci_info *info;
> @@ -2964,6 +3137,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
>  	struct ti_sci_core_ops *core_ops = &ops->core_ops;
>  	struct ti_sci_dev_ops *dops = &ops->dev_ops;
>  	struct ti_sci_clk_ops *cops = &ops->clk_ops;
> +	struct ti_sci_pm_ops *pmops = &ops->pm_ops;
>  	struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops;
>  	struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops;
>  	struct ti_sci_rm_ringacc_ops *rops = &ops->rm_ring_ops;
> @@ -3003,6 +3177,13 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
>  	cops->set_freq = ti_sci_cmd_clk_set_freq;
>  	cops->get_freq = ti_sci_cmd_clk_get_freq;
>  
> +	if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) {
> +		pr_debug("detected DM managed LPM in fw_caps\n");
> +		pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason;
> +		pmops->set_device_constraint = ti_sci_cmd_set_device_constraint;
> +		pmops->set_latency_constraint = ti_sci_cmd_set_latency_constraint;
> +	}
> +
>  	rm_core_ops->get_range = ti_sci_cmd_get_resource_range;
>  	rm_core_ops->get_range_from_shost =
>  				ti_sci_cmd_get_resource_range_from_shost;
> @@ -3490,12 +3671,20 @@ static int ti_sci_resume_noirq(struct device *dev)
>  {
>  	struct ti_sci_info *info = dev_get_drvdata(dev);
>  	int ret = 0;
> +	u32 source;
> +	u64 time;
> +	u8 pin;
> +	u8 mode;
>  
>  	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
>  	if (ret)
>  		return ret;
>  	dev_dbg(dev, "%s: disable isolation: %d\n", __func__, ret);
>  
> +	ti_sci_msg_cmd_lpm_wake_reason(&info->handle, &source, &time, &pin, &mode);

No handling of error?

> +	dev_info(dev, "ti_sci: wakeup source:0x%x, pin:0x%x, mode:0x%x\n",
> +		 source, pin, mode);
> +
>  	return 0;
>  }
>  
	[...]
>  
>  /**
> diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> index 1f1871e23f76..4ba9e520a28f 100644
> --- a/include/linux/soc/ti/ti_sci_protocol.h
> +++ b/include/linux/soc/ti/ti_sci_protocol.h
> @@ -199,6 +199,47 @@ struct ti_sci_clk_ops {
>  #define TISCI_MSG_VALUE_IO_ENABLE			1
>  #define TISCI_MSG_VALUE_IO_DISABLE			0
>  
> +/* TISCI LPM wake up sources */
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_I2C0	0x00
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_UART0	0x10
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_GPIO0	0x20
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_ICEMELTER0	0x30
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER0	0x40
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER1	0x41
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_RTC0	0x50
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_RESET		0x60
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB0		0x70
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB1		0x71
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO		0x80
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_IO		0x81

^^ I assume these are daisy chain sources. - will these remain OR do we
have to consider the wake sources SoC dependent? If so, the
sci_protocol.h will be SoC agnostic..


> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO		0x82
> +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID		0xFF
> +
> +/* TISCI LPM constraint state values */
> +#define TISCI_MSG_CONSTRAINT_SET			1
> +#define TISCI_MSG_CONSTRAINT_CLR			0
> +

[...]

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

Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Markus Schneider-Pargmann]

Hi Nishanth,

thanks for your review. I will integrate fixes for all your comments
into the next version.

On Mon, Aug 26, 2024 at 11:43:46AM GMT, Nishanth Menon wrote:
> On 08:39-20240814, Kevin Hilman wrote:
> [...]
> > diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> > index 1f1871e23f76..4ba9e520a28f 100644
> > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > @@ -199,6 +199,47 @@ struct ti_sci_clk_ops {
> >  #define TISCI_MSG_VALUE_IO_ENABLE			1
> >  #define TISCI_MSG_VALUE_IO_DISABLE			0
> >  
> > +/* TISCI LPM wake up sources */
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_I2C0	0x00
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_UART0	0x10
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_GPIO0	0x20
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_ICEMELTER0	0x30
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER0	0x40
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER1	0x41
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_RTC0	0x50
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_RESET		0x60
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB0		0x70
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB1		0x71
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO		0x80
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_IO		0x81
> 
> ^^ I assume these are daisy chain sources. - will these remain OR do we
> have to consider the wake sources SoC dependent? If so, the
> sci_protocol.h will be SoC agnostic..

These are all wakeup sources from LPM for the AM62 family of SoCs, not
only daisy chain sources. The currently defined wakeup sources are
relevant for am62x/a/p but will also be reused for others where
possible. Otherwise these can be extended in the future for other wakeup
sources.

Best
Markus

> 
> 
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO		0x82
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID		0xFF
> > +
> > +/* TISCI LPM constraint state values */
> > +#define TISCI_MSG_CONSTRAINT_SET			1
> > +#define TISCI_MSG_CONSTRAINT_CLR			0
> > +
> 
> [...]
> 
> -- 
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Nishanth Menon]

On 21:54-20240828, Markus Schneider-Pargmann wrote:
> Hi Nishanth,
> 
> thanks for your review. I will integrate fixes for all your comments
> into the next version.
> 
> On Mon, Aug 26, 2024 at 11:43:46AM GMT, Nishanth Menon wrote:
> > On 08:39-20240814, Kevin Hilman wrote:
> > [...]
> > > diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> > > index 1f1871e23f76..4ba9e520a28f 100644
> > > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > > @@ -199,6 +199,47 @@ struct ti_sci_clk_ops {
> > >  #define TISCI_MSG_VALUE_IO_ENABLE			1
> > >  #define TISCI_MSG_VALUE_IO_DISABLE			0
> > >  
> > > +/* TISCI LPM wake up sources */
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_I2C0	0x00
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_UART0	0x10
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_GPIO0	0x20
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_ICEMELTER0	0x30
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER0	0x40
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER1	0x41
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_RTC0	0x50
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_RESET		0x60
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB0		0x70
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB1		0x71
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO		0x80
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_IO		0x81
> > 
> > ^^ I assume these are daisy chain sources. - will these remain OR do we
> > have to consider the wake sources SoC dependent? If so, the
> > sci_protocol.h will be SoC agnostic..
> 
> These are all wakeup sources from LPM for the AM62 family of SoCs, not
> only daisy chain sources. The currently defined wakeup sources are
> relevant for am62x/a/p but will also be reused for others where
> possible. Otherwise these can be extended in the future for other wakeup
> sources.
> 

OK -> I should have been clear, but, I think you also caught on it
when you said am62x/a/p extended for future wakeup sources - sure..
with 32 bits you can indeed reach a large range.. BUT:

MAIN_DOMAIN, MCU_DOMAIN are specific to a SoC part architecture, it is
not a generic K3 SoC family architecture concept - the power domains
will vary depending on device - same with the list of peripherals used
as wakeup source - is WKUP_I2C0 always present in all devices with
wakeup, Do all K3 devices have USB0 and 1 always? We should not bet on
that.

ti_sci_protocol.h is meant as a generic SoC family level header. I see
these as possibly hardware specific description.

Lastly, I do not see the macros used either in the patch series.. I
understand the ti_sci_protocol.h is meant to standardize stuff in
other driver (I tried searching to see if some other series used
it[1], but I could not find a reference)..

So, wondering: Is DT the right place for that - With a DT header ABI
header that is shared between driver and dt? I don't understand the
modelling of how wakeup_reason is getting used from this series, so I
cannot comment better..

[1] https://lore.kernel.org/all/?q=TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Markus Schneider-Pargmann]

Hi Nishanth,

On Wed, Aug 28, 2024 at 10:24:23PM GMT, Nishanth Menon wrote:
> On 21:54-20240828, Markus Schneider-Pargmann wrote:
> > Hi Nishanth,
> > 
> > thanks for your review. I will integrate fixes for all your comments
> > into the next version.
> > 
> > On Mon, Aug 26, 2024 at 11:43:46AM GMT, Nishanth Menon wrote:
> > > On 08:39-20240814, Kevin Hilman wrote:
> > > [...]
> > > > diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> > > > index 1f1871e23f76..4ba9e520a28f 100644
> > > > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > > > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > > > @@ -199,6 +199,47 @@ struct ti_sci_clk_ops {
> > > >  #define TISCI_MSG_VALUE_IO_ENABLE			1
> > > >  #define TISCI_MSG_VALUE_IO_DISABLE			0
> > > >  
> > > > +/* TISCI LPM wake up sources */
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_I2C0	0x00
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_UART0	0x10
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_GPIO0	0x20
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_ICEMELTER0	0x30
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER0	0x40
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER1	0x41
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_RTC0	0x50
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_RESET		0x60
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB0		0x70
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB1		0x71
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO		0x80
> > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_IO		0x81
> > > 
> > > ^^ I assume these are daisy chain sources. - will these remain OR do we
> > > have to consider the wake sources SoC dependent? If so, the
> > > sci_protocol.h will be SoC agnostic..
> > 
> > These are all wakeup sources from LPM for the AM62 family of SoCs, not
> > only daisy chain sources. The currently defined wakeup sources are
> > relevant for am62x/a/p but will also be reused for others where
> > possible. Otherwise these can be extended in the future for other wakeup
> > sources.
> > 
> 
> OK -> I should have been clear, but, I think you also caught on it
> when you said am62x/a/p extended for future wakeup sources - sure..
> with 32 bits you can indeed reach a large range.. BUT:
> 
> MAIN_DOMAIN, MCU_DOMAIN are specific to a SoC part architecture, it is
> not a generic K3 SoC family architecture concept - the power domains
> will vary depending on device - same with the list of peripherals used
> as wakeup source - is WKUP_I2C0 always present in all devices with
> wakeup, Do all K3 devices have USB0 and 1 always? We should not bet on
> that.
> 
> ti_sci_protocol.h is meant as a generic SoC family level header. I see
> these as possibly hardware specific description.
> 
> Lastly, I do not see the macros used either in the patch series.. I
> understand the ti_sci_protocol.h is meant to standardize stuff in
> other driver (I tried searching to see if some other series used
> it[1], but I could not find a reference)..
> 
> So, wondering: Is DT the right place for that - With a DT header ABI
> header that is shared between driver and dt? I don't understand the
> modelling of how wakeup_reason is getting used from this series, so I
> cannot comment better..

Thanks for explaining. So should I add the header already with this
series although it is unused right now, or should we add it together
with the first actual user later on, so there is no unused header in the
meantime?

Best
Markus

> 
> [1] https://lore.kernel.org/all/?q=TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO
> -- 
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

[Nishanth Menon]

On 10:47-20240829, Markus Schneider-Pargmann wrote:
[...]
> Thanks for explaining. So should I add the header already with this
> series although it is unused right now, or should we add it together
> with the first actual user later on, so there is no unused header in the
> meantime?

Thinking deeper: we have two options:
a) dt bindings update with the property without knowing how the driver
  changes will be accepted or not.
b) drop the header changes for the macros.

I think (a) at this point is risky given the driver usage model is
un-clear - the APIs are abstract enough to be used in any way of choice,
but we do not want to be stuck with binding that then has to be
backward-forward compatible fixup..

So. (b) is better approach, IMHO..

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