Re: [PATCH v6 4/5] arm64: dts: qcom: Add CMN PLL node for IPQ9574 SoC

[Jie Luo <quic_luoj@quicinc.com>]

On 12/13/2024 2:32 AM, Konrad Dybcio wrote:
> On 7.11.2024 10:50 AM, Luo Jie wrote:
>> The CMN PLL clock controller allows selection of an input clock rate
>> from a defined set of input clock rates. It in-turn supplies fixed
>> rate output clocks to the hardware blocks that provide the ethernet
>> functions such as PPE (Packet Process Engine) and connected switch or
>> PHY, and to GCC.
>>
>> The reference clock of CMN PLL is routed from XO to the CMN PLL through
>> the internal WiFi block.
>> .XO (48 MHZ or 96 MHZ)-->WiFi (multiplier/divider)-->48 MHZ to CMN PLL.
>>
>> The reference input clock from WiFi to CMN PLL is fully controlled by
>> the bootstrap pins which select the XO frequency (48 MHZ or 96 MHZ).
>> Based on this frequency, the divider in the internal Wi-Fi block is
>> automatically configured by hardware (1 for 48 MHZ, 2 for 96 MHZ), to
>> ensure output clock to CMN PLL is 48 MHZ.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi | 16 ++++++++++++++-
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi            | 26 +++++++++++++++++++++++-
>>   2 files changed, 40 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 91e104b0f865..78f6a2e053d5 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
>> @@ -3,7 +3,7 @@
>>    * IPQ9574 RDP board common device tree source
>>    *
>>    * Copyright (c) 2020-2021 The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>>   /dts-v1/;
>> @@ -164,6 +164,20 @@ &usb3 {
>>   	status = "okay";
>>   };
>>   
>> +/*
>> + * The bootstrap pins for the board select the XO clock frequency,
>> + * which automatically enables the right dividers to ensure the
>> + * reference clock output from WiFi is 48 MHZ.
> 
> I'm a bit puzzled by this comment. Does it mean this clock could
> run at some different speeds?

The reference clock of CMN PLL is routed from XO to the CMN PLL through
the internal WiFi block.

.XO (48 MHZ or 96 MHZ)-->WiFi (multiplier/divider)-->48 MHZ to CMN PLL.

The CMN PLL reference clock from WiFi always runs at 48 MHZ on IPQ9574,
but the XO clock could be 48 MHZ or 96 MHZ on different IPQ9574 boards.
The bootstrap pins select the right divider to ensure eventual clock
rate from Wi-Fi is always 48 MHZ.

To avoid confusion, I will update this comment to mention that the
XO clock frequency could run at 48 MHZ or 96 MHZ.

> 
> [...]
> 
>>   
>> +		cmn_pll: clock-controller@9b000 {
>> +			compatible = "qcom,ipq9574-cmn-pll";
>> +			reg = <0x0009b000 0x800>;
>> +			clocks = <&ref_48mhz_clk>,
>> +				 <&gcc GCC_CMN_12GPLL_AHB_CLK>,
>> +				 <&gcc GCC_CMN_12GPLL_SYS_CLK>;
>> +			clock-names = "ref", "ahb", "sys";
>> +			#clock-cells = <1>;
>> +			assigned-clocks = <&cmn_pll CMN_PLL_CLK>;
>> +			assigned-clock-rates-u64 = /bits/ 64 <12000000000>;
> 
> Does devlink not complain about self-referencing the clock here?
> 
> Konrad

This code is validated on IPQ9574 RDP433 reference board, there is no
complaint reported about this self-referencing the clock of the clock
supplier DT node. It seems the API of_clk_set_defaults(struct
device_node *node, bool clk_supplier) called by this DT property
"assigned-clocks" allows this kind of self-reference.