+ Johan,
On 19-10-22, 19:29, Manivannan Sadhasivam wrote:
> Hello,
>
> This series adds clock provider support to the Qcom CPUFreq driver for
> supplying the clocks to the CPU cores in Qcom SoCs.
>
> The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> clocks to the CPU cores. But this is not represented clearly in devicetree.
> There is no clock coming out of the CPUFreq HW node to the CPU. This created
> an issue [1] with the OPP core when a recent enhancement series was submitted.
> Eventhough the issue got fixed in the OPP framework in the meantime, that's
> not a proper solution and this series aims to fix it properly.
>
> There was also an attempt made by Viresh [2] to fix the issue by moving the
> clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
> since those clocks belong to the CPUFreq HW node only.
>
> The proposal here is to add clock provider support to the Qcom CPUFreq HW
> driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
> This correctly reflects the hardware implementation.
>
> The clock provider is a simple one that just provides the frequency of the
> clocks supplied to each frequency domain in the SoC using .recalc_rate()
> callback. The frequency supplied by the driver will be the actual frequency
> that comes out of the EPSS/OSM block after the DCVS operation. This frequency
> is not same as what the CPUFreq framework has set but it is the one that gets
> supplied to the CPUs after throttling by LMh.
>
> This series has been tested on SM8450 based dev board and hence there is a DTS
> change only for that platform. Once this series gets accepted, rest of the
> platform DTS can also be modified and finally the hack on the OPP core can be
> dropped.
Thanks for working on this Mani.
Can you also test the below code over your series ? This shouldn't
result in issues that Johan reported earlier [1][2]. Below is the hack I
am carrying in the OPP core for Qcom SoCs at the moment.
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e87567dbe99f..b7158d33c13d 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1384,20 +1384,6 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
}
if (ret == -ENOENT) {
- /*
- * There are few platforms which don't want the OPP core to
- * manage device's clock settings. In such cases neither the
- * platform provides the clks explicitly to us, nor the DT
- * contains a valid clk entry. The OPP nodes in DT may still
- * contain "opp-hz" property though, which we need to parse and
- * allow the platform to find an OPP based on freq later on.
- *
- * This is a simple solution to take care of such corner cases,
- * i.e. make the clk_count 1, which lets us allocate space for
- * frequency in opp->rates and also parse the entries in DT.
- */
- opp_table->clk_count = 1;
-
dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret);
return opp_table;
}
diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c
index 96a30a032c5f..402c507edac7 100644
--- a/drivers/opp/debugfs.c
+++ b/drivers/opp/debugfs.c
@@ -138,7 +138,7 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
* - For some devices rate isn't available or there are multiple, use
* index instead for them.
*/
- if (likely(opp_table->clk_count == 1 && opp->rates[0]))
+ if (likely(opp_table->clk_count == 1))
id = opp->rates[0];
else
id = _get_opp_count(opp_table);
--
viresh
[1] https://lore.kernel.org/all/YsxSkswzsqgMOc0l@xxxxxxxxxxxxxxxxxxxx/
[2] https://lore.kernel.org/all/Ys2FZa6YDwt7d%2FZc@xxxxxxxxxxxxxxxxxxxx/
