| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250509002201.g2db6cf5w4mtow6k@bryanbrattlof.com>
Date: Thu, 8 May 2025 19:22:01 -0500
From: Bryan Brattlof <bb@...com>
To: Paresh Bhagat <p-bhagat@...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>
Subject: Re: [PATCH v3 3/3] arm64: dts: ti: Add support for AM62D2-EVM
On May 8, 2025 thus sayeth Paresh Bhagat:
> AM62D-EVM evaluation module (EVM) is a low-cost expandable platform board
> designed for TI’s AM62D2 SoC. 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 µ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
>
> AM62A7 and AM62D2 SoCs share several peripherals in wakeup, mcu, thermal,
> and portions of the main domain. To improve reuse and reduce duplication,
> common *-wakeup.dtsi, *-mcu.dtsi, *-thermal.dtsi, and *-main.dtsi files
> have been introduced. Each board will have a dedicated DTS file that
> includes both the shared and SoC-specific .dtsi files.
>
> Schematics Link - https://www.ti.com/lit/zip/sprcal5
>
> Signed-off-by: Paresh Bhagat <p-bhagat@...com>
> ---
> arch/arm64/boot/dts/ti/Makefile | 3 +
> .../dts/ti/k3-am62a-am62d-common-main.dtsi | 1013 +++++++++++++++++
> ...cu.dtsi => k3-am62a-am62d-common-mcu.dtsi} | 26 +-
> ...tsi => k3-am62a-am62d-common-thermal.dtsi} | 2 +-
> ...dtsi => k3-am62a-am62d-common-wakeup.dtsi} | 2 +-
> arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 1005 ----------------
> arch/arm64/boot/dts/ti/k3-am62a.dtsi | 9 +-
> arch/arm64/boot/dts/ti/k3-am62d.dtsi | 123 ++
> arch/arm64/boot/dts/ti/k3-am62d2-evm.dts | 533 +++++++++
> arch/arm64/boot/dts/ti/k3-am62d2.dtsi | 155 +++
> 10 files changed, 1837 insertions(+), 1034 deletions(-)
> create mode 100644 arch/arm64/boot/dts/ti/k3-am62a-am62d-common-main.dtsi
> rename arch/arm64/boot/dts/ti/{k3-am62a-mcu.dtsi => k3-am62a-am62d-common-mcu.dtsi} (86%)
> rename arch/arm64/boot/dts/ti/{k3-am62a-thermal.dtsi => k3-am62a-am62d-common-thermal.dtsi} (94%)
> rename arch/arm64/boot/dts/ti/{k3-am62a-wakeup.dtsi => k3-am62a-am62d-common-wakeup.dtsi} (97%)
> create mode 100644 arch/arm64/boot/dts/ti/k3-am62d.dtsi
> create mode 100644 arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
> create mode 100644 arch/arm64/boot/dts/ti/k3-am62d2.dtsi
>
So this diff was kinda a slog and I'm not sure I went through everything
but I called out some things I saw.
I'll let others chime in here with their opinions but we should probably
find a way to split this up a little more next time if possible.
...
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-am62d-common-main.dtsi
> b/arch/arm64/boot/dts/ti/k3-am62a-am62d-common-main.dtsi
> new file mode 100644
> index 000000000000..570a6413165d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am62a-am62d-common-main.dtsi
> @@ -0,0 +1,1013 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Device Tree file for the MAIN domain peripherals shared by AM62A and AM62D
> + *
> + * Copyright (C) 2022-2024 Texas Instruments Incorporated - https://www.ti.com/
My thinking is this is a new file so we should probably update the
copyright year while we're here
...
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a.dtsi b/arch/arm64/boot/dts/ti/k3-am62a.dtsi
> index 4d79b3e9486a..e9f28343a4c1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62a.dtsi
> @@ -118,10 +118,13 @@ cbass_wakeup: bus@...000 {
> };
> };
>
> - #include "k3-am62a-thermal.dtsi"
> + #include "k3-am62a-am62d-common-thermal.dtsi"
I know we're trying to reduce the boilerplate for very similar chips but
I think a better way to split this would be to have an industrial and
automotive qualified thermal #include here rather than a 62D/62A
I'm assuming from the marketing material this will be the same
automotive qualified AMB package so we could call it something along
those lines if we want.
...
>
> /* Now include the peripherals for each bus segments */
> +#include "k3-am62a-am62d-common-main.dtsi"
> +#include "k3-am62a-am62d-common-mcu.dtsi"
> +#include "k3-am62a-am62d-common-wakeup.dtsi"
> +
> +/* Include AM62P specific peripherals */
s/AM62P/AM62D/
...
> diff --git a/arch/arm64/boot/dts/ti/k3-am62d.dtsi b/arch/arm64/boot/dts/ti/k3-am62d.dtsi
> new file mode 100644
> index 000000000000..606da1c1f1bc
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am62d.dtsi
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Device Tree Source for AM62D SoC Family
> + *
> + * Copyright (C) 2022-2024 Texas Instruments Incorporated - https://www.ti.com/
2025
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/soc/ti,sci_pm_domain.h>
> +
> +#include "k3-pinctrl.h"
> +
> +/ {
> + model = "Texas Instruments K3 AM62D SoC";
> + compatible = "ti,am62d2";
> + interrupt-parent = <&gic500>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
No need for this
...
> diff --git a/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts b/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
> new file mode 100644
> index 000000000000..0873c2523607
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
> @@ -0,0 +1,533 @@
...
> +
> + chosen {
> + stdout-path = "serial2:115200n8";
> + };
Look at the 62P and how we used a &phandle for this.
...
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + /* global cma region */
> + linux,cma {
> + compatible = "shared-dma-pool";
> + reusable;
> + size = <0x00 0x2000000>;
> + alloc-ranges = <0x00 0xc0000000 0x00 0x2000000>;
> + linux,cma-default;
> + };
> +
> + c7x_0_dma_memory_region: c7x-dma-memory@...00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x99800000 0x00 0x100000>;
> + no-map;
> + };
> +
> + c7x_0_memory_region: c7x-memory@...00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x99900000 0x00 0xf00000>;
> + no-map;
> + };
> +
> + mcu_r5fss0_core0_dma_memory_region: r5f-dma-memory@...00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9b800000 0x00 0x100000>;
> + no-map;
> + };
> +
> + mcu_r5fss0_core0_memory_region: r5f-dma-memory@...00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9b900000 0x00 0xf00000>;
> + no-map;
> + };
> +
> + wkup_r5fss0_core0_dma_memory_region: r5f-dma-memory@...00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9c800000 0x00 0x100000>;
> + no-map;
> + };
> +
> + wkup_r5fss0_core0_memory_region: r5f-dma-memory@...00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0x9c900000 0x00 0xf00000>;
> + no-map;
> + };
> +
> + secure_tfa_ddr: tfa@...00000 {
> + reg = <0x00 0x80000000 0x00 0x80000>;
Just a nit pick but while we're respinning this series could you sort
this? It just makes our lives easier when we inevitably changes this
when we want to experiment with some other firmware or try to move
things when someone wants to use a smaller DDR part
> + alignment = <0x1000>;
> + no-map;
> + };
> +
> + secure_ddr: optee@...00000 {
> + reg = <0x00 0x9e800000 0x00 0x01800000>; /* for OP-TEE */
> + alignment = <0x1000>;
> + no-map;
> + };
> + };
...
> +
> +&mcu_pmx0 {
> + bootph-all;
Look at how the 62P does this. Newer U-Boot versions are smart enough to
only prune things if none of their children have an appropriate bootph-*
flag. So as long as we have a bootph-all flag in this following
&wkup_uart0_pins_default the &mcu_pmx0 will not be pruned
It makes things much cleaner and avoids us having to pull in SoC.dtsi
stuff here to make the bootloaders happy
> +
> + wkup_uart0_pins_default: wkup-uart0-default-pins {
> + pinctrl-single,pins = <
> + AM62DX_MCU_IOPAD(0x0024, PIN_INPUT, 0) /* (C9) WKUP_UART0_RXD */
> + AM62DX_MCU_IOPAD(0x0028, PIN_OUTPUT, 0) /* (E9) WKUP_UART0_TXD */
> + AM62DX_MCU_IOPAD(0x002c, PIN_INPUT, 0) /* (C10) WKUP_UART0_CTSn */
> + AM62DX_MCU_IOPAD(0x0030, PIN_OUTPUT, 0) /* (C8) WKUP_UART0_RTSn */
> + >;
> + bootph-all;
> + };
> +};
> +
> +/* WKUP UART0 is used for DM firmware logs */
> +&wkup_uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&wkup_uart0_pins_default>;
> + status = "reserved";
> + bootph-all;
> +};
> +
> +&main_pmx0 {
> + bootph-all;
same here
...
> +&secure_proxy_main {
> + bootph-all;
> +};
So this is where things get weird. The only way U-Boot's R5 SPL will
talk to TIFS is via the secure proxy thread. It's a requirement of any
board that uses both the 62A and 63D chip well all the 62* really.
So rather than defining this on every board we should probably just
throw it in with the &secure_proxy_main node rather than calling it out
here.
> +
> +&dmsc {
> + bootph-all;
> +};
> +
> +&k3_pds {
> + bootph-all;
> +};
> +
> +&k3_clks {
> + bootph-all;
> +};
> +
> +&k3_reset {
> + bootph-all;
> +};
> +
> +&wkup_conf {
> + bootph-all;
> +};
> +
> +&chipid {
> + bootph-all;
> +};
Same with these. The chipid, clocks, power domains will all be needed
for booting the SoC regardless of how someone wires the chip. These
should just go in their respective nodes and be done with it rather than
each board calling this out.
...
> +
> +&k3_reset {
> + bootph-all;
> +};
> +
> +&dmsc {
> + bootph-all;
> +};
I've already snipped this email quite a bit, but I know I've seen these
duplicates. Double check we're not defining things multiple times.
...
> 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..47566fea4157
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am62d2.dtsi
> @@ -0,0 +1,155 @@
> +// 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/zip/spruj16
> + *
> + * Copyright (C) 2020-2024 Texas Instruments Incorporated - https://www.ti.com/
s/2024/2025
~Bryan
Powered by blists - more mailing lists