| Message ID | 20210512151209.27560-2-kishon@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | J721E: Use external clock in EVM for SERDES | expand |
On 20:42-20210512, Kishon Vijay Abraham I wrote: > Rename the external refclk inputs to the SERDES from > dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 > respectively. Also move the external refclk DT nodes outside the > cbass_main DT node. Since in j721e common processor board, only the > cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. > > Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") Assume we want this part of 5.13 fixes? > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- > 2 files changed, 34 insertions(+), 28 deletions(-) > > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > index 60764366e22b..86f7ab511ee8 100644 > --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > @@ -635,6 +635,10 @@ > status = "disabled"; > }; > > +&cmn_refclk1 { > + clock-frequency = <100000000>; > +}; > + > &serdes0 { > serdes0_pcie_link: link@0 { > reg = <0>; > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > index c2aa45a3ac79..002a0c1520ee 100644 > --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > @@ -8,6 +8,20 @@ > #include <dt-bindings/mux/mux.h> > #include <dt-bindings/mux/ti-serdes.h> > > +/ { > + cmn_refclk: cmn-refclk { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <0>; > + }; > + > + cmn_refclk1: cmn-refclk1 { Just curious: why cant we use the standard nodenames with clock? > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <0>; > + }; > +}; > + > &cbass_main { > msmc_ram: sram@70000000 { > compatible = "mmio-sram"; > @@ -336,24 +350,12 @@ > pinctrl-single,function-mask = <0xffffffff>; > }; > > - dummy_cmn_refclk: dummy-cmn-refclk { > - #clock-cells = <0>; > - compatible = "fixed-clock"; > - clock-frequency = <100000000>; > - }; > - > - dummy_cmn_refclk1: dummy-cmn-refclk1 { > - #clock-cells = <0>; > - compatible = "fixed-clock"; > - clock-frequency = <100000000>; > - }; > - > serdes_wiz0: wiz@5000000 { > compatible = "ti,j721e-wiz-16g"; > #address-cells = <1>; > #size-cells = <1>; > power-domains = <&k3_pds 292 TI_SCI_PD_EXCLUSIVE>; > - clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&cmn_refclk>; > clock-names = "fck", "core_ref_clk", "ext_ref_clk"; > assigned-clocks = <&k3_clks 292 11>, <&k3_clks 292 0>; > assigned-clock-parents = <&k3_clks 292 15>, <&k3_clks 292 4>; > @@ -362,21 +364,21 @@ > ranges = <0x5000000 0x0 0x5000000 0x10000>; > > wiz0_pll0_refclk: pll0-refclk { > - clocks = <&k3_clks 292 11>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 292 11>, <&cmn_refclk>; > #clock-cells = <0>; > assigned-clocks = <&wiz0_pll0_refclk>; > assigned-clock-parents = <&k3_clks 292 11>; > }; > > wiz0_pll1_refclk: pll1-refclk { > - clocks = <&k3_clks 292 0>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 292 0>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz0_pll1_refclk>; > assigned-clock-parents = <&k3_clks 292 0>; > }; > > wiz0_refclk_dig: refclk-dig { > - clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&cmn_refclk>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz0_refclk_dig>; > assigned-clock-parents = <&k3_clks 292 11>; > @@ -410,7 +412,7 @@ > #address-cells = <1>; > #size-cells = <1>; > power-domains = <&k3_pds 293 TI_SCI_PD_EXCLUSIVE>; > - clocks = <&k3_clks 293 5>, <&k3_clks 293 13>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 293 5>, <&k3_clks 293 13>, <&cmn_refclk>; > clock-names = "fck", "core_ref_clk", "ext_ref_clk"; > assigned-clocks = <&k3_clks 293 13>, <&k3_clks 293 0>; > assigned-clock-parents = <&k3_clks 293 17>, <&k3_clks 293 4>; > @@ -419,21 +421,21 @@ > ranges = <0x5010000 0x0 0x5010000 0x10000>; > > wiz1_pll0_refclk: pll0-refclk { > - clocks = <&k3_clks 293 13>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 293 13>, <&cmn_refclk>; > #clock-cells = <0>; > assigned-clocks = <&wiz1_pll0_refclk>; > assigned-clock-parents = <&k3_clks 293 13>; > }; > > wiz1_pll1_refclk: pll1-refclk { > - clocks = <&k3_clks 293 0>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 293 0>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz1_pll1_refclk>; > assigned-clock-parents = <&k3_clks 293 0>; > }; > > wiz1_refclk_dig: refclk-dig { > - clocks = <&k3_clks 293 13>, <&k3_clks 293 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 293 13>, <&k3_clks 293 0>, <&cmn_refclk>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz1_refclk_dig>; > assigned-clock-parents = <&k3_clks 293 13>; > @@ -467,7 +469,7 @@ > #address-cells = <1>; > #size-cells = <1>; > power-domains = <&k3_pds 294 TI_SCI_PD_EXCLUSIVE>; > - clocks = <&k3_clks 294 5>, <&k3_clks 294 11>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 294 5>, <&k3_clks 294 11>, <&cmn_refclk>; > clock-names = "fck", "core_ref_clk", "ext_ref_clk"; > assigned-clocks = <&k3_clks 294 11>, <&k3_clks 294 0>; > assigned-clock-parents = <&k3_clks 294 15>, <&k3_clks 294 4>; > @@ -476,21 +478,21 @@ > ranges = <0x5020000 0x0 0x5020000 0x10000>; > > wiz2_pll0_refclk: pll0-refclk { > - clocks = <&k3_clks 294 11>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 294 11>, <&cmn_refclk>; > #clock-cells = <0>; > assigned-clocks = <&wiz2_pll0_refclk>; > assigned-clock-parents = <&k3_clks 294 11>; > }; > > wiz2_pll1_refclk: pll1-refclk { > - clocks = <&k3_clks 294 0>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 294 0>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz2_pll1_refclk>; > assigned-clock-parents = <&k3_clks 294 0>; > }; > > wiz2_refclk_dig: refclk-dig { > - clocks = <&k3_clks 294 11>, <&k3_clks 294 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 294 11>, <&k3_clks 294 0>, <&cmn_refclk>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz2_refclk_dig>; > assigned-clock-parents = <&k3_clks 294 11>; > @@ -524,7 +526,7 @@ > #address-cells = <1>; > #size-cells = <1>; > power-domains = <&k3_pds 295 TI_SCI_PD_EXCLUSIVE>; > - clocks = <&k3_clks 295 5>, <&k3_clks 295 9>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 295 5>, <&k3_clks 295 9>, <&cmn_refclk>; > clock-names = "fck", "core_ref_clk", "ext_ref_clk"; > assigned-clocks = <&k3_clks 295 9>, <&k3_clks 295 0>; > assigned-clock-parents = <&k3_clks 295 13>, <&k3_clks 295 4>; > @@ -533,21 +535,21 @@ > ranges = <0x5030000 0x0 0x5030000 0x10000>; > > wiz3_pll0_refclk: pll0-refclk { > - clocks = <&k3_clks 295 9>, <&dummy_cmn_refclk>; > + clocks = <&k3_clks 295 9>, <&cmn_refclk>; > #clock-cells = <0>; > assigned-clocks = <&wiz3_pll0_refclk>; > assigned-clock-parents = <&k3_clks 295 9>; > }; > > wiz3_pll1_refclk: pll1-refclk { > - clocks = <&k3_clks 295 0>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 295 0>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz3_pll1_refclk>; > assigned-clock-parents = <&k3_clks 295 0>; > }; > > wiz3_refclk_dig: refclk-dig { > - clocks = <&k3_clks 295 9>, <&k3_clks 295 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; > + clocks = <&k3_clks 295 9>, <&k3_clks 295 0>, <&cmn_refclk>, <&cmn_refclk1>; > #clock-cells = <0>; > assigned-clocks = <&wiz3_refclk_dig>; > assigned-clock-parents = <&k3_clks 295 9>; > -- > 2.17.1 >
Hi Nishanth, On 13/05/21 12:21 am, Nishanth Menon wrote: > On 20:42-20210512, Kishon Vijay Abraham I wrote: >> Rename the external refclk inputs to the SERDES from >> dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 >> respectively. Also move the external refclk DT nodes outside the >> cbass_main DT node. Since in j721e common processor board, only the >> cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. >> >> Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") > > Assume we want this part of 5.13 fixes? This doesn't fix any functionality. Okay for me to go in 5.14 along with the rest of the series. > >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ >> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- >> 2 files changed, 34 insertions(+), 28 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >> index 60764366e22b..86f7ab511ee8 100644 >> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >> @@ -635,6 +635,10 @@ >> status = "disabled"; >> }; >> >> +&cmn_refclk1 { >> + clock-frequency = <100000000>; >> +}; >> + >> &serdes0 { >> serdes0_pcie_link: link@0 { >> reg = <0>; >> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >> index c2aa45a3ac79..002a0c1520ee 100644 >> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >> @@ -8,6 +8,20 @@ >> #include <dt-bindings/mux/mux.h> >> #include <dt-bindings/mux/ti-serdes.h> >> >> +/ { >> + cmn_refclk: cmn-refclk { >> + #clock-cells = <0>; >> + compatible = "fixed-clock"; >> + clock-frequency = <0>; >> + }; >> + >> + cmn_refclk1: cmn-refclk1 { > > Just curious: why cant we use the standard nodenames with clock? We can use standard names here. Is there any defined nodename for clocks? clk or clock? Don't see $nodename defined for clocks in dt-schema repository. Thanks Kishon
On 17:41-20210513, Kishon Vijay Abraham I wrote: > Hi Nishanth, > > On 13/05/21 12:21 am, Nishanth Menon wrote: > > On 20:42-20210512, Kishon Vijay Abraham I wrote: > >> Rename the external refclk inputs to the SERDES from > >> dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 > >> respectively. Also move the external refclk DT nodes outside the > >> cbass_main DT node. Since in j721e common processor board, only the > >> cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. > >> > >> Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") > > > > Assume we want this part of 5.13 fixes? > > This doesn't fix any functionality. Okay for me to go in 5.14 along with > the rest of the series. > > > >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > >> --- > >> .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ > >> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- > >> 2 files changed, 34 insertions(+), 28 deletions(-) > >> > >> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >> index 60764366e22b..86f7ab511ee8 100644 > >> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >> @@ -635,6 +635,10 @@ > >> status = "disabled"; > >> }; > >> > >> +&cmn_refclk1 { > >> + clock-frequency = <100000000>; > >> +}; > >> + > >> &serdes0 { > >> serdes0_pcie_link: link@0 { > >> reg = <0>; > >> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >> index c2aa45a3ac79..002a0c1520ee 100644 > >> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >> @@ -8,6 +8,20 @@ > >> #include <dt-bindings/mux/mux.h> > >> #include <dt-bindings/mux/ti-serdes.h> > >> > >> +/ { > >> + cmn_refclk: cmn-refclk { > >> + #clock-cells = <0>; > >> + compatible = "fixed-clock"; > >> + clock-frequency = <0>; > >> + }; > >> + > >> + cmn_refclk1: cmn-refclk1 { > > > > Just curious: why cant we use the standard nodenames with clock? > > We can use standard names here. Is there any defined nodename for > clocks? clk or clock? Don't see $nodename defined for clocks in > dt-schema repository. Looking at the fixed-clock example, lets go with clock
Hi Nishanth, On 13/05/21 7:31 pm, Nishanth Menon wrote: > On 17:41-20210513, Kishon Vijay Abraham I wrote: >> Hi Nishanth, >> >> On 13/05/21 12:21 am, Nishanth Menon wrote: >>> On 20:42-20210512, Kishon Vijay Abraham I wrote: >>>> Rename the external refclk inputs to the SERDES from >>>> dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 >>>> respectively. Also move the external refclk DT nodes outside the >>>> cbass_main DT node. Since in j721e common processor board, only the >>>> cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. >>>> >>>> Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") >>> >>> Assume we want this part of 5.13 fixes? >> >> This doesn't fix any functionality. Okay for me to go in 5.14 along with >> the rest of the series. > > >>> >>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>>> --- >>>> .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ >>>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- >>>> 2 files changed, 34 insertions(+), 28 deletions(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >>>> index 60764366e22b..86f7ab511ee8 100644 >>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >>>> @@ -635,6 +635,10 @@ >>>> status = "disabled"; >>>> }; >>>> >>>> +&cmn_refclk1 { >>>> + clock-frequency = <100000000>; >>>> +}; >>>> + >>>> &serdes0 { >>>> serdes0_pcie_link: link@0 { >>>> reg = <0>; >>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >>>> index c2aa45a3ac79..002a0c1520ee 100644 >>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >>>> @@ -8,6 +8,20 @@ >>>> #include <dt-bindings/mux/mux.h> >>>> #include <dt-bindings/mux/ti-serdes.h> >>>> >>>> +/ { >>>> + cmn_refclk: cmn-refclk { >>>> + #clock-cells = <0>; >>>> + compatible = "fixed-clock"; >>>> + clock-frequency = <0>; >>>> + }; >>>> + >>>> + cmn_refclk1: cmn-refclk1 { >>> >>> Just curious: why cant we use the standard nodenames with clock? >> >> We can use standard names here. Is there any defined nodename for >> clocks? clk or clock? Don't see $nodename defined for clocks in >> dt-schema repository. > > Looking at the fixed-clock example, lets go with clock Since I have two clocks here adding clock@0 and clock@1 introduces the following error. /home/a0393678/repos/linux-wip/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dt.yaml: /: clock@0: 'anyOf' conditional failed, one must be fixed: 'reg' is a required property 'ranges' is a required property The current "fixed-clock" binding doesn't allow adding "reg" property. We'll stick to non standard names? or do you think the binding has to be fixed? Thanks Kishon
On 14:00-20210517, Kishon Vijay Abraham I wrote: > Hi Nishanth, > > On 13/05/21 7:31 pm, Nishanth Menon wrote: > > On 17:41-20210513, Kishon Vijay Abraham I wrote: > >> Hi Nishanth, > >> > >> On 13/05/21 12:21 am, Nishanth Menon wrote: > >>> On 20:42-20210512, Kishon Vijay Abraham I wrote: > >>>> Rename the external refclk inputs to the SERDES from > >>>> dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 > >>>> respectively. Also move the external refclk DT nodes outside the > >>>> cbass_main DT node. Since in j721e common processor board, only the > >>>> cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. > >>>> > >>>> Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") > >>> > >>> Assume we want this part of 5.13 fixes? > >> > >> This doesn't fix any functionality. Okay for me to go in 5.14 along with > >> the rest of the series. > > > > > >>> > >>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > >>>> --- > >>>> .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ > >>>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- > >>>> 2 files changed, 34 insertions(+), 28 deletions(-) > >>>> > >>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >>>> index 60764366e22b..86f7ab511ee8 100644 > >>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >>>> @@ -635,6 +635,10 @@ > >>>> status = "disabled"; > >>>> }; > >>>> > >>>> +&cmn_refclk1 { > >>>> + clock-frequency = <100000000>; > >>>> +}; > >>>> + > >>>> &serdes0 { > >>>> serdes0_pcie_link: link@0 { > >>>> reg = <0>; > >>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >>>> index c2aa45a3ac79..002a0c1520ee 100644 > >>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >>>> @@ -8,6 +8,20 @@ > >>>> #include <dt-bindings/mux/mux.h> > >>>> #include <dt-bindings/mux/ti-serdes.h> > >>>> > >>>> +/ { > >>>> + cmn_refclk: cmn-refclk { > >>>> + #clock-cells = <0>; > >>>> + compatible = "fixed-clock"; > >>>> + clock-frequency = <0>; > >>>> + }; > >>>> + > >>>> + cmn_refclk1: cmn-refclk1 { > >>> > >>> Just curious: why cant we use the standard nodenames with clock? > >> > >> We can use standard names here. Is there any defined nodename for > >> clocks? clk or clock? Don't see $nodename defined for clocks in > >> dt-schema repository. > > > > Looking at the fixed-clock example, lets go with clock > > Since I have two clocks here adding clock@0 and clock@1 introduces the > following error. > /home/a0393678/repos/linux-wip/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dt.yaml: > /: clock@0: 'anyOf' conditional failed, one must be fixed: > 'reg' is a required property > 'ranges' is a required property > > The current "fixed-clock" binding doesn't allow adding "reg" property. > We'll stick to non standard names? or do you think the binding has to be > fixed? Look at other fixed-clock examples in other arm64 examples https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mm.dtsi#n147 is a good one.. Binding is fine, IMHO.
Hi Nishanth, On 17/05/21 7:35 pm, Nishanth Menon wrote: > On 14:00-20210517, Kishon Vijay Abraham I wrote: >> Hi Nishanth, >> >> On 13/05/21 7:31 pm, Nishanth Menon wrote: >>> On 17:41-20210513, Kishon Vijay Abraham I wrote: >>>> Hi Nishanth, >>>> >>>> On 13/05/21 12:21 am, Nishanth Menon wrote: >>>>> On 20:42-20210512, Kishon Vijay Abraham I wrote: >>>>>> Rename the external refclk inputs to the SERDES from >>>>>> dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 >>>>>> respectively. Also move the external refclk DT nodes outside the >>>>>> cbass_main DT node. Since in j721e common processor board, only the >>>>>> cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. >>>>>> >>>>>> Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") >>>>> >>>>> Assume we want this part of 5.13 fixes? >>>> >>>> This doesn't fix any functionality. Okay for me to go in 5.14 along with >>>> the rest of the series. >>> >>> >>>>> >>>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>>>>> --- >>>>>> .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ >>>>>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- >>>>>> 2 files changed, 34 insertions(+), 28 deletions(-) >>>>>> >>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >>>>>> index 60764366e22b..86f7ab511ee8 100644 >>>>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >>>>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts >>>>>> @@ -635,6 +635,10 @@ >>>>>> status = "disabled"; >>>>>> }; >>>>>> >>>>>> +&cmn_refclk1 { >>>>>> + clock-frequency = <100000000>; >>>>>> +}; >>>>>> + >>>>>> &serdes0 { >>>>>> serdes0_pcie_link: link@0 { >>>>>> reg = <0>; >>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >>>>>> index c2aa45a3ac79..002a0c1520ee 100644 >>>>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >>>>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi >>>>>> @@ -8,6 +8,20 @@ >>>>>> #include <dt-bindings/mux/mux.h> >>>>>> #include <dt-bindings/mux/ti-serdes.h> >>>>>> >>>>>> +/ { >>>>>> + cmn_refclk: cmn-refclk { >>>>>> + #clock-cells = <0>; >>>>>> + compatible = "fixed-clock"; >>>>>> + clock-frequency = <0>; >>>>>> + }; >>>>>> + >>>>>> + cmn_refclk1: cmn-refclk1 { >>>>> >>>>> Just curious: why cant we use the standard nodenames with clock? >>>> >>>> We can use standard names here. Is there any defined nodename for >>>> clocks? clk or clock? Don't see $nodename defined for clocks in >>>> dt-schema repository. >>> >>> Looking at the fixed-clock example, lets go with clock >> >> Since I have two clocks here adding clock@0 and clock@1 introduces the >> following error. >> /home/a0393678/repos/linux-wip/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dt.yaml: >> /: clock@0: 'anyOf' conditional failed, one must be fixed: >> 'reg' is a required property >> 'ranges' is a required property >> >> The current "fixed-clock" binding doesn't allow adding "reg" property. >> We'll stick to non standard names? or do you think the binding has to be >> fixed? > > Look at other fixed-clock examples in other arm64 examples > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mm.dtsi#n147 > is a good one.. Binding is fine, IMHO. Ah Thanks. It only has to be prefixed with clock-. Thanks Kishon
On 18:48-20210520, Kishon Vijay Abraham I wrote: > Hi Nishanth, > > On 17/05/21 7:35 pm, Nishanth Menon wrote: > > On 14:00-20210517, Kishon Vijay Abraham I wrote: > >> Hi Nishanth, > >> > >> On 13/05/21 7:31 pm, Nishanth Menon wrote: > >>> On 17:41-20210513, Kishon Vijay Abraham I wrote: > >>>> Hi Nishanth, > >>>> > >>>> On 13/05/21 12:21 am, Nishanth Menon wrote: > >>>>> On 20:42-20210512, Kishon Vijay Abraham I wrote: > >>>>>> Rename the external refclk inputs to the SERDES from > >>>>>> dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 > >>>>>> respectively. Also move the external refclk DT nodes outside the > >>>>>> cbass_main DT node. Since in j721e common processor board, only the > >>>>>> cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. > >>>>>> > >>>>>> Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") > >>>>> > >>>>> Assume we want this part of 5.13 fixes? > >>>> > >>>> This doesn't fix any functionality. Okay for me to go in 5.14 along with > >>>> the rest of the series. > >>> > >>> > >>>>> > >>>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > >>>>>> --- > >>>>>> .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ > >>>>>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- > >>>>>> 2 files changed, 34 insertions(+), 28 deletions(-) > >>>>>> > >>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >>>>>> index 60764366e22b..86f7ab511ee8 100644 > >>>>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >>>>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > >>>>>> @@ -635,6 +635,10 @@ > >>>>>> status = "disabled"; > >>>>>> }; > >>>>>> > >>>>>> +&cmn_refclk1 { > >>>>>> + clock-frequency = <100000000>; > >>>>>> +}; > >>>>>> + > >>>>>> &serdes0 { > >>>>>> serdes0_pcie_link: link@0 { > >>>>>> reg = <0>; > >>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >>>>>> index c2aa45a3ac79..002a0c1520ee 100644 > >>>>>> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >>>>>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > >>>>>> @@ -8,6 +8,20 @@ > >>>>>> #include <dt-bindings/mux/mux.h> > >>>>>> #include <dt-bindings/mux/ti-serdes.h> > >>>>>> > >>>>>> +/ { > >>>>>> + cmn_refclk: cmn-refclk { > >>>>>> + #clock-cells = <0>; > >>>>>> + compatible = "fixed-clock"; > >>>>>> + clock-frequency = <0>; > >>>>>> + }; > >>>>>> + > >>>>>> + cmn_refclk1: cmn-refclk1 { > >>>>> > >>>>> Just curious: why cant we use the standard nodenames with clock? > >>>> > >>>> We can use standard names here. Is there any defined nodename for > >>>> clocks? clk or clock? Don't see $nodename defined for clocks in > >>>> dt-schema repository. > >>> > >>> Looking at the fixed-clock example, lets go with clock > >> > >> Since I have two clocks here adding clock@0 and clock@1 introduces the > >> following error. > >> /home/a0393678/repos/linux-wip/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dt.yaml: > >> /: clock@0: 'anyOf' conditional failed, one must be fixed: > >> 'reg' is a required property > >> 'ranges' is a required property > >> > >> The current "fixed-clock" binding doesn't allow adding "reg" property. > >> We'll stick to non standard names? or do you think the binding has to be > >> fixed? > > > > Look at other fixed-clock examples in other arm64 examples > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mm.dtsi#n147 > > is a good one.. Binding is fine, IMHO. > > Ah Thanks. It only has to be prefixed with clock-. Yep - also, though I think it is self evident, I will explicitly state as well: since dts should represent hardware, using names like "dummy" does'nt belong to dts - it would indicate something of a software construct. Knowing what you are trying to describe, I do understand it is not a "software construct", but please avoid using similar naming which may create misunderstanding.
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts index 60764366e22b..86f7ab511ee8 100644 --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts @@ -635,6 +635,10 @@ status = "disabled"; }; +&cmn_refclk1 { + clock-frequency = <100000000>; +}; + &serdes0 { serdes0_pcie_link: link@0 { reg = <0>; diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi index c2aa45a3ac79..002a0c1520ee 100644 --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi @@ -8,6 +8,20 @@ #include <dt-bindings/mux/mux.h> #include <dt-bindings/mux/ti-serdes.h> +/ { + cmn_refclk: cmn-refclk { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <0>; + }; + + cmn_refclk1: cmn-refclk1 { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <0>; + }; +}; + &cbass_main { msmc_ram: sram@70000000 { compatible = "mmio-sram"; @@ -336,24 +350,12 @@ pinctrl-single,function-mask = <0xffffffff>; }; - dummy_cmn_refclk: dummy-cmn-refclk { - #clock-cells = <0>; - compatible = "fixed-clock"; - clock-frequency = <100000000>; - }; - - dummy_cmn_refclk1: dummy-cmn-refclk1 { - #clock-cells = <0>; - compatible = "fixed-clock"; - clock-frequency = <100000000>; - }; - serdes_wiz0: wiz@5000000 { compatible = "ti,j721e-wiz-16g"; #address-cells = <1>; #size-cells = <1>; power-domains = <&k3_pds 292 TI_SCI_PD_EXCLUSIVE>; - clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&cmn_refclk>; clock-names = "fck", "core_ref_clk", "ext_ref_clk"; assigned-clocks = <&k3_clks 292 11>, <&k3_clks 292 0>; assigned-clock-parents = <&k3_clks 292 15>, <&k3_clks 292 4>; @@ -362,21 +364,21 @@ ranges = <0x5000000 0x0 0x5000000 0x10000>; wiz0_pll0_refclk: pll0-refclk { - clocks = <&k3_clks 292 11>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 292 11>, <&cmn_refclk>; #clock-cells = <0>; assigned-clocks = <&wiz0_pll0_refclk>; assigned-clock-parents = <&k3_clks 292 11>; }; wiz0_pll1_refclk: pll1-refclk { - clocks = <&k3_clks 292 0>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 292 0>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz0_pll1_refclk>; assigned-clock-parents = <&k3_clks 292 0>; }; wiz0_refclk_dig: refclk-dig { - clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&cmn_refclk>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz0_refclk_dig>; assigned-clock-parents = <&k3_clks 292 11>; @@ -410,7 +412,7 @@ #address-cells = <1>; #size-cells = <1>; power-domains = <&k3_pds 293 TI_SCI_PD_EXCLUSIVE>; - clocks = <&k3_clks 293 5>, <&k3_clks 293 13>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 293 5>, <&k3_clks 293 13>, <&cmn_refclk>; clock-names = "fck", "core_ref_clk", "ext_ref_clk"; assigned-clocks = <&k3_clks 293 13>, <&k3_clks 293 0>; assigned-clock-parents = <&k3_clks 293 17>, <&k3_clks 293 4>; @@ -419,21 +421,21 @@ ranges = <0x5010000 0x0 0x5010000 0x10000>; wiz1_pll0_refclk: pll0-refclk { - clocks = <&k3_clks 293 13>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 293 13>, <&cmn_refclk>; #clock-cells = <0>; assigned-clocks = <&wiz1_pll0_refclk>; assigned-clock-parents = <&k3_clks 293 13>; }; wiz1_pll1_refclk: pll1-refclk { - clocks = <&k3_clks 293 0>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 293 0>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz1_pll1_refclk>; assigned-clock-parents = <&k3_clks 293 0>; }; wiz1_refclk_dig: refclk-dig { - clocks = <&k3_clks 293 13>, <&k3_clks 293 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 293 13>, <&k3_clks 293 0>, <&cmn_refclk>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz1_refclk_dig>; assigned-clock-parents = <&k3_clks 293 13>; @@ -467,7 +469,7 @@ #address-cells = <1>; #size-cells = <1>; power-domains = <&k3_pds 294 TI_SCI_PD_EXCLUSIVE>; - clocks = <&k3_clks 294 5>, <&k3_clks 294 11>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 294 5>, <&k3_clks 294 11>, <&cmn_refclk>; clock-names = "fck", "core_ref_clk", "ext_ref_clk"; assigned-clocks = <&k3_clks 294 11>, <&k3_clks 294 0>; assigned-clock-parents = <&k3_clks 294 15>, <&k3_clks 294 4>; @@ -476,21 +478,21 @@ ranges = <0x5020000 0x0 0x5020000 0x10000>; wiz2_pll0_refclk: pll0-refclk { - clocks = <&k3_clks 294 11>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 294 11>, <&cmn_refclk>; #clock-cells = <0>; assigned-clocks = <&wiz2_pll0_refclk>; assigned-clock-parents = <&k3_clks 294 11>; }; wiz2_pll1_refclk: pll1-refclk { - clocks = <&k3_clks 294 0>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 294 0>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz2_pll1_refclk>; assigned-clock-parents = <&k3_clks 294 0>; }; wiz2_refclk_dig: refclk-dig { - clocks = <&k3_clks 294 11>, <&k3_clks 294 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 294 11>, <&k3_clks 294 0>, <&cmn_refclk>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz2_refclk_dig>; assigned-clock-parents = <&k3_clks 294 11>; @@ -524,7 +526,7 @@ #address-cells = <1>; #size-cells = <1>; power-domains = <&k3_pds 295 TI_SCI_PD_EXCLUSIVE>; - clocks = <&k3_clks 295 5>, <&k3_clks 295 9>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 295 5>, <&k3_clks 295 9>, <&cmn_refclk>; clock-names = "fck", "core_ref_clk", "ext_ref_clk"; assigned-clocks = <&k3_clks 295 9>, <&k3_clks 295 0>; assigned-clock-parents = <&k3_clks 295 13>, <&k3_clks 295 4>; @@ -533,21 +535,21 @@ ranges = <0x5030000 0x0 0x5030000 0x10000>; wiz3_pll0_refclk: pll0-refclk { - clocks = <&k3_clks 295 9>, <&dummy_cmn_refclk>; + clocks = <&k3_clks 295 9>, <&cmn_refclk>; #clock-cells = <0>; assigned-clocks = <&wiz3_pll0_refclk>; assigned-clock-parents = <&k3_clks 295 9>; }; wiz3_pll1_refclk: pll1-refclk { - clocks = <&k3_clks 295 0>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 295 0>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz3_pll1_refclk>; assigned-clock-parents = <&k3_clks 295 0>; }; wiz3_refclk_dig: refclk-dig { - clocks = <&k3_clks 295 9>, <&k3_clks 295 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; + clocks = <&k3_clks 295 9>, <&k3_clks 295 0>, <&cmn_refclk>, <&cmn_refclk1>; #clock-cells = <0>; assigned-clocks = <&wiz3_refclk_dig>; assigned-clock-parents = <&k3_clks 295 9>;
Rename the external refclk inputs to the SERDES from dummy_cmn_refclk/dummy_cmn_refclk1 to cmn_refclk/cmn_refclk1 respectively. Also move the external refclk DT nodes outside the cbass_main DT node. Since in j721e common processor board, only the cmn_refclk1 is connected to 100MHz clock, fix the clock frequency. Fixes: afd094ebe69f ("arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes") Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- .../dts/ti/k3-j721e-common-proc-board.dts | 4 ++ arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 58 ++++++++++--------- 2 files changed, 34 insertions(+), 28 deletions(-)