Re: [PATCH 2/3] arm64: dts: amlogic: t7: Add UART controllers nodes

[Ronald Claveau <linux-kernel-dev@aliel.fr>]

Hi Xianwei,

On 4/15/26 1:38 PM, Xianwei Zhao wrote:
> Hi Ronald,
> 
> On 2026/4/15 19:16, Ronald Claveau wrote:
>> Add device tree nodes for UART B through F (serial@7a000 to
>> serial@82000), completing the UART controller description for the T7
>> SoC. Each node includes the peripheral clock.
>>
>> While at it, move the uart_a node to its correct position in the
>> bus address order (0x78000) to comply with the DT requirement that
>> nodes be sorted by their reg address. Complete the
>> uart_a node with its peripheral clock (CLKID_SYS_UART_A) and the
>> associated clock-names, matching the vendor default clock assignment,
>> consistent with the other UART nodes.
>>
>> Signed-off-by: Ronald Claveau<linux-kernel-dev@aliel.fr>
>> ---
>>   arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 61 +++++++++++++++++++
>> ++++++----
>>   1 file changed, 54 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/
>> boot/dts/amlogic/amlogic-t7.dtsi
>> index 531931cc1437c..56b015cfbd6d1 100644
>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>> @@ -577,13 +577,6 @@ gpio_intc: interrupt-controller@4080 {
>>                                          <10 11 12 13 14 15 16 17 18
>> 19 20 21>;
>>                          };
>>
>> -                       uart_a: serial@78000 {
>> -                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> -                               reg = <0x0 0x78000 0x0 0x18>;
>> -                               interrupts = <GIC_SPI 168
>> IRQ_TYPE_EDGE_RISING>;
>> -                               status = "disabled";
>> -                       };
>> -
>>                          gp0: clock-controller@8080 {
>>                                  compatible = "amlogic,t7-gp0-pll";
>>                                  reg = <0x0 0x8080 0x0 0x20>;
>> @@ -713,6 +706,60 @@ pwm_ao_cd: pwm@60000 {
>>                                  status = "disabled";
>>                          };
>>
>> +                       uart_a: serial@78000 {
>> +                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> +                               reg = <0x0 0x78000 0x0 0x18>;
>> +                               interrupts = <GIC_SPI 168
>> IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&xtal>, <&clkc_periphs
>> CLKID_SYS_UART_A>, <&xtal>;
>> +                               clock-names = "xtal", "pclk", "baud";
> 
> The xtal clock is defined in the board-level DTS file, while it is
> referenced in the DTSI file, which seems a bit unusual.
> 
> On other chips, the xtal clock is usually defined directly in the DTSI
> file.
> 

Thanks for your feedback.
I have tested with clock removed in the DTS, and it is ok.
I will send the modification in V2.

>> +                               status = "disabled";
>> +                       };
>> +
>> +                       uart_b: serial@7a000 {
>> +                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> +                               reg = <0x0 0x7a000 0x0 0x18>;
>> +                               interrupts = <GIC_SPI 169
>> IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&xtal>, <&clkc_periphs
>> CLKID_SYS_UART_B>, <&xtal>;
>> +                               clock-names = "xtal", "pclk", "baud";
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       uart_c: serial@7c000 {
>> +                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> +                               reg = <0x0 0x7c000 0x0 0x18>;
>> +                               interrupts = <GIC_SPI 170
>> IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&xtal>, <&clkc_periphs
>> CLKID_SYS_UART_C>, <&xtal>;
>> +                               clock-names = "xtal", "pclk", "baud";
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       uart_d: serial@7e000 {
>> +                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> +                               reg = <0x0 0x7e000 0x0 0x18>;
>> +                               interrupts = <GIC_SPI 171
>> IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&xtal>, <&clkc_periphs
>> CLKID_SYS_UART_D>, <&xtal>;
>> +                               clock-names = "xtal", "pclk", "baud";
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       uart_e: serial@80000 {
>> +                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> +                               reg = <0x0 0x80000 0x0 0x18>;
>> +                               interrupts = <GIC_SPI 172
>> IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&xtal>, <&clkc_periphs
>> CLKID_SYS_UART_E>, <&xtal>;
>> +                               clock-names = "xtal", "pclk", "baud";
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       uart_f: serial@82000 {
>> +                               compatible = "amlogic,t7-uart",
>> "amlogic,meson-s4-uart";
>> +                               reg = <0x0 0x82000 0x0 0x18>;
>> +                               interrupts = <GIC_SPI 173
>> IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&xtal>, <&clkc_periphs
>> CLKID_SYS_UART_F>, <&xtal>;
>> +                               clock-names = "xtal", "pclk", "baud";
>> +                               status = "disabled";
>> +                       };
>> +
>>                          sd_emmc_a: mmc@88000 {
>>                                  compatible = "amlogic,t7-mmc",
>> "amlogic,meson-axg-mmc";
>>                                  reg = <0x0 0x88000 0x0 0x800>;


-- 
Best regards,
Ronald