On 4.02.2025 7:28 AM, Sricharan Ramabadhran wrote:
>
>
> On 2/1/2025 8:55 PM, Konrad Dybcio wrote:
>> On 30.01.2025 11:03 AM, Sricharan Ramabadhran wrote:
>>>
>>>
>>> On 1/28/2025 5:29 PM, Konrad Dybcio wrote:
>>>> On 27.01.2025 10:31 AM, Sricharan R wrote:
>>>>> From: Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx>
>>>>>
>>>>> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
>>>>> Add support for the APSS PLL, RCG and clock enable for ipq5424.
>>>>> The PLL, RCG register space are clubbed. Hence adding new APSS driver
>>>>> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
>>>>> and needs to be scaled along with the CPU.
>>>>>
>>>>> Co-developed-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
>>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
>>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx>
>>>>> ---
>>
>> [...]
>>
>>>>> + clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
>>>>> +
>>>>> + clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
>>>>> +
>>>>> + ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
>>>>> +
>>>>> + apss_ipq5424_cfg->dev = dev;
>>>>> + apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
>>>>> + apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
>>>>> +
>>>>> + apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
>>>>> + if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
>>>>> + dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
>>>>> + PTR_ERR(apss_ipq5424_cfg->l3_clk));
>>>>> + return PTR_ERR(apss_ipq5424_cfg->l3_clk);
>>>>> + }
>>>>
>>>> Now that you'll use OPP, you can drop all this getting.. maybe even the
>>>> apss_ipq5424_cfg struct could be let go
>>>
>>> ok, is the suggestion here to use devm_pm_opp_set_config ?
>>
>> Since what you tried to do here is binding CPU and L3 frequencies together,
>> yeah, we can just scale two clocks from OPP.
>>
>> On some newer platforms using the epss-l3 driver, or on msm8996 with a more
>> complex setup, we expose the L3 voter as an interconnect, but here it would
>> seem that we directly control the clock that feeds it.
>
> ok, will update and check.
+Dmitry
Giving it yet another thought, we now have infrastructure in clk/qcom/common.c
to register icc clocks. We can register the L3 one as such and make the
description like:
cpu0: cpu@0 {
[...]
interconnects = <&apss L3_CLK>;
[...]
};
cpu_opp_table: opp-table {
opp-1234000 {
opp-hz = /bits/ 64 <1234000>;
opp-peak-kBps = <SOME_L3_FREQ>;
};
};
as that will both match how we modeled msm8996 & require less code changes
Dmitry, Bjorn?
Konrad
