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

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

[Dhruva Gole]

Abstract
********

This series introduces necessary ti_sci driver functionality to support
DeepSleep mode (suspend to mem) on TI K3 AM62x. DeepSleep mode is
described in section "6.2.4.4 DeepSleep" of the AM62x Technical Reference
Manual [0].

Summary
*******

This V6 series is a fixup and rebase of the patch series by
Dave Gerlach [1]. It applies on top of next-20230731.

The kernel triggers entry to DeepSleep 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
  R5 when it starts up. If using U-Boot, then it requires tispl.bin to
  contain the TIFS stub. Refer to ti-u-boot patch [3] for further
  details. The supported firmware version is from TI Processor SDK
  >= 09.00 ie. tag 09.00.00.006 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.

* The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP
  message in order to provide details about suspend, so we must add the
  ability to send this message. We also add TISCI_MSG_LPM_WAKE_REASON
  and TISCI_MSG_SET_IO_ISOLATION messages as part of a new PM ops. These
  messages are part of the TISCI PM Low Power Mode API [2]. (Patch 2)

* A memory address must be provided to the firmware using the above
  message, which is allocated and managed by dma_alloc_attrs()
  and friends. This memory address can be used by the firmware to
  save necessary context at that physical location in the DDR RAM. (Patch 3)

* While entering DeepSleep, it's also good to take precautions inorder to
  prevent any external current from directly entering the internal IPs and
  potentially damaging them. Hence, we send a ti_sci_cmd_set_io_isolation
  which essentially tells the DM R5 Firmware to isolate all the I/O's or pads
  from the IPs that they were connected to in active state. This is also
  something that is required when for example we want an I/O daisychain wakeup
  to wake the system from DeepSleep. More on this in the AM62x Processors TRM [0]
  under section "6.2.4.11 I/O Power Management and Daisy Chaining" (Patch 1)

* Finally, the ti_sci driver must actually send TISCI_MSG_PREPARE_SLEEP
  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. (Patch 4)

It currently enables only DeepSleep mode, but even if any additional
modes are needed to be supported in future, they would not require any
changes to the TISCI LPM APIs [2]. The enabling of additional modes
would be done via GenPD changes that is currently in the works.

Testing:
*******

In can be tested with the following branch:
https://github.com/DhruvaG2000/v-linux/commits/lpm-upstream-6.5

Tested on SK-AM62B [6] here:
https://gist.github.com/DhruvaG2000/8410fac048c677c40cd94f5169b5b0b4

Limitations:
************

Currently, DeepSleep is only supported on SK-AM62B with DDR4.
Boards with LPDDR part like Beagle Play and AM62x LP have a known FW issue.

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

commit ec8939156379 (tag: next-20230731) ("Add linux-next specific files for 20230731")

origin:
linux-next      https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

Changelog:
**********

v6:
- 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:
- link [5]
- 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://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
[3] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04&id=91886b68025c7ad121e62d1fc1fa4601eeb736cd
[4] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/?h=ti-linux-firmware-next&id=905eb58564581d951d148f45828e8c8a142a5938
[5] https://lore.kernel.org/all/20221128140522.49474-1-g-vlaev@ti.com/
[6] https://www.ti.com/tool/SK-AM62B


Cc: Vibhore <vibhore@ti.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Vignesh R <vigneshr@ti.com>
Cc: Tony Lindgren <tony@atomide.com>

Dave Gerlach (2):
  firmware: ti_sci: Introduce Power Management Ops
  firmware: ti_sci: Allocate memory for Low Power Modes

Dhruva Gole (1):
  firmware: ti_sci: Introduce system suspend resume support

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

 drivers/firmware/ti_sci.c              | 336 +++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  90 ++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  44 ++++
 3 files changed, 469 insertions(+), 1 deletion(-)

-- 
2.34.1

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

[Dhruva Gole]

Abstract
********

This series introduces necessary ti_sci driver functionality to support
DeepSleep mode (suspend to mem) on TI K3 AM62x. DeepSleep mode is
described in section "6.2.4.4 DeepSleep" of the AM62x Technical Reference
Manual [0].

Summary
*******

This V6 series is a fixup and rebase of the patch series by
Dave Gerlach [1]. It applies on top of next-20230731.

The kernel triggers entry to DeepSleep 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
  R5 when it starts up. If using U-Boot, then it requires tispl.bin to
  contain the TIFS stub. Refer to ti-u-boot patch [3] for further
  details. The supported firmware version is from TI Processor SDK
  >= 09.00 ie. tag 09.00.00.006 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.

* The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP
  message in order to provide details about suspend, so we must add the
  ability to send this message. We also add TISCI_MSG_LPM_WAKE_REASON
  and TISCI_MSG_SET_IO_ISOLATION messages as part of a new PM ops. These
  messages are part of the TISCI PM Low Power Mode API [2]. (Patch 2)

* A memory address must be provided to the firmware using the above
  message, which is allocated and managed by dma_alloc_attrs()
  and friends. This memory address can be used by the firmware to
  save necessary context at that physical location in the DDR RAM. (Patch 3)

* While entering DeepSleep, it's also good to take precautions inorder to
  prevent any external current from directly entering the internal IPs and
  potentially damaging them. Hence, we send a ti_sci_cmd_set_io_isolation
  which essentially tells the DM R5 Firmware to isolate all the I/O's or pads
  from the IPs that they were connected to in active state. This is also
  something that is required when for example we want an I/O daisychain wakeup
  to wake the system from DeepSleep. More on this in the AM62x Processors TRM [0]
  under section "6.2.4.11 I/O Power Management and Daisy Chaining" (Patch 1)

* Finally, the ti_sci driver must actually send TISCI_MSG_PREPARE_SLEEP
  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. (Patch 4)

It currently enables only DeepSleep mode, but even if any additional
modes are needed to be supported in future, they would not require any
changes to the TISCI LPM APIs [2]. The enabling of additional modes
would be done via GenPD changes that is currently in the works.

Testing:
*******

In can be tested with the following branch:
https://github.com/DhruvaG2000/v-linux/commits/lpm-upstream-6.5

Tested on SK-AM62B [6] here:
https://gist.github.com/DhruvaG2000/8410fac048c677c40cd94f5169b5b0b4

Limitations:
************

Currently, DeepSleep is only supported on SK-AM62B with DDR4.
Boards with LPDDR part like Beagle Play and AM62x LP have a known FW issue.

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

commit ec8939156379 (tag: next-20230731) ("Add linux-next specific files for 20230731")

origin:
linux-next      https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

Changelog:
**********

v6:
- 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:
- link [5]
- 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://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
[3] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04&id=91886b68025c7ad121e62d1fc1fa4601eeb736cd
[4] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/?h=ti-linux-firmware-next&id=905eb58564581d951d148f45828e8c8a142a5938
[5] https://lore.kernel.org/all/20221128140522.49474-1-g-vlaev@ti.com/
[6] https://www.ti.com/tool/SK-AM62B


Cc: Vibhore <vibhore@ti.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Vignesh R <vigneshr@ti.com>
Cc: Tony Lindgren <tony@atomide.com>

Dave Gerlach (2):
  firmware: ti_sci: Introduce Power Management Ops
  firmware: ti_sci: Allocate memory for Low Power Modes

Dhruva Gole (1):
  firmware: ti_sci: Introduce system suspend resume support

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

 drivers/firmware/ti_sci.c              | 336 +++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  90 ++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  44 ++++
 3 files changed, 469 insertions(+), 1 deletion(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Dhruva Gole]

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

Introduce power management ops supported by the TISCI
Low Power Mode API [1]. These messages are currently
supported only on AM62x platforms.

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_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.

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

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

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
[g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support]
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Tested-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/firmware/ti_sci.c              | 175 +++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  64 ++++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  44 +++++++
 3 files changed, 282 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 26a37f47f4ca..31a71613ca54 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1664,6 +1664,176 @@ 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_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
+ *
+ * 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)
+{
+	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;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	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;
@@ -2806,6 +2976,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;
@@ -2845,6 +3016,10 @@ 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;
 
+	pmops->prepare_sleep = ti_sci_cmd_prepare_sleep;
+	pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason;
+	pmops->set_io_isolation = ti_sci_cmd_set_io_isolation;
+
 	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;
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index ef3a8214d002..e4bfe146c43d 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: http://processors.wiki.ti.com/index.php/TISCI for details
  *
- * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C)  2015-2022 Texas Instruments Incorporated - https://www.ti.com/
  */
 
 #ifndef __TI_SCI_H
@@ -35,6 +35,11 @@
 #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_LPM_WAKE_REASON	0x0306
+#define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
+
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
@@ -545,6 +550,63 @@ 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
+
+/**
+ * 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;
+
+/**
+ * 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.
+ *
+ * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON,
+ * used to query the wake up source from low power mode.
+ */
+struct ti_sci_msg_resp_lpm_wake_reason {
+	struct ti_sci_msg_hdr hdr;
+	u32 wake_source;
+	u64 wake_timestamp;
+} __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..f2d1d74ab8fc 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
 			u64 *current_freq);
 };
 
+/* 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 IO isolation control values */
+#define TISCI_MSG_VALUE_IO_ENABLE			1
+#define TISCI_MSG_VALUE_IO_DISABLE			0
+
+/**
+ * struct ti_sci_pm_ops - Low Power Mode (LPM) control operations
+ * @prepare_sleep: Prepare to enter low power mode
+ *		- mode: Low power mode to enter.
+ *		- ctx_lo: Low 32-bits of physical address for context save.
+ *		- ctx_hi: High 32-bits of physical address for context save.
+ *		- ctx_lo: 'true' if frequency change is desired.
+ *		- debug_flags: JTAG control flags for debug.
+ * @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_io_isolation: Enable or disable IO isolation
+ *		- state: The desired state of the IO isolation.
+ */
+struct ti_sci_pm_ops {
+	int (*prepare_sleep)(const struct ti_sci_handle *handle, u8 mode,
+			     u32 ctx_lo, u32 ctx_hi, u32 flags);
+	int (*lpm_wake_reason)(const struct ti_sci_handle *handle,
+			       u32 *source, u64 *timestamp);
+	int (*set_io_isolation)(const struct ti_sci_handle *handle,
+				u8 state);
+};
+
 /**
  * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
  * @start:	Start index of the first resource range.
@@ -539,6 +582,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.34.1

[PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Dhruva Gole]

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

Introduce power management ops supported by the TISCI
Low Power Mode API [1]. These messages are currently
supported only on AM62x platforms.

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_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.

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

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

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
[g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support]
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Tested-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/firmware/ti_sci.c              | 175 +++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  64 ++++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  44 +++++++
 3 files changed, 282 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 26a37f47f4ca..31a71613ca54 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1664,6 +1664,176 @@ 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_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
+ *
+ * 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)
+{
+	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;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	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;
@@ -2806,6 +2976,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;
@@ -2845,6 +3016,10 @@ 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;
 
+	pmops->prepare_sleep = ti_sci_cmd_prepare_sleep;
+	pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason;
+	pmops->set_io_isolation = ti_sci_cmd_set_io_isolation;
+
 	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;
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index ef3a8214d002..e4bfe146c43d 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: http://processors.wiki.ti.com/index.php/TISCI for details
  *
- * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C)  2015-2022 Texas Instruments Incorporated - https://www.ti.com/
  */
 
 #ifndef __TI_SCI_H
@@ -35,6 +35,11 @@
 #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_LPM_WAKE_REASON	0x0306
+#define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
+
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
@@ -545,6 +550,63 @@ 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
+
+/**
+ * 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;
+
+/**
+ * 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.
+ *
+ * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON,
+ * used to query the wake up source from low power mode.
+ */
+struct ti_sci_msg_resp_lpm_wake_reason {
+	struct ti_sci_msg_hdr hdr;
+	u32 wake_source;
+	u64 wake_timestamp;
+} __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..f2d1d74ab8fc 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
 			u64 *current_freq);
 };
 
+/* 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 IO isolation control values */
+#define TISCI_MSG_VALUE_IO_ENABLE			1
+#define TISCI_MSG_VALUE_IO_DISABLE			0
+
+/**
+ * struct ti_sci_pm_ops - Low Power Mode (LPM) control operations
+ * @prepare_sleep: Prepare to enter low power mode
+ *		- mode: Low power mode to enter.
+ *		- ctx_lo: Low 32-bits of physical address for context save.
+ *		- ctx_hi: High 32-bits of physical address for context save.
+ *		- ctx_lo: 'true' if frequency change is desired.
+ *		- debug_flags: JTAG control flags for debug.
+ * @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_io_isolation: Enable or disable IO isolation
+ *		- state: The desired state of the IO isolation.
+ */
+struct ti_sci_pm_ops {
+	int (*prepare_sleep)(const struct ti_sci_handle *handle, u8 mode,
+			     u32 ctx_lo, u32 ctx_hi, u32 flags);
+	int (*lpm_wake_reason)(const struct ti_sci_handle *handle,
+			       u32 *source, u64 *timestamp);
+	int (*set_io_isolation)(const struct ti_sci_handle *handle,
+				u8 state);
+};
+
 /**
  * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
  * @start:	Start index of the first resource range.
@@ -539,6 +582,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.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V6 2/4] firmware: ti_sci: Add support for querying the firmware caps

[Dhruva Gole]

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

This patch adds 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.

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

Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
---
 drivers/firmware/ti_sci.c | 56 +++++++++++++++++++++++++++++++++++++++
 drivers/firmware/ti_sci.h | 26 ++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 31a71613ca54..3b40f9336b3f 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1723,6 +1723,62 @@ static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
 	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;
+}
+
 /**
  * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM
  * @handle:		Pointer to TI SCI handle
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index e4bfe146c43d..d5b23d91b9b9 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
@@ -137,6 +138,31 @@ 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_DEEP_SLEEP: Deep Sleep LPM
+ *		MSG_FLAG_CAPS_LPM_MCU_ONLY: MCU only LPM
+ *		MSG_FLAG_CAPS_LPM_STANDBY: Standby LPM
+ *		MSG_FLAG_CAPS_LPM_PARTIAL_IO: Partial IO in LPM
+ *
+ * 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_DEEP_SLEEP	TI_SCI_MSG_FLAG(1)
+#define MSG_FLAG_CAPS_LPM_MCU_ONLY	TI_SCI_MSG_FLAG(2)
+#define MSG_FLAG_CAPS_LPM_STANDBY	TI_SCI_MSG_FLAG(3)
+#define MSG_FLAG_CAPS_LPM_PARTIAL_IO	TI_SCI_MSG_FLAG(4)
+#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.34.1

[PATCH V6 2/4] firmware: ti_sci: Add support for querying the firmware caps

[Dhruva Gole]

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

This patch adds 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.

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

Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
---
 drivers/firmware/ti_sci.c | 56 +++++++++++++++++++++++++++++++++++++++
 drivers/firmware/ti_sci.h | 26 ++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 31a71613ca54..3b40f9336b3f 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1723,6 +1723,62 @@ static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
 	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;
+}
+
 /**
  * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM
  * @handle:		Pointer to TI SCI handle
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index e4bfe146c43d..d5b23d91b9b9 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
@@ -137,6 +138,31 @@ 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_DEEP_SLEEP: Deep Sleep LPM
+ *		MSG_FLAG_CAPS_LPM_MCU_ONLY: MCU only LPM
+ *		MSG_FLAG_CAPS_LPM_STANDBY: Standby LPM
+ *		MSG_FLAG_CAPS_LPM_PARTIAL_IO: Partial IO in LPM
+ *
+ * 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_DEEP_SLEEP	TI_SCI_MSG_FLAG(1)
+#define MSG_FLAG_CAPS_LPM_MCU_ONLY	TI_SCI_MSG_FLAG(2)
+#define MSG_FLAG_CAPS_LPM_STANDBY	TI_SCI_MSG_FLAG(3)
+#define MSG_FLAG_CAPS_LPM_PARTIAL_IO	TI_SCI_MSG_FLAG(4)
+#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.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

[Dhruva Gole]

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

A region of memory in DDR must be used during Deep Sleep for saving
of some system context when using the ti_sci firmware. From DM's point
of view, this can be any contiguous region in the DDR, so can allocate
512KB of DMA reserved memory in probe(), instead of another carveout.

Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
probe to determine if any low power modes are supported and if
ti_sci_init_suspend should be called based on the response received.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Tested-by: Roger Quadros <rogerq@kernel.org>
[d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 3b40f9336b3f..0334ade19868 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -10,6 +10,7 @@
 
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
+#include <linux/dma-mapping.h>
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -25,6 +26,9 @@
 
 #include "ti_sci.h"
 
+/* Low power mode memory context size */
+#define LPM_CTX_MEM_SIZE 0x80000
+
 /* List of all TI SCI devices active in system */
 static LIST_HEAD(ti_sci_list);
 /* Protection for the entire list */
@@ -96,6 +100,9 @@ struct ti_sci_desc {
  * @minfo:	Message info
  * @node:	list head
  * @host_id:	Host ID
+ * @ctx_mem_addr: Low power context memory phys address
+ * @ctx_mem_buf: Low power context memory buffer
+ * @fw_caps:	FW/SoC low power capabilities
  * @users:	Number of users of this instance
  */
 struct ti_sci_info {
@@ -113,6 +120,9 @@ struct ti_sci_info {
 	struct ti_sci_xfers_info minfo;
 	struct list_head node;
 	u8 host_id;
+	dma_addr_t ctx_mem_addr;
+	void *ctx_mem_buf;
+	u64 fw_caps;
 	/* protected by ti_sci_list_mutex */
 	int users;
 };
@@ -3511,6 +3521,25 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 	return NOTIFY_BAD;
 }
 
+static int ti_sci_init_suspend(struct platform_device *pdev,
+			       struct ti_sci_info *info)
+{
+	struct device *dev = &pdev->dev;
+
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	info->ctx_mem_buf = dma_alloc_attrs(info->dev, LPM_CTX_MEM_SIZE,
+					    &info->ctx_mem_addr,
+					    GFP_KERNEL,
+					    DMA_ATTR_NO_KERNEL_MAPPING |
+					    DMA_ATTR_FORCE_CONTIGUOUS);
+	if (!info->ctx_mem_buf) {
+		dev_err(info->dev, "Failed to allocate LPM context memory\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * Check if the firmware supports any optional low power modes
+	 * and initialize them if present. 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_MASK_CAPS_LPM))
+		ti_sci_init_suspend(pdev, info);
+
 	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,
@@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
 		mbox_free_channel(info->chan_rx);
 	}
 
+	if (info->ctx_mem_buf)
+		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
+				  info->ctx_mem_buf,
+				  info->ctx_mem_addr);
 	return ret;
 }
 
-- 
2.34.1

[PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

[Dhruva Gole]

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

A region of memory in DDR must be used during Deep Sleep for saving
of some system context when using the ti_sci firmware. From DM's point
of view, this can be any contiguous region in the DDR, so can allocate
512KB of DMA reserved memory in probe(), instead of another carveout.

Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
probe to determine if any low power modes are supported and if
ti_sci_init_suspend should be called based on the response received.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Tested-by: Roger Quadros <rogerq@kernel.org>
[d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 3b40f9336b3f..0334ade19868 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -10,6 +10,7 @@
 
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
+#include <linux/dma-mapping.h>
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -25,6 +26,9 @@
 
 #include "ti_sci.h"
 
+/* Low power mode memory context size */
+#define LPM_CTX_MEM_SIZE 0x80000
+
 /* List of all TI SCI devices active in system */
 static LIST_HEAD(ti_sci_list);
 /* Protection for the entire list */
@@ -96,6 +100,9 @@ struct ti_sci_desc {
  * @minfo:	Message info
  * @node:	list head
  * @host_id:	Host ID
+ * @ctx_mem_addr: Low power context memory phys address
+ * @ctx_mem_buf: Low power context memory buffer
+ * @fw_caps:	FW/SoC low power capabilities
  * @users:	Number of users of this instance
  */
 struct ti_sci_info {
@@ -113,6 +120,9 @@ struct ti_sci_info {
 	struct ti_sci_xfers_info minfo;
 	struct list_head node;
 	u8 host_id;
+	dma_addr_t ctx_mem_addr;
+	void *ctx_mem_buf;
+	u64 fw_caps;
 	/* protected by ti_sci_list_mutex */
 	int users;
 };
@@ -3511,6 +3521,25 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 	return NOTIFY_BAD;
 }
 
+static int ti_sci_init_suspend(struct platform_device *pdev,
+			       struct ti_sci_info *info)
+{
+	struct device *dev = &pdev->dev;
+
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	info->ctx_mem_buf = dma_alloc_attrs(info->dev, LPM_CTX_MEM_SIZE,
+					    &info->ctx_mem_addr,
+					    GFP_KERNEL,
+					    DMA_ATTR_NO_KERNEL_MAPPING |
+					    DMA_ATTR_FORCE_CONTIGUOUS);
+	if (!info->ctx_mem_buf) {
+		dev_err(info->dev, "Failed to allocate LPM context memory\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * Check if the firmware supports any optional low power modes
+	 * and initialize them if present. 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_MASK_CAPS_LPM))
+		ti_sci_init_suspend(pdev, info);
+
 	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,
@@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
 		mbox_free_channel(info->chan_rx);
 	}
 
+	if (info->ctx_mem_buf)
+		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
+				  info->ctx_mem_buf,
+				  info->ctx_mem_addr);
 	return ret;
 }
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

Introduce system suspend resume calls that will allow the ti_sci
driver to support deep sleep low power mode when the user space issues a
suspend to mem.

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 is being entered and the address for allocated memory for
storing the context during Deep Sleep.

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.

Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 0334ade19868..172b726e00a6 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -22,6 +22,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/reboot.h>
 
 #include "ti_sci.h"
@@ -3521,6 +3522,67 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 	return NOTIFY_BAD;
 }
 
