| Message ID | 20240119152338.3047620-1-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] firmware: arm_scmi: Warn if domain frequency multiplier is 0 or rounded off | expand |
On Fri, Jan 19, 2024 at 03:23:37PM +0000, Sudeep Holla wrote: > When (sustained_freq_khz * 1000) is less than sustained_perf_level, the > multiplier will be less than 1 and hence rounded down as 0. Similarly if > it is not multiple of sustained_perf_level the dom_info->mult_factor will > contain rounded down value and will end up impacting all the frequency > calculations done using it. > > Add warning if and when the domain frequency multiplier is 0 or rounded > down so that it gives a clue to get the firmware tables fixed. > > Suggested-by: Pierre Gondois <Pierre.Gondois@arm.com> > Reviewed-by: Pierre Gondois <pierre.gondois@arm.com> > Reviewed-by: Cristian Marussi <cristian.marussi@arm.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- LGTM. Thanks, Cristian > drivers/firmware/arm_scmi/perf.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > v1->v2: > - Added comment regarding the requirements on sustained_freq_khz > and sustained_perf_level obtained from the SCMI firmware > - Changed pr_warn to dev_warn > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 8ea2a7b3d35d..9fe123e1f3c7 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -270,15 +270,30 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, > le32_to_cpu(attr->sustained_freq_khz); > dom_info->sustained_perf_level = > le32_to_cpu(attr->sustained_perf_level); > + /* > + * sustained_freq_khz = mult_factor * sustained_perf_level > + * mult_factor must be non zero positive integer(not fraction) > + */ > if (!dom_info->sustained_freq_khz || > !dom_info->sustained_perf_level || > - dom_info->level_indexing_mode) > + dom_info->level_indexing_mode) { > /* CPUFreq converts to kHz, hence default 1000 */ > dom_info->mult_factor = 1000; > - else > + } else { > dom_info->mult_factor = > (dom_info->sustained_freq_khz * 1000UL) > / dom_info->sustained_perf_level; > + if ((dom_info->sustained_freq_khz * 1000UL) % > + dom_info->sustained_perf_level) > + dev_warn(ph->dev, > + "multiplier for domain %d rounded\n", > + dom_info->id); > + } > + if (!dom_info->mult_factor) > + dev_warn(ph->dev, > + "Wrong sustained perf/frequency(domain %d)\n", > + dom_info->id); > + > strscpy(dom_info->info.name, attr->name, > SCMI_SHORT_NAME_MAX_SIZE); > } > -- > 2.43.0 >
On Fri, 19 Jan 2024 15:23:37 +0000, Sudeep Holla wrote: > When (sustained_freq_khz * 1000) is less than sustained_perf_level, the > multiplier will be less than 1 and hence rounded down as 0. Similarly if > it is not multiple of sustained_perf_level the dom_info->mult_factor will > contain rounded down value and will end up impacting all the frequency > calculations done using it. > > Add warning if and when the domain frequency multiplier is 0 or rounded > down so that it gives a clue to get the firmware tables fixed. > > [...] Applied to sudeep.holla/linux (for-next/scmi/updates), thanks! [1/1] firmware: arm_scmi: Warn if domain frequency multiplier is 0 or rounded off https://git.kernel.org/sudeep.holla/c/534224b958df -- Regards, Sudeep
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 8ea2a7b3d35d..9fe123e1f3c7 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -270,15 +270,30 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, le32_to_cpu(attr->sustained_freq_khz); dom_info->sustained_perf_level = le32_to_cpu(attr->sustained_perf_level); + /* + * sustained_freq_khz = mult_factor * sustained_perf_level + * mult_factor must be non zero positive integer(not fraction) + */ if (!dom_info->sustained_freq_khz || !dom_info->sustained_perf_level || - dom_info->level_indexing_mode) + dom_info->level_indexing_mode) { /* CPUFreq converts to kHz, hence default 1000 */ dom_info->mult_factor = 1000; - else + } else { dom_info->mult_factor = (dom_info->sustained_freq_khz * 1000UL) / dom_info->sustained_perf_level; + if ((dom_info->sustained_freq_khz * 1000UL) % + dom_info->sustained_perf_level) + dev_warn(ph->dev, + "multiplier for domain %d rounded\n", + dom_info->id); + } + if (!dom_info->mult_factor) + dev_warn(ph->dev, + "Wrong sustained perf/frequency(domain %d)\n", + dom_info->id); + strscpy(dom_info->info.name, attr->name, SCMI_SHORT_NAME_MAX_SIZE); }