Re: [PATCH v6 5/5] arm64: dts: qcom: Update IPQ9574 xo_board_clk to use fixed factor clock

[Jie Luo <quic_luoj@quicinc.com>]

On 12/20/2024 5:28 AM, Konrad Dybcio wrote:
> On 18.12.2024 12:17 PM, Jie Luo wrote:
>>
>>
>> On 12/13/2024 6:30 PM, Jie Luo wrote:
>>>
>>>
>>> On 12/13/2024 2:33 AM, Konrad Dybcio wrote:
>>>> On 7.11.2024 10:50 AM, Luo Jie wrote:
>>>>> xo_board_clk is fixed to 24 MHZ, which is routed from WiFi output clock
>>>>> 48 MHZ (also being the reference clock of CMN PLL) divided 2 by analog
>>>>> block routing channel.
>>>>>
>>>>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>>>>> ---
>>>>>    arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi | 7 ++++++-
>>>>>    arch/arm64/boot/dts/qcom/ipq9574.dtsi            | 3 ++-
>>>>>    2 files changed, 8 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi b/arch/ arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
>>>>> index 78f6a2e053d5..9a8692377176 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
>>>>> @@ -174,8 +174,13 @@ &ref_48mhz_clk {
>>>>>        clock-mult = <1>;
>>>>>    };
>>>>> +/*
>>>>> + * The frequency of xo_board_clk is fixed to 24 MHZ, which is routed
>>>>> + * from WiFi output clock 48 MHZ divided by 2.
>>>>> + */
>>>>>    &xo_board_clk {
>>>>> -    clock-frequency = <24000000>;
>>>>> +    clock-div = <2>;
>>>>> +    clock-mult = <1>;
>>>>>    };
>>>>>    &xo_clk {
>>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/ dts/qcom/ipq9574.dtsi
>>>>> index 8246a00a3e3e..25aed33e9358 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>>> @@ -32,7 +32,8 @@ sleep_clk: sleep-clk {
>>>>>            };
>>>>>            xo_board_clk: xo-board-clk {
>>>>> -            compatible = "fixed-clock";
>>>>> +            compatible = "fixed-factor-clock";
>>>>> +            clocks = <&ref_48mhz_clk>;
>>>>
>>>> This must be squashed with the previous patch, you can't introduce
>>>> code and replace it immediately afterwards.
>>>>
>>>> Konrad
>>
>> Hi Konrad,
>>
>> Looking at this comment again, there may have been some
>> misunderstanding. We are not introducing xo_board_clk in patch 4 of this
>> series. xo_board_clk is a pre-existing node.
>>
>> As part of this additional patch 5, we wanted to address Dmitry's
>> comment earlier in v5 (reference to comment below), by converting the
>> xo_board_clk as well to a fixed-factor clock. So it is better to keep
>> this change as a separate patch in my view. Hope this is OK.
> 
> Oh okay..
> 
> What's the difference between xo_board_clk and xo_clk then, if the
> divider is accounted for in xo_board_clk?
> 
> Konrad

Here is the clock chain for the relationship between the clocks:
xo_clk (48 MHZ or 96 MHZ)-->WiFi (mul/div)--> 48 MHZ-->CMN PLL
				               |
					       +-->fixed factor(div by 2) --> xo_board_clk (24 MHZ)

So there are two dividers (fixed factor clocks): first for generating
the 48 MHZ clock (ref-48mhz-clk) from the fixed clock source 'xo_clk',
and a second for generating the 24 MHZ 'xo_board_clk' from the 48 MHZ
ref clock.