Message-ID: <cb52029a-d1c8-47c8-bf4b-0de44601a9ef@ti.com>
Date: Fri, 4 Jul 2025 14:30:33 +0530
From: Paresh Bhagat <p-bhagat@...com>
To: Bryan Brattlof <bb@...com>
CC: <nm@...com>, <vigneshr@...com>, <praneeth@...com>, <kristo@...nel.org>,
        <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <khasim@...com>, <v-singh1@...com>,
        <afd@...com>, <devarsht@...com>, <s-vadapalli@...com>,
        <andrew@...n.ch>
Subject: Re: [PATCH v5 4/4] arm64: dts: ti: Add support for AM62D2-EVM

Hi Bryan,


On 03/07/25 17:50, Bryan Brattlof wrote:
> On July  3, 2025 thus sayeth Paresh Bhagat:
>> Hi Bryan,
>>
>>
>> On 01/07/25 21:55, Bryan Brattlof wrote:
>>> On June 27, 2025 thus sayeth Paresh Bhagat:
>>>> AM62D-EVM evaluation module (EVM) is a low-cost expandable platform board
>>>> designed for AM62D2 SoC from TI. It supports the following interfaces:
>>>>
>>>> * 4 GB LPDDR4 RAM
>>>> * x2 Gigabit Ethernet expansion connectors
>>>> * x4 3.5mm TRS Audio Jack Line In
>>>> * x4 3.5mm TRS Audio Jack Line Out
>>>> * x2 Audio expansion connectors
>>>> * x1 Type-A USB 2.0, x1 Type-C dual-role device (DRD) USB 2.0
>>>> * x1 UHS-1 capable micro SD card slot
>>>> * 32 GB eMMC Flash
>>>> * 512 Mb OSPI NOR flash
>>>> * x4 UARTs via USB 2.0-B
>>>> * XDS110 for onboard JTAG debug using USB
>>>> * Temperature sensors, user push buttons and LEDs
>>>>
>>>> Although AM62D2 and AM62A7 differ in peripheral capabilities example
>>>> multimedia, VPAC, and display subsystems, the core architecture remains
>>>> same. To reduce duplication, AM62D support reuses the AM62A dtsi and the
>>>> necessary overrides will be handled in SOC specific dtsi file and a
>>>> board specific dts.
>>>>
>>>> Add basic support for AM62D2-EVM.
>>>>
>>>> Schematics Link - https://www.ti.com/lit/zip/sprcal5
>>>>
>>>> Signed-off-by: Paresh Bhagat <p-bhagat@...com>
>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
>>>> ---
>>>>    arch/arm64/boot/dts/ti/Makefile          |   3 +
>>>>    arch/arm64/boot/dts/ti/k3-am62d2-evm.dts | 599 +++++++++++++++++++++++
>>>>    arch/arm64/boot/dts/ti/k3-am62d2.dtsi    |  25 +
>>>>    3 files changed, 627 insertions(+)
>>>>    create mode 100644 arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
>>>>    create mode 100644 arch/arm64/boot/dts/ti/k3-am62d2.dtsi
>>>>
>>> ...
>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62d2.dtsi
>>>> b/arch/arm64/boot/dts/ti/k3-am62d2.dtsi
>>>> new file mode 100644
>>>> index 000000000000..70aeb40872a9
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am62d2.dtsi
>>>> @@ -0,0 +1,25 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
>>>> +/*
>>>> + * Device Tree Source for AM62D2 SoC family in Quad core configuration
>>>> + *
>>>> + * TRM: https://www.ti.com/lit/pdf/sprujd4
>>>> + *
>>>> + * Copyright (C) 2025 Texas Instruments Incorporated - https://www.ti.com/
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +#include "k3-am62a7.dtsi"
>>> If we want to reuse the AM62A chassis I think we should probably reused
>>> the AM62AX_IOPAD() macro instead of creating a new one.
>>
>> AM62D does not necessarily have the same pin configuration compared to
>> AM62A. While it is a macro which could be shareable across many boards, i
>> think its preferable we maintain separate definitions to highlight the new
>> SoCs. AM62D is a separate package, with some components reused from AM62a.
> I guess I don't understand the need to create a new padconfig macro when
> we say, in device tree syntax, the AM62D and AM62A uses the same RTL
> chassis. The pinout will always change with packaging changes but this
> will not change the padconfig MMR layout.
>
> All that said. It's just a name and honestly when you look at all these
> macros we haven't changed the padconfig layout for any K3 chip so it not
> a big deal to me. If it helps people when grepping around i'll relent ;)


Makes sense. Thanks
>
>>
>>>> +
>>>> +/ {
>>>> +	model = "Texas Instruments K3 AM62D SoC";
>>>> +	compatible = "ti,am62d2";
>>>> +};
>>>> +
>>>> +&vpu {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&e5010 {
>>>> +	status = "disabled";
>>>> +};
>>> So I could be a little out of date on the style guidelines here, but my
>>> intuition is device trees, much like real trees, can only grow, so we
>>> can't inherit the am62a.dtsi and remove things.
>>>
>>> My understanding is we have to create a full am62d.dtsi with its
>>> features that the am62a.dtsi can extend with the vpu{} and e5010{} nodes
>>>
>>> ~Bryan
>>
>> Agree we should ideally keep the device trees extending. But in this case it
>> will involve changes not only in am62a.dtsi but ideally it will change
>> k3-am62a-main.dtsi and k3-am62a-mcu.dtsi as well. This moves us back to
>> version 3 of this series
>> https://lore.kernel.org/all/20250508091422.288876-1-p-bhagat@ti.com/ where i
>> created *common*.dtsi files which looks a bit complex.
>>
>>
>> The current method also ensures that customers can start their development
>> of a new board with k3-am62d2.dtsi, while maintaining less complexity and is
>> a easier to follow approach.
> The issue I take with this approach is what does 'status = "disabled"'
> mean now. Historically (for TI SoCs at least) it indicated the node was
> incomplete and would need to be extended in the board.dts to function
> properly. But now we're trying to say for these two nodes the hardware
> doesn't exist on this SoC and bad things will happen if you enable them.
>
> My recommendation is to try to flip this around. The am62a7.dtsi should
> inherit the am62d2.dtsi and add the vpu{} and e5010{} nodes. I agree we
> don't need to try to combine the two as we did in v3 just yet but we
> should try to keep the device trees growing as we inherit things.
>
> ~Bryan


Yep makes sense. I guess deleting the node will be better than having 
'status = "disabled"' for components which are not supported/absent. 
Also it will avoid the need to flip around and change the existing am62a 
structure. I will send a new version with the updates. Thanks.
>
>>

Powered by blists - more mailing lists