| Message ID | 20230627-sm6125-dpu-v2-12-03e430a2078c@somainline.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel | expand |
On 27/06/2023 23:14, Marijn Suijten wrote: > We have a working RPM XO clock; no other driver except rpmcc should be > parenting directly to the fixed-factor xo_board clock nor should it be > reachable by that global name. Remove the name to that effect, so that > every clock relation is explicitly defined in DTS. > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi > index 722dde560bec..edb03508dba3 100644 > --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi > @@ -22,7 +22,6 @@ xo_board: xo-board { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <19200000>; > - clock-output-names = "xo_board"; Why? I'd say, leave it. With that fixed: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > }; > > sleep_clk: sleep-clk { > @@ -306,6 +305,8 @@ rpm_requests: rpm-requests { > rpmcc: clock-controller { > compatible = "qcom,rpmcc-sm6125", "qcom,rpmcc"; > #clock-cells = <1>; > + clocks = <&xo_board>; > + clock-names = "xo"; > }; > > rpmpd: power-controller { > @@ -713,7 +714,7 @@ sdhc_1: mmc@4744000 { > > clocks = <&gcc GCC_SDCC1_AHB_CLK>, > <&gcc GCC_SDCC1_APPS_CLK>, > - <&xo_board>; > + <&rpmcc RPM_SMD_XO_CLK_SRC>; > clock-names = "iface", "core", "xo"; > iommus = <&apps_smmu 0x160 0x0>; > > @@ -740,7 +741,7 @@ sdhc_2: mmc@4784000 { > > clocks = <&gcc GCC_SDCC2_AHB_CLK>, > <&gcc GCC_SDCC2_APPS_CLK>, > - <&xo_board>; > + <&rpmcc RPM_SMD_XO_CLK_SRC>; > clock-names = "iface", "core", "xo"; > iommus = <&apps_smmu 0x180 0x0>; > >
On 2023-06-29 13:55:28, Dmitry Baryshkov wrote: > On 27/06/2023 23:14, Marijn Suijten wrote: > > We have a working RPM XO clock; no other driver except rpmcc should be > > parenting directly to the fixed-factor xo_board clock nor should it be > > reachable by that global name. Remove the name to that effect, so that > > every clock relation is explicitly defined in DTS. > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > --- > > arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi > > index 722dde560bec..edb03508dba3 100644 > > --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi > > @@ -22,7 +22,6 @@ xo_board: xo-board { > > compatible = "fixed-clock"; > > #clock-cells = <0>; > > clock-frequency = <19200000>; > > - clock-output-names = "xo_board"; > > Why? I'd say, leave it. The exact reason is explained in the commit message. > > With that fixed: Hence I don't think it makes sense to "fix" this. - Marijn > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Thu, 29 Jun 2023 at 15:09, Marijn Suijten <marijn.suijten@somainline.org> wrote: > > On 2023-06-29 13:55:28, Dmitry Baryshkov wrote: > > On 27/06/2023 23:14, Marijn Suijten wrote: > > > We have a working RPM XO clock; no other driver except rpmcc should be > > > parenting directly to the fixed-factor xo_board clock nor should it be > > > reachable by that global name. Remove the name to that effect, so that > > > every clock relation is explicitly defined in DTS. > > > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > > --- > > > arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi > > > index 722dde560bec..edb03508dba3 100644 > > > --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi > > > @@ -22,7 +22,6 @@ xo_board: xo-board { > > > compatible = "fixed-clock"; > > > #clock-cells = <0>; > > > clock-frequency = <19200000>; > > > - clock-output-names = "xo_board"; > > > > Why? I'd say, leave it. > > The exact reason is explained in the commit message. Usually we do no not kill the xo_board name for the sake of anybody still looking for the old name. Weak argument, I know. > > > > > With that fixed: > > Hence I don't think it makes sense to "fix" this. > > - Marijn > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 29.06.2023 14:26, Dmitry Baryshkov wrote: > On Thu, 29 Jun 2023 at 15:09, Marijn Suijten > <marijn.suijten@somainline.org> wrote: >> >> On 2023-06-29 13:55:28, Dmitry Baryshkov wrote: >>> On 27/06/2023 23:14, Marijn Suijten wrote: >>>> We have a working RPM XO clock; no other driver except rpmcc should be >>>> parenting directly to the fixed-factor xo_board clock nor should it be >>>> reachable by that global name. Remove the name to that effect, so that >>>> every clock relation is explicitly defined in DTS. >>>> >>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >>>> --- >>>> arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++--- >>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi >>>> index 722dde560bec..edb03508dba3 100644 >>>> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi >>>> @@ -22,7 +22,6 @@ xo_board: xo-board { >>>> compatible = "fixed-clock"; >>>> #clock-cells = <0>; >>>> clock-frequency = <19200000>; >>>> - clock-output-names = "xo_board"; >>> >>> Why? I'd say, leave it. >> >> The exact reason is explained in the commit message. > > Usually we do no not kill the xo_board name for the sake of anybody > still looking for the old name. Weak argument, I know. The only users are (rg -l '"xo_board"' drivers): drivers/clk/qcom/mmcc-msm8974.c drivers/clk/qcom/a53-pll.c drivers/clk/qcom/gcc-msm8974.c drivers/clk/qcom/clk-smd-rpm.c drivers/clk/qcom/mmcc-msm8996.c drivers/clk/qcom/gcc-msm8916.c drivers/clk/qcom/gcc-apq8084.c drivers/clk/qcom/gcc-msm8996.c drivers/clk/qcom/mmcc-apq8084.c drivers/clk/qcom/clk-rpmh.c drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c This platform only binds clk-smd-rpm, but patch 11 provides a direct reference in the DT. Konrad > >> >>> >>> With that fixed: >> >> Hence I don't think it makes sense to "fix" this. >> >> - Marijn >> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > >
On 2023-06-29 21:14:47, Konrad Dybcio wrote: > On 29.06.2023 14:26, Dmitry Baryshkov wrote: > > On Thu, 29 Jun 2023 at 15:09, Marijn Suijten > > <marijn.suijten@somainline.org> wrote: > >> > >> On 2023-06-29 13:55:28, Dmitry Baryshkov wrote: > >>> On 27/06/2023 23:14, Marijn Suijten wrote: > >>>> We have a working RPM XO clock; no other driver except rpmcc should be > >>>> parenting directly to the fixed-factor xo_board clock nor should it be > >>>> reachable by that global name. Remove the name to that effect, so that > >>>> every clock relation is explicitly defined in DTS. > >>>> > >>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > >>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > >>>> --- > >>>> arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++--- > >>>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi > >>>> index 722dde560bec..edb03508dba3 100644 > >>>> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi > >>>> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi > >>>> @@ -22,7 +22,6 @@ xo_board: xo-board { > >>>> compatible = "fixed-clock"; > >>>> #clock-cells = <0>; > >>>> clock-frequency = <19200000>; > >>>> - clock-output-names = "xo_board"; > >>> > >>> Why? I'd say, leave it. > >> > >> The exact reason is explained in the commit message. > > > > Usually we do no not kill the xo_board name for the sake of anybody > > still looking for the old name. Weak argument, I know. > The only users are (rg -l '"xo_board"' drivers): > > drivers/clk/qcom/mmcc-msm8974.c > drivers/clk/qcom/a53-pll.c > drivers/clk/qcom/gcc-msm8974.c > drivers/clk/qcom/clk-smd-rpm.c > drivers/clk/qcom/mmcc-msm8996.c > drivers/clk/qcom/gcc-msm8916.c > drivers/clk/qcom/gcc-apq8084.c > drivers/clk/qcom/gcc-msm8996.c > drivers/clk/qcom/mmcc-apq8084.c > drivers/clk/qcom/clk-rpmh.c > drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c > > This platform only binds clk-smd-rpm, but patch 11 provides a > direct reference in the DT. And following a quick check, those occurrences all have .fw_name="xo",.name="xo_board", allowing the clock to be provided via DT. For sm6125, I'd like it to be required like that: all dt-bindings require an "xo" board where relevant, after all. - Marijn > > Konrad > > > > >> > >>> > >>> With that fixed: > >> > >> Hence I don't think it makes sense to "fix" this. > >> > >> - Marijn > >> > >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > >
diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi index 722dde560bec..edb03508dba3 100644 --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi @@ -22,7 +22,6 @@ xo_board: xo-board { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <19200000>; - clock-output-names = "xo_board"; }; sleep_clk: sleep-clk { @@ -306,6 +305,8 @@ rpm_requests: rpm-requests { rpmcc: clock-controller { compatible = "qcom,rpmcc-sm6125", "qcom,rpmcc"; #clock-cells = <1>; + clocks = <&xo_board>; + clock-names = "xo"; }; rpmpd: power-controller { @@ -713,7 +714,7 @@ sdhc_1: mmc@4744000 { clocks = <&gcc GCC_SDCC1_AHB_CLK>, <&gcc GCC_SDCC1_APPS_CLK>, - <&xo_board>; + <&rpmcc RPM_SMD_XO_CLK_SRC>; clock-names = "iface", "core", "xo"; iommus = <&apps_smmu 0x160 0x0>; @@ -740,7 +741,7 @@ sdhc_2: mmc@4784000 { clocks = <&gcc GCC_SDCC2_AHB_CLK>, <&gcc GCC_SDCC2_APPS_CLK>, - <&xo_board>; + <&rpmcc RPM_SMD_XO_CLK_SRC>; clock-names = "iface", "core", "xo"; iommus = <&apps_smmu 0x180 0x0>;