Re: [PATCH 3/3] arm64: dts: amlogic: t7: Add clock controller nodes

[Jian Hu <jian.hu@amlogic.com>]

Hi, ferass


Thanks for your review.


On 3/11/2026 1:30 AM, Ferass El Hafidi wrote:
> [You don't often get email from funderscore@postmarketos.org. Learn 
> why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> [ EXTERNAL EMAIL ]
>
> On Thu, 05 Mar 2026 07:43, Jian Hu <jian.hu@amlogic.com> wrote:
>> Add the required clock controller nodes for Amlogic T7 SoC family:
>> - SCMI clock controller
>> - PLL clock controller
>> - Peripheral clock controller
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>> arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 125 ++++++++++++++++++++
>> 1 file changed, 125 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi 
>> b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>> index 6510068bcff9..6ea1b583b13d 100644
>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>> @@ -6,6 +6,9 @@
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> #include <dt-bindings/power/amlogic,t7-pwrc.h>
>> #include "amlogic-t7-reset.h"
>> +#include <dt-bindings/clock/amlogic,t7-scmi.h>
>> +#include <dt-bindings/clock/amlogic,t7-pll-clkc.h>
>> +#include <dt-bindings/clock/amlogic,t7-peripherals-clkc.h>
>>
>> / {
>>       interrupt-parent = <&gic>;
>> @@ -201,6 +204,33 @@ pwrc: power-controller {
>>               };
>>       };
>>
>> +      sram@f7042000 {
>> +              compatible = "mmio-sram";
>> +              #address-cells = <1>;
>> +              #size-cells = <1>;
>> +              ranges = <0 0x0 0xf7042000 0x100>;
>> +
>> +              scmi_shmem: sram@0 {
>> +                      compatible = "arm,scmi-shmem";
>> +                      reg = <0x0 0x100>;
>> +              };
>> +      };
>> +
>> +      firmware {
>> +              scmi: scmi {
>> +                      compatible = "arm,scmi-smc";
>> +                      arm,smc-id = <0x820000c1>;
>> +                      shmem = <&scmi_shmem>;
>> +                      #address-cells = <1>;
>> +                      #size-cells = <0>;
>> +
>> +                      scmi_clk: protocol@14 {
>> +                              reg = <0x14>;
>> +                              #clock-cells = <1>;
>> +                      };
>> +              };
>> +      };
>> +
>>       soc {
>>               compatible = "simple-bus";
>>               #address-cells = <2>;
>> @@ -224,6 +254,42 @@ apb4: bus@fe000000 {
>>                       #size-cells = <2>;
>>                       ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
>>
>> +                      clkc_periphs:clock-controller@0 {
>> +                              compatible = 
>> "amlogic,t7-peripherals-clkc";
>> +                              reg = <0x0 0x0 0x0 0x1c8>;
>> +                              #clock-cells = <1>;
>> +                              clocks = <&xtal>,
>> +                                       <&scmi_clk CLKID_SYS_CLK>,
>> +                                       <&scmi_clk CLKID_FIXED_PLL>,
>> +                                       <&scmi_clk CLKID_FCLK_DIV2>,
>> +                                       <&scmi_clk CLKID_FCLK_DIV2P5>,
>> +                                       <&scmi_clk CLKID_FCLK_DIV3>,
>> +                                       <&scmi_clk CLKID_FCLK_DIV4>,
>> +                                       <&scmi_clk CLKID_FCLK_DIV5>,
>> +                                       <&scmi_clk CLKID_FCLK_DIV7>,
>> +                                       <&hifi CLKID_HIFI_PLL>,
>> +                                       <&gp0 CLKID_GP0_PLL>,
>> +                                       <&gp1 CLKID_GP1_PLL>,
>> +                                       <&mpll CLKID_MPLL1>,
>> +                                       <&mpll CLKID_MPLL2>,
>> +                                       <&mpll CLKID_MPLL3>;
>> +                              clock-names = "xtal",
>> +                                            "sys",
>> +                                            "fix",
>> +                                            "fdiv2",
>> +                                            "fdiv2p5",
>> +                                            "fdiv3",
>> +                                            "fdiv4",
>> +                                            "fdiv5",
>> +                                            "fdiv7",
>> +                                            "hifi",
>> +                                            "gp0",
>> +                                            "gp1",
>> +                                            "mpll1",
>> +                                            "mpll2",
>> +                                            "mpll3";
>> +                      };
>> +
>>                       reset: reset-controller@2000 {
>>                               compatible = "amlogic,t7-reset";
>>                               reg = <0x0 0x2000 0x0 0x98>;
>> @@ -234,6 +300,7 @@ watchdog@2100 {
>>                               compatible = "amlogic,t7-wdt";
>>                               reg = <0x0 0x2100 0x0 0x10>;
>>                               clocks = <&xtal>;
>> +
>>                       };
>>
>>                       periphs_pinctrl: pinctrl@4000 {
>> @@ -269,6 +336,64 @@ uart_a: serial@78000 {
>>                               status = "disabled";
>>                       };
>>
>> +                      gp0:clock-controller@8080 {
>> +                              compatible = "amlogic,t7-gp0-pll";
>> +                              reg = <0x0 0x8080 0x0 0x20>;
>> +                              clocks = <&scmi_clk CLKID_TOP_PLL_OSC>;
>> +                              clock-names = "in0";
>> +                              #clock-cells = <1>;
>> +                      };
>
> I would separate `gp0:` and `clock-controller@8080` with a space, like 
> so:
>
>                        gp0: clock-controller@8080 {
>
> Same for the others below (and `clkc_periphs:clock-controller@0` above).


Thanks for the great suggestion! I completely agree that consistent 
spacing improves readability.
I'll update the format to add a space for `gp0: clock-controller@8080`, 
`clkc_periphs: clock-controller@0`
and all other related entries in v2.

>
>> +
>> +                      gp1:clock-controller@80c0 {
>> +                              compatible = "amlogic,t7-gp1-pll";
>> +                              reg = <0x0 0x80c0 0x0 0x14>;
>> +                              clocks = <&scmi_clk CLKID_TOP_PLL_OSC>;
>> +                              clock-names = "in0";
>> +                              #clock-cells = <1>;
>> +                      };
>> +
>> +                      hifi:clock-controller@8100 {
>> +                              compatible = "amlogic,t7-hifi-pll";
>> +                              reg = <0x0 0x8100 0x0 0x20>;
>> +                              clocks = <&scmi_clk CLKID_TOP_PLL_OSC>;
>> +                              clock-names = "in0";
>> +                              #clock-cells = <1>;
>> +                      };
>> +
>> +                      pcie:clock-controller@8140 {
>> +                              compatible = "amlogic,t7-pcie-pll";
>> +                              reg = <0x0 0x8140 0x0 0x1c>;
>> +                              clocks = <&scmi_clk CLKID_PCIE_OSC>;
>> +                              clock-names = "in0";
>> +                              #clock-cells = <1>;
>> +                      };
>> +
>> +                      mpll:clock-controller@8180 {
>> +                              compatible = "amlogic,t7-mpll";
>> +                              reg = <0x0 0x8180 0x0 0x28>;
>> +                              clocks = <&scmi_clk CLKID_FIXED_PLL_DCO>;
>> +                              clock-names = "in0";
>> +                              #clock-cells = <1>;
>> +                      };
>> +
>> +                      hdmi:clock-controller@81c0 {
>> +                              compatible = "amlogic,t7-hdmi-pll";
>> +                              reg = <0x0 0x81c0 0x0 0x20>;
>> +                              clocks = <&scmi_clk CLKID_HDMI_PLL_OSC>;
>> +                              clock-names = "in0";
>> +                              #clock-cells = <1>;
>> +                      };
>> +
>> +                      mclk:clock-controller@8300 {
>> +                              compatible = "amlogic,t7-mclk-pll";
>> +                              reg = <0x0 0x8300 0x0 0x18>;
>> +                              clocks = <&scmi_clk CLKID_MCLK_PLL_OSC>,
>> +                                       <&xtal>,
>> +                                       <&scmi_clk CLKID_FCLK_50M>;
>> +                              clock-names = "in0", "in1", "in2";
>> +                              #clock-cells = <1>;
>> +                      };
>> +
>>                       sec_ao: ao-secure@10220 {
>>                               compatible = "amlogic,t7-ao-secure",
>> "amlogic,meson-gx-ao-secure",
>> -- 
>> 2.47.1
>>
>>
>
> Best regards,
> Ferass
>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic