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

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

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