linux-kernel - Re: [PATCH v3 2/4] clk: qcom: Add CMN PLL clock controller driver for IPQ SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ecc34401-68c2-463f-b630-6a81ad95625e@quicinc.com>
Date: Mon, 2 Sep 2024 23:33:57 +0800
From: Jie Luo <quic_luoj@...cinc.com>
To: Stephen Boyd <sboyd@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        "Krzysztof
 Kozlowski" <krzk+dt@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Rob Herring <robh@...nel.org>, Will Deacon <will@...nel.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <quic_kkumarcs@...cinc.com>,
        <quic_suruchia@...cinc.com>, <quic_pavir@...cinc.com>,
        <quic_linchen@...cinc.com>, <quic_leiwei@...cinc.com>,
        <bartosz.golaszewski@...aro.org>, <srinivas.kandagatla@...aro.org>
Subject: Re: [PATCH v3 2/4] clk: qcom: Add CMN PLL clock controller driver for
 IPQ SoC



On 8/31/2024 6:24 AM, Stephen Boyd wrote:
> Quoting Jie Luo (2024-08-30 09:14:28)
>> Hi Stephen,
>> Please find below a minor update to my earlier message on clk_ops usage.
> 
> Ok. Next time you can trim the reply to save me time.

OK.

> 
>> On 8/28/2024 1:44 PM, Jie Luo wrote:
>>> On 8/28/2024 7:50 AM, Stephen Boyd wrote:
>>>> Quoting Luo Jie (2024-08-27 05:46:00)
>>>>> +       case 48000000:
>>>>> +               val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7);
>>>>> +               break;
>>>>> +       case 50000000:
>>>>> +               val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 8);
>>>>> +               break;
>>>>> +       case 96000000:
>>>>> +               val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7);
>>>>> +               val &= ~CMN_PLL_REFCLK_DIV;
>>>>> +               val |= FIELD_PREP(CMN_PLL_REFCLK_DIV, 2);
>>>>> +               break;
>>>>> +       default:
>>>>> +               return -EINVAL;
>>>>> +       }
>>>>
>>>> Why isn't this done with struct clk_ops::set_rate() or clk_ops::init()?
>>>
>>> OK, I will move this code into the clk_ops::init().
>>
>> This code is expected to be executed once for initializing the CMN PLL
>> to enable output clocks, and requires the parent clock rate to be
>> available. However the parent clock rate is not available in the
>> clk_ops::init(). Hence clk_ops::set_rate() seems to be the right option
>> for this. Please let us know if this approach is fine. Thanks.
> 
> Sure. It actually sounds like the PLL has a mux to select different
> reference clks. Is that right? If so, it seems like there should be
> multiple 'clocks' for the DT property and many parents possible. If
> that's the case then it should be possible to have something like
> 
> 	clocks = <0>, <&refclk>, <0>;
> 
> in the DT node and then have clk_set_rate() from the consumer actually
> set the parent index in hardware. If that's all static then it can be
> done with assigned-clock-parents or assigned-clock-rates.

Thanks Stephen. The CMN PLL block always uses a single input reference
clock pin on any given IPQ SoC, however its rate may be different on
different IPQ SoC. For example, its rate is 48MHZ on IPQ9574 and 96MHZ
on IPQ5018.

Your second suggestion seems more apt for this device. I can define the
DT property 'assigned-clock-parents' to configure the clock parent of
CMN PLL. The code for reference clock selection will be added in
clk_ops::set_parent(). Please let us know if this approach is fine.