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

[Neil Armstrong <neil.armstrong@linaro.org>]

On 8/25/26 10:54, Ronald Claveau wrote:
> 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<tanure@linux.com>
>>>>>>> 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-
>>>>> v4-0-9505df0e7016@aliel.fr/
>>>>>
>>>>> 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.

This is what was done for qualcomm SoCs, we could do it for Amlogic aswell.

> 
> 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)?
> 

No since it describes a SoC configuration using XTAL, it should be in the dtsi.

Neil


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