* [PATCH] arm64: dts: meson: add A1 periphs and PLL clock nodes
@ 2019-12-11 7:08 Jian Hu
2019-12-11 9:43 ` Jerome Brunet
0 siblings, 1 reply; 3+ messages in thread
From: Jian Hu @ 2019-12-11 7:08 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong
Cc: Rob Herring, Victor Wan, Jianxin Pan, devicetree,
Martin Blumenstingl, Kevin Hilman, Michael Turquette,
linux-kernel, Stephen Boyd, Jian Hu, linux-arm-kernel,
Qiufang Dai, linux-amlogic, linux-clk, Chandle Zou
Add A1 periphs and PLL clock controller nodes, Some clocks
in periphs controller are the parents of PLL clocks, Meanwhile
some clocks in PLL controller are those of periphs clocks.
They rely on each other. Compared with the previous series,
the register region is only for the clock. So syscon is not
used in A1.
Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 26 +++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 7210ad049d1d..de43a010fa6e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -5,6 +5,8 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/a1-pll-clkc.h>
+#include <dt-bindings/clock/a1-clkc.h>
/ {
compatible = "amlogic,a1";
@@ -74,6 +76,30 @@
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
+ clkc_periphs: periphs-clock-controller@800 {
+ compatible = "amlogic,a1-periphs-clkc";
+ #clock-cells = <1>;
+ reg = <0 0x800 0 0x104>;
+ clocks = <&clkc_pll CLKID_FCLK_DIV2>,
+ <&clkc_pll CLKID_FCLK_DIV3>,
+ <&clkc_pll CLKID_FCLK_DIV5>,
+ <&clkc_pll CLKID_FCLK_DIV7>,
+ <&clkc_pll CLKID_HIFI_PLL>,
+ <&xtal>;
+ clock-names = "fclk_div2", "fclk_div3",
+ "fclk_div5", "fclk_div7",
+ "hifi_pll", "xtal";
+ };
+
+ clkc_pll: pll-clock-controller@7c80 {
+ compatible = "amlogic,a1-pll-clkc";
+ #clock-cells = <1>;
+ reg = <0 0x7c80 0 0x21c>;
+ clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
+ <&clkc_periphs CLKID_XTAL_HIFIPLL>;
+ clock-names = "xtal_fixpll", "xtal_hifipll";
+ };
+
uart_AO: serial@1c00 {
compatible = "amlogic,meson-gx-uart",
"amlogic,meson-ao-uart";
--
2.24.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] arm64: dts: meson: add A1 periphs and PLL clock nodes
2019-12-11 7:08 [PATCH] arm64: dts: meson: add A1 periphs and PLL clock nodes Jian Hu
@ 2019-12-11 9:43 ` Jerome Brunet
2019-12-11 13:16 ` Jian Hu
0 siblings, 1 reply; 3+ messages in thread
From: Jerome Brunet @ 2019-12-11 9:43 UTC (permalink / raw)
To: Jian Hu, Neil Armstrong
Cc: Rob Herring, Victor Wan, Jianxin Pan, devicetree,
Martin Blumenstingl, Kevin Hilman, Michael Turquette,
linux-kernel, Stephen Boyd, Qiufang Dai, Chandle Zou,
linux-amlogic, linux-clk, linux-arm-kernel
On Wed 11 Dec 2019 at 08:08, Jian Hu <jian.hu@amlogic.com> wrote:
> Add A1 periphs and PLL clock controller nodes, Some clocks
> in periphs controller are the parents of PLL clocks, Meanwhile
> some clocks in PLL controller are those of periphs clocks.
> They rely on each other.
> Compared with the previous series,
> the register region is only for the clock. So syscon is not
> used in A1.
Again, while this is valuable information for the maintainer to keep up,
it is not something that should appear in the commit description.
The evolution of your commit should be described after the '---'
Also, this obviously depends on another series. It should be mentioned
accordingly
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 26 +++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> index 7210ad049d1d..de43a010fa6e 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -5,6 +5,8 @@
>
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/a1-pll-clkc.h>
> +#include <dt-bindings/clock/a1-clkc.h>
When possible, please order the includes alpha-numerically
>
> / {
> compatible = "amlogic,a1";
> @@ -74,6 +76,30 @@
> #size-cells = <2>;
> ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
>
> + clkc_periphs: periphs-clock-controller@800 {
^
From DT spec: "The name of a node should be somewhat generic, reflecting
the function of the device and not its precise programming model."
Here, an appropriate node name would be "clock-controller", not
"periphs-clock-controller"
> + compatible = "amlogic,a1-periphs-clkc";
> + #clock-cells = <1>;
> + reg = <0 0x800 0 0x104>;
> + clocks = <&clkc_pll CLKID_FCLK_DIV2>,
> + <&clkc_pll CLKID_FCLK_DIV3>,
> + <&clkc_pll CLKID_FCLK_DIV5>,
> + <&clkc_pll CLKID_FCLK_DIV7>,
> + <&clkc_pll CLKID_HIFI_PLL>,
> + <&xtal>;
> + clock-names = "fclk_div2", "fclk_div3",
> + "fclk_div5", "fclk_div7",
> + "hifi_pll", "xtal";
> + };
> +
> + clkc_pll: pll-clock-controller@7c80 {
Please order nodes by address when they have one.
This clock controller should appear after the uarts
> + compatible = "amlogic,a1-pll-clkc";
> + #clock-cells = <1>;
> + reg = <0 0x7c80 0 0x21c>;
> + clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
> + <&clkc_periphs CLKID_XTAL_HIFIPLL>;
> + clock-names = "xtal_fixpll", "xtal_hifipll";
> + };
> +
> uart_AO: serial@1c00 {
> compatible = "amlogic,meson-gx-uart",
> "amlogic,meson-ao-uart";
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] arm64: dts: meson: add A1 periphs and PLL clock nodes
2019-12-11 9:43 ` Jerome Brunet
@ 2019-12-11 13:16 ` Jian Hu
0 siblings, 0 replies; 3+ messages in thread
From: Jian Hu @ 2019-12-11 13:16 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong
Cc: Rob Herring, Victor Wan, Jianxin Pan, devicetree,
Martin Blumenstingl, Kevin Hilman, Michael Turquette,
linux-kernel, Stephen Boyd, Qiufang Dai, Chandle Zou,
linux-amlogic, linux-clk, linux-arm-kernel
On 2019/12/11 17:43, Jerome Brunet wrote:
>
> On Wed 11 Dec 2019 at 08:08, Jian Hu <jian.hu@amlogic.com> wrote:
>
>> Add A1 periphs and PLL clock controller nodes, Some clocks
>> in periphs controller are the parents of PLL clocks, Meanwhile
>> some clocks in PLL controller are those of periphs clocks.
>> They rely on each other.
>
>> Compared with the previous series,
>> the register region is only for the clock. So syscon is not
>> used in A1.
>
> Again, while this is valuable information for the maintainer to keep up,
> it is not something that should appear in the commit description.
>
> The evolution of your commit should be described after the '---'
>
OK, I will put the compared message after the '---'
> Also, this obviously depends on another series. It should be mentioned
> accordingly
OK, I will add the dependent clock patchset.
>
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 26 +++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> index 7210ad049d1d..de43a010fa6e 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> @@ -5,6 +5,8 @@
>>
>> #include <dt-bindings/interrupt-controller/irq.h>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/a1-pll-clkc.h>
>> +#include <dt-bindings/clock/a1-clkc.h>
>
> When possible, please order the includes alpha-numerically
>
OK, I will reorder it.
>>
>> / {
>> compatible = "amlogic,a1";
>> @@ -74,6 +76,30 @@
>> #size-cells = <2>;
>> ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
>>
>> + clkc_periphs: periphs-clock-controller@800 {
> ^
>>From DT spec: "The name of a node should be somewhat generic, reflecting
> the function of the device and not its precise programming model."
>
> Here, an appropriate node name would be "clock-controller", not
> "periphs-clock-controller"
OK, I will change the node name.
>
>> + compatible = "amlogic,a1-periphs-clkc";
>> + #clock-cells = <1>;
>> + reg = <0 0x800 0 0x104>;
>> + clocks = <&clkc_pll CLKID_FCLK_DIV2>,
>> + <&clkc_pll CLKID_FCLK_DIV3>,
>> + <&clkc_pll CLKID_FCLK_DIV5>,
>> + <&clkc_pll CLKID_FCLK_DIV7>,
>> + <&clkc_pll CLKID_HIFI_PLL>,
>> + <&xtal>;
>> + clock-names = "fclk_div2", "fclk_div3",
>> + "fclk_div5", "fclk_div7",
>> + "hifi_pll", "xtal";
>> + };
>> +
>> + clkc_pll: pll-clock-controller@7c80 {
>
> Please order nodes by address when they have one.
> This clock controller should appear after the uarts
OK, I will reorder it.
>
>> + compatible = "amlogic,a1-pll-clkc";
>> + #clock-cells = <1>;
>> + reg = <0 0x7c80 0 0x21c>;
>> + clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
>> + <&clkc_periphs CLKID_XTAL_HIFIPLL>;
>> + clock-names = "xtal_fixpll", "xtal_hifipll";
>> + };
>> +
>> uart_AO: serial@1c00 {
>> compatible = "amlogic,meson-gx-uart",
>> "amlogic,meson-ao-uart";
>
> .
>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-11 13:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 7:08 [PATCH] arm64: dts: meson: add A1 periphs and PLL clock nodes Jian Hu
2019-12-11 9:43 ` Jerome Brunet
2019-12-11 13:16 ` Jian Hu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®