firmware: arm_scmi: Fixup perf microwatt support

Message ID 20230811204818.30928-1-quic_sibis@quicinc.com
State New
Headers
Series firmware: arm_scmi: Fixup perf microwatt support |

Commit Message

Sibi Sankar Aug. 11, 2023, 8:48 p.m. UTC
  The perf power scale value would currently be reported as bogowatts if the
platform firmware supports microwatt power scale and meets the perf major
version requirements. Fix this by populating version information in the
driver private data before the call to protocol attributes is made.

CC: Chandra Sekhar Lingutla <quic_lingutla@quicinc.com>
Fixes: 3630cd8130ce ("firmware: arm_scmi: Add SCMI v3.1 perf power-cost in microwatts")
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/firmware/arm_scmi/perf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Cristian Marussi Aug. 14, 2023, 7:54 a.m. UTC | #1
On Sat, Aug 12, 2023 at 02:18:18AM +0530, Sibi Sankar wrote:
> The perf power scale value would currently be reported as bogowatts if the
> platform firmware supports microwatt power scale and meets the perf major
> version requirements. Fix this by populating version information in the
> driver private data before the call to protocol attributes is made.
> 
> CC: Chandra Sekhar Lingutla <quic_lingutla@quicinc.com>
> Fixes: 3630cd8130ce ("firmware: arm_scmi: Add SCMI v3.1 perf power-cost in microwatts")
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
>  drivers/firmware/arm_scmi/perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Hi,

LGTM.

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index c0cd556fbaae..30dedd6ebfde 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -1080,6 +1080,8 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
>  	if (!pinfo)
>  		return -ENOMEM;
>  
> +	pinfo->version = version;
> +
>  	ret = scmi_perf_attributes_get(ph, pinfo);
>  	if (ret)
>  		return ret;
> @@ -1104,8 +1106,6 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
>  	if (ret)
>  		return ret;
>  
> -	pinfo->version = version;
> -
>  	return ph->set_priv(ph, pinfo);
>  }
>  
> -- 
> 2.17.1
>
  
Sudeep Holla Aug. 14, 2023, 9:25 a.m. UTC | #2
On Sat, Aug 12, 2023 at 02:18:18AM +0530, Sibi Sankar wrote:
> The perf power scale value would currently be reported as bogowatts if the
> platform firmware supports microwatt power scale and meets the perf major
> version requirements. Fix this by populating version information in the
> driver private data before the call to protocol attributes is made.
> 
> CC: Chandra Sekhar Lingutla <quic_lingutla@quicinc.com>
> Fixes: 3630cd8130ce ("firmware: arm_scmi: Add SCMI v3.1 perf power-cost in microwatts")
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
>  drivers/firmware/arm_scmi/perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index c0cd556fbaae..30dedd6ebfde 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -1080,6 +1080,8 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)

Please rebase any patch when posting upstream on upstream kernel tree.
This definitely looks like some downstream tree. I will adjust and apply
this time. This file never crossed 1000 line and this patch indicates
otherwise, so I am sure Qcom has some downstream changes in this file now.
  
Sibi Sankar Aug. 14, 2023, 4:13 p.m. UTC | #3
Hey Sudeep,

On 8/14/23 14:55, Sudeep Holla wrote:
> On Sat, Aug 12, 2023 at 02:18:18AM +0530, Sibi Sankar wrote:
>> The perf power scale value would currently be reported as bogowatts if the
>> platform firmware supports microwatt power scale and meets the perf major
>> version requirements. Fix this by populating version information in the
>> driver private data before the call to protocol attributes is made.
>>
>> CC: Chandra Sekhar Lingutla <quic_lingutla@quicinc.com>
>> Fixes: 3630cd8130ce ("firmware: arm_scmi: Add SCMI v3.1 perf power-cost in microwatts")
>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
>> ---
>>   drivers/firmware/arm_scmi/perf.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
>> index c0cd556fbaae..30dedd6ebfde 100644
>> --- a/drivers/firmware/arm_scmi/perf.c
>> +++ b/drivers/firmware/arm_scmi/perf.c
>> @@ -1080,6 +1080,8 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
> 
> Please rebase any patch when posting upstream on upstream kernel tree.
> This definitely looks like some downstream tree. I will adjust and apply
> this time. This file never crossed 1000 line and this patch indicates
> otherwise, so I am sure Qcom has some downstream changes in this file now.

Thanks for taking time to review the patch but zero points for the ^^
deduction lol. The correct phrasing would be v6.5 rc6 hasn't crossed 1k
lines but the patch was based on next-20230809 which has the perf level
indexing mode support in addition :)

- Sibi

>
  
Sudeep Holla Aug. 15, 2023, 8:48 a.m. UTC | #4
On Mon, Aug 14, 2023 at 09:43:37PM +0530, Sibi Sankar wrote:
> Hey Sudeep,
> 
> On 8/14/23 14:55, Sudeep Holla wrote:
> > On Sat, Aug 12, 2023 at 02:18:18AM +0530, Sibi Sankar wrote:
> > > The perf power scale value would currently be reported as bogowatts if the
> > > platform firmware supports microwatt power scale and meets the perf major
> > > version requirements. Fix this by populating version information in the
> > > driver private data before the call to protocol attributes is made.
> > > 
> > > CC: Chandra Sekhar Lingutla <quic_lingutla@quicinc.com>
> > > Fixes: 3630cd8130ce ("firmware: arm_scmi: Add SCMI v3.1 perf power-cost in microwatts")
> > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> > > ---
> > >   drivers/firmware/arm_scmi/perf.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> > > index c0cd556fbaae..30dedd6ebfde 100644
> > > --- a/drivers/firmware/arm_scmi/perf.c
> > > +++ b/drivers/firmware/arm_scmi/perf.c
> > > @@ -1080,6 +1080,8 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
> > 
> > Please rebase any patch when posting upstream on upstream kernel tree.
> > This definitely looks like some downstream tree. I will adjust and apply
> > this time. This file never crossed 1000 line and this patch indicates
> > otherwise, so I am sure Qcom has some downstream changes in this file now.
> 
> Thanks for taking time to review the patch but zero points for the ^^
> deduction lol. The correct phrasing would be v6.5 rc6 hasn't crossed 1k
> lines but the patch was based on next-20230809 which has the perf level
> indexing mode support in addition :)

You are right. Cristian reminded me about v3.2 perf changes in -next after
I replied on this thread. I saw this as fix and was looking at only upstream
tree and this didn't apply cleanly.
  
Sudeep Holla Sept. 20, 2023, 1:42 p.m. UTC | #5
On Sat, 12 Aug 2023 02:18:18 +0530, Sibi Sankar wrote:
> The perf power scale value would currently be reported as bogowatts if the
> platform firmware supports microwatt power scale and meets the perf major
> version requirements. Fix this by populating version information in the
> driver private data before the call to protocol attributes is made.
>

Applied to sudeep.holla/linux (for-next/scmi/fixes), thanks!

[1/1] firmware: arm_scmi: Fixup perf microwatt support
      https://git.kernel.org/sudeep.holla/c/c3638b851bc1
--
Regards,
Sudeep
  

Patch

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index c0cd556fbaae..30dedd6ebfde 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -1080,6 +1080,8 @@  static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
 	if (!pinfo)
 		return -ENOMEM;
 
+	pinfo->version = version;
+
 	ret = scmi_perf_attributes_get(ph, pinfo);
 	if (ret)
 		return ret;
@@ -1104,8 +1106,6 @@  static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
 	if (ret)
 		return ret;
 
-	pinfo->version = version;
-
 	return ph->set_priv(ph, pinfo);
 }