| Message ID | 20241130-fix-board-clocks-v2-0-b9a35858657e@linaro.org |
|---|---|
| Headers | show |
| Series | arm64: dts: qcom: move board clocks to SoC DTSI files | expand |
On 30/11/2024 02:44, Dmitry Baryshkov wrote: > SM8650 is one of the platforms where board-level clocks (XO, sleep) > definitions are split between the SoC dtsi file and the board file. > This is not optimal, as the clocks are a part of the SoC + PMICs design. > Frequencies are common for the whole set of devices using the same SoC. > Remove the split and move frequencies to the SoC DTSI file. > > Suggested-by: Bjorn Andersson <andersson@kernel.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Nacked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On Sat, Nov 30, 2024 at 11:21:56AM +0100, Stephan Gerhold wrote: > On Sat, Nov 30, 2024 at 03:44:13AM +0200, Dmitry Baryshkov wrote: > > The MSM8916 platform uses PM8916 to provide sleep clock. According to the > > documentation, that clock has 32.7645 kHz frequency. Correct the sleep > > clock definition. > > > > Fixes: f4fb6aeafaaa ("arm64: dts: qcom: msm8916: Add fixed rate on-board oscillators") > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Thanks for spotting this! This fix looks good independent of the more > controversial "arm64: dts: qcom: move board clocks to SoC DTSI files" > changes. Maybe move these to a separate series? > > > --- > > arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi > > index 5e558bcc9d87893486352e5e211f131d4a1f67e5..8f35c9af18782aa1da7089988692e6588c4b7c5d 100644 > > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi > > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi > > @@ -125,7 +125,7 @@ xo_board: xo-board { > > sleep_clk: sleep-clk { > > compatible = "fixed-clock"; > > #clock-cells = <0>; > > - clock-frequency = <32768>; > > + clock-frequency = <32764>; > > To be precise the PM8916 specification says the sleep clock is "The 19.2 > MHz XO divided by 586". Maybe we can actually describe it that way with > a fixed-factor-clock? > > sleep_clk: sleep-clk { > compatible = "fixed-factor-clock"; > clocks = <&xo_board>; > #clock-cells = <0>; > clock-div = <586>; > clock-mult = <1>; > }; I thought about it, but then it's also not complete truth (at least for some of PMICs, don't remember if that's the case for PM8916): there is an external XO and also there is an on-PMIC RC, which is further divided with PMIC actually selecting which source to use as a source for sleep_clk. > > If we keep the fixed-clock with the hardcoded frequency I wonder if we > should put 32765 instead of 32764. If you calculate it exactly it's > slightly closer to 32765 than 32764. :-) > > 19200000/586 = 32764.505119453926 = ~32765 Well, I think according to the most typical rounding rules it is 32764. > > Thanks, > Stephan
Multiple Qualcomm platforms play strange tricks with board-level clocks (XO, sleep) definitions. On some (older) platforms such clocks are completely defined within SoC.dtsi file (although these clocks are not a part of the SoC). On other platforms definitions of such clocks are split between the SoC dtsi file and the board file. Several obscure platforms define those clocks completely in the board files. Unify the design and move complete description of those clocks to the SoC DTSI file. The XO clock is (usually) an external crystal used by the external PMIC, which then provides RF CLK and LN BB CLK to the main SoC. However for technical reasons this part of the PMIC is modelled as a part of the SoC as RPM or RPMh clock controllers. It makes it impractical to describe XO clock as being used or being connected to the PMIC. Sleep clock is a 32.764 kHz RC oscillator provided by one of PMICs. However pushing it into the PMIC might interact badly with fw devlink, causing unnecessary probe delays and/or devlink loops. One of the possible solutions might be to move it to the corresponding PMIC.dtsi, but model the clock outside of the PMIC node, providing /clocks/sleep-clk node from that file. Note, the series includes a set of fixes for the sleep clocks frequencies. For several platforms I wasn't able to find corresponding document and as such I didn't change defined clocks. These platforms are: IPQ5018, IPQ5332, IPQ5424, IPQ6018, IPQ8074, IPQ9574, MSM8953. Also several MSM8996 / MSM8994 devices define divisor clocks at 32.768 kHz. Most likely these clocks are also generated by dividing the 19.2 MHz clock and should have the frequency 32.764 kHz, but being not 100% sure I decided to leave those as is for now. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Changes in v2: - Move clocks to SoC DTSI (offline discussion with Bjorn) - Link to v1: https://lore.kernel.org/r/20241115-fix-board-clocks-v1-0-8cb00a4f57c2@linaro.org --- Dmitry Baryshkov (31): arm64: dts: qcom: msm8916: correct sleep clock frequency arm64: dts: qcom: msm8939: correct sleep clock frequency arm64: dts: qcom: msm8994: correct sleep clock frequency arm64: dts: qcom: qcs404: correct sleep clock frequency arm64: dts: qcom: q[dr]u1000: correct sleep clock frequency arm64: dts: qcom: qrb4210-rb2: correct sleep clock frequency arm64: dts: qcom: sar2130p: correct sleep clock frequency arm64: dts: qcom: sc7280: correct sleep clock frequency arm64: dts: qcom: sdx75: correct sleep clock frequency arm64: dts: qcom: sm4450: correct sleep clock frequency arm64: dts: qcom: sm6125: correct sleep clock frequency arm64: dts: qcom: sm6375: correct sleep clock frequency arm64: dts: qcom: sm8250: correct sleep clock frequency arm64: dts: qcom: sm8350: correct sleep clock frequency arm64: dts: qcom: sm8450: correct sleep clock frequency arm64: dts: qcom: sm8550: correct sleep clock frequency arm64: dts: qcom: sm8650: correct sleep clock frequency arm64: dts: qcom: x1e80100: correct sleep clock frequency arm64: dts: qcom: sc8180x: drop extra XO clock frequencies arm64: dts: qcom: ipq5018: move board clocks to ipq5018.dtsi file arm64: dts: qcom: ipq5332: move board clocks to ipq5332.dtsi file arm64: dts: qcom: ipq5424: move board clocks to ipq5424.dtsi file arm64: dts: qcom: ipq9574: move board clocks to ipq9574.dtsi file arm64: dts: qcom: qcm2290: move board clocks to qcm2290.dtsi file arm64: dts: qcom: sc8280xp: move board clocks to sc8280xp.dtsi file arm64: dts: qcom: sm6115: move board clocks to sm6115.dtsi file arm64: dts: qcom: sm6375: move board clocks to sm6375.dtsi file arm64: dts: qcom: sm8550: move board clocks to sm8550.dtsi file arm64: dts: qcom: sm8650: move board clocks to sm8650.dtsi file arm64: dts: qcom: sdm670: move board clocks to sdm670.dtsi file arm64: dts: qcom: q[dr]u1000: move board clocks to qdu1000.dtsi file arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts | 8 -------- arch/arm64/boot/dts/qcom/ipq5018-tplink-archer-ax55-v1.dts | 8 -------- arch/arm64/boot/dts/qcom/ipq5018.dtsi | 2 ++ arch/arm64/boot/dts/qcom/ipq5332-rdp-common.dtsi | 8 -------- arch/arm64/boot/dts/qcom/ipq5332.dtsi | 2 ++ arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 9 --------- arch/arm64/boot/dts/qcom/ipq5424.dtsi | 2 ++ arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi | 8 -------- arch/arm64/boot/dts/qcom/ipq9574.dtsi | 2 ++ arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 +- arch/arm64/boot/dts/qcom/msm8939.dtsi | 2 +- arch/arm64/boot/dts/qcom/msm8994.dtsi | 2 +- arch/arm64/boot/dts/qcom/qcm2290.dtsi | 1 + arch/arm64/boot/dts/qcom/qcs404.dtsi | 2 +- arch/arm64/boot/dts/qcom/qcs8550-aim300.dtsi | 8 -------- arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 14 -------------- arch/arm64/boot/dts/qcom/qdu1000.dtsi | 14 ++++++++++++++ arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 4 ---- arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 8 -------- arch/arm64/boot/dts/qcom/qru1000-idp.dts | 14 -------------- arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 4 ---- arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 4 ---- arch/arm64/boot/dts/qcom/sar2130p.dtsi | 2 +- arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- arch/arm64/boot/dts/qcom/sc8180x-lenovo-flex-5g.dts | 4 ---- arch/arm64/boot/dts/qcom/sc8180x-primus.dts | 4 ---- arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 4 ---- arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 4 ---- arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dts | 4 ---- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 1 + arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts | 14 -------------- arch/arm64/boot/dts/qcom/sdm670.dtsi | 14 ++++++++++++++ arch/arm64/boot/dts/qcom/sdx75.dtsi | 2 +- arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 8 -------- arch/arm64/boot/dts/qcom/sm4450.dtsi | 2 +- arch/arm64/boot/dts/qcom/sm6115-fxtec-pro1x.dts | 8 -------- arch/arm64/boot/dts/qcom/sm6115.dtsi | 2 ++ arch/arm64/boot/dts/qcom/sm6115p-lenovo-j606f.dts | 8 -------- arch/arm64/boot/dts/qcom/sm6125.dtsi | 2 +- .../boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts | 4 ---- arch/arm64/boot/dts/qcom/sm6375.dtsi | 3 ++- arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +- arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 +- arch/arm64/boot/dts/qcom/sm8450.dtsi | 2 +- arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8550-mtp.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts | 8 -------- .../arm64/boot/dts/qcom/sm8550-sony-xperia-yodo-pdx234.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 ++ arch/arm64/boot/dts/qcom/sm8650-hdk.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8650-mtp.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 8 -------- arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 ++ arch/arm64/boot/dts/qcom/x1e80100.dtsi | 2 +- 55 files changed, 59 insertions(+), 237 deletions(-) --- base-commit: cfba9f07a1d6aeca38f47f1f472cfb0ba133d341 change-id: 20241115-fix-board-clocks-e3afe520627c Best regards,