[PATCH v3 0/4] firmware: ti_sci: Introduce BOARDCFG_MANAGED mode for Jacinto family

[PATCH v3 0/4] firmware: ti_sci: Introduce BOARDCFG_MANAGED mode for Jacinto family

[Thomas Richard (TI.com)]

This is the fourth iteration. In this new version the sci-clk driver also
restores the clock rate. I rebased the series on linux-next so we should
not have conflict with next rc1.

Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
---
Changes in v3:
- rebased on linux-next
- sci-clk: context_restore() operation restores also rate.
- Link to v2: https://lore.kernel.org/r/20251127-ti-sci-jacinto-s2r-restore-irq-v2-0-a487fa3ff221@bootlin.com

Changes in v2:
- ti_sci: use hlist to store IRQs.
- sci-clk: add context_restore operation
- ti_sci: restore clock parents during resume 
- Link to v1: https://lore.kernel.org/r/20251017-ti-sci-jacinto-s2r-restore-irq-v1-0-34d4339d247a@bootlin.com

---
Thomas Richard (TI.com) (4):
      firmware: ti_sci: add BOARDCFG_MANAGED mode support
      firmware: ti_sci: handle IRQ restore in BOARDCFG_MANAGED mode during resume
      clk: keystone: sci-clk: add restore_context() operation
      firmware: ti_sci: restore clock context during resume in BOARDCFG_MANAGED mode

 drivers/clk/keystone/sci-clk.c |  42 ++++++++---
 drivers/firmware/ti_sci.c      | 160 +++++++++++++++++++++++++++++++++++++----
 drivers/firmware/ti_sci.h      |   2 +
 3 files changed, 184 insertions(+), 20 deletions(-)
---
base-commit: 6c95d7e679d5d01fa48d6a8b8bdb92b6effe34f6
change-id: 20251010-ti-sci-jacinto-s2r-restore-irq-428e008fd10c

Best regards,
-- 
Thomas Richard (TI.com) <thomas.richard@bootlin.com>

[PATCH v3 1/4] firmware: ti_sci: add BOARDCFG_MANAGED mode support

[Thomas Richard (TI.com)]

In BOARDCFG_MANAGED mode, the low power mode configuration is done
statically for the DM via the boardcfg. Constraints are not supported, and
prepare_sleep() is not needed.

Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
---
 drivers/firmware/ti_sci.c | 10 +++++++---
 drivers/firmware/ti_sci.h |  2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 4a277937211f6..d77b631d9c855 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -3772,8 +3772,11 @@ static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
 			return ti_sci_cmd_prepare_sleep(&info->handle,
 							TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED,
 							0, 0, 0);
+		} else if (info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED) {
+			/* Nothing to do in the BOARDCFG_MANAGED mode */
+			return 0;
 		} else {
-			/* DM Managed is not supported by the firmware. */
+			/* DM Managed and BoardCfg Managed are not supported by the firmware. */
 			dev_err(info->dev, "Suspend to memory is not supported by the firmware\n");
 			return -EOPNOTSUPP;
 		}
@@ -4011,12 +4014,13 @@ static int ti_sci_probe(struct platform_device *pdev)
 	}
 
 	ti_sci_msg_cmd_query_fw_caps(&info->handle, &info->fw_caps);
