diff mbox series

[v2,12/15] arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock

Message ID 20230627-sm6125-dpu-v2-12-03e430a2078c@somainline.org (mailing list archive)
State New, archived
Headers show
Series drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel | expand

Commit Message

Marijn Suijten June 27, 2023, 8:14 p.m. UTC 
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(-)

Comments

Dmitry Baryshkov June 29, 2023, 10:55 a.m. UTC | #1 
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>;
>   
>
Marijn Suijten June 29, 2023, 12:09 p.m. UTC | #2 
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>
Dmitry Baryshkov June 29, 2023, 12:26 p.m. UTC | #3 
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>
Konrad Dybcio June 29, 2023, 7:14 p.m. UTC | #4 
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>
> 
> 
>
Marijn Suijten July 18, 2023, 9:04 p.m. UTC | #5 
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 mbox series

Patch

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>;