+static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
+{
+#if IS_ENABLED(CONFIG_SUSPEND)
+	u8 mode;
+
+	/* Map and validate the target Linux suspend state to TISCI LPM. */
+	switch (pm_suspend_target_state) {
+	case PM_SUSPEND_MEM:
+		/* S2MEM is not supported by the firmware. */
+		if (!(info->fw_caps & MSG_FLAG_CAPS_LPM_DEEP_SLEEP))
+			return 0;
+		mode = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP;
+		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,
+					(u32)(info->ctx_mem_addr & 0xffffffff),
+					(u32)((u64)info->ctx_mem_addr >> 32), 0);
+#else
+	return 0;
+#endif
+}
+
+static int ti_sci_suspend(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
+	if (ret)
+		return ret;
+	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
+
+	ret = ti_sci_prepare_system_suspend(info);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ti_sci_resume(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
+	if (ret)
+		return ret;
+	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
+
 static int ti_sci_init_suspend(struct platform_device *pdev,
 			       struct ti_sci_info *info)
 {
@@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
 	.driver = {
 		   .name = "ti-sci",
 		   .of_match_table = of_match_ptr(ti_sci_of_match),
+		   .pm = &ti_sci_pm_ops,
 	},
 };
 module_platform_driver(ti_sci_driver);
-- 
2.34.1

[PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

Introduce system suspend resume calls that will allow the ti_sci
driver to support deep sleep low power mode when the user space issues a
suspend to mem.

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 is being entered and the address for allocated memory for
storing the context during Deep Sleep.

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.

Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 0334ade19868..172b726e00a6 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -22,6 +22,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/reboot.h>
 
 #include "ti_sci.h"
@@ -3521,6 +3522,67 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 	return NOTIFY_BAD;
 }
 
+static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
+{
+#if IS_ENABLED(CONFIG_SUSPEND)
+	u8 mode;
+
+	/* Map and validate the target Linux suspend state to TISCI LPM. */
+	switch (pm_suspend_target_state) {
+	case PM_SUSPEND_MEM:
+		/* S2MEM is not supported by the firmware. */
+		if (!(info->fw_caps & MSG_FLAG_CAPS_LPM_DEEP_SLEEP))
+			return 0;
+		mode = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP;
+		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,
+					(u32)(info->ctx_mem_addr & 0xffffffff),
+					(u32)((u64)info->ctx_mem_addr >> 32), 0);
+#else
+	return 0;
+#endif
+}
+
+static int ti_sci_suspend(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
+	if (ret)
+		return ret;
+	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
+
+	ret = ti_sci_prepare_system_suspend(info);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ti_sci_resume(struct device *dev)
+{
+	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
+	if (ret)
+		return ret;
+	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
+
 static int ti_sci_init_suspend(struct platform_device *pdev,
 			       struct ti_sci_info *info)
 {
@@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
 	.driver = {
 		   .name = "ti-sci",
 		   .of_match_table = of_match_ptr(ti_sci_of_match),
+		   .pm = &ti_sci_pm_ops,
 	},
 };
 module_platform_driver(ti_sci_driver);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
> 
> Introduce power management ops supported by the TISCI
> Low Power Mode API [1]. These messages are currently
> supported only on AM62x platforms.
> 
> 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_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.
> 
> 3) TISCI_MSG_SET_IO_ISOLATION
> Control the IO isolation for Low Power Mode.
> 
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> [g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support]
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> Tested-by: Roger Quadros <rogerq@kernel.org>
> ---
>   drivers/firmware/ti_sci.c              | 175 +++++++++++++++++++++++++
>   drivers/firmware/ti_sci.h              |  64 ++++++++-
>   include/linux/soc/ti/ti_sci_protocol.h |  44 +++++++
>   3 files changed, 282 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 26a37f47f4ca..31a71613ca54 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -1664,6 +1664,176 @@ 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_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
> + *
> + * 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)
> +{
> +	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;
> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	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;
> @@ -2806,6 +2976,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;
> @@ -2845,6 +3016,10 @@ 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;
>   
> +	pmops->prepare_sleep = ti_sci_cmd_prepare_sleep;
> +	pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason;
> +	pmops->set_io_isolation = ti_sci_cmd_set_io_isolation;
> +
>   	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;
> diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
> index ef3a8214d002..e4bfe146c43d 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: http://processors.wiki.ti.com/index.php/TISCI for details
>    *
> - * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
> + * Copyright (C)  2015-2022 Texas Instruments Incorporated - https://www.ti.com/
>    */
>   
>   #ifndef __TI_SCI_H
> @@ -35,6 +35,11 @@
>   #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_LPM_WAKE_REASON	0x0306
> +#define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
> +
>   /* Resource Management Requests */
>   #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
>   
> @@ -545,6 +550,63 @@ 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

Feels like excessive alignment tabs..

> +
> +/**
> + * 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;
> +
> +/**
> + * 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.
> + *
> + * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON,
> + * used to query the wake up source from low power mode.
> + */
> +struct ti_sci_msg_resp_lpm_wake_reason {
> +	struct ti_sci_msg_hdr hdr;
> +	u32 wake_source;
> +	u64 wake_timestamp;
> +} __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..f2d1d74ab8fc 100644
> --- a/include/linux/soc/ti/ti_sci_protocol.h
> +++ b/include/linux/soc/ti/ti_sci_protocol.h
> @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
>   			u64 *current_freq);
>   };
>   
> +/* 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

I don't see these used in this series, do you need them? lpm_wake_reason()
doesn't seem used either and I'm not sure how you plan to use it, could
you detail that?

Andrew

> +
> +/* TISCI LPM IO isolation control values */
> +#define TISCI_MSG_VALUE_IO_ENABLE			1
> +#define TISCI_MSG_VALUE_IO_DISABLE			0
> +
> +/**
> + * struct ti_sci_pm_ops - Low Power Mode (LPM) control operations
> + * @prepare_sleep: Prepare to enter low power mode
> + *		- mode: Low power mode to enter.
> + *		- ctx_lo: Low 32-bits of physical address for context save.
> + *		- ctx_hi: High 32-bits of physical address for context save.
> + *		- ctx_lo: 'true' if frequency change is desired.
> + *		- debug_flags: JTAG control flags for debug.
> + * @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_io_isolation: Enable or disable IO isolation
> + *		- state: The desired state of the IO isolation.
> + */
> +struct ti_sci_pm_ops {
> +	int (*prepare_sleep)(const struct ti_sci_handle *handle, u8 mode,
> +			     u32 ctx_lo, u32 ctx_hi, u32 flags);
> +	int (*lpm_wake_reason)(const struct ti_sci_handle *handle,
> +			       u32 *source, u64 *timestamp);
> +	int (*set_io_isolation)(const struct ti_sci_handle *handle,
> +				u8 state);
> +};
> +
>   /**
>    * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
>    * @start:	Start index of the first resource range.
> @@ -539,6 +582,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;

Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
> 
> Introduce power management ops supported by the TISCI
> Low Power Mode API [1]. These messages are currently
> supported only on AM62x platforms.
> 
> 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_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.
> 
> 3) TISCI_MSG_SET_IO_ISOLATION
> Control the IO isolation for Low Power Mode.
> 
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> [g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support]
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> Tested-by: Roger Quadros <rogerq@kernel.org>
> ---
>   drivers/firmware/ti_sci.c              | 175 +++++++++++++++++++++++++
>   drivers/firmware/ti_sci.h              |  64 ++++++++-
>   include/linux/soc/ti/ti_sci_protocol.h |  44 +++++++
>   3 files changed, 282 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 26a37f47f4ca..31a71613ca54 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -1664,6 +1664,176 @@ 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_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
> + *
> + * 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)
> +{
> +	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;
> +
> +fail:
> +	ti_sci_put_one_xfer(&info->minfo, xfer);
> +
> +	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;
> @@ -2806,6 +2976,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;
> @@ -2845,6 +3016,10 @@ 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;
>   
> +	pmops->prepare_sleep = ti_sci_cmd_prepare_sleep;
> +	pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason;
> +	pmops->set_io_isolation = ti_sci_cmd_set_io_isolation;
> +
>   	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;
> diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
> index ef3a8214d002..e4bfe146c43d 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: http://processors.wiki.ti.com/index.php/TISCI for details
>    *
> - * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
> + * Copyright (C)  2015-2022 Texas Instruments Incorporated - https://www.ti.com/
>    */
>   
>   #ifndef __TI_SCI_H
> @@ -35,6 +35,11 @@
>   #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_LPM_WAKE_REASON	0x0306
> +#define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
> +
>   /* Resource Management Requests */
>   #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
>   
> @@ -545,6 +550,63 @@ 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

Feels like excessive alignment tabs..

> +
> +/**
> + * 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;
> +
> +/**
> + * 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.
> + *
> + * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON,
> + * used to query the wake up source from low power mode.
> + */
> +struct ti_sci_msg_resp_lpm_wake_reason {
> +	struct ti_sci_msg_hdr hdr;
> +	u32 wake_source;
> +	u64 wake_timestamp;
> +} __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..f2d1d74ab8fc 100644
> --- a/include/linux/soc/ti/ti_sci_protocol.h
> +++ b/include/linux/soc/ti/ti_sci_protocol.h
> @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
>   			u64 *current_freq);
>   };
>   
> +/* 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

I don't see these used in this series, do you need them? lpm_wake_reason()
doesn't seem used either and I'm not sure how you plan to use it, could
you detail that?

Andrew

> +
> +/* TISCI LPM IO isolation control values */
> +#define TISCI_MSG_VALUE_IO_ENABLE			1
> +#define TISCI_MSG_VALUE_IO_DISABLE			0
> +
> +/**
> + * struct ti_sci_pm_ops - Low Power Mode (LPM) control operations
> + * @prepare_sleep: Prepare to enter low power mode
> + *		- mode: Low power mode to enter.
> + *		- ctx_lo: Low 32-bits of physical address for context save.
> + *		- ctx_hi: High 32-bits of physical address for context save.
> + *		- ctx_lo: 'true' if frequency change is desired.
> + *		- debug_flags: JTAG control flags for debug.
> + * @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_io_isolation: Enable or disable IO isolation
> + *		- state: The desired state of the IO isolation.
> + */
> +struct ti_sci_pm_ops {
> +	int (*prepare_sleep)(const struct ti_sci_handle *handle, u8 mode,
> +			     u32 ctx_lo, u32 ctx_hi, u32 flags);
> +	int (*lpm_wake_reason)(const struct ti_sci_handle *handle,
> +			       u32 *source, u64 *timestamp);
> +	int (*set_io_isolation)(const struct ti_sci_handle *handle,
> +				u8 state);
> +};
> +
>   /**
>    * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
>    * @start:	Start index of the first resource range.
> @@ -539,6 +582,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;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> Abstract
> ********
> 
> This series introduces necessary ti_sci driver functionality to support
> DeepSleep mode (suspend to mem) on TI K3 AM62x. DeepSleep mode is
> described in section "6.2.4.4 DeepSleep" of the AM62x Technical Reference
> Manual [0].
> 
> Summary
> *******
> 
> This V6 series is a fixup and rebase of the patch series by
> Dave Gerlach [1]. It applies on top of next-20230731.
> 
> The kernel triggers entry to DeepSleep 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
>    R5 when it starts up. If using U-Boot, then it requires tispl.bin to
>    contain the TIFS stub. Refer to ti-u-boot patch [3] for further
>    details. The supported firmware version is from TI Processor SDK
>    >= 09.00 ie. tag 09.00.00.006 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.

What happens if we do not have this TIFS stub or not using a TF-A that
supports PSCI_SYSTEM_SUSPEND (or mistakenly thinks it does due to firmware
bugs (this is a known issue for AM64, I'm only phrasing it like a hypothetical :)
)), can we safely return if suspend fails here?

Andrew

> 
> * The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP
>    message in order to provide details about suspend, so we must add the
>    ability to send this message. We also add TISCI_MSG_LPM_WAKE_REASON
>    and TISCI_MSG_SET_IO_ISOLATION messages as part of a new PM ops. These
>    messages are part of the TISCI PM Low Power Mode API [2]. (Patch 2)
> 
> * A memory address must be provided to the firmware using the above
>    message, which is allocated and managed by dma_alloc_attrs()
>    and friends. This memory address can be used by the firmware to
>    save necessary context at that physical location in the DDR RAM. (Patch 3)
> 
> * While entering DeepSleep, it's also good to take precautions inorder to
>    prevent any external current from directly entering the internal IPs and
>    potentially damaging them. Hence, we send a ti_sci_cmd_set_io_isolation
>    which essentially tells the DM R5 Firmware to isolate all the I/O's or pads
>    from the IPs that they were connected to in active state. This is also
>    something that is required when for example we want an I/O daisychain wakeup
>    to wake the system from DeepSleep. More on this in the AM62x Processors TRM [0]
>    under section "6.2.4.11 I/O Power Management and Daisy Chaining" (Patch 1)
> 
> * Finally, the ti_sci driver must actually send TISCI_MSG_PREPARE_SLEEP
>    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. (Patch 4)
> 
> It currently enables only DeepSleep mode, but even if any additional
> modes are needed to be supported in future, they would not require any
> changes to the TISCI LPM APIs [2]. The enabling of additional modes
> would be done via GenPD changes that is currently in the works.
> 
> Testing:
> *******
> 
> In can be tested with the following branch:
> https://github.com/DhruvaG2000/v-linux/commits/lpm-upstream-6.5
> 
> Tested on SK-AM62B [6] here:
> https://gist.github.com/DhruvaG2000/8410fac048c677c40cd94f5169b5b0b4
> 
> Limitations:
> ************
> 
> Currently, DeepSleep is only supported on SK-AM62B with DDR4.
> Boards with LPDDR part like Beagle Play and AM62x LP have a known FW issue.
> 
> Base commit:
> ************
> 
> commit ec8939156379 (tag: next-20230731) ("Add linux-next specific files for 20230731")
> 
> origin:
> linux-next      https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> 
> Changelog:
> **********
> 
> v6:
> - 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:
> - link [5]
> - 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://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
> [3] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04&id=91886b68025c7ad121e62d1fc1fa4601eeb736cd
> [4] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/?h=ti-linux-firmware-next&id=905eb58564581d951d148f45828e8c8a142a5938
> [5] https://lore.kernel.org/all/20221128140522.49474-1-g-vlaev@ti.com/
> [6] https://www.ti.com/tool/SK-AM62B
> 
> 
> Cc: Vibhore <vibhore@ti.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> 
> Dave Gerlach (2):
>    firmware: ti_sci: Introduce Power Management Ops
>    firmware: ti_sci: Allocate memory for Low Power Modes
> 
> Dhruva Gole (1):
>    firmware: ti_sci: Introduce system suspend resume support
> 
> Georgi Vlaev (1):
>    firmware: ti_sci: Add support for querying the firmware caps
> 
>   drivers/firmware/ti_sci.c              | 336 +++++++++++++++++++++++++
>   drivers/firmware/ti_sci.h              |  90 ++++++-
>   include/linux/soc/ti/ti_sci_protocol.h |  44 ++++
>   3 files changed, 469 insertions(+), 1 deletion(-)
> 

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

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> Abstract
> ********
> 
> This series introduces necessary ti_sci driver functionality to support
> DeepSleep mode (suspend to mem) on TI K3 AM62x. DeepSleep mode is
> described in section "6.2.4.4 DeepSleep" of the AM62x Technical Reference
> Manual [0].
> 
> Summary
> *******
> 
> This V6 series is a fixup and rebase of the patch series by
> Dave Gerlach [1]. It applies on top of next-20230731.
> 
> The kernel triggers entry to DeepSleep 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
>    R5 when it starts up. If using U-Boot, then it requires tispl.bin to
>    contain the TIFS stub. Refer to ti-u-boot patch [3] for further
>    details. The supported firmware version is from TI Processor SDK
>    >= 09.00 ie. tag 09.00.00.006 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.

What happens if we do not have this TIFS stub or not using a TF-A that
supports PSCI_SYSTEM_SUSPEND (or mistakenly thinks it does due to firmware
bugs (this is a known issue for AM64, I'm only phrasing it like a hypothetical :)
)), can we safely return if suspend fails here?

Andrew

> 
> * The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP
>    message in order to provide details about suspend, so we must add the
>    ability to send this message. We also add TISCI_MSG_LPM_WAKE_REASON
>    and TISCI_MSG_SET_IO_ISOLATION messages as part of a new PM ops. These
>    messages are part of the TISCI PM Low Power Mode API [2]. (Patch 2)
> 
> * A memory address must be provided to the firmware using the above
>    message, which is allocated and managed by dma_alloc_attrs()
>    and friends. This memory address can be used by the firmware to
>    save necessary context at that physical location in the DDR RAM. (Patch 3)
> 
> * While entering DeepSleep, it's also good to take precautions inorder to
>    prevent any external current from directly entering the internal IPs and
>    potentially damaging them. Hence, we send a ti_sci_cmd_set_io_isolation
>    which essentially tells the DM R5 Firmware to isolate all the I/O's or pads
>    from the IPs that they were connected to in active state. This is also
>    something that is required when for example we want an I/O daisychain wakeup
>    to wake the system from DeepSleep. More on this in the AM62x Processors TRM [0]
>    under section "6.2.4.11 I/O Power Management and Daisy Chaining" (Patch 1)
> 
> * Finally, the ti_sci driver must actually send TISCI_MSG_PREPARE_SLEEP
>    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. (Patch 4)
> 
> It currently enables only DeepSleep mode, but even if any additional
> modes are needed to be supported in future, they would not require any
> changes to the TISCI LPM APIs [2]. The enabling of additional modes
> would be done via GenPD changes that is currently in the works.
> 
> Testing:
> *******
> 
> In can be tested with the following branch:
> https://github.com/DhruvaG2000/v-linux/commits/lpm-upstream-6.5
> 
> Tested on SK-AM62B [6] here:
> https://gist.github.com/DhruvaG2000/8410fac048c677c40cd94f5169b5b0b4
> 
> Limitations:
> ************
> 
> Currently, DeepSleep is only supported on SK-AM62B with DDR4.
> Boards with LPDDR part like Beagle Play and AM62x LP have a known FW issue.
> 
> Base commit:
> ************
> 
> commit ec8939156379 (tag: next-20230731) ("Add linux-next specific files for 20230731")
> 
> origin:
> linux-next      https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> 
> Changelog:
> **********
> 
> v6:
> - 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:
> - link [5]
> - 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://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
> [3] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04&id=91886b68025c7ad121e62d1fc1fa4601eeb736cd
> [4] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/?h=ti-linux-firmware-next&id=905eb58564581d951d148f45828e8c8a142a5938
> [5] https://lore.kernel.org/all/20221128140522.49474-1-g-vlaev@ti.com/
> [6] https://www.ti.com/tool/SK-AM62B
> 
> 
> Cc: Vibhore <vibhore@ti.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> 
> Dave Gerlach (2):
>    firmware: ti_sci: Introduce Power Management Ops
>    firmware: ti_sci: Allocate memory for Low Power Modes
> 
> Dhruva Gole (1):
>    firmware: ti_sci: Introduce system suspend resume support
> 
> Georgi Vlaev (1):
>    firmware: ti_sci: Add support for querying the firmware caps
> 
>   drivers/firmware/ti_sci.c              | 336 +++++++++++++++++++++++++
>   drivers/firmware/ti_sci.h              |  90 ++++++-
>   include/linux/soc/ti/ti_sci_protocol.h |  44 ++++
>   3 files changed, 469 insertions(+), 1 deletion(-)
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 2/4] firmware: ti_sci: Add support for querying the firmware caps

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> From: Georgi Vlaev <g-vlaev@ti.com>
> 
> This patch adds 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.

The API is also buggy on SYSFW 9.00 and below, maybe we should
check the firmware version in this function and return "not
supported" or all 0s, instead of returning known bad values.

Andrew

> 
> 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.
> 
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html
> 
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> ---
>   drivers/firmware/ti_sci.c | 56 +++++++++++++++++++++++++++++++++++++++
>   drivers/firmware/ti_sci.h | 26 ++++++++++++++++++
>   2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 31a71613ca54..3b40f9336b3f 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -1723,6 +1723,62 @@ static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
>   	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;
> +}
> +
>   /**
>    * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM
>    * @handle:		Pointer to TI SCI handle
> diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
> index e4bfe146c43d..d5b23d91b9b9 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
> @@ -137,6 +138,31 @@ 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_DEEP_SLEEP: Deep Sleep LPM
> + *		MSG_FLAG_CAPS_LPM_MCU_ONLY: MCU only LPM
> + *		MSG_FLAG_CAPS_LPM_STANDBY: Standby LPM
> + *		MSG_FLAG_CAPS_LPM_PARTIAL_IO: Partial IO in LPM
> + *
> + * 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_DEEP_SLEEP	TI_SCI_MSG_FLAG(1)
> +#define MSG_FLAG_CAPS_LPM_MCU_ONLY	TI_SCI_MSG_FLAG(2)
> +#define MSG_FLAG_CAPS_LPM_STANDBY	TI_SCI_MSG_FLAG(3)
> +#define MSG_FLAG_CAPS_LPM_PARTIAL_IO	TI_SCI_MSG_FLAG(4)
> +#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

Re: [PATCH V6 2/4] firmware: ti_sci: Add support for querying the firmware caps

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> From: Georgi Vlaev <g-vlaev@ti.com>
> 
> This patch adds 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.

The API is also buggy on SYSFW 9.00 and below, maybe we should
check the firmware version in this function and return "not
supported" or all 0s, instead of returning known bad values.

Andrew

> 
> 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.
> 
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html
> 
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> ---
>   drivers/firmware/ti_sci.c | 56 +++++++++++++++++++++++++++++++++++++++
>   drivers/firmware/ti_sci.h | 26 ++++++++++++++++++
>   2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 31a71613ca54..3b40f9336b3f 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -1723,6 +1723,62 @@ static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
>   	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;
> +}
> +
>   /**
>    * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM
>    * @handle:		Pointer to TI SCI handle
> diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
> index e4bfe146c43d..d5b23d91b9b9 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
> @@ -137,6 +138,31 @@ 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_DEEP_SLEEP: Deep Sleep LPM
> + *		MSG_FLAG_CAPS_LPM_MCU_ONLY: MCU only LPM
> + *		MSG_FLAG_CAPS_LPM_STANDBY: Standby LPM
> + *		MSG_FLAG_CAPS_LPM_PARTIAL_IO: Partial IO in LPM
> + *
> + * 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_DEEP_SLEEP	TI_SCI_MSG_FLAG(1)
> +#define MSG_FLAG_CAPS_LPM_MCU_ONLY	TI_SCI_MSG_FLAG(2)
> +#define MSG_FLAG_CAPS_LPM_STANDBY	TI_SCI_MSG_FLAG(3)
> +#define MSG_FLAG_CAPS_LPM_PARTIAL_IO	TI_SCI_MSG_FLAG(4)
> +#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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
> 
> A region of memory in DDR must be used during Deep Sleep for saving
> of some system context when using the ti_sci firmware. From DM's point
> of view, this can be any contiguous region in the DDR, so can allocate
> 512KB of DMA reserved memory in probe(), instead of another carveout.
> 
> Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
> probe to determine if any low power modes are supported and if
> ti_sci_init_suspend should be called based on the response received.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> Tested-by: Roger Quadros <rogerq@kernel.org>
> [d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>   drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 3b40f9336b3f..0334ade19868 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -10,6 +10,7 @@
>   
>   #include <linux/bitmap.h>
>   #include <linux/debugfs.h>
> +#include <linux/dma-mapping.h>
>   #include <linux/export.h>
>   #include <linux/io.h>
>   #include <linux/iopoll.h>
> @@ -25,6 +26,9 @@
>   
>   #include "ti_sci.h"
>   
> +/* Low power mode memory context size */
> +#define LPM_CTX_MEM_SIZE 0x80000
> +
>   /* List of all TI SCI devices active in system */
>   static LIST_HEAD(ti_sci_list);
>   /* Protection for the entire list */
> @@ -96,6 +100,9 @@ struct ti_sci_desc {
>    * @minfo:	Message info
>    * @node:	list head
>    * @host_id:	Host ID
> + * @ctx_mem_addr: Low power context memory phys address
> + * @ctx_mem_buf: Low power context memory buffer
> + * @fw_caps:	FW/SoC low power capabilities
>    * @users:	Number of users of this instance
>    */
>   struct ti_sci_info {
> @@ -113,6 +120,9 @@ struct ti_sci_info {
>   	struct ti_sci_xfers_info minfo;
>   	struct list_head node;
>   	u8 host_id;
> +	dma_addr_t ctx_mem_addr;
> +	void *ctx_mem_buf;
> +	u64 fw_caps;
>   	/* protected by ti_sci_list_mutex */
>   	int users;
>   };
> @@ -3511,6 +3521,25 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
>   	return NOTIFY_BAD;
>   }
>   
> +static int ti_sci_init_suspend(struct platform_device *pdev,
> +			       struct ti_sci_info *info)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	info->ctx_mem_buf = dma_alloc_attrs(info->dev, LPM_CTX_MEM_SIZE,
> +					    &info->ctx_mem_addr,
> +					    GFP_KERNEL,
> +					    DMA_ATTR_NO_KERNEL_MAPPING |
> +					    DMA_ATTR_FORCE_CONTIGUOUS);
> +	if (!info->ctx_mem_buf) {
> +		dev_err(info->dev, "Failed to allocate LPM context memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
>   /* Description for K2G */
>   static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>   	.default_host_id = 2,
> @@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	/*
> +	 * Check if the firmware supports any optional low power modes
> +	 * and initialize them if present. 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_MASK_CAPS_LPM))
> +		ti_sci_init_suspend(pdev, info);
> +
>   	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,
> @@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
>   		mbox_free_channel(info->chan_rx);
>   	}
>   
> +	if (info->ctx_mem_buf)
> +		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,

You allocated with dma_alloc_attrs() you should free with dma_free_attrs().

Andrew

> +				  info->ctx_mem_buf,
> +				  info->ctx_mem_addr);
>   	return ret;
>   }
>   

Re: [PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
> 
> A region of memory in DDR must be used during Deep Sleep for saving
> of some system context when using the ti_sci firmware. From DM's point
> of view, this can be any contiguous region in the DDR, so can allocate
> 512KB of DMA reserved memory in probe(), instead of another carveout.
> 
> Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
> probe to determine if any low power modes are supported and if
> ti_sci_init_suspend should be called based on the response received.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> Tested-by: Roger Quadros <rogerq@kernel.org>
> [d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>   drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 3b40f9336b3f..0334ade19868 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -10,6 +10,7 @@
>   
>   #include <linux/bitmap.h>
>   #include <linux/debugfs.h>
> +#include <linux/dma-mapping.h>
>   #include <linux/export.h>
>   #include <linux/io.h>
>   #include <linux/iopoll.h>
> @@ -25,6 +26,9 @@
>   
>   #include "ti_sci.h"
>   
> +/* Low power mode memory context size */
> +#define LPM_CTX_MEM_SIZE 0x80000
> +
>   /* List of all TI SCI devices active in system */
>   static LIST_HEAD(ti_sci_list);
>   /* Protection for the entire list */
> @@ -96,6 +100,9 @@ struct ti_sci_desc {
>    * @minfo:	Message info
>    * @node:	list head
>    * @host_id:	Host ID
> + * @ctx_mem_addr: Low power context memory phys address
> + * @ctx_mem_buf: Low power context memory buffer
> + * @fw_caps:	FW/SoC low power capabilities
>    * @users:	Number of users of this instance
>    */
>   struct ti_sci_info {
> @@ -113,6 +120,9 @@ struct ti_sci_info {
>   	struct ti_sci_xfers_info minfo;
>   	struct list_head node;
>   	u8 host_id;
> +	dma_addr_t ctx_mem_addr;
> +	void *ctx_mem_buf;
> +	u64 fw_caps;
>   	/* protected by ti_sci_list_mutex */
>   	int users;
>   };
> @@ -3511,6 +3521,25 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
>   	return NOTIFY_BAD;
>   }
>   
> +static int ti_sci_init_suspend(struct platform_device *pdev,
> +			       struct ti_sci_info *info)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	info->ctx_mem_buf = dma_alloc_attrs(info->dev, LPM_CTX_MEM_SIZE,
> +					    &info->ctx_mem_addr,
> +					    GFP_KERNEL,
> +					    DMA_ATTR_NO_KERNEL_MAPPING |
> +					    DMA_ATTR_FORCE_CONTIGUOUS);
> +	if (!info->ctx_mem_buf) {
> +		dev_err(info->dev, "Failed to allocate LPM context memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
>   /* Description for K2G */
>   static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>   	.default_host_id = 2,
> @@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	/*
> +	 * Check if the firmware supports any optional low power modes
> +	 * and initialize them if present. 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_MASK_CAPS_LPM))
> +		ti_sci_init_suspend(pdev, info);
> +
>   	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,
> @@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
>   		mbox_free_channel(info->chan_rx);
>   	}
>   
> +	if (info->ctx_mem_buf)
> +		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,

You allocated with dma_alloc_attrs() you should free with dma_free_attrs().

Andrew

> +				  info->ctx_mem_buf,
> +				  info->ctx_mem_addr);
>   	return ret;
>   }
>   

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> Introduce system suspend resume calls that will allow the ti_sci
> driver to support deep sleep low power mode when the user space issues a
> suspend to mem.
> 
> 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 is being entered and the address for allocated memory for
> storing the context during Deep Sleep.
> 
> 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.
> 
> Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>   drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 0334ade19868..172b726e00a6 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -22,6 +22,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/reboot.h>
>   
>   #include "ti_sci.h"
> @@ -3521,6 +3522,67 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
>   	return NOTIFY_BAD;
>   }
>   
> +static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
> +{
> +#if IS_ENABLED(CONFIG_SUSPEND)
> +	u8 mode;
> +
> +	/* Map and validate the target Linux suspend state to TISCI LPM. */
> +	switch (pm_suspend_target_state) {
> +	case PM_SUSPEND_MEM:
> +		/* S2MEM is not supported by the firmware. */
> +		if (!(info->fw_caps & MSG_FLAG_CAPS_LPM_DEEP_SLEEP))
> +			return 0;
> +		mode = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP;
> +		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,
> +					(u32)(info->ctx_mem_addr & 0xffffffff),
> +					(u32)((u64)info->ctx_mem_addr >> 32), 0);
> +#else
> +	return 0;
> +#endif
> +}
> +
> +static int ti_sci_suspend(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);

After this the will the IOs be in isolation? Or does the firmware wait
until power down begins later?

Andrew

> +	if (ret)
> +		return ret;
> +	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
> +
> +	ret = ti_sci_prepare_system_suspend(info);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ti_sci_resume(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
> +	if (ret)
> +		return ret;
> +	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
> +
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
> +
>   static int ti_sci_init_suspend(struct platform_device *pdev,
>   			       struct ti_sci_info *info)
>   {
> @@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
>   	.driver = {
>   		   .name = "ti-sci",
>   		   .of_match_table = of_match_ptr(ti_sci_of_match),
> +		   .pm = &ti_sci_pm_ops,
>   	},
>   };
>   module_platform_driver(ti_sci_driver);

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Andrew Davis]

On 8/3/23 1:42 AM, Dhruva Gole wrote:
> Introduce system suspend resume calls that will allow the ti_sci
> driver to support deep sleep low power mode when the user space issues a
> suspend to mem.
> 
> 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 is being entered and the address for allocated memory for
> storing the context during Deep Sleep.
> 
> 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.
> 
> Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>   drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 0334ade19868..172b726e00a6 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -22,6 +22,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/reboot.h>
>   
>   #include "ti_sci.h"
> @@ -3521,6 +3522,67 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
>   	return NOTIFY_BAD;
>   }
>   
> +static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
> +{
> +#if IS_ENABLED(CONFIG_SUSPEND)
> +	u8 mode;
> +
> +	/* Map and validate the target Linux suspend state to TISCI LPM. */
> +	switch (pm_suspend_target_state) {
> +	case PM_SUSPEND_MEM:
> +		/* S2MEM is not supported by the firmware. */
> +		if (!(info->fw_caps & MSG_FLAG_CAPS_LPM_DEEP_SLEEP))
> +			return 0;
> +		mode = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP;
> +		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,
> +					(u32)(info->ctx_mem_addr & 0xffffffff),
> +					(u32)((u64)info->ctx_mem_addr >> 32), 0);
> +#else
> +	return 0;
> +#endif
> +}
> +
> +static int ti_sci_suspend(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);

After this the will the IOs be in isolation? Or does the firmware wait
until power down begins later?

Andrew

> +	if (ret)
> +		return ret;
> +	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
> +
> +	ret = ti_sci_prepare_system_suspend(info);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ti_sci_resume(struct device *dev)
> +{
> +	struct ti_sci_info *info = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
> +	if (ret)
> +		return ret;
> +	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
> +
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
> +
>   static int ti_sci_init_suspend(struct platform_device *pdev,
>   			       struct ti_sci_info *info)
>   {
> @@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
>   	.driver = {
>   		   .name = "ti-sci",
>   		   .of_match_table = of_match_ptr(ti_sci_of_match),
> +		   .pm = &ti_sci_pm_ops,
>   	},
>   };
>   module_platform_driver(ti_sci_driver);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Dhruva Gole]

On Aug 03, 2023 at 10:14:03 -0500, Andrew Davis wrote:
[..snip..]
> >   #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..f2d1d74ab8fc 100644
> > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
> >   			u64 *current_freq);
> >   };
> > +/* 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
> 
> I don't see these used in this series, do you need them? lpm_wake_reason()

True, we are not currently using these macros. They _maybe required in
future.
I can remove them if required?

> doesn't seem used either and I'm not sure how you plan to use it, could
> you detail that?

When the system wakes from suspend-to-mem we can check which
subsystem has woken us up with the TISCI LPM_WAKEUP_REASON message.
There's no hardware event generated and we have to ask the firmware
for the actual wake reason.

We may want to add support for a wake up interrupt controller that will
generate an interrupt for other subsystems. This might end up using this
lpm_wake_reason API, hence even though the function maybe unused today
it will be required for above described scenario.

However if you prefer that I remove it and then add it in future when we
finally write a working interrupt controller driver, then do let me
know.

> 
> Andrew
> 
> > +
> > +/* TISCI LPM IO isolation control values */
> > +#define TISCI_MSG_VALUE_IO_ENABLE			1
> > +#define TISCI_MSG_VALUE_IO_DISABLE			0
> > +
[..snip..]

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

Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Dhruva Gole]

On Aug 03, 2023 at 10:14:03 -0500, Andrew Davis wrote:
[..snip..]
> >   #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..f2d1d74ab8fc 100644
> > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
> >   			u64 *current_freq);
> >   };
> > +/* 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
> 
> I don't see these used in this series, do you need them? lpm_wake_reason()

True, we are not currently using these macros. They _maybe required in
future.
I can remove them if required?

> doesn't seem used either and I'm not sure how you plan to use it, could
> you detail that?

When the system wakes from suspend-to-mem we can check which
subsystem has woken us up with the TISCI LPM_WAKEUP_REASON message.
There's no hardware event generated and we have to ask the firmware
for the actual wake reason.

We may want to add support for a wake up interrupt controller that will
generate an interrupt for other subsystems. This might end up using this
lpm_wake_reason API, hence even though the function maybe unused today
it will be required for above described scenario.

However if you prefer that I remove it and then add it in future when we
finally write a working interrupt controller driver, then do let me
know.

> 
> Andrew
> 
> > +
> > +/* TISCI LPM IO isolation control values */
> > +#define TISCI_MSG_VALUE_IO_ENABLE			1
> > +#define TISCI_MSG_VALUE_IO_DISABLE			0
> > +
[..snip..]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
> On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > Introduce system suspend resume calls that will allow the ti_sci
> > driver to support deep sleep low power mode when the user space issues a
> > suspend to mem.
> > 
> > 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 is being entered and the address for allocated memory for
> > storing the context during Deep Sleep.
> > 
> > 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.
> > 
> > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > ---
> >   drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 63 insertions(+)
> > 
[..snip..]
> > +static int ti_sci_suspend(struct device *dev)
> > +{
> > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> 
> After this the will the IOs be in isolation? Or does the firmware wait
> until power down begins later?

From what I understand,
IOs will be in isolation immediately

> 
> Andrew
> 
> > +	if (ret)
> > +		return ret;
> > +	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
> > +
> > +	ret = ti_sci_prepare_system_suspend(info);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_sci_resume(struct device *dev)
> > +{
> > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
> > +	if (ret)
> > +		return ret;
> > +	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
> > +
> > +	return 0;
> > +}
> > +
> > +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
> > +
> >   static int ti_sci_init_suspend(struct platform_device *pdev,
> >   			       struct ti_sci_info *info)
> >   {
> > @@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
> >   	.driver = {
> >   		   .name = "ti-sci",
> >   		   .of_match_table = of_match_ptr(ti_sci_of_match),
> > +		   .pm = &ti_sci_pm_ops,
> >   	},
> >   };
> >   module_platform_driver(ti_sci_driver);

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

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
> On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > Introduce system suspend resume calls that will allow the ti_sci
> > driver to support deep sleep low power mode when the user space issues a
> > suspend to mem.
> > 
> > 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 is being entered and the address for allocated memory for
> > storing the context during Deep Sleep.
> > 
> > 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.
> > 
> > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > ---
> >   drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 63 insertions(+)
> > 
[..snip..]
> > +static int ti_sci_suspend(struct device *dev)
> > +{
> > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> 
> After this the will the IOs be in isolation? Or does the firmware wait
> until power down begins later?

From what I understand,
IOs will be in isolation immediately

> 
> Andrew
> 
> > +	if (ret)
> > +		return ret;
> > +	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
> > +
> > +	ret = ti_sci_prepare_system_suspend(info);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_sci_resume(struct device *dev)
> > +{
> > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
> > +	if (ret)
> > +		return ret;
> > +	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
> > +
> > +	return 0;
> > +}
> > +
> > +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
> > +
> >   static int ti_sci_init_suspend(struct platform_device *pdev,
> >   			       struct ti_sci_info *info)
> >   {
> > @@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
> >   	.driver = {
> >   		   .name = "ti-sci",
> >   		   .of_match_table = of_match_ptr(ti_sci_of_match),
> > +		   .pm = &ti_sci_pm_ops,
> >   	},
> >   };
> >   module_platform_driver(ti_sci_driver);

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

[Dhruva Gole]

On Aug 03, 2023 at 10:23:47 -0500, Andrew Davis wrote:
> On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > From: Dave Gerlach <d-gerlach@ti.com>
> > 
> > A region of memory in DDR must be used during Deep Sleep for saving
> > of some system context when using the ti_sci firmware. From DM's point
> > of view, this can be any contiguous region in the DDR, so can allocate
> > 512KB of DMA reserved memory in probe(), instead of another carveout.
> > 
> > Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
> > probe to determine if any low power modes are supported and if
> > ti_sci_init_suspend should be called based on the response received.
> > 
> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > Tested-by: Roger Quadros <rogerq@kernel.org>
> > [d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
> > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > ---
> >   drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 42 insertions(+)
> > 
[..snip..]
> >   static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
> >   	.default_host_id = 2,
> > @@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
> >   		}
> >   	}
> > +	/*
> > +	 * Check if the firmware supports any optional low power modes
> > +	 * and initialize them if present. 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_MASK_CAPS_LPM))
> > +		ti_sci_init_suspend(pdev, info);
> > +
> >   	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,
> > @@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
> >   		mbox_free_channel(info->chan_rx);
> >   	}
> > +	if (info->ctx_mem_buf)
> > +		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
> 
> You allocated with dma_alloc_attrs() you should free with dma_free_attrs().

Good catch, will fix this in next revision

> 
> Andrew
> 
> > +				  info->ctx_mem_buf,
> > +				  info->ctx_mem_addr);
> >   	return ret;
> >   }

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

Re: [PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

[Dhruva Gole]

On Aug 03, 2023 at 10:23:47 -0500, Andrew Davis wrote:
> On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > From: Dave Gerlach <d-gerlach@ti.com>
> > 
> > A region of memory in DDR must be used during Deep Sleep for saving
> > of some system context when using the ti_sci firmware. From DM's point
> > of view, this can be any contiguous region in the DDR, so can allocate
> > 512KB of DMA reserved memory in probe(), instead of another carveout.
> > 
> > Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
> > probe to determine if any low power modes are supported and if
> > ti_sci_init_suspend should be called based on the response received.
> > 
> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > Tested-by: Roger Quadros <rogerq@kernel.org>
> > [d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
> > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > ---
> >   drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 42 insertions(+)
> > 
[..snip..]
> >   static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
> >   	.default_host_id = 2,
> > @@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
> >   		}
> >   	}
> > +	/*
> > +	 * Check if the firmware supports any optional low power modes
> > +	 * and initialize them if present. 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_MASK_CAPS_LPM))
> > +		ti_sci_init_suspend(pdev, info);
> > +
> >   	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,
> > @@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
> >   		mbox_free_channel(info->chan_rx);
> >   	}
> > +	if (info->ctx_mem_buf)
> > +		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
> 
> You allocated with dma_alloc_attrs() you should free with dma_free_attrs().

Good catch, will fix this in next revision

> 
> Andrew
> 
> > +				  info->ctx_mem_buf,
> > +				  info->ctx_mem_addr);
> >   	return ret;
> >   }

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Andrew Davis]

On 8/3/23 10:42 AM, Dhruva Gole wrote:
> On Aug 03, 2023 at 10:14:03 -0500, Andrew Davis wrote:
> [..snip..]
>>>    #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..f2d1d74ab8fc 100644
>>> --- a/include/linux/soc/ti/ti_sci_protocol.h
>>> +++ b/include/linux/soc/ti/ti_sci_protocol.h
>>> @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
>>>    			u64 *current_freq);
>>>    };
>>> +/* 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
>>
>> I don't see these used in this series, do you need them? lpm_wake_reason()
> 
> True, we are not currently using these macros. They _maybe required in
> future.
> I can remove them if required?
> 
>> doesn't seem used either and I'm not sure how you plan to use it, could
>> you detail that?
> 
> When the system wakes from suspend-to-mem we can check which
> subsystem has woken us up with the TISCI LPM_WAKEUP_REASON message.
> There's no hardware event generated and we have to ask the firmware
> for the actual wake reason.
> 
> We may want to add support for a wake up interrupt controller that will
> generate an interrupt for other subsystems. This might end up using this
> lpm_wake_reason API, hence even though the function maybe unused today
> it will be required for above described scenario.
> 
> However if you prefer that I remove it and then add it in future when we
> finally write a working interrupt controller driver, then do let me
> know.
> 

It's easier to review code that is used. We may go with a
completely different approach by then, will be easier to unwind
this if you don't start with it, only add it when used.

Andrew

>>
>> Andrew
>>
>>> +
>>> +/* TISCI LPM IO isolation control values */
>>> +#define TISCI_MSG_VALUE_IO_ENABLE			1
>>> +#define TISCI_MSG_VALUE_IO_DISABLE			0
>>> +
> [..snip..]
> 

Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops

[Andrew Davis]

On 8/3/23 10:42 AM, Dhruva Gole wrote:
> On Aug 03, 2023 at 10:14:03 -0500, Andrew Davis wrote:
> [..snip..]
>>>    #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..f2d1d74ab8fc 100644
>>> --- a/include/linux/soc/ti/ti_sci_protocol.h
>>> +++ b/include/linux/soc/ti/ti_sci_protocol.h
>>> @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
>>>    			u64 *current_freq);
>>>    };
>>> +/* 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
>>
>> I don't see these used in this series, do you need them? lpm_wake_reason()
> 
> True, we are not currently using these macros. They _maybe required in
> future.
> I can remove them if required?
> 
>> doesn't seem used either and I'm not sure how you plan to use it, could
>> you detail that?
> 
> When the system wakes from suspend-to-mem we can check which
> subsystem has woken us up with the TISCI LPM_WAKEUP_REASON message.
> There's no hardware event generated and we have to ask the firmware
> for the actual wake reason.
> 
> We may want to add support for a wake up interrupt controller that will
> generate an interrupt for other subsystems. This might end up using this
> lpm_wake_reason API, hence even though the function maybe unused today
> it will be required for above described scenario.
> 
> However if you prefer that I remove it and then add it in future when we
> finally write a working interrupt controller driver, then do let me
> know.
> 

It's easier to review code that is used. We may go with a
completely different approach by then, will be easier to unwind
this if you don't start with it, only add it when used.

Andrew

>>
>> Andrew
>>
>>> +
>>> +/* TISCI LPM IO isolation control values */
>>> +#define TISCI_MSG_VALUE_IO_ENABLE			1
>>> +#define TISCI_MSG_VALUE_IO_DISABLE			0
>>> +
> [..snip..]
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Andrew Davis]

On 8/3/23 10:55 AM, Dhruva Gole wrote:
> On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
>> On 8/3/23 1:42 AM, Dhruva Gole wrote:
>>> Introduce system suspend resume calls that will allow the ti_sci
>>> driver to support deep sleep low power mode when the user space issues a
>>> suspend to mem.
>>>
>>> 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 is being entered and the address for allocated memory for
>>> storing the context during Deep Sleep.
>>>
>>> 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.
>>>
>>> Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>>> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>>> Signed-off-by: Dhruva Gole <d-gole@ti.com>
>>> ---
>>>    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 63 insertions(+)
>>>
> [..snip..]
>>> +static int ti_sci_suspend(struct device *dev)
>>> +{
>>> +	struct ti_sci_info *info = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
>>
>> After this the will the IOs be in isolation? Or does the firmware wait
>> until power down begins later?
> 
>  From what I understand,
> IOs will be in isolation immediately
> 

That is what I understand too, so then any device that may need to do some
external communication for its suspend will not function, this must be the
last driver _suspend() the system calls, how do you enforce that?

Andrew

>>
>> Andrew
>>
>>> +	if (ret)
>>> +		return ret;
>>> +	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
>>> +
>>> +	ret = ti_sci_prepare_system_suspend(info);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int ti_sci_resume(struct device *dev)
>>> +{
>>> +	struct ti_sci_info *info = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
>>> +	if (ret)
>>> +		return ret;
>>> +	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
>>> +
>>>    static int ti_sci_init_suspend(struct platform_device *pdev,
>>>    			       struct ti_sci_info *info)
>>>    {
>>> @@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
>>>    	.driver = {
>>>    		   .name = "ti-sci",
>>>    		   .of_match_table = of_match_ptr(ti_sci_of_match),
>>> +		   .pm = &ti_sci_pm_ops,
>>>    	},
>>>    };
>>>    module_platform_driver(ti_sci_driver);
> 

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Andrew Davis]

On 8/3/23 10:55 AM, Dhruva Gole wrote:
> On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
>> On 8/3/23 1:42 AM, Dhruva Gole wrote:
>>> Introduce system suspend resume calls that will allow the ti_sci
>>> driver to support deep sleep low power mode when the user space issues a
>>> suspend to mem.
>>>
>>> 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 is being entered and the address for allocated memory for
>>> storing the context during Deep Sleep.
>>>
>>> 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.
>>>
>>> Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>>> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>>> Signed-off-by: Dhruva Gole <d-gole@ti.com>
>>> ---
>>>    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 63 insertions(+)
>>>
> [..snip..]
>>> +static int ti_sci_suspend(struct device *dev)
>>> +{
>>> +	struct ti_sci_info *info = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
>>
>> After this the will the IOs be in isolation? Or does the firmware wait
>> until power down begins later?
> 
>  From what I understand,
> IOs will be in isolation immediately
> 

That is what I understand too, so then any device that may need to do some
external communication for its suspend will not function, this must be the
last driver _suspend() the system calls, how do you enforce that?

Andrew

>>
>> Andrew
>>
>>> +	if (ret)
>>> +		return ret;
>>> +	dev_dbg(dev, "%s: set I/O isolation: %d\n", __func__, ret);
>>> +
>>> +	ret = ti_sci_prepare_system_suspend(info);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int ti_sci_resume(struct device *dev)
>>> +{
>>> +	struct ti_sci_info *info = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE);
>>> +	if (ret)
>>> +		return ret;
>>> +	dev_dbg(dev, "%s: disable I/O isolation: %d\n", __func__, ret);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
>>> +
>>>    static int ti_sci_init_suspend(struct platform_device *pdev,
>>>    			       struct ti_sci_info *info)
>>>    {
>>> @@ -3759,6 +3821,7 @@ static struct platform_driver ti_sci_driver = {
>>>    	.driver = {
>>>    		   .name = "ti-sci",
>>>    		   .of_match_table = of_match_ptr(ti_sci_of_match),
>>> +		   .pm = &ti_sci_pm_ops,
>>>    	},
>>>    };
>>>    module_platform_driver(ti_sci_driver);
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
> On 8/3/23 10:55 AM, Dhruva Gole wrote:
> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > > > Introduce system suspend resume calls that will allow the ti_sci
> > > > driver to support deep sleep low power mode when the user space issues a
> > > > suspend to mem.
> > > > 
> > > > 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 is being entered and the address for allocated memory for
> > > > storing the context during Deep Sleep.
> > > > 
> > > > 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.
> > > > 
> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > > > ---
> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
> > > >    1 file changed, 63 insertions(+)
> > > > 
> > [..snip..]
> > > > +static int ti_sci_suspend(struct device *dev)
> > > > +{
> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> > > > +	int ret;
> > > > +
> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> > > 
> > > After this the will the IOs be in isolation? Or does the firmware wait
> > > until power down begins later?
> > 
> >  From what I understand,
> > IOs will be in isolation immediately
> > 
> 
> That is what I understand too, so then any device that may need to do some
> external communication for its suspend will not function, this must be the
> last driver _suspend() the system calls, how do you enforce that?

I will make use of .suspend_noirq callbacks in that case. Does that
sound better, or is there anything else I may not be aware of?

> 
> Andrew
> 
> > > 
> > > Andrew
> > > 
[..snip..]
> > 

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

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
> On 8/3/23 10:55 AM, Dhruva Gole wrote:
> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > > > Introduce system suspend resume calls that will allow the ti_sci
> > > > driver to support deep sleep low power mode when the user space issues a
> > > > suspend to mem.
> > > > 
> > > > 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 is being entered and the address for allocated memory for
> > > > storing the context during Deep Sleep.
> > > > 
> > > > 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.
> > > > 
> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > > > ---
> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
> > > >    1 file changed, 63 insertions(+)
> > > > 
> > [..snip..]
> > > > +static int ti_sci_suspend(struct device *dev)
> > > > +{
> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> > > > +	int ret;
> > > > +
> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> > > 
> > > After this the will the IOs be in isolation? Or does the firmware wait
> > > until power down begins later?
> > 
> >  From what I understand,
> > IOs will be in isolation immediately
> > 
> 
> That is what I understand too, so then any device that may need to do some
> external communication for its suspend will not function, this must be the
> last driver _suspend() the system calls, how do you enforce that?

I will make use of .suspend_noirq callbacks in that case. Does that
sound better, or is there anything else I may not be aware of?

> 
> Andrew
> 
> > > 
> > > Andrew
> > > 
[..snip..]
> > 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Kevin Hilman]

Dhruva Gole <d-gole@ti.com> writes:

> On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
>> On 8/3/23 10:55 AM, Dhruva Gole wrote:
>> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
>> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
>> > > > Introduce system suspend resume calls that will allow the ti_sci
>> > > > driver to support deep sleep low power mode when the user space issues a
>> > > > suspend to mem.
>> > > > 
>> > > > 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 is being entered and the address for allocated memory for
>> > > > storing the context during Deep Sleep.
>> > > > 
>> > > > 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.
>> > > > 
>> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
>> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
>> > > > ---
>> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>> > > >    1 file changed, 63 insertions(+)
>> > > > 
>> > [..snip..]
>> > > > +static int ti_sci_suspend(struct device *dev)
>> > > > +{
>> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
>> > > > +	int ret;
>> > > > +
>> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
>> > > 
>> > > After this the will the IOs be in isolation? Or does the firmware wait
>> > > until power down begins later?
>> > 
>> >  From what I understand,
>> > IOs will be in isolation immediately
>> > 
>> 
>> That is what I understand too, so then any device that may need to do some
>> external communication for its suspend will not function, this must be the
>> last driver _suspend() the system calls, how do you enforce that?
>
> I will make use of .suspend_noirq callbacks in that case. Does that
> sound better, or is there anything else I may not be aware of?

Using _noirq just moves the problem.  What if other drivers are also
using _noirq callbacks and run after the SCI driver?  You still cannot
guarantee ordering.

It seems to me that the IO isolation stuff is a system-wide operation,
and should probably be handled at the platform suspend_ops level
(e.g. suspend_ops->prepare_late()).   This would ensure that it runs
*after* all the driver hooks (even driver _noirq hooks.) and right
before the full suspend (or s2idle.)

Now, all that being said, I noticed that in v7, you didn't move this to
_noirq, but instead suggested that this be handled by TF-A.  I suppose
that's an option also, but my suggestion above should work also.

Kevin

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Kevin Hilman]

Dhruva Gole <d-gole@ti.com> writes:

> On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
>> On 8/3/23 10:55 AM, Dhruva Gole wrote:
>> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
>> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
>> > > > Introduce system suspend resume calls that will allow the ti_sci
>> > > > driver to support deep sleep low power mode when the user space issues a
>> > > > suspend to mem.
>> > > > 
>> > > > 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 is being entered and the address for allocated memory for
>> > > > storing the context during Deep Sleep.
>> > > > 
>> > > > 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.
>> > > > 
>> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
>> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
>> > > > ---
>> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>> > > >    1 file changed, 63 insertions(+)
>> > > > 
>> > [..snip..]
>> > > > +static int ti_sci_suspend(struct device *dev)
>> > > > +{
>> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
>> > > > +	int ret;
>> > > > +
>> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
>> > > 
>> > > After this the will the IOs be in isolation? Or does the firmware wait
>> > > until power down begins later?
>> > 
>> >  From what I understand,
>> > IOs will be in isolation immediately
>> > 
>> 
>> That is what I understand too, so then any device that may need to do some
>> external communication for its suspend will not function, this must be the
>> last driver _suspend() the system calls, how do you enforce that?
>
> I will make use of .suspend_noirq callbacks in that case. Does that
> sound better, or is there anything else I may not be aware of?

Using _noirq just moves the problem.  What if other drivers are also
using _noirq callbacks and run after the SCI driver?  You still cannot
guarantee ordering.

It seems to me that the IO isolation stuff is a system-wide operation,
and should probably be handled at the platform suspend_ops level
(e.g. suspend_ops->prepare_late()).   This would ensure that it runs
*after* all the driver hooks (even driver _noirq hooks.) and right
before the full suspend (or s2idle.)

Now, all that being said, I noticed that in v7, you didn't move this to
_noirq, but instead suggested that this be handled by TF-A.  I suppose
that's an option also, but my suggestion above should work also.

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

Kevin,

Thanks for the suggestion. I have a rough proposal inline, please can
you take a look? I will test those changes and respin this series
accordingly

On Aug 07, 2023 at 14:57:05 -0700, Kevin Hilman wrote:
> Dhruva Gole <d-gole@ti.com> writes:
> 
> > On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
> >> On 8/3/23 10:55 AM, Dhruva Gole wrote:
> >> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
> >> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
> >> > > > Introduce system suspend resume calls that will allow the ti_sci
> >> > > > driver to support deep sleep low power mode when the user space issues a
> >> > > > suspend to mem.
> >> > > > 
> >> > > > 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 is being entered and the address for allocated memory for
> >> > > > storing the context during Deep Sleep.
> >> > > > 
> >> > > > 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.
> >> > > > 
> >> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> >> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> >> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> >> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> >> > > > ---
> >> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
> >> > > >    1 file changed, 63 insertions(+)
> >> > > > 
> >> > [..snip..]
> >> > > > +static int ti_sci_suspend(struct device *dev)
> >> > > > +{
> >> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> >> > > > +	int ret;
> >> > > > +
> >> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> >> > > 
> >> > > After this the will the IOs be in isolation? Or does the firmware wait
> >> > > until power down begins later?
> >> > 
> >> >  From what I understand,
> >> > IOs will be in isolation immediately
> >> > 
> >> 
> >> That is what I understand too, so then any device that may need to do some
> >> external communication for its suspend will not function, this must be the
> >> last driver _suspend() the system calls, how do you enforce that?
> >
> > I will make use of .suspend_noirq callbacks in that case. Does that
> > sound better, or is there anything else I may not be aware of?
> 
> Using _noirq just moves the problem.  What if other drivers are also
> using _noirq callbacks and run after the SCI driver?  You still cannot

True, this thought occurred to me as well which is why I was thinking
that moving it to ATF might be a better choice.

> guarantee ordering.
> 
> It seems to me that the IO isolation stuff is a system-wide operation,
> and should probably be handled at the platform suspend_ops level
> (e.g. suspend_ops->prepare_late()).   This would ensure that it runs

I must have missed this approach! Are you suggesting something like what
was done for am335?

static const struct platform_suspend_ops am33xx_pm_ops

have a similar code for tisci..?

static const struct platform_suspend_ops tisci_pm_ops = {
	.prepare_late = tisci_set_io_isolation
	};

And then while resuming we may want the pinctrl driver to scan for the
wk_evt bit[0] before the isolation is disabled, so we want the
tisci_resume/ remove isolation to be called later than that.

So I a wondering if the code below makes sense?

static const struct platform_suspend_ops tisci_pm_ops = {
	.prepare_late = tisci_suspend // also includes set isolation
	.end = tisci_resume 	// Disables isolation
	};

However a minor drawback here maybe that the serial logs on the resume
path may not appear when using a serial console for example. However
they should be able to easily access using dmesg.

> *after* all the driver hooks (even driver _noirq hooks.) and right
> before the full suspend (or s2idle.)
> 
> Now, all that being said, I noticed that in v7, you didn't move this to
> _noirq, but instead suggested that this be handled by TF-A.  I suppose
> that's an option also, but my suggestion above should work also.

Thanks for the pointer! I do believe it will make more sense to do it
from linux itself unless we have no way to do it in linux.

> 
> Kevin


[0] Table 5-517. Description Of The Pad Configuration Register Bits
https://www.ti.com/lit/pdf/spruid7

NOTE: The hardware works in such a way that as soon as the IO isolation
is disabled the wake_evt information is lost so the pinctrl-single
driver won't be able to know what pin woke it up if we disable io
isolation before it has the chance to look at the padconf registers

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Dhruva Gole]

Kevin,

Thanks for the suggestion. I have a rough proposal inline, please can
you take a look? I will test those changes and respin this series
accordingly

On Aug 07, 2023 at 14:57:05 -0700, Kevin Hilman wrote:
> Dhruva Gole <d-gole@ti.com> writes:
> 
> > On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
> >> On 8/3/23 10:55 AM, Dhruva Gole wrote:
> >> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
> >> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
> >> > > > Introduce system suspend resume calls that will allow the ti_sci
> >> > > > driver to support deep sleep low power mode when the user space issues a
> >> > > > suspend to mem.
> >> > > > 
> >> > > > 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 is being entered and the address for allocated memory for
> >> > > > storing the context during Deep Sleep.
> >> > > > 
> >> > > > 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.
> >> > > > 
> >> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
> >> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> >> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> >> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> >> > > > ---
> >> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
> >> > > >    1 file changed, 63 insertions(+)
> >> > > > 
> >> > [..snip..]
> >> > > > +static int ti_sci_suspend(struct device *dev)
> >> > > > +{
> >> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
> >> > > > +	int ret;
> >> > > > +
> >> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
> >> > > 
> >> > > After this the will the IOs be in isolation? Or does the firmware wait
> >> > > until power down begins later?
> >> > 
> >> >  From what I understand,
> >> > IOs will be in isolation immediately
> >> > 
> >> 
> >> That is what I understand too, so then any device that may need to do some
> >> external communication for its suspend will not function, this must be the
> >> last driver _suspend() the system calls, how do you enforce that?
> >
> > I will make use of .suspend_noirq callbacks in that case. Does that
> > sound better, or is there anything else I may not be aware of?
> 
> Using _noirq just moves the problem.  What if other drivers are also
> using _noirq callbacks and run after the SCI driver?  You still cannot

True, this thought occurred to me as well which is why I was thinking
that moving it to ATF might be a better choice.

> guarantee ordering.
> 
> It seems to me that the IO isolation stuff is a system-wide operation,
> and should probably be handled at the platform suspend_ops level
> (e.g. suspend_ops->prepare_late()).   This would ensure that it runs

I must have missed this approach! Are you suggesting something like what
was done for am335?

static const struct platform_suspend_ops am33xx_pm_ops

have a similar code for tisci..?

static const struct platform_suspend_ops tisci_pm_ops = {
	.prepare_late = tisci_set_io_isolation
	};

And then while resuming we may want the pinctrl driver to scan for the
wk_evt bit[0] before the isolation is disabled, so we want the
tisci_resume/ remove isolation to be called later than that.

So I a wondering if the code below makes sense?

static const struct platform_suspend_ops tisci_pm_ops = {
	.prepare_late = tisci_suspend // also includes set isolation
	.end = tisci_resume 	// Disables isolation
	};

However a minor drawback here maybe that the serial logs on the resume
path may not appear when using a serial console for example. However
they should be able to easily access using dmesg.

> *after* all the driver hooks (even driver _noirq hooks.) and right
> before the full suspend (or s2idle.)
> 
> Now, all that being said, I noticed that in v7, you didn't move this to
> _noirq, but instead suggested that this be handled by TF-A.  I suppose
> that's an option also, but my suggestion above should work also.

Thanks for the pointer! I do believe it will make more sense to do it
from linux itself unless we have no way to do it in linux.

> 
> Kevin


[0] Table 5-517. Description Of The Pad Configuration Register Bits
https://www.ti.com/lit/pdf/spruid7

NOTE: The hardware works in such a way that as soon as the IO isolation
is disabled the wake_evt information is lost so the pinctrl-single
driver won't be able to know what pin woke it up if we disable io
isolation before it has the chance to look at the padconf registers

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

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Kevin Hilman]

Hi Dhruva,

Dhruva Gole <d-gole@ti.com> writes:

> Kevin,
>
> Thanks for the suggestion. I have a rough proposal inline, please can
> you take a look? I will test those changes and respin this series
> accordingly
>
> On Aug 07, 2023 at 14:57:05 -0700, Kevin Hilman wrote:
>> Dhruva Gole <d-gole@ti.com> writes:
>> 
>> > On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
>> >> On 8/3/23 10:55 AM, Dhruva Gole wrote:
>> >> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
>> >> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
>> >> > > > Introduce system suspend resume calls that will allow the ti_sci
>> >> > > > driver to support deep sleep low power mode when the user space issues a
>> >> > > > suspend to mem.
>> >> > > > 
>> >> > > > 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 is being entered and the address for allocated memory for
>> >> > > > storing the context during Deep Sleep.
>> >> > > > 
>> >> > > > 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.
>> >> > > > 
>> >> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
>> >> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> >> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>> >> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>> >> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
>> >> > > > ---
>> >> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>> >> > > >    1 file changed, 63 insertions(+)
>> >> > > > 
>> >> > [..snip..]
>> >> > > > +static int ti_sci_suspend(struct device *dev)
>> >> > > > +{
>> >> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
>> >> > > > +	int ret;
>> >> > > > +
>> >> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
>> >> > > 
>> >> > > After this the will the IOs be in isolation? Or does the firmware wait
>> >> > > until power down begins later?
>> >> > 
>> >> >  From what I understand,
>> >> > IOs will be in isolation immediately
>> >> > 
>> >> 
>> >> That is what I understand too, so then any device that may need to do some
>> >> external communication for its suspend will not function, this must be the
>> >> last driver _suspend() the system calls, how do you enforce that?
>> >
>> > I will make use of .suspend_noirq callbacks in that case. Does that
>> > sound better, or is there anything else I may not be aware of?
>> 
>> Using _noirq just moves the problem.  What if other drivers are also
>> using _noirq callbacks and run after the SCI driver?  You still cannot
>
> True, this thought occurred to me as well which is why I was thinking
> that moving it to ATF might be a better choice.
>
>> guarantee ordering.
>> 
>> It seems to me that the IO isolation stuff is a system-wide operation,
>> and should probably be handled at the platform suspend_ops level
>> (e.g. suspend_ops->prepare_late()).   This would ensure that it runs
>
> I must have missed this approach! Are you suggesting something like what
> was done for am335?
>
> static const struct platform_suspend_ops am33xx_pm_ops
>
> have a similar code for tisci..?
>
> static const struct platform_suspend_ops tisci_pm_ops = {
> 	.prepare_late = tisci_set_io_isolation
> 	};

Yes, with the minor nit that I would make a tisci_prepare_late()
function, which then calls _set_io_isolation(), since there may be some
other things we want to do in the "late" prepare for other LPM modes.

Also, for the additional LPM modes (more than DeepSleep), we're looking
at using suspend-to-idle (s2idle) to manage those.  So in addition to
platform_suspend_ops->prepare_late(), you should add this function to
s2idle_ops->prepare_late() also.

> And then while resuming we may want the pinctrl driver to scan for the
> wk_evt bit[0] before the isolation is disabled, so we want the
> tisci_resume/ remove isolation to be called later than that.
>
> So I a wondering if the code below makes sense?
>
> static const struct platform_suspend_ops tisci_pm_ops = {
> 	.prepare_late = tisci_suspend // also includes set isolation
> 	.end = tisci_resume 	// Disables isolation
> 	};
>
> However a minor drawback here maybe that the serial logs on the resume
> path may not appear when using a serial console for example. However
> they should be able to easily access using dmesg.

I'm not sure I fully understand this usecase, but using ->end() seems
drastic.  If IO isolation is disabled that long, won't that cause
problems for driver's ->resume callbacks that want to touch hardware?

To me, it sounds like you might want to use ->resume_early() or maybe
->resume_noirq() in the pinctrl driver for this so that IO isolation can
be disabled sooner?

>> *after* all the driver hooks (even driver _noirq hooks.) and right
>> before the full suspend (or s2idle.)
>> 
>> Now, all that being said, I noticed that in v7, you didn't move this to
>> _noirq, but instead suggested that this be handled by TF-A.  I suppose
>> that's an option also, but my suggestion above should work also.
>
> Thanks for the pointer! I do believe it will make more sense to do it
> from linux itself unless we have no way to do it in linux.
>
>> 
>> Kevin
>
>
> [0] Table 5-517. Description Of The Pad Configuration Register Bits
> https://www.ti.com/lit/pdf/spruid7
>
> NOTE: The hardware works in such a way that as soon as the IO isolation
> is disabled the wake_evt information is lost so the pinctrl-single
> driver won't be able to know what pin woke it up if we disable io
> isolation before it has the chance to look at the padconf registers

Ah, OK.  So yeah, as I hinted at above, what about using
->resume_noirq() in the pinctrl driver to get the wake_evt information,
and then use the s2idle_ops->restore_early() to disable IO isolation,
since this happens after all the driver's noirq hooks have run.

Kevin

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Kevin Hilman]

Hi Dhruva,

Dhruva Gole <d-gole@ti.com> writes:

> Kevin,
>
> Thanks for the suggestion. I have a rough proposal inline, please can
> you take a look? I will test those changes and respin this series
> accordingly
>
> On Aug 07, 2023 at 14:57:05 -0700, Kevin Hilman wrote:
>> Dhruva Gole <d-gole@ti.com> writes:
>> 
>> > On Aug 03, 2023 at 11:00:11 -0500, Andrew Davis wrote:
>> >> On 8/3/23 10:55 AM, Dhruva Gole wrote:
>> >> > On Aug 03, 2023 at 10:26:32 -0500, Andrew Davis wrote:
>> >> > > On 8/3/23 1:42 AM, Dhruva Gole wrote:
>> >> > > > Introduce system suspend resume calls that will allow the ti_sci
>> >> > > > driver to support deep sleep low power mode when the user space issues a
>> >> > > > suspend to mem.
>> >> > > > 
>> >> > > > 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 is being entered and the address for allocated memory for
>> >> > > > storing the context during Deep Sleep.
>> >> > > > 
>> >> > > > 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.
>> >> > > > 
>> >> > > > Co-developed-by: Dave Gerlach <d-gerlach@ti.com>
>> >> > > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> >> > > > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>> >> > > > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>> >> > > > Signed-off-by: Dhruva Gole <d-gole@ti.com>
>> >> > > > ---
>> >> > > >    drivers/firmware/ti_sci.c | 63 +++++++++++++++++++++++++++++++++++++++
>> >> > > >    1 file changed, 63 insertions(+)
>> >> > > > 
>> >> > [..snip..]
>> >> > > > +static int ti_sci_suspend(struct device *dev)
>> >> > > > +{
>> >> > > > +	struct ti_sci_info *info = dev_get_drvdata(dev);
>> >> > > > +	int ret;
>> >> > > > +
>> >> > > > +	ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_ENABLE);
>> >> > > 
>> >> > > After this the will the IOs be in isolation? Or does the firmware wait
>> >> > > until power down begins later?
>> >> > 
>> >> >  From what I understand,
>> >> > IOs will be in isolation immediately
>> >> > 
>> >> 
>> >> That is what I understand too, so then any device that may need to do some
>> >> external communication for its suspend will not function, this must be the
>> >> last driver _suspend() the system calls, how do you enforce that?
>> >
>> > I will make use of .suspend_noirq callbacks in that case. Does that
>> > sound better, or is there anything else I may not be aware of?
>> 
>> Using _noirq just moves the problem.  What if other drivers are also
>> using _noirq callbacks and run after the SCI driver?  You still cannot
>
> True, this thought occurred to me as well which is why I was thinking
> that moving it to ATF might be a better choice.
>
>> guarantee ordering.
>> 
>> It seems to me that the IO isolation stuff is a system-wide operation,
>> and should probably be handled at the platform suspend_ops level
>> (e.g. suspend_ops->prepare_late()).   This would ensure that it runs
>
> I must have missed this approach! Are you suggesting something like what
> was done for am335?
>
> static const struct platform_suspend_ops am33xx_pm_ops
>
> have a similar code for tisci..?
>
> static const struct platform_suspend_ops tisci_pm_ops = {
> 	.prepare_late = tisci_set_io_isolation
> 	};

Yes, with the minor nit that I would make a tisci_prepare_late()
function, which then calls _set_io_isolation(), since there may be some
other things we want to do in the "late" prepare for other LPM modes.

Also, for the additional LPM modes (more than DeepSleep), we're looking
at using suspend-to-idle (s2idle) to manage those.  So in addition to
platform_suspend_ops->prepare_late(), you should add this function to
s2idle_ops->prepare_late() also.

> And then while resuming we may want the pinctrl driver to scan for the
> wk_evt bit[0] before the isolation is disabled, so we want the
> tisci_resume/ remove isolation to be called later than that.
>
> So I a wondering if the code below makes sense?
>
> static const struct platform_suspend_ops tisci_pm_ops = {
> 	.prepare_late = tisci_suspend // also includes set isolation
> 	.end = tisci_resume 	// Disables isolation
> 	};
>
> However a minor drawback here maybe that the serial logs on the resume
> path may not appear when using a serial console for example. However
> they should be able to easily access using dmesg.

I'm not sure I fully understand this usecase, but using ->end() seems
drastic.  If IO isolation is disabled that long, won't that cause
problems for driver's ->resume callbacks that want to touch hardware?

To me, it sounds like you might want to use ->resume_early() or maybe
->resume_noirq() in the pinctrl driver for this so that IO isolation can
be disabled sooner?

>> *after* all the driver hooks (even driver _noirq hooks.) and right
>> before the full suspend (or s2idle.)
>> 
>> Now, all that being said, I noticed that in v7, you didn't move this to
>> _noirq, but instead suggested that this be handled by TF-A.  I suppose
>> that's an option also, but my suggestion above should work also.
>
> Thanks for the pointer! I do believe it will make more sense to do it
> from linux itself unless we have no way to do it in linux.
>
>> 
>> Kevin
>
>
> [0] Table 5-517. Description Of The Pad Configuration Register Bits
> https://www.ti.com/lit/pdf/spruid7
>
> NOTE: The hardware works in such a way that as soon as the IO isolation
> is disabled the wake_evt information is lost so the pinctrl-single
> driver won't be able to know what pin woke it up if we disable io
> isolation before it has the chance to look at the padconf registers

Ah, OK.  So yeah, as I hinted at above, what about using
->resume_noirq() in the pinctrl driver to get the wake_evt information,
and then use the s2idle_ops->restore_early() to disable IO isolation,
since this happens after all the driver's noirq hooks have run.

Kevin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [230809 00:20]:
> To me, it sounds like you might want to use ->resume_early() or maybe
> ->resume_noirq() in the pinctrl driver for this so that IO isolation can
> be disabled sooner?

For calls that need to happen just before the SoC is disabled or first
thing on resume path, cpu_cluster_pm_enter() and cpu_cluster_pm_exit()
notifiers work nice and allow distributing the code across the related
SoC specific code and device drivers. See for example the usage in
drivers/irqchip/irq-gic.c for CPU_CLUSTER_PM_ENTER.

Regards,

Tony

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [230809 00:20]:
> To me, it sounds like you might want to use ->resume_early() or maybe
> ->resume_noirq() in the pinctrl driver for this so that IO isolation can
> be disabled sooner?

For calls that need to happen just before the SoC is disabled or first
thing on resume path, cpu_cluster_pm_enter() and cpu_cluster_pm_exit()
notifiers work nice and allow distributing the code across the related
SoC specific code and device drivers. See for example the usage in
drivers/irqchip/irq-gic.c for CPU_CLUSTER_PM_ENTER.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [230809 00:20]:
>> To me, it sounds like you might want to use ->resume_early() or maybe
>> ->resume_noirq() in the pinctrl driver for this so that IO isolation can
>> be disabled sooner?
>
> For calls that need to happen just before the SoC is disabled or first
> thing on resume path, cpu_cluster_pm_enter() and cpu_cluster_pm_exit()
> notifiers work nice and allow distributing the code across the related
> SoC specific code and device drivers. See for example the usage in
> drivers/irqchip/irq-gic.c for CPU_CLUSTER_PM_ENTER.

Indeed, this is an option too, but for things that already have "full"
drivers (e.g. not an irqchip), they already have a full range of PM
callbacks, and adding another set of callbacks/notifiers for cpu_pm_* is
a bit clunky IMO.

That being said, for things like this IO isolation stuff that is
system-wide, and needs to happen very late in suspend (and/or very early
in suspend), cpu_pm_ is worth considering if the same cannot be done
with the normal PM callbacks.

Kevin

Re: [PATCH V6 4/4] firmware: ti_sci: Introduce system suspend resume support

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [230809 00:20]:
>> To me, it sounds like you might want to use ->resume_early() or maybe
>> ->resume_noirq() in the pinctrl driver for this so that IO isolation can
>> be disabled sooner?
>
> For calls that need to happen just before the SoC is disabled or first
> thing on resume path, cpu_cluster_pm_enter() and cpu_cluster_pm_exit()
> notifiers work nice and allow distributing the code across the related
> SoC specific code and device drivers. See for example the usage in
> drivers/irqchip/irq-gic.c for CPU_CLUSTER_PM_ENTER.

Indeed, this is an option too, but for things that already have "full"
drivers (e.g. not an irqchip), they already have a full range of PM
callbacks, and adding another set of callbacks/notifiers for cpu_pm_* is
a bit clunky IMO.

That being said, for things like this IO isolation stuff that is
system-wide, and needs to happen very late in suspend (and/or very early
in suspend), cpu_pm_ is worth considering if the same cannot be done
with the normal PM callbacks.

Kevin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel