Re: [PATCH v3 1/8] soc: qcom: geni-se: Populate clk_perf_tbl with SE source clock frequencies from perf OPP table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] soc: qcom: geni-se: clk_perf_tbl populated with OPP levels instead of frequencies
--

commit 2006551310c45129142551fa77079692b29b39f8
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

soc: qcom: geni-se: Populate clk_perf_tbl with SE source clock frequencies from perf OPP table

This commit aims to populate clk_perf_tbl using the OPP table exposed on
the perf domain device. This is intended to allow protocol drivers on the
firmware managed path to calculate required dividers and pick supported
source clock frequencies similarly to the Linux clock managed path.

> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 074087c592cc3..95eeb9438a292 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
[ ... ]
> @@ -1170,7 +1171,12 @@ int geni_se_domain_attach(struct geni_se *se)
[ ... ]
> +	level = 0;
> +	for (i = 0; i < num_opps; i++) {
> +		opp = dev_pm_opp_find_level_ceil(perf_dev, &level);
> +		if (IS_ERR(opp))
> +			return PTR_ERR(opp);
> +
> +		se->clk_perf_tbl[i] = level;

[Severity: High]
Does this populate clk_perf_tbl with abstract performance levels instead of
actual clock frequencies?

The commit message states clk_perf_tbl should be populated with actual source
clock frequencies so protocol drivers can calculate dividers and set the rate
via dev_pm_opp_set_rate(). However, by using dev_pm_opp_find_level_ceil()
in geni_se_domain_attach(), it appears this loop stores the performance
level (an abstract index like 16 or 64) rather than the frequency in Hz.

For SCMI performance domains, this abstract level will break the divider math
in geni_se_clk_freq_match(), leading to massive deltas or invalid dividers.
It will also cause dev_pm_opp_set_rate() to receive tiny abstract levels
instead of Hz. Because dev_pm_opp_set_rate() searches the OPP table for a
matching frequency, passing the abstract level will incorrectly match the
lowest available frequency and likely break protocol communication.

Should this use dev_pm_opp_find_freq_ceil() and track a frequency variable
to store the actual rates in clk_perf_tbl?

> +		dev_pm_opp_put(opp);
> +		level++;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-derive_clk_perf_tbl_from_perf_domain_opp_table-v3-0-5da1096243c9@oss.qualcomm.com?part=1