* [PATCH 1/2] ARM: dts: s3c64xx: factor out external fixed clocks [not found] <20190907024719.16974-1-ylhuajnu@outlook.com> @ 2019-09-07 2:48 ` Yao Lihua 2019-09-09 18:51 ` krzk 2019-09-07 2:48 ` [PATCH 2/2] ARM: dts: s3c64xx: specify dependency of clock providers Yao Lihua 1 sibling, 1 reply; 5+ messages in thread From: Yao Lihua @ 2019-09-07 2:48 UTC (permalink / raw) To: krzk, kgene, linux-samsung-soc; +Cc: Yao Lihua, linux-arm-kernel From: Lihua Yao <ylhuajnu@outlook.com> As per arch/arm/mach-s3c64xx/common.c, the external oscillators of S3C6400 and S3C6410 are identical. Move them to s3c64xx.dtsi and place under root node directly. This introduces side effect of changing the initialization order of fin_pll and clock-controller@7e00f000. As of commit 3f6d439f2022 ("clk: reverse default clk provider initialization order in of_clk_init()"), clock providers are initialized in the orders they are present in the device tree unless the clocks' dependencies are specified explicitly. without this patch: [ 0.000000] S3C6410 clocks: apll = 0, mpll = 0 [ 0.000000] epll = 0, arm_clk = 0 with this patch: [ 0.000000] S3C6410 clocks: apll = 532000000, mpll = 532000000 [ 0.000000] epll = 24000000, arm_clk = 532000000 Fixes: 3f6d439f2022 ("clk: reverse default clk provider initialization order in of_clk_init()") Signed-off-by: Lihua Yao <ylhuajnu@outlook.com> --- arch/arm/boot/dts/s3c6410-mini6410.dts | 22 ---------------------- arch/arm/boot/dts/s3c6410-smdk6410.dts | 22 ---------------------- arch/arm/boot/dts/s3c64xx.dtsi | 14 ++++++++++++++ 3 files changed, 14 insertions(+), 44 deletions(-) diff --git a/arch/arm/boot/dts/s3c6410-mini6410.dts b/arch/arm/boot/dts/s3c6410-mini6410.dts index 5201512054c4..7028507b7076 100644 --- a/arch/arm/boot/dts/s3c6410-mini6410.dts +++ b/arch/arm/boot/dts/s3c6410-mini6410.dts @@ -28,28 +28,6 @@ bootargs = "console=ttySAC0,115200n8 earlyprintk root=/dev/nfs rw nfsroot=192.168.31.2:/srv/nfs/tiny6410,nfsvers=3 ip=dhcp"; }; - clocks { - compatible = "simple-bus"; - #address-cells = <1>; - #size-cells = <0>; - - fin_pll: oscillator@0 { - compatible = "fixed-clock"; - reg = <0>; - clock-frequency = <12000000>; - clock-output-names = "fin_pll"; - #clock-cells = <0>; - }; - - xusbxti: oscillator@1 { - compatible = "fixed-clock"; - reg = <1>; - clock-output-names = "xusbxti"; - clock-frequency = <48000000>; - #clock-cells = <0>; - }; - }; - srom-cs1@18000000 { compatible = "simple-bus"; #address-cells = <1>; diff --git a/arch/arm/boot/dts/s3c6410-smdk6410.dts b/arch/arm/boot/dts/s3c6410-smdk6410.dts index a9a5689dc462..10a854b488a8 100644 --- a/arch/arm/boot/dts/s3c6410-smdk6410.dts +++ b/arch/arm/boot/dts/s3c6410-smdk6410.dts @@ -28,28 +28,6 @@ bootargs = "console=ttySAC0,115200n8 earlyprintk rootwait root=/dev/mmcblk0p1"; }; - clocks { - compatible = "simple-bus"; - #address-cells = <1>; - #size-cells = <0>; - - fin_pll: oscillator@0 { - compatible = "fixed-clock"; - reg = <0>; - clock-frequency = <12000000>; - clock-output-names = "fin_pll"; - #clock-cells = <0>; - }; - - xusbxti: oscillator@1 { - compatible = "fixed-clock"; - reg = <1>; - clock-output-names = "xusbxti"; - clock-frequency = <48000000>; - #clock-cells = <0>; - }; - }; - srom-cs1@18000000 { compatible = "simple-bus"; #address-cells = <1>; diff --git a/arch/arm/boot/dts/s3c64xx.dtsi b/arch/arm/boot/dts/s3c64xx.dtsi index 2e611df37911..672764133cea 100644 --- a/arch/arm/boot/dts/s3c64xx.dtsi +++ b/arch/arm/boot/dts/s3c64xx.dtsi @@ -39,6 +39,20 @@ }; }; + fin_pll: oscillator-0 { + compatible = "fixed-clock"; + clock-frequency = <12000000>; + clock-output-names = "fin_pll"; + #clock-cells = <0>; + }; + + xusbxti: oscillator-1 { + compatible = "fixed-clock"; + clock-frequency = <48000000>; + clock-output-names = "xusbxti"; + #clock-cells = <0>; + }; + soc: soc { compatible = "simple-bus"; #address-cells = <1>; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ARM: dts: s3c64xx: factor out external fixed clocks 2019-09-07 2:48 ` [PATCH 1/2] ARM: dts: s3c64xx: factor out external fixed clocks Yao Lihua @ 2019-09-09 18:51 ` krzk 2019-09-10 13:07 ` Lihua Yao 0 siblings, 1 reply; 5+ messages in thread From: krzk @ 2019-09-09 18:51 UTC (permalink / raw) To: Yao Lihua; +Cc: linux-samsung-soc, kgene, linux-arm-kernel On Sat, Sep 07, 2019 at 02:48:08AM +0000, Yao Lihua wrote: > From: Lihua Yao <ylhuajnu@outlook.com> > > As per arch/arm/mach-s3c64xx/common.c, the external oscillators > of S3C6400 and S3C6410 are identical. Move them to s3c64xx.dtsi > and place under root node directly. Hi, Thanks for patches! These are external oscillators so they are not a SoC property. They should be external. They could be moved to their own shared DTSI but I am not sure how much benefit it will bring - it is rather small code duplication. You need to fix the error in different way. However I do not quite understand why moving them to the end of DTS fixed the error - they should be now registered at the end... Best regards, Krzysztof > This introduces side effect of changing the initialization order of > fin_pll and clock-controller@7e00f000. As of commit 3f6d439f2022 > ("clk: reverse default clk provider initialization order in of_clk_init()"), > clock providers are initialized in the orders they are present in the > device tree unless the clocks' dependencies are specified explicitly. > > without this patch: > [ 0.000000] S3C6410 clocks: apll = 0, mpll = 0 > [ 0.000000] epll = 0, arm_clk = 0 > > with this patch: > [ 0.000000] S3C6410 clocks: apll = 532000000, mpll = 532000000 > [ 0.000000] epll = 24000000, arm_clk = 532000000 > > Fixes: 3f6d439f2022 ("clk: reverse default clk provider initialization order in of_clk_init()") > Signed-off-by: Lihua Yao <ylhuajnu@outlook.com> > --- > arch/arm/boot/dts/s3c6410-mini6410.dts | 22 ---------------------- > arch/arm/boot/dts/s3c6410-smdk6410.dts | 22 ---------------------- > arch/arm/boot/dts/s3c64xx.dtsi | 14 ++++++++++++++ > 3 files changed, 14 insertions(+), 44 deletions(-) > > diff --git a/arch/arm/boot/dts/s3c6410-mini6410.dts b/arch/arm/boot/dts/s3c6410-mini6410.dts > index 5201512054c4..7028507b7076 100644 > --- a/arch/arm/boot/dts/s3c6410-mini6410.dts > +++ b/arch/arm/boot/dts/s3c6410-mini6410.dts > @@ -28,28 +28,6 @@ > bootargs = "console=ttySAC0,115200n8 earlyprintk root=/dev/nfs rw nfsroot=192.168.31.2:/srv/nfs/tiny6410,nfsvers=3 ip=dhcp"; > }; > > - clocks { > - compatible = "simple-bus"; > - #address-cells = <1>; > - #size-cells = <0>; > - > - fin_pll: oscillator@0 { > - compatible = "fixed-clock"; > - reg = <0>; > - clock-frequency = <12000000>; > - clock-output-names = "fin_pll"; > - #clock-cells = <0>; > - }; > - > - xusbxti: oscillator@1 { > - compatible = "fixed-clock"; > - reg = <1>; > - clock-output-names = "xusbxti"; > - clock-frequency = <48000000>; > - #clock-cells = <0>; > - }; > - }; > - > srom-cs1@18000000 { > compatible = "simple-bus"; > #address-cells = <1>; > diff --git a/arch/arm/boot/dts/s3c6410-smdk6410.dts b/arch/arm/boot/dts/s3c6410-smdk6410.dts > index a9a5689dc462..10a854b488a8 100644 > --- a/arch/arm/boot/dts/s3c6410-smdk6410.dts > +++ b/arch/arm/boot/dts/s3c6410-smdk6410.dts > @@ -28,28 +28,6 @@ > bootargs = "console=ttySAC0,115200n8 earlyprintk rootwait root=/dev/mmcblk0p1"; > }; > > - clocks { > - compatible = "simple-bus"; > - #address-cells = <1>; > - #size-cells = <0>; > - > - fin_pll: oscillator@0 { > - compatible = "fixed-clock"; > - reg = <0>; > - clock-frequency = <12000000>; > - clock-output-names = "fin_pll"; > - #clock-cells = <0>; > - }; > - > - xusbxti: oscillator@1 { > - compatible = "fixed-clock"; > - reg = <1>; > - clock-output-names = "xusbxti"; > - clock-frequency = <48000000>; > - #clock-cells = <0>; > - }; > - }; > - > srom-cs1@18000000 { > compatible = "simple-bus"; > #address-cells = <1>; > diff --git a/arch/arm/boot/dts/s3c64xx.dtsi b/arch/arm/boot/dts/s3c64xx.dtsi > index 2e611df37911..672764133cea 100644 > --- a/arch/arm/boot/dts/s3c64xx.dtsi > +++ b/arch/arm/boot/dts/s3c64xx.dtsi > @@ -39,6 +39,20 @@ > }; > }; > > + fin_pll: oscillator-0 { > + compatible = "fixed-clock"; > + clock-frequency = <12000000>; > + clock-output-names = "fin_pll"; > + #clock-cells = <0>; > + }; > + > + xusbxti: oscillator-1 { > + compatible = "fixed-clock"; > + clock-frequency = <48000000>; > + clock-output-names = "xusbxti"; > + #clock-cells = <0>; > + }; > + > soc: soc { > compatible = "simple-bus"; > #address-cells = <1>; > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ARM: dts: s3c64xx: factor out external fixed clocks 2019-09-09 18:51 ` krzk @ 2019-09-10 13:07 ` Lihua Yao 0 siblings, 0 replies; 5+ messages in thread From: Lihua Yao @ 2019-09-10 13:07 UTC (permalink / raw) To: krzk; +Cc: linux-samsung-soc, kgene, linux-arm-kernel Hi Krzysztof, On 10/9/2019 2:51 AM, krzk@kernel.org wrote: > On Sat, Sep 07, 2019 at 02:48:08AM +0000, Yao Lihua wrote: >> From: Lihua Yao <ylhuajnu@outlook.com> >> >> As per arch/arm/mach-s3c64xx/common.c, the external oscillators >> of S3C6400 and S3C6410 are identical. Move them to s3c64xx.dtsi >> and place under root node directly. > Hi, > > Thanks for patches! > > These are external oscillators so they are not a SoC property. They > should be external. > > They could be moved to their own shared DTSI but I am not sure how much > benefit it will bring - it is rather small code duplication. OK. > > You need to fix the error in different way. However I do not quite > understand why moving them to the end of DTS fixed the error - they > should be now registered at the end... I moved them to the begin of DTSI so fin_pll will be placed before clock-controller@7e00f000. > > Best regards, > Krzysztof > >> This introduces side effect of changing the initialization order of >> fin_pll and clock-controller@7e00f000. As of commit 3f6d439f2022 >> ("clk: reverse default clk provider initialization order in of_clk_init()"), >> clock providers are initialized in the orders they are present in the >> device tree unless the clocks' dependencies are specified explicitly. >> >> without this patch: >> [ 0.000000] S3C6410 clocks: apll = 0, mpll = 0 >> [ 0.000000] epll = 0, arm_clk = 0 >> >> with this patch: >> [ 0.000000] S3C6410 clocks: apll = 532000000, mpll = 532000000 >> [ 0.000000] epll = 24000000, arm_clk = 532000000 >> >> Fixes: 3f6d439f2022 ("clk: reverse default clk provider initialization order in of_clk_init()") >> Signed-off-by: Lihua Yao <ylhuajnu@outlook.com> >> --- >> arch/arm/boot/dts/s3c6410-mini6410.dts | 22 ---------------------- >> arch/arm/boot/dts/s3c6410-smdk6410.dts | 22 ---------------------- >> arch/arm/boot/dts/s3c64xx.dtsi | 14 ++++++++++++++ >> 3 files changed, 14 insertions(+), 44 deletions(-) >> >> diff --git a/arch/arm/boot/dts/s3c6410-mini6410.dts b/arch/arm/boot/dts/s3c6410-mini6410.dts >> index 5201512054c4..7028507b7076 100644 >> --- a/arch/arm/boot/dts/s3c6410-mini6410.dts >> +++ b/arch/arm/boot/dts/s3c6410-mini6410.dts >> @@ -28,28 +28,6 @@ >> bootargs = "console=ttySAC0,115200n8 earlyprintk root=/dev/nfs rw nfsroot=192.168.31.2:/srv/nfs/tiny6410,nfsvers=3 ip=dhcp"; >> }; >> >> - clocks { >> - compatible = "simple-bus"; >> - #address-cells = <1>; >> - #size-cells = <0>; >> - >> - fin_pll: oscillator@0 { >> - compatible = "fixed-clock"; >> - reg = <0>; >> - clock-frequency = <12000000>; >> - clock-output-names = "fin_pll"; >> - #clock-cells = <0>; >> - }; >> - >> - xusbxti: oscillator@1 { >> - compatible = "fixed-clock"; >> - reg = <1>; >> - clock-output-names = "xusbxti"; >> - clock-frequency = <48000000>; >> - #clock-cells = <0>; >> - }; >> - }; >> - >> srom-cs1@18000000 { >> compatible = "simple-bus"; >> #address-cells = <1>; >> diff --git a/arch/arm/boot/dts/s3c6410-smdk6410.dts b/arch/arm/boot/dts/s3c6410-smdk6410.dts >> index a9a5689dc462..10a854b488a8 100644 >> --- a/arch/arm/boot/dts/s3c6410-smdk6410.dts >> +++ b/arch/arm/boot/dts/s3c6410-smdk6410.dts >> @@ -28,28 +28,6 @@ >> bootargs = "console=ttySAC0,115200n8 earlyprintk rootwait root=/dev/mmcblk0p1"; >> }; >> >> - clocks { >> - compatible = "simple-bus"; >> - #address-cells = <1>; >> - #size-cells = <0>; >> - >> - fin_pll: oscillator@0 { >> - compatible = "fixed-clock"; >> - reg = <0>; >> - clock-frequency = <12000000>; >> - clock-output-names = "fin_pll"; >> - #clock-cells = <0>; >> - }; >> - >> - xusbxti: oscillator@1 { >> - compatible = "fixed-clock"; >> - reg = <1>; >> - clock-output-names = "xusbxti"; >> - clock-frequency = <48000000>; >> - #clock-cells = <0>; >> - }; >> - }; >> - >> srom-cs1@18000000 { >> compatible = "simple-bus"; >> #address-cells = <1>; >> diff --git a/arch/arm/boot/dts/s3c64xx.dtsi b/arch/arm/boot/dts/s3c64xx.dtsi >> index 2e611df37911..672764133cea 100644 >> --- a/arch/arm/boot/dts/s3c64xx.dtsi >> +++ b/arch/arm/boot/dts/s3c64xx.dtsi >> @@ -39,6 +39,20 @@ >> }; >> }; >> >> + fin_pll: oscillator-0 { >> + compatible = "fixed-clock"; >> + clock-frequency = <12000000>; >> + clock-output-names = "fin_pll"; >> + #clock-cells = <0>; >> + }; >> + >> + xusbxti: oscillator-1 { >> + compatible = "fixed-clock"; >> + clock-frequency = <48000000>; >> + clock-output-names = "xusbxti"; >> + #clock-cells = <0>; >> + }; >> + >> soc: soc { >> compatible = "simple-bus"; >> #address-cells = <1>; >> -- >> 2.17.1 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] ARM: dts: s3c64xx: specify dependency of clock providers [not found] <20190907024719.16974-1-ylhuajnu@outlook.com> 2019-09-07 2:48 ` [PATCH 1/2] ARM: dts: s3c64xx: factor out external fixed clocks Yao Lihua @ 2019-09-07 2:48 ` Yao Lihua 2019-09-09 18:53 ` krzk 1 sibling, 1 reply; 5+ messages in thread From: Yao Lihua @ 2019-09-07 2:48 UTC (permalink / raw) To: krzk, kgene, linux-samsung-soc; +Cc: Yao Lihua, linux-arm-kernel From: Lihua Yao <ylhuajnu@outlook.com> fin_pll is the parent of clock-controller@7e00f000, specify the dependency to ensure proper initialization order of clock providers. Fixes: 3f6d439f2022 ("clk: reverse default clk provider initialization order in of_clk_init()") Signed-off-by: Lihua Yao <ylhuajnu@outlook.com> --- arch/arm/boot/dts/s3c6400.dtsi | 1 + arch/arm/boot/dts/s3c6410.dtsi | 1 + 2 files changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/s3c6400.dtsi b/arch/arm/boot/dts/s3c6400.dtsi index 8c28e8a0c824..ef5a8fa3555c 100644 --- a/arch/arm/boot/dts/s3c6400.dtsi +++ b/arch/arm/boot/dts/s3c6400.dtsi @@ -34,5 +34,6 @@ compatible = "samsung,s3c6400-clock"; reg = <0x7e00f000 0x1000>; #clock-cells = <1>; + clocks = <&fin_pll>; }; }; diff --git a/arch/arm/boot/dts/s3c6410.dtsi b/arch/arm/boot/dts/s3c6410.dtsi index a766d6de696c..b201b71d45b5 100644 --- a/arch/arm/boot/dts/s3c6410.dtsi +++ b/arch/arm/boot/dts/s3c6410.dtsi @@ -38,6 +38,7 @@ compatible = "samsung,s3c6410-clock"; reg = <0x7e00f000 0x1000>; #clock-cells = <1>; + clocks = <&fin_pll>; }; i2c1: i2c@7f00f000 { -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ARM: dts: s3c64xx: specify dependency of clock providers 2019-09-07 2:48 ` [PATCH 2/2] ARM: dts: s3c64xx: specify dependency of clock providers Yao Lihua @ 2019-09-09 18:53 ` krzk 0 siblings, 0 replies; 5+ messages in thread From: krzk @ 2019-09-09 18:53 UTC (permalink / raw) To: Yao Lihua; +Cc: linux-samsung-soc, kgene, linux-arm-kernel On Sat, Sep 07, 2019 at 02:48:12AM +0000, Yao Lihua wrote: > From: Lihua Yao <ylhuajnu@outlook.com> > > fin_pll is the parent of clock-controller@7e00f000, specify > the dependency to ensure proper initialization order of clock > providers. > > Fixes: 3f6d439f2022 ("clk: reverse default clk provider initialization order in of_clk_init()") > Signed-off-by: Lihua Yao <ylhuajnu@outlook.com> > --- > arch/arm/boot/dts/s3c6400.dtsi | 1 + > arch/arm/boot/dts/s3c6410.dtsi | 1 + > 2 files changed, 2 insertions(+) Idea looks good but should go to each of DTS files. Best regards, Krzysztof > > diff --git a/arch/arm/boot/dts/s3c6400.dtsi b/arch/arm/boot/dts/s3c6400.dtsi > index 8c28e8a0c824..ef5a8fa3555c 100644 > --- a/arch/arm/boot/dts/s3c6400.dtsi > +++ b/arch/arm/boot/dts/s3c6400.dtsi > @@ -34,5 +34,6 @@ > compatible = "samsung,s3c6400-clock"; > reg = <0x7e00f000 0x1000>; > #clock-cells = <1>; > + clocks = <&fin_pll>; > }; > }; > diff --git a/arch/arm/boot/dts/s3c6410.dtsi b/arch/arm/boot/dts/s3c6410.dtsi > index a766d6de696c..b201b71d45b5 100644 > --- a/arch/arm/boot/dts/s3c6410.dtsi > +++ b/arch/arm/boot/dts/s3c6410.dtsi > @@ -38,6 +38,7 @@ > compatible = "samsung,s3c6410-clock"; > reg = <0x7e00f000 0x1000>; > #clock-cells = <1>; > + clocks = <&fin_pll>; > }; > > i2c1: i2c@7f00f000 { > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-10 13:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190907024719.16974-1-ylhuajnu@outlook.com>
2019-09-07 2:48 ` [PATCH 1/2] ARM: dts: s3c64xx: factor out external fixed clocks Yao Lihua
2019-09-09 18:51 ` krzk
2019-09-10 13:07 ` Lihua Yao
2019-09-07 2:48 ` [PATCH 2/2] ARM: dts: s3c64xx: specify dependency of clock providers Yao Lihua
2019-09-09 18:53 ` krzk
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).