[PATCH] firmware: ti_sci: Convert CPU latency constraint from us to ms

[PATCH] firmware: ti_sci: Convert CPU latency constraint from us to ms

[Kendall Willis]

Fix CPU resume latency constraint units sent to TI SCI firmware.
CPU latency constraints are set using the PM QoS framework. The PM QoS
framework uses usecs as the units for latency whereas the device manager
uses msecs, so a conversion is needed before passing to device manager.

Signed-off-by: Kendall Willis <k-willis@ti.com>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
---
Test log [1] shows entry to MCU Only low power mode by sending a CPU
resume latency constraint of 100000 us using PM QoS. MCU Only is shown
to be entered by 0x1 as the printed mode.

[1] https://gist.github.com/kwillis01/059a2ca38232387b414bc6f4b87c7475
---
 drivers/firmware/ti_sci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 806a975fff22..bc138a837430 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -3670,6 +3670,7 @@ static int __maybe_unused ti_sci_suspend(struct device *dev)
 	struct ti_sci_info *info = dev_get_drvdata(dev);
 	struct device *cpu_dev, *cpu_dev_max = NULL;
 	s32 val, cpu_lat = 0;
+	u16 cpu_lat_ms;
 	int i, ret;
 
 	if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) {
@@ -3682,9 +3683,13 @@ static int __maybe_unused ti_sci_suspend(struct device *dev)
 			}
 		}
 		if (cpu_dev_max) {
-			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u\n", __func__, cpu_lat);
+			/* PM QoS latency unit is usecs, TI SCI uses msecs */
+			cpu_lat_ms = cpu_lat / USEC_PER_MSEC;
+			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u ms\n", __func__,
+				cpu_lat_ms);
 			ret = ti_sci_cmd_set_latency_constraint(&info->handle,
-								cpu_lat, TISCI_MSG_CONSTRAINT_SET);
+								cpu_lat_ms,
+								TISCI_MSG_CONSTRAINT_SET);
 			if (ret)
 				return ret;
 		}

base-commit: 393d0c54cae31317deaa9043320c5fd9454deabc
-- 
2.34.1

Re: [PATCH] firmware: ti_sci: Convert CPU latency constraint from us to ms

[Nishanth Menon]

On 10:37-20250425, Kendall Willis wrote:
> Fix CPU resume latency constraint units sent to TI SCI firmware.
> CPU latency constraints are set using the PM QoS framework. The PM QoS
> framework uses usecs as the units for latency whereas the device manager
> uses msecs, so a conversion is needed before passing to device manager.
> 

If this is a bug fix (sounds like it), follow the stable kernel rules.

Also please do not expect reviewers in the community know what this
means, I think you intent to point folks to the url
https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-lpm-set-latency-constraint

If so, add the reference to your commit message.


> Signed-off-by: Kendall Willis <k-willis@ti.com>
> Reviewed-by: Dhruva Gole <d-gole@ti.com>
> ---
> Test log [1] shows entry to MCU Only low power mode by sending a CPU
> resume latency constraint of 100000 us using PM QoS. MCU Only is shown
> to be entered by 0x1 as the printed mode.
> 
> [1] https://gist.github.com/kwillis01/059a2ca38232387b414bc6f4b87c7475
> ---
>  drivers/firmware/ti_sci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 806a975fff22..bc138a837430 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -3670,6 +3670,7 @@ static int __maybe_unused ti_sci_suspend(struct device *dev)
>  	struct ti_sci_info *info = dev_get_drvdata(dev);
>  	struct device *cpu_dev, *cpu_dev_max = NULL;
>  	s32 val, cpu_lat = 0;
> +	u16 cpu_lat_ms;
>  	int i, ret;
>  
>  	if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) {
> @@ -3682,9 +3683,13 @@ static int __maybe_unused ti_sci_suspend(struct device *dev)
>  			}
>  		}
>  		if (cpu_dev_max) {
> -			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u\n", __func__, cpu_lat);
> +			/* PM QoS latency unit is usecs, TI SCI uses msecs */
> +			cpu_lat_ms = cpu_lat / USEC_PER_MSEC;

round_down or a round_up? I assume you intent round_down, please
document that in the comments.

> +			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u ms\n", __func__,
> +				cpu_lat_ms);
>  			ret = ti_sci_cmd_set_latency_constraint(&info->handle,
> -								cpu_lat, TISCI_MSG_CONSTRAINT_SET);
> +								cpu_lat_ms,
> +								TISCI_MSG_CONSTRAINT_SET);
>  			if (ret)
>  				return ret;
>  		}
> 
> base-commit: 393d0c54cae31317deaa9043320c5fd9454deabc
> -- 
> 2.34.1
> 

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

Re: [PATCH] firmware: ti_sci: Convert CPU latency constraint from us to ms

[Kendall Willis]

On 4/25/25 14:08, Nishanth Menon wrote:
> On 10:37-20250425, Kendall Willis wrote:
>> Fix CPU resume latency constraint units sent to TI SCI firmware.
>> CPU latency constraints are set using the PM QoS framework. The PM QoS
>> framework uses usecs as the units for latency whereas the device manager
>> uses msecs, so a conversion is needed before passing to device manager.
>>
> 
> If this is a bug fix (sounds like it), follow the stable kernel rules.
> 
> Also please do not expect reviewers in the community know what this
> means, I think you intent to point folks to the url
> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-lpm-set-latency-constraint
> 
> If so, add the reference to your commit message.
>  >
>> Signed-off-by: Kendall Willis <k-willis@ti.com>
>> Reviewed-by: Dhruva Gole <d-gole@ti.com>
>> ---
>> Test log [1] shows entry to MCU Only low power mode by sending a CPU
>> resume latency constraint of 100000 us using PM QoS. MCU Only is shown
>> to be entered by 0x1 as the printed mode.
>>
>> [1] https://gist.github.com/kwillis01/059a2ca38232387b414bc6f4b87c7475
>> ---
>>   drivers/firmware/ti_sci.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index 806a975fff22..bc138a837430 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -3670,6 +3670,7 @@ static int __maybe_unused ti_sci_suspend(struct device *dev)
>>   	struct ti_sci_info *info = dev_get_drvdata(dev);
>>   	struct device *cpu_dev, *cpu_dev_max = NULL;
>>   	s32 val, cpu_lat = 0;
>> +	u16 cpu_lat_ms;
>>   	int i, ret;
>>   
>>   	if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) {
>> @@ -3682,9 +3683,13 @@ static int __maybe_unused ti_sci_suspend(struct device *dev)
>>   			}
>>   		}
>>   		if (cpu_dev_max) {
>> -			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u\n", __func__, cpu_lat);
>> +			/* PM QoS latency unit is usecs, TI SCI uses msecs */
>> +			cpu_lat_ms = cpu_lat / USEC_PER_MSEC;
> 
> round_down or a round_up? I assume you intent round_down, please
> document that in the comments.
> 
>> +			dev_dbg(cpu_dev_max, "%s: sending max CPU latency=%u ms\n", __func__,
>> +				cpu_lat_ms);
>>   			ret = ti_sci_cmd_set_latency_constraint(&info->handle,
>> -								cpu_lat, TISCI_MSG_CONSTRAINT_SET);
>> +								cpu_lat_ms,
>> +								TISCI_MSG_CONSTRAINT_SET);
>>   			if (ret)
>>   				return ret;
>>   		}
>>
>> base-commit: 393d0c54cae31317deaa9043320c5fd9454deabc
>> -- 
>> 2.34.1
>>
> 

Thanks for looking this over, Nishanth. I will implement your comments 
in V2.

Best,
Kendall Willis