Re: [PATCH 1/2] arm64: dts: amlogic: t7: use the real UART pclk

On 8/24/26 11:32 AM, Lucas Tanure wrote:
> On 24/08/2026 10:15, Ronald Claveau wrote:
>> On 8/23/26 1:39 PM, Lucas Tanure wrote:
>>> On 19/08/2026 08:25, Ronald Claveau wrote:
>>>> On 8/19/26 4:39 AM, Xianwei Zhao wrote:
>>>>> Hi Lucas,
>>>>>
>>>>> On 2026/8/19 03:11, Lucas Tanure wrote:
>>>>>> uart_a lists the 24MHz crystal for all three of its clocks, because
>>>>>> the T7 clock controller driver did not exist when these boards were
>>>>>> added.
>>>>>>
>>>>>> That leaves the real UART bus clock without a user, so the kernel
>>>>>> turns it off when it disables unused clocks at the end of boot, and
>>>>>> the board hangs.
>>>>>>
>>>>>> Point uart_a at the real clocks, the way meson-s4.dtsi does, and drop
>>>>>> the placeholders from the two board files.
>>>>>>
>>>>>> Fixes: 4fef056588f5 ("arm64: dts: amlogic-t7-a311d2-khadas-vim4: add
>>>>>> initial device-tree")
>>>>>> Fixes: 6f048cc7a635 ("arm64: dts: add board AN400")
>>>>>> Signed-off-by: Lucas Tanure<[email protected]>
>>>>>> Assisted-by: Claude:claude-fable-5
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-an400.dts      
>>>>>> | 2 --
>>>>>>     arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
>>>>>> | 2 --
>>>>>>     arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi                   | 4
>>>>>> ++++
>>>>>>     3 files changed, 4 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-an400.dts
>>>>>> b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-an400.dts
>>>>>> index cab2ee9ea0d3..dcbcd08a78b9 100644
>>>>>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-an400.dts
>>>>>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-an400.dts
>>>>>> @@ -33,7 +33,5 @@ xtal: xtal-clk {
>>>>>>     };
>>>>>>
>>>>>>     &uart_a {
>>>>>> -       clocks = <&xtal>, <&xtal>,
>>>>>> <&xtal>;
>>>>>> -       clock-names = "xtal", "pclk", "baud";
>>>>>>            status = "okay";
>>>>>>     };
>>>>>> diff --git
>>>>>> a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
>>>>>> b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
>>>>>> index c41525a34b72..677069e58f30 100644
>>>>>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
>>>>>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
>>>>>> @@ -266,6 +266,4 @@ &sd_emmc_c {
>>>>>>
>>>>>>     &uart_a {
>>>>>>            status = "okay";
>>>>>> -       clocks = <&xtal>, <&xtal>,
>>>>>> <&xtal>;
>>>>>> -       clock-names = "xtal", "pclk", "baud";
>>>>>>     };
>>>>>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>>>>> b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>>>>> index cc371fcd1896..7847582e77ed 100644
>>>>>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>>>>> @@ -587,6 +587,10 @@ 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";
>>>>>>                                    status = "disabled";
>>>>>>                            };
>>>>>
>>>>> I agree with moving the UART clock configuration to the DTSI file.
>>>>> However, it seems a little odd to keep the XTAL clock definition in
>>>>> the
>>>>> DTS, as the alias may not be consistent across different boards, which
>>>>> could result in compilation errors. Could we move the XTAL clock
>>>>> definition to the DTSI as well, similar to other Amlogic SoCs?
>>>>
>>>> It seems similar to changes in this series :
>>>>
>>>> https://lore.kernel.org/all/20260420-add-bluetooth-t7-vim4-
>>>> [email protected]/
>>>>
>>>> What do you think ?
>>>>
>>> Thanks for pointing out that series, though I wasn't aware of it.
>>> And after reading I don't agree with it. The clocks are not redundant,
>>> they are there because we agreed sometime ago that xtal belongs to the
>>> board files.
>>> So I still vote for my change in v2, that I will send in a few minutes.
>>
>> I don't get why it would be better to define the exact same clocks in
>> each DTS file.
>>
>> The xtal recommended characteristics provided at the SOC level can be
>> defined in the DTSI and overridden, if really necessary, in the DTS.
>>
> Hi Ronald,
> 
> The preference for keeping xtal in board-specific .dts files rather than
> the SoC .dtsi comes down to DT hardware modeling principles and upstream
> maintainer guidelines:
> 
> Hardware Topology: The xtal is physically located on the board PCB, not
> inside the SoC silicon. The .dtsi file models the SoC silicon internal
> architecture (clock controllers, IPs, registers), whereas .dts files
> describe the physical board layout surrounding it.
> 
> Avoiding Implicit Assumptions: Defining a default 24MHz xtal in .dtsi
> assumes every T7 board will use the same oscillator configuration. If a
> custom board design uses a different crystal frequency or an external
> clock generator/TCXO, overriding a .dtsi-level node can lead to messy DT
> overrides or silent clock rate mismatches if forgotten.
> 
> Upstream DT Maintainer Policy: Kernel DT maintainers consistently push
> to keep board-level hardware explicitly declared in the board .dts files
> to reflect actual physical board components.
> 
> That said, I acknowledge the trade-off is minor boilerplate duplication
> across board DTS files. If the Amlogic SoC maintainers prefer setting a
> default 24MHz xtal in amlogic-t7.dtsi to match older SoC generations, I
> am open to updating it—provided the maintainers explicitly prefer that
> tradeoff over strict DT topology modeling.
> 

Hi Lucas,

Thank you for this detailed explanation.
I was referring to the last paragraph of the DTS coding style:

Hardware components that are present on the board shall be placed in the
board DTS, not in the SoC or SoM DTSI. A partial exception is a common
external reference SoC input clock, which could be coded as a
fixed-clock in the SoC DTSI with its frequency provided by each board DTS.

I understand going to strict topology is better, that means we must
apply the same rules to new Amlogic SOCs, right ?

Do we need to move other clocks and assigned-clocks properties which use
the xtal to the board DTS as well (such as clkc_periphs or sd_emmc)?

-- 
Best regards,
Ronald