[v6] Add CMN PLL clock controller driver for IPQ9574

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

Posted by Luo Jie 1 year, 3 months ago

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>;
 			#clock-cells = <0>;
 		};
 

-- 
2.34.1

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

Posted by Konrad Dybcio 1 year, 1 month ago

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

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

Posted by Jie Luo 1 year, 1 month ago


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

Ok, I will update the patch series with this patch dislodged, and push
this single patch in next update. Thanks.

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

Posted by Jie Luo 1 year, 1 month ago


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.

https://lore.kernel.org/linux-arm-msm/CAA8EJpoQO7=v8QWeH8MAgX4uU=m4VJqfC3J5PKyySM2TBcHWiw@mail.gmail.com/

> 
> Ok, I will update the patch series with this patch dislodged, and push
> this single patch in next update. Thanks.
> 
>

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

Posted by Konrad Dybcio 1 year, 1 month ago

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

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

Posted by Jie Luo 1 year, 1 month ago


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.

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

Posted by Konrad Dybcio 1 year, 1 month ago

On 20.12.2024 7:43 AM, Jie Luo wrote:
> 
> 
> 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.

Ok, thanks for the explanation

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad