| Message ID | 20241107-qcom_ipq_cmnpll-v6-5-a5cfe09de485@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add CMN PLL clock controller driver for IPQ9574 | expand |
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
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.
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. > >
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
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
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>; };
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(-)