-	dev_dbg(dev, "Detected firmware capabilities: %s%s%s%s%s\n",
+	dev_dbg(dev, "Detected firmware capabilities: %s%s%s%s%s%s\n",
 		info->fw_caps & MSG_FLAG_CAPS_GENERIC ? "Generic" : "",
 		info->fw_caps & MSG_FLAG_CAPS_LPM_PARTIAL_IO ? " Partial-IO" : "",
 		info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED ? " DM-Managed" : "",
 		info->fw_caps & MSG_FLAG_CAPS_LPM_ABORT ? " LPM-Abort" : "",
-		info->fw_caps & MSG_FLAG_CAPS_IO_ISOLATION ? " IO-Isolation" : ""
+		info->fw_caps & MSG_FLAG_CAPS_IO_ISOLATION ? " IO-Isolation" : "",
+		info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED ? " BoardConfig-Managed" : ""
 	);
 
 	ti_sci_setup_ops(info);
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index 91f234550c438..5f5c1ae7a19a8 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -150,6 +150,7 @@ struct ti_sci_msg_req_reboot {
  *		MSG_FLAG_CAPS_LPM_DM_MANAGED: LPM can be managed by DM
  *		MSG_FLAG_CAPS_LPM_ABORT: Abort entry to LPM
  *		MSG_FLAG_CAPS_IO_ISOLATION: IO Isolation support
+ *		MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED: LPM config done statically for the DM via boardcfg
  *
  * Response to a generic message with message type TI_SCI_MSG_QUERY_FW_CAPS
  * providing currently available SOC/firmware capabilities. SoC that don't
@@ -162,6 +163,7 @@ struct ti_sci_msg_resp_query_fw_caps {
 #define MSG_FLAG_CAPS_LPM_DM_MANAGED	TI_SCI_MSG_FLAG(5)
 #define MSG_FLAG_CAPS_LPM_ABORT		TI_SCI_MSG_FLAG(9)
 #define MSG_FLAG_CAPS_IO_ISOLATION	TI_SCI_MSG_FLAG(7)
+#define MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED	TI_SCI_MSG_FLAG(10)
 #define MSG_MASK_CAPS_LPM		GENMASK_ULL(4, 1)
 	u64 fw_caps;
 } __packed;

-- 
2.51.0

Re: [PATCH v3 1/4] firmware: ti_sci: add BOARDCFG_MANAGED mode support

[Kumar, Udit]

On 12/5/2025 7:58 PM, Thomas Richard (TI.com) wrote:
> In BOARDCFG_MANAGED mode, the low power mode configuration is done
> statically for the DM via the boardcfg. Constraints are not supported, and
> prepare_sleep() is not needed.
>
> Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
> ---
>   drivers/firmware/ti_sci.c | 10 +++++++---
>   drivers/firmware/ti_sci.h |  2 ++
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 4a277937211f6..d77b631d9c855 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -3772,8 +3772,11 @@ static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
>   			return ti_sci_cmd_prepare_sleep(&info->handle,
>   							TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED,
>   							0, 0, 0);
> +		} else if (info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED) {
> +			/* Nothing to do in the BOARDCFG_MANAGED mode */
> +			return 0;
>   		} else {
> -			/* DM Managed is not supported by the firmware. */
> +			/* DM Managed and BoardCfg Managed are not supported by the firmware. */
>   			dev_err(info->dev, "Suspend to memory is not supported by the firmware\n");
>   			return -EOPNOTSUPP;
>   		}
> @@ -4011,12 +4014,13 @@ static int ti_sci_probe(struct platform_device *pdev)
>   	}
>   
>   	ti_sci_msg_cmd_query_fw_caps(&info->handle, &info->fw_caps);
> -	dev_dbg(dev, "Detected firmware capabilities: %s%s%s%s%s\n",
> +	dev_dbg(dev, "Detected firmware capabilities: %s%s%s%s%s%s\n",
>   		info->fw_caps & MSG_FLAG_CAPS_GENERIC ? "Generic" : "",
>   		info->fw_caps & MSG_FLAG_CAPS_LPM_PARTIAL_IO ? " Partial-IO" : "",
>   		info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED ? " DM-Managed" : "",
>   		info->fw_caps & MSG_FLAG_CAPS_LPM_ABORT ? " LPM-Abort" : "",
> -		info->fw_caps & MSG_FLAG_CAPS_IO_ISOLATION ? " IO-Isolation" : ""
> +		info->fw_caps & MSG_FLAG_CAPS_IO_ISOLATION ? " IO-Isolation" : "",
> +		info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED ? " BoardConfig-Managed" : ""
>   	);
>   
>   	ti_sci_setup_ops(info);
> diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
> index 91f234550c438..5f5c1ae7a19a8 100644
> --- a/drivers/firmware/ti_sci.h
> +++ b/drivers/firmware/ti_sci.h
> @@ -150,6 +150,7 @@ struct ti_sci_msg_req_reboot {
>    *		MSG_FLAG_CAPS_LPM_DM_MANAGED: LPM can be managed by DM
>    *		MSG_FLAG_CAPS_LPM_ABORT: Abort entry to LPM
>    *		MSG_FLAG_CAPS_IO_ISOLATION: IO Isolation support
> + *		MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED: LPM config done statically for the DM via boardcfg
>    *
>    * Response to a generic message with message type TI_SCI_MSG_QUERY_FW_CAPS
>    * providing currently available SOC/firmware capabilities. SoC that don't
> @@ -162,6 +163,7 @@ struct ti_sci_msg_resp_query_fw_caps {
>   #define MSG_FLAG_CAPS_LPM_DM_MANAGED	TI_SCI_MSG_FLAG(5)
>   #define MSG_FLAG_CAPS_LPM_ABORT		TI_SCI_MSG_FLAG(9)
>   #define MSG_FLAG_CAPS_IO_ISOLATION	TI_SCI_MSG_FLAG(7)
> +#define MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED	TI_SCI_MSG_FLAG(10)


I think, you already noticed, this should be bit-12


>   #define MSG_MASK_CAPS_LPM		GENMASK_ULL(4, 1)
>   	u64 fw_caps;
>   } __packed;
>

[PATCH v3 2/4] firmware: ti_sci: handle IRQ restore in BOARDCFG_MANAGED mode during resume

[Thomas Richard (TI.com)]

In BOARDCFG_MANAGED mode, the firmware cannot restore IRQs during
resume. This responsibility is delegated to the ti_sci driver,
which maintains an internal list of all requested IRQs. This list
is updated on each set/free operation, and all IRQs are restored
during the resume_noirq() phase.

Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
---
 drivers/firmware/ti_sci.c | 147 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 138 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index d77b631d9c855..8d94745376e2a 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -12,6 +12,7 @@
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
 #include <linux/export.h>
+#include <linux/hashtable.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -87,6 +88,16 @@ struct ti_sci_desc {
 	int max_msg_size;
 };
 
+/**
+ * struct ti_sci_irq - Description of allocated irqs
+ * @node: Link to hash table
+ * @desc: Description of the irq
+ */
+struct ti_sci_irq {
+	struct hlist_node node;
+	struct ti_sci_msg_req_manage_irq desc;
+};
+
 /**
  * struct ti_sci_info - Structure representing a TI SCI instance
  * @dev:	Device pointer
@@ -101,6 +112,7 @@ struct ti_sci_desc {
  * @chan_rx:	Receive mailbox channel
  * @minfo:	Message info
  * @node:	list head
+ * @irqs:	List of allocated irqs
  * @host_id:	Host ID
  * @fw_caps:	FW/SoC low power capabilities
  * @users:	Number of users of this instance
@@ -117,6 +129,7 @@ struct ti_sci_info {
 	struct mbox_chan *chan_tx;
 	struct mbox_chan *chan_rx;
 	struct ti_sci_xfers_info minfo;
+	DECLARE_HASHTABLE(irqs, 8);
 	struct list_head node;
 	u8 host_id;
 	u64 fw_caps;
@@ -2301,6 +2314,32 @@ static int ti_sci_manage_irq(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_irq_hash() - Helper API to compute irq hash for the hash table.
+ * @irq:	irq to hash
+ *
+ * Return: the computed hash value.
+ */
+static int ti_sci_irq_hash(struct ti_sci_msg_req_manage_irq *irq)
+{
+	return irq->src_id ^ irq->src_index;
+}
+
+/**
+ * ti_sci_irq_equal() - Helper API to compare two irqs (generic headers are not
+ *                       compared)
+ * @irq_a:	irq_a to compare
+ * @irq_b:	irq_b to compare
+ *
+ * Return: true if the two irqs are equal, else false.
+ */
+static bool ti_sci_irq_equal(struct ti_sci_msg_req_manage_irq *irq_a,
+			     struct ti_sci_msg_req_manage_irq *irq_b)
+{
+	return !memcmp(&irq_a->valid_params, &irq_b->valid_params,
+		       sizeof(*irq_a) - sizeof(irq_a->hdr));
+}
+
 /**
  * ti_sci_set_irq() - Helper api to configure the irq route between the
  *		      requested source and destination
@@ -2324,15 +2363,43 @@ static int ti_sci_set_irq(const struct ti_sci_handle *handle, u32 valid_params,
 			  u16 dst_host_irq, u16 ia_id, u16 vint,
 			  u16 global_event, u8 vint_status_bit, u8 s_host)
 {
+	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
+	struct ti_sci_msg_req_manage_irq *desc;
+	struct ti_sci_irq *irq;
+	int ret;
+
 	pr_debug("%s: IRQ set with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
 		 __func__, valid_params, src_id, src_index,
 		 dst_id, dst_host_irq, ia_id, vint, global_event,
 		 vint_status_bit);
 
-	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
-				 dst_id, dst_host_irq, ia_id, vint,
-				 global_event, vint_status_bit, s_host,
-				 TI_SCI_MSG_SET_IRQ);
+	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
+				dst_id, dst_host_irq, ia_id, vint,
+				global_event, vint_status_bit, s_host,
+				TI_SCI_MSG_SET_IRQ);
+
+	if (ret)
+		return ret;
+
+	irq = kzalloc(sizeof(*irq), GFP_KERNEL);
+	if (!irq)
+		return -ENOMEM;
+
+	desc = &irq->desc;
+	desc->valid_params = valid_params;
+	desc->src_id = src_id;
+	desc->src_index = src_index;
+	desc->dst_id = dst_id;
+	desc->dst_host_irq = dst_host_irq;
+	desc->ia_id = ia_id;
+	desc->vint = vint;
+	desc->global_event = global_event;
+	desc->vint_status_bit = vint_status_bit;
+	desc->secondary_host = s_host;
+
+	hash_add(info->irqs, &irq->node, ti_sci_irq_hash(desc));
+
+	return 0;
 }
 
 /**
@@ -2358,15 +2425,46 @@ static int ti_sci_free_irq(const struct ti_sci_handle *handle, u32 valid_params,
 			   u16 dst_host_irq, u16 ia_id, u16 vint,
 			   u16 global_event, u8 vint_status_bit, u8 s_host)
 {
+	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
+	struct ti_sci_msg_req_manage_irq irq_desc;
+	struct ti_sci_irq *this_irq;
+	struct hlist_node *tmp_node;
+	int ret;
+
 	pr_debug("%s: IRQ release with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
 		 __func__, valid_params, src_id, src_index,
 		 dst_id, dst_host_irq, ia_id, vint, global_event,
 		 vint_status_bit);
 
-	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
-				 dst_id, dst_host_irq, ia_id, vint,
-				 global_event, vint_status_bit, s_host,
-				 TI_SCI_MSG_FREE_IRQ);
+	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
+				dst_id, dst_host_irq, ia_id, vint,
+				global_event, vint_status_bit, s_host,
+				TI_SCI_MSG_FREE_IRQ);
+
+	if (ret)
+		return ret;
+
+	irq_desc.valid_params = valid_params;
+	irq_desc.src_id = src_id;
+	irq_desc.src_index = src_index;
+	irq_desc.dst_id = dst_id;
+	irq_desc.dst_host_irq = dst_host_irq;
+	irq_desc.ia_id = ia_id;
+	irq_desc.vint = vint;
+	irq_desc.global_event = global_event;
+	irq_desc.vint_status_bit = vint_status_bit;
+	irq_desc.secondary_host = s_host;
+
+	hash_for_each_possible_safe(info->irqs, this_irq, tmp_node, node,
+				    ti_sci_irq_hash(&irq_desc)) {
+		if (ti_sci_irq_equal(&irq_desc, &this_irq->desc)) {
+			hlist_del(&this_irq->node);
+			kfree(this_irq);
+			return 0;
+		}
+	}
+
+	return 0;
 }
 
 /**
@@ -3847,7 +3945,10 @@ static int ti_sci_suspend_noirq(struct device *dev)
 static int ti_sci_resume_noirq(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
-	int ret = 0;
+	struct ti_sci_msg_req_manage_irq *irq_desc;
+	struct ti_sci_irq *irq;
+	struct hlist_node *tmp_node;
+	int ret = 0, i;
 	u32 source;
 	u64 time;
 	u8 pin;
@@ -3859,6 +3960,32 @@ static int ti_sci_resume_noirq(struct device *dev)
 			return ret;
 	}
 
+	switch (pm_suspend_target_state) {
+	case PM_SUSPEND_MEM:
+		if (info->fw_caps & MSG_FLAG_CAPS_LPM_BOARDCFG_MANAGED) {
+			hash_for_each_safe(info->irqs, i, tmp_node, irq, node) {
+				irq_desc = &irq->desc;
+				ret = ti_sci_manage_irq(&info->handle,
+							irq_desc->valid_params,
+							irq_desc->src_id,
+							irq_desc->src_index,
+							irq_desc->dst_id,
+							irq_desc->dst_host_irq,
+							irq_desc->ia_id,
+							irq_desc->vint,
+							irq_desc->global_event,
+							irq_desc->vint_status_bit,
+							irq_desc->secondary_host,
+							TI_SCI_MSG_SET_IRQ);
+				if (ret)
+					return ret;
+			}
+		}
+		break;
+	default:
+		break;
+	}
+
 	ret = ti_sci_msg_cmd_lpm_wake_reason(&info->handle, &source, &time, &pin, &mode);
 	/* Do not fail to resume on error as the wake reason is not critical */
 	if (!ret)
@@ -4053,6 +4180,8 @@ static int ti_sci_probe(struct platform_device *pdev)
 	list_add_tail(&info->node, &ti_sci_list);
 	mutex_unlock(&ti_sci_list_mutex);
 
+	hash_init(info->irqs);
+
 	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
 	if (ret) {
 		dev_err(dev, "platform_populate failed %pe\n", ERR_PTR(ret));

-- 
2.51.0

Re: [PATCH v3 2/4] firmware: ti_sci: handle IRQ restore in BOARDCFG_MANAGED mode during resume

[Kumar, Udit]

On 12/5/2025 7:58 PM, Thomas Richard (TI.com) wrote:
> In BOARDCFG_MANAGED mode, the firmware cannot restore IRQs during
> resume. This responsibility is delegated to the ti_sci driver,
> which maintains an internal list of all requested IRQs. This list
> is updated on each set/free operation, and all IRQs are restored
> during the resume_noirq() phase.
>
> Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
> ---
>   drivers/firmware/ti_sci.c | 147 +++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 138 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index d77b631d9c855..8d94745376e2a 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -12,6 +12,7 @@
>   #include <linux/cpu.h>
>   #include <linux/debugfs.h>
>   #include <linux/export.h>
> +#include <linux/hashtable.h>
>   #include <linux/io.h>
>   #include <linux/iopoll.h>
>   #include <linux/kernel.h>
> @@ -87,6 +88,16 @@ struct ti_sci_desc {
>   	int max_msg_size;
>   };
>   
> +/**
> + * struct ti_sci_irq - Description of allocated irqs
> + * @node: Link to hash table
> + * @desc: Description of the irq
> + */
> +struct ti_sci_irq {
> +	struct hlist_node node;
> +	struct ti_sci_msg_req_manage_irq desc;
> +};
> +
>   /**
>    * struct ti_sci_info - Structure representing a TI SCI instance
>    * @dev:	Device pointer
> @@ -101,6 +112,7 @@ struct ti_sci_desc {
>    * @chan_rx:	Receive mailbox channel
>    * @minfo:	Message info
>    * @node:	list head
> + * @irqs:	List of allocated irqs
>    * @host_id:	Host ID
>    * @fw_caps:	FW/SoC low power capabilities
>    * @users:	Number of users of this instance
> @@ -117,6 +129,7 @@ struct ti_sci_info {
>   	struct mbox_chan *chan_tx;
>   	struct mbox_chan *chan_rx;
>   	struct ti_sci_xfers_info minfo;
> +	DECLARE_HASHTABLE(irqs, 8);
>   	struct list_head node;
>   	u8 host_id;
>   	u64 fw_caps;
> @@ -2301,6 +2314,32 @@ static int ti_sci_manage_irq(const struct ti_sci_handle *handle,
>   	return ret;
>   }
>   
> +/**
> + * ti_sci_irq_hash() - Helper API to compute irq hash for the hash table.
> + * @irq:	irq to hash
> + *
> + * Return: the computed hash value.
> + */
> +static int ti_sci_irq_hash(struct ti_sci_msg_req_manage_irq *irq)
> +{
> +	return irq->src_id ^ irq->src_index;
> +}
> +
> +/**
> + * ti_sci_irq_equal() - Helper API to compare two irqs (generic headers are not
> + *                       compared)
> + * @irq_a:	irq_a to compare
> + * @irq_b:	irq_b to compare
> + *
> + * Return: true if the two irqs are equal, else false.
> + */
> +static bool ti_sci_irq_equal(struct ti_sci_msg_req_manage_irq *irq_a,
> +			     struct ti_sci_msg_req_manage_irq *irq_b)
> +{
> +	return !memcmp(&irq_a->valid_params, &irq_b->valid_params,
> +		       sizeof(*irq_a) - sizeof(irq_a->hdr));
> +}
> +
>   /**
>    * ti_sci_set_irq() - Helper api to configure the irq route between the
>    *		      requested source and destination
> @@ -2324,15 +2363,43 @@ static int ti_sci_set_irq(const struct ti_sci_handle *handle, u32 valid_params,
>   			  u16 dst_host_irq, u16 ia_id, u16 vint,
>   			  u16 global_event, u8 vint_status_bit, u8 s_host)
>   {
> +	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
> +	struct ti_sci_msg_req_manage_irq *desc;
> +	struct ti_sci_irq *irq;
> +	int ret;
> +
>   	pr_debug("%s: IRQ set with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
>   		 __func__, valid_params, src_id, src_index,
>   		 dst_id, dst_host_irq, ia_id, vint, global_event,
>   		 vint_status_bit);
>   
> -	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> -				 dst_id, dst_host_irq, ia_id, vint,
> -				 global_event, vint_status_bit, s_host,
> -				 TI_SCI_MSG_SET_IRQ);
> +	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> +				dst_id, dst_host_irq, ia_id, vint,
> +				global_event, vint_status_bit, s_host,
> +				TI_SCI_MSG_SET_IRQ);
> +
> +	if (ret)
> +		return ret;
> +
> +	irq = kzalloc(sizeof(*irq), GFP_KERNEL);
> +	if (!irq)
> +		return -ENOMEM;
> +
> +	desc = &irq->desc;
> +	desc->valid_params = valid_params;
> +	desc->src_id = src_id;
> +	desc->src_index = src_index;
> +	desc->dst_id = dst_id;
> +	desc->dst_host_irq = dst_host_irq;
> +	desc->ia_id = ia_id;
> +	desc->vint = vint;
> +	desc->global_event = global_event;
> +	desc->vint_status_bit = vint_status_bit;
> +	desc->secondary_host = s_host;
> +
> +	hash_add(info->irqs, &irq->node, ti_sci_irq_hash(desc));
> +
> +	return 0;
>   }
>   
>   /**
> @@ -2358,15 +2425,46 @@ static int ti_sci_free_irq(const struct ti_sci_handle *handle, u32 valid_params,
>   			   u16 dst_host_irq, u16 ia_id, u16 vint,
>   			   u16 global_event, u8 vint_status_bit, u8 s_host)
>   {
> +	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
> +	struct ti_sci_msg_req_manage_irq irq_desc;
> +	struct ti_sci_irq *this_irq;
> +	struct hlist_node *tmp_node;
> +	int ret;
> +
>   	pr_debug("%s: IRQ release with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
>   		 __func__, valid_params, src_id, src_index,
>   		 dst_id, dst_host_irq, ia_id, vint, global_event,
>   		 vint_status_bit);
>   
> -	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> -				 dst_id, dst_host_irq, ia_id, vint,
> -				 global_event, vint_status_bit, s_host,
> -				 TI_SCI_MSG_FREE_IRQ);
> +	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> +				dst_id, dst_host_irq, ia_id, vint,
> +				global_event, vint_status_bit, s_host,
> +				TI_SCI_MSG_FREE_IRQ);
> +
> +	if (ret)
> +		return ret;
> +
> +	irq_desc.valid_params = valid_params;
> +	irq_desc.src_id = src_id;
> +	irq_desc.src_index = src_index;
> +	irq_desc.dst_id = dst_id;
> +	irq_desc.dst_host_irq = dst_host_irq;
> +	irq_desc.ia_id = ia_id;
> +	irq_desc.vint = vint;
> +	irq_desc.global_event = global_event;
> +	irq_desc.vint_status_bit = vint_status_bit;
> +	irq_desc.secondary_host = s_host;
> +
> +	hash_for_each_possible_safe(info->irqs, this_irq, tmp_node, node,
> +				    ti_sci_irq_hash(&irq_desc)) {
> +		if (ti_sci_irq_equal(&irq_desc, &this_irq->desc)) {
> +			hlist_del(&this_irq->node);
> +			kfree(this_irq);
> +			return 0;


IMO,  you can restrict saving of irq and list management to fw having

BOARDCFG_MANAGED capability.

Dhurva ?


> +		}
> +	}
> +
> +	return 0;
>   }
>   
>   /**
> [..]

Re: [PATCH v3 2/4] firmware: ti_sci: handle IRQ restore in BOARDCFG_MANAGED mode during resume

[Dhruva Gole]

On Dec 16, 2025 at 08:17:24 +0530, Kumar, Udit wrote:
> 
> On 12/5/2025 7:58 PM, Thomas Richard (TI.com) wrote:
> > In BOARDCFG_MANAGED mode, the firmware cannot restore IRQs during
> > resume. This responsibility is delegated to the ti_sci driver,
> > which maintains an internal list of all requested IRQs. This list
> > is updated on each set/free operation, and all IRQs are restored
> > during the resume_noirq() phase.
> > 
> > Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
> > ---
> >   drivers/firmware/ti_sci.c | 147 +++++++++++++++++++++++++++++++++++++++++++---
> >   1 file changed, 138 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> > index d77b631d9c855..8d94745376e2a 100644
> > --- a/drivers/firmware/ti_sci.c
> > +++ b/drivers/firmware/ti_sci.c
> > @@ -12,6 +12,7 @@
> >   #include <linux/cpu.h>
> >   #include <linux/debugfs.h>
> >   #include <linux/export.h>
> > +#include <linux/hashtable.h>
> >   #include <linux/io.h>
> >   #include <linux/iopoll.h>
> >   #include <linux/kernel.h>
> > @@ -87,6 +88,16 @@ struct ti_sci_desc {
> >   	int max_msg_size;
> >   };
> > +/**
> > + * struct ti_sci_irq - Description of allocated irqs
> > + * @node: Link to hash table
> > + * @desc: Description of the irq
> > + */
> > +struct ti_sci_irq {
> > +	struct hlist_node node;
> > +	struct ti_sci_msg_req_manage_irq desc;
> > +};
> > +
> >   /**
> >    * struct ti_sci_info - Structure representing a TI SCI instance
> >    * @dev:	Device pointer
> > @@ -101,6 +112,7 @@ struct ti_sci_desc {
> >    * @chan_rx:	Receive mailbox channel
> >    * @minfo:	Message info
> >    * @node:	list head
> > + * @irqs:	List of allocated irqs
> >    * @host_id:	Host ID
> >    * @fw_caps:	FW/SoC low power capabilities
> >    * @users:	Number of users of this instance
> > @@ -117,6 +129,7 @@ struct ti_sci_info {
> >   	struct mbox_chan *chan_tx;
> >   	struct mbox_chan *chan_rx;
> >   	struct ti_sci_xfers_info minfo;
> > +	DECLARE_HASHTABLE(irqs, 8);
> >   	struct list_head node;
> >   	u8 host_id;
> >   	u64 fw_caps;
> > @@ -2301,6 +2314,32 @@ static int ti_sci_manage_irq(const struct ti_sci_handle *handle,
> >   	return ret;
> >   }
> > +/**
> > + * ti_sci_irq_hash() - Helper API to compute irq hash for the hash table.
> > + * @irq:	irq to hash
> > + *
> > + * Return: the computed hash value.
> > + */
> > +static int ti_sci_irq_hash(struct ti_sci_msg_req_manage_irq *irq)
> > +{
> > +	return irq->src_id ^ irq->src_index;
> > +}
> > +
> > +/**
> > + * ti_sci_irq_equal() - Helper API to compare two irqs (generic headers are not
> > + *                       compared)
> > + * @irq_a:	irq_a to compare
> > + * @irq_b:	irq_b to compare
> > + *
> > + * Return: true if the two irqs are equal, else false.
> > + */
> > +static bool ti_sci_irq_equal(struct ti_sci_msg_req_manage_irq *irq_a,
> > +			     struct ti_sci_msg_req_manage_irq *irq_b)
> > +{
> > +	return !memcmp(&irq_a->valid_params, &irq_b->valid_params,
> > +		       sizeof(*irq_a) - sizeof(irq_a->hdr));
> > +}
> > +
> >   /**
> >    * ti_sci_set_irq() - Helper api to configure the irq route between the
> >    *		      requested source and destination
> > @@ -2324,15 +2363,43 @@ static int ti_sci_set_irq(const struct ti_sci_handle *handle, u32 valid_params,
> >   			  u16 dst_host_irq, u16 ia_id, u16 vint,
> >   			  u16 global_event, u8 vint_status_bit, u8 s_host)
> >   {
> > +	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
> > +	struct ti_sci_msg_req_manage_irq *desc;
> > +	struct ti_sci_irq *irq;
> > +	int ret;
> > +
> >   	pr_debug("%s: IRQ set with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
> >   		 __func__, valid_params, src_id, src_index,
> >   		 dst_id, dst_host_irq, ia_id, vint, global_event,
> >   		 vint_status_bit);
> > -	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> > -				 dst_id, dst_host_irq, ia_id, vint,
> > -				 global_event, vint_status_bit, s_host,
> > -				 TI_SCI_MSG_SET_IRQ);
> > +	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> > +				dst_id, dst_host_irq, ia_id, vint,
> > +				global_event, vint_status_bit, s_host,
> > +				TI_SCI_MSG_SET_IRQ);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	irq = kzalloc(sizeof(*irq), GFP_KERNEL);
> > +	if (!irq)
> > +		return -ENOMEM;
> > +
> > +	desc = &irq->desc;
> > +	desc->valid_params = valid_params;
> > +	desc->src_id = src_id;
> > +	desc->src_index = src_index;
> > +	desc->dst_id = dst_id;
> > +	desc->dst_host_irq = dst_host_irq;
> > +	desc->ia_id = ia_id;
> > +	desc->vint = vint;
> > +	desc->global_event = global_event;
> > +	desc->vint_status_bit = vint_status_bit;
> > +	desc->secondary_host = s_host;
> > +
> > +	hash_add(info->irqs, &irq->node, ti_sci_irq_hash(desc));
> > +
> > +	return 0;
> >   }
> >   /**
> > @@ -2358,15 +2425,46 @@ static int ti_sci_free_irq(const struct ti_sci_handle *handle, u32 valid_params,
> >   			   u16 dst_host_irq, u16 ia_id, u16 vint,
> >   			   u16 global_event, u8 vint_status_bit, u8 s_host)
> >   {
> > +	struct ti_sci_info *info = handle_to_ti_sci_info(handle);
> > +	struct ti_sci_msg_req_manage_irq irq_desc;
> > +	struct ti_sci_irq *this_irq;
> > +	struct hlist_node *tmp_node;
> > +	int ret;
> > +
> >   	pr_debug("%s: IRQ release with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
> >   		 __func__, valid_params, src_id, src_index,
> >   		 dst_id, dst_host_irq, ia_id, vint, global_event,
> >   		 vint_status_bit);
> > -	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> > -				 dst_id, dst_host_irq, ia_id, vint,
> > -				 global_event, vint_status_bit, s_host,
> > -				 TI_SCI_MSG_FREE_IRQ);
> > +	ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
> > +				dst_id, dst_host_irq, ia_id, vint,
> > +				global_event, vint_status_bit, s_host,
> > +				TI_SCI_MSG_FREE_IRQ);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	irq_desc.valid_params = valid_params;
> > +	irq_desc.src_id = src_id;
> > +	irq_desc.src_index = src_index;
> > +	irq_desc.dst_id = dst_id;
> > +	irq_desc.dst_host_irq = dst_host_irq;
> > +	irq_desc.ia_id = ia_id;
> > +	irq_desc.vint = vint;
> > +	irq_desc.global_event = global_event;
> > +	irq_desc.vint_status_bit = vint_status_bit;
> > +	irq_desc.secondary_host = s_host;
> > +
> > +	hash_for_each_possible_safe(info->irqs, this_irq, tmp_node, node,
> > +				    ti_sci_irq_hash(&irq_desc)) {
> > +		if (ti_sci_irq_equal(&irq_desc, &this_irq->desc)) {
> > +			hlist_del(&this_irq->node);
> > +			kfree(this_irq);
> > +			return 0;
> 
> 
> IMO,  you can restrict saving of irq and list management to fw having
> 
> BOARDCFG_MANAGED capability.
> 
> Dhurva ?

Yes I agree with Udit, we should gate hash operations by firmware capability.
Everywhere else you'll need to make it conditional accordingly.

Also, how much is the IRQ count usually? If IRQ count is typically small (< 50),
then won't a simple linked list be more efficient than a hash table? The
code becomes a bit more readable too that way IMO.
Take a call based on if there's really that many IRQs that LL's become
less practical.

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

Re: [PATCH v3 2/4] firmware: ti_sci: handle IRQ restore in BOARDCFG_MANAGED mode during resume

[Thomas Richard]

On 12/17/25 6:29 AM, Dhruva Gole wrote:
> On Dec 16, 2025 at 08:17:24 +0530, Kumar, Udit wrote:
>>
>> On 12/5/2025 7:58 PM, Thomas Richard (TI.com) wrote:
>>> In BOARDCFG_MANAGED mode, the firmware cannot restore IRQs during
>>> resume. This responsibility is delegated to the ti_sci driver,
>>> which maintains an internal list of all requested IRQs. This list
>>> is updated on each set/free operation, and all IRQs are restored
>>> during the resume_noirq() phase.
>>>
>>> Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>

[...]

>>> +
>>> +	hash_for_each_possible_safe(info->irqs, this_irq, tmp_node, node,
>>> +				    ti_sci_irq_hash(&irq_desc)) {
>>> +		if (ti_sci_irq_equal(&irq_desc, &this_irq->desc)) {
>>> +			hlist_del(&this_irq->node);
>>> +			kfree(this_irq);
>>> +			return 0;
>>
>>
>> IMO,  you can restrict saving of irq and list management to fw having
>>
>> BOARDCFG_MANAGED capability.
>>
>> Dhurva ?
> 
> Yes I agree with Udit, we should gate hash operations by firmware capability.
> Everywhere else you'll need to make it conditional accordingly.

ack

> 
> Also, how much is the IRQ count usually? If IRQ count is typically small (< 50),
> then won't a simple linked list be more efficient than a hash table? The
> code becomes a bit more readable too that way IMO.
> Take a call based on if there's really that many IRQs that LL's become
> less practical.
> 

I tested a TI kernel on J721S2 and I got 60 entries. I guess we can
expect this number to grow with the next SOCs. But maybe the test does
not reflect the usual cases.

Best Regards,
Thomas

[PATCH v3 3/4] clk: keystone: sci-clk: add restore_context() operation

[Thomas Richard (TI.com)]

Implement the restore_context() operation to restore the clock rate and the
clock parent state. The clock rate is saved in sci_clk struct during
set_rate() operation. The parent index is saved in sci_clk struct during
set_parent() operation. During clock registration, the core retrieves each
clock’s parent using get_parent() operation to ensure the internal clock
tree reflects the actual hardware state, including any configurations made
by the bootloader. So we also save the parent index in get_parent().

Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
---
 drivers/clk/keystone/sci-clk.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index 9d5071223f4cb..428050a05de31 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -47,6 +47,8 @@ struct sci_clk_provider {
  * @node:	 Link for handling clocks probed via DT
  * @cached_req:	 Cached requested freq for determine rate calls
  * @cached_res:	 Cached result freq for determine rate calls
+ * @parent_id:	 Parent index for this clock
+ * @rate:	 Clock rate
  */
 struct sci_clk {
 	struct clk_hw hw;
@@ -58,6 +60,8 @@ struct sci_clk {
 	struct list_head node;
 	unsigned long cached_req;
 	unsigned long cached_res;
+	u8 parent_id;
+	unsigned long rate;
 };
 
 #define to_sci_clk(_hw) container_of(_hw, struct sci_clk, hw)
@@ -210,10 +214,16 @@ static int sci_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 			    unsigned long parent_rate)
 {
 	struct sci_clk *clk = to_sci_clk(hw);
+	int ret;
+
+	ret = clk->provider->ops->set_freq(clk->provider->sci, clk->dev_id,
+					   clk->clk_id, rate / 10 * 9, rate,
+					   rate / 10 * 11);
 
-	return clk->provider->ops->set_freq(clk->provider->sci, clk->dev_id,
-					    clk->clk_id, rate / 10 * 9, rate,
-					    rate / 10 * 11);
+	if (!ret)
+		clk->rate = rate;
+
+	return ret;
 }
 
 /**
@@ -237,9 +247,9 @@ static u8 sci_clk_get_parent(struct clk_hw *hw)
 		return 0;
 	}
 
-	parent_id = parent_id - clk->clk_id - 1;
+	clk->parent_id = (u8)(parent_id - clk->clk_id - 1);
 
-	return (u8)parent_id;
+	return clk->parent_id;
 }
 
 /**
@@ -252,12 +262,27 @@ static u8 sci_clk_get_parent(struct clk_hw *hw)
 static int sci_clk_set_parent(struct clk_hw *hw, u8 index)
 {
 	struct sci_clk *clk = to_sci_clk(hw);
+	int ret;
 
 	clk->cached_req = 0;
 
-	return clk->provider->ops->set_parent(clk->provider->sci, clk->dev_id,
-					      clk->clk_id,
-					      index + 1 + clk->clk_id);
+	ret = clk->provider->ops->set_parent(clk->provider->sci, clk->dev_id,
+					     clk->clk_id,
+					     index + 1 + clk->clk_id);
+	if (!ret)
+		clk->parent_id = index;
+
+	return ret;
+}
+
+static void sci_clk_restore_context(struct clk_hw *hw)
+{
+	struct sci_clk *clk = to_sci_clk(hw);
+
+	sci_clk_set_parent(hw, clk->parent_id);
+
+	if (clk->rate)
+		sci_clk_set_rate(hw, clk->rate, 0);
 }
 
 static const struct clk_ops sci_clk_ops = {
@@ -269,6 +294,7 @@ static const struct clk_ops sci_clk_ops = {
 	.set_rate = sci_clk_set_rate,
 	.get_parent = sci_clk_get_parent,
 	.set_parent = sci_clk_set_parent,
+	.restore_context = sci_clk_restore_context,
 };
 
 /**

-- 
2.51.0

Re: [PATCH v3 3/4] clk: keystone: sci-clk: add restore_context() operation

[Kumar, Udit]

On 12/5/2025 7:58 PM, Thomas Richard (TI.com) wrote:
> Implement the restore_context() operation to restore the clock rate and the
> clock parent state. The clock rate is saved in sci_clk struct during
> set_rate() operation. The parent index is saved in sci_clk struct during
> set_parent() operation. During clock registration, the core retrieves each
> clock’s parent using get_parent() operation to ensure the internal clock
> tree reflects the actual hardware state, including any configurations made
> by the bootloader. So we also save the parent index in get_parent().
>
> Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
> ---
>   drivers/clk/keystone/sci-clk.c | 42 ++++++++++++++++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
> index 9d5071223f4cb..428050a05de31 100644
> --- a/drivers/clk/keystone/sci-clk.c
> +++ b/drivers/clk/keystone/sci-clk.c
> @@ -47,6 +47,8 @@ struct sci_clk_provider {
>    * @node:	 Link for handling clocks probed via DT
>    * @cached_req:	 Cached requested freq for determine rate calls
>    * @cached_res:	 Cached result freq for determine rate calls
> + * @parent_id:	 Parent index for this clock
> + * @rate:	 Clock rate
>    */
>   struct sci_clk {
>   	struct clk_hw hw;
> @@ -58,6 +60,8 @@ struct sci_clk {
>   	struct list_head node;
>   	unsigned long cached_req;
>   	unsigned long cached_res;
> +	u8 parent_id;
> +	unsigned long rate;
>   };
>   
>   #define to_sci_clk(_hw) container_of(_hw, struct sci_clk, hw)
> @@ -210,10 +214,16 @@ static int sci_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>   			    unsigned long parent_rate)
>   {
>   	struct sci_clk *clk = to_sci_clk(hw);
> +	int ret;
> +
> +	ret = clk->provider->ops->set_freq(clk->provider->sci, clk->dev_id,
> +					   clk->clk_id, rate / 10 * 9, rate,
> +					   rate / 10 * 11);
>   
> -	return clk->provider->ops->set_freq(clk->provider->sci, clk->dev_id,
> -					    clk->clk_id, rate / 10 * 9, rate,
> -					    rate / 10 * 11);
> +	if (!ret)
> +		clk->rate = rate;
> +
> +	return ret;
>   }
>   
>   /**
> @@ -237,9 +247,9 @@ static u8 sci_clk_get_parent(struct clk_hw *hw)
>   		return 0;
>   	}
>   
> -	parent_id = parent_id - clk->clk_id - 1;
> +	clk->parent_id = (u8)(parent_id - clk->clk_id - 1);
>   
> -	return (u8)parent_id;
> +	return clk->parent_id;
>   }
>   
>   /**
> @@ -252,12 +262,27 @@ static u8 sci_clk_get_parent(struct clk_hw *hw)
>   static int sci_clk_set_parent(struct clk_hw *hw, u8 index)
>   {
>   	struct sci_clk *clk = to_sci_clk(hw);
> +	int ret;
>   
>   	clk->cached_req = 0;
>   
> -	return clk->provider->ops->set_parent(clk->provider->sci, clk->dev_id,
> -					      clk->clk_id,
> -					      index + 1 + clk->clk_id);
> +	ret = clk->provider->ops->set_parent(clk->provider->sci, clk->dev_id,
> +					     clk->clk_id,
> +					     index + 1 + clk->clk_id);
> +	if (!ret)
> +		clk->parent_id = index;
> +
> +	return ret;
> +}
> +
> +static void sci_clk_restore_context(struct clk_hw *hw)
> +{
> +	struct sci_clk *clk = to_sci_clk(hw);
> +
> +	sci_clk_set_parent(hw, clk->parent_id);
> +
> +	if (clk->rate)
> +		sci_clk_set_rate(hw, clk->rate, 0);


Mostly looks ok ,  Please add some warning prints in case of resume path 
either of

sci_clk_set_parent/sci_clk_set_rate returned failure


>   }
>   
>   static const struct clk_ops sci_clk_ops = {
> @@ -269,6 +294,7 @@ static const struct clk_ops sci_clk_ops = {
>   	.set_rate = sci_clk_set_rate,
>   	.get_parent = sci_clk_get_parent,
>   	.set_parent = sci_clk_set_parent,
> +	.restore_context = sci_clk_restore_context,
>   };
>   
>   /**
>

[PATCH v3 4/4] firmware: ti_sci: restore clock context during resume in BOARDCFG_MANAGED mode

[Thomas Richard (TI.com)]

In BOARDCFG_MANAGED mode, the firmware cannot restore the clock rates and
the clock parents. This responsibility is therefore delegated to the ti_sci
driver, which uses clk_restore_context() to trigger the context_restore()
operation for all registered clocks, including those managed by the sci-clk
driver. The sci-clk driver implements the context_restore() operation to
ensure rates and clock parents are correctly restored.

Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
---
 drivers/firmware/ti_sci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 8d94745376e2a..6ef687e481c49 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -9,6 +9,7 @@
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
 #include <linux/bitmap.h>
+#include <linux/clk.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
 #include <linux/export.h>
@@ -3980,6 +3981,8 @@ static int ti_sci_resume_noirq(struct device *dev)
 				if (ret)
 					return ret;
 			}
+
+			clk_restore_context();
 		}
 		break;
 	default:

-- 
2.51.0

Re: [PATCH v3 4/4] firmware: ti_sci: restore clock context during resume in BOARDCFG_MANAGED mode

[Dhruva Gole]

On Dec 05, 2025 at 15:28:26 +0100, Thomas Richard (TI.com) wrote:
> In BOARDCFG_MANAGED mode, the firmware cannot restore the clock rates and
> the clock parents. This responsibility is therefore delegated to the ti_sci
> driver, which uses clk_restore_context() to trigger the context_restore()
> operation for all registered clocks, including those managed by the sci-clk
> driver. The sci-clk driver implements the context_restore() operation to
> ensure rates and clock parents are correctly restored.
> 
> Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
> ---
>  drivers/firmware/ti_sci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 8d94745376e2a..6ef687e481c49 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -9,6 +9,7 @@
>  #define pr_fmt(fmt) "%s: " fmt, __func__
>  
>  #include <linux/bitmap.h>
> +#include <linux/clk.h>
>  #include <linux/cpu.h>
>  #include <linux/debugfs.h>
>  #include <linux/export.h>
> @@ -3980,6 +3981,8 @@ static int ti_sci_resume_noirq(struct device *dev)
>  				if (ret)
>  					return ret;
>  			}
> +
> +			clk_restore_context();

Here as well, make it conditional to only BOARDCFG_MANAGED. Other
platforms/ firmwares have lived without this for a while now, and it's
evident that we don't always need this.

Thinking more about this, I think we're over using this BOARDCFG_MANAGED
mode a bit much. We should really just come up with new FW caps for
this, one for clk_restore , other for the previous IRQ restore patch.

That's the only way I can see this scaling. In future if we ever need
more devices that may actually be BOARDCFG_MANAGED, but don't need the
IRQ or clock restoration then the current approach won't work.

MODE should only be passed in the prepare_sleep, where it makes sense.
Using it for anything else just does not feel clean to me.

Thoughts?

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

Re: [PATCH v3 4/4] firmware: ti_sci: restore clock context during resume in BOARDCFG_MANAGED mode

[Thomas Richard]

On 12/17/25 7:07 AM, Dhruva Gole wrote:
> On Dec 05, 2025 at 15:28:26 +0100, Thomas Richard (TI.com) wrote:
>> In BOARDCFG_MANAGED mode, the firmware cannot restore the clock rates and
>> the clock parents. This responsibility is therefore delegated to the ti_sci
>> driver, which uses clk_restore_context() to trigger the context_restore()
>> operation for all registered clocks, including those managed by the sci-clk
>> driver. The sci-clk driver implements the context_restore() operation to
>> ensure rates and clock parents are correctly restored.
>>
>> Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
>> ---
>>  drivers/firmware/ti_sci.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index 8d94745376e2a..6ef687e481c49 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -9,6 +9,7 @@
>>  #define pr_fmt(fmt) "%s: " fmt, __func__
>>  
>>  #include <linux/bitmap.h>
>> +#include <linux/clk.h>
>>  #include <linux/cpu.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/export.h>
>> @@ -3980,6 +3981,8 @@ static int ti_sci_resume_noirq(struct device *dev)
>>  				if (ret)
>>  					return ret;
>>  			}
>> +
>> +			clk_restore_context();
> 
> Here as well, make it conditional to only BOARDCFG_MANAGED. Other
> platforms/ firmwares have lived without this for a while now, and it's
> evident that we don't always need this.

It is already conditionally done to only BOARDCFG_MANAGED.

> 
> Thinking more about this, I think we're over using this BOARDCFG_MANAGED
> mode a bit much. We should really just come up with new FW caps for
> this, one for clk_restore , other for the previous IRQ restore patch.
> 
> That's the only way I can see this scaling. In future if we ever need
> more devices that may actually be BOARDCFG_MANAGED, but don't need the
> IRQ or clock restoration then the current approach won't work.
> 
> MODE should only be passed in the prepare_sleep, where it makes sense.
> Using it for anything else just does not feel clean to me.

Fair point. Restoring clocks and IRQs is more related to the fact that
DM-Firmware on Jacinto platforms does not have suspend-resume support
than the BOARDCFG_MANAGED mode. I guess we could imagine in the future
having suspend-resume support in Jacinto DM-Firmware, so no need to
restore clocks and IRQs anymore, but the mode remains BOARDCFG_MANAGED.

Best Regards,
Thomas