Re: [PATCH v4 2/3] arm64: dts: ti: k3-am62l: add initial infrastructure

From: Bryan Brattlof
Date: Fri Apr 11 2025 - 14:26:29 EST

On April 9, 2025 thus sayeth krzk@xxxxxxxxxx:
> On Mon, Apr 07, 2025 at 10:34:39AM GMT, Bryan Brattlof wrote:
> > From: Vignesh Raghavendra <vigneshr@xxxxxx>
> >
> > Add the initial infrastructure needed for the AM62L. ALl of which can be
> > found in the Technical Reference Manual (TRM) located here:
> >
> > https://www.ti.com/lit/pdf/sprujb4
> >
> > Signed-off-by: Vignesh Raghavendra <vigneshr@xxxxxx>
> > Signed-off-by: Bryan Brattlof <bb@xxxxxx>
> > ---
> > Changes in v3:
> > - Added more nodes now that the SCMI interface is ready
> >
> > Changes in v1:
> > - switched to non-direct links to TRM updates are automatic
> > - fixed white space indent issues with a few nodes
> > - separated out device tree bindings
> > ---
> > arch/arm64/boot/dts/ti/Makefile | 3 +
> > arch/arm64/boot/dts/ti/k3-am62l-main.dtsi | 672 +++++++++++++++++++++++++++
> > arch/arm64/boot/dts/ti/k3-am62l-thermal.dtsi | 19 +
> > arch/arm64/boot/dts/ti/k3-am62l-wakeup.dtsi | 144 ++++++
> > arch/arm64/boot/dts/ti/k3-am62l.dtsi | 121 +++++
> > arch/arm64/boot/dts/ti/k3-am62l3.dtsi | 67 +++
> > arch/arm64/boot/dts/ti/k3-pinctrl.h | 2 +
> > 7 files changed, 1028 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
> > index 03d4cecfc001c..93df47282add3 100644
> > --- a/arch/arm64/boot/dts/ti/Makefile
> > +++ b/arch/arm64/boot/dts/ti/Makefile
> > @@ -32,6 +32,9 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62-lp-sk-nand.dtbo
> > dtb-$(CONFIG_ARCH_K3) += k3-am62a7-sk.dtb
> > dtb-$(CONFIG_ARCH_K3) += k3-am62a7-phyboard-lyra-rdk.dtb
> >
> > +# Boards with AM62Lx SoCs
> > +dtb-$(CONFIG_ARCH_K3) += k3-am62l3-evm.dtb
> > +
> > # Boards with AM62Px SoC
> > dtb-$(CONFIG_ARCH_K3) += k3-am62p5-sk.dtb
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi
> > new file mode 100644
> > index 0000000000000..697181c2e7f51
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/ti/k3-am62l-main.dtsi
> > @@ -0,0 +1,672 @@
> > +// SPDX-License-Identifier: GPL-2.0-only or MIT
> > +/*
> > + * Device Tree file for the AM62L main domain peripherals
> > + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
> > + *
> > + * Technical Reference Manual: https://www.ti.com/lit/pdf/sprujb4
> > + */
> > +
> > +&cbass_main {
> > + gic500: interrupt-controller@1800000 {
> > + compatible = "arm,gic-v3";
> > + reg = <0x00 0x01800000 0x00 0x10000>, /* GICD */
> > + <0x00 0x01840000 0x00 0xc0000>, /* GICR */
> > + <0x01 0x00000000 0x00 0x2000>, /* GICC */
> > + <0x01 0x00010000 0x00 0x1000>, /* GICH */
> > + <0x01 0x00020000 0x00 0x2000>; /* GICV */
> > + ranges;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + #interrupt-cells = <3>;
> > + interrupt-controller;
> > + /*
> > + * vcpumntirq:
> > + * virtual CPU interface maintenance interrupt
> > + */
> > + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > + gic_its: msi-controller@1820000 {
> > + compatible = "arm,gic-v3-its";
> > + reg = <0x00 0x01820000 0x00 0x10000>;
> > + socionext,synquacer-pre-its = <0x1000000 0x400000>;
> > + msi-controller;
> > + #msi-cells = <1>;
> > + };
> > + };
> > +
> > + gpio0: gpio@600000 {
> > + compatible = "ti,am64-gpio", "ti,keystone-gpio";
> > + reg = <0x00 0x00600000 0x00 0x100>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + interrupt-parent = <&gic500>;
> > + interrupts = <GIC_SPI 260 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 261 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 266 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 267 IRQ_TYPE_EDGE_RISING>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + power-domains = <&scmi_pds 34>;
> > + clocks = <&scmi_clk 140>;
> > + clock-names = "gpio";
> > + ti,ngpio = <126>;
> > + ti,davinci-gpio-unbanked = <0>;
> > + status = "disabled";
> > + };
> > +
> > + gpio2: gpio@610000 {
> > + compatible = "ti,am64-gpio", "ti,keystone-gpio";
>
> 64 or 62?
> > + reg = <0x00 0x00610000 0x00 0x100>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + interrupt-parent = <&gic500>;
> > + interrupts = <GIC_SPI 280 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 281 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 282 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 283 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 284 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 285 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 286 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 287 IRQ_TYPE_EDGE_RISING>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + power-domains = <&scmi_pds 35>;
> > + clocks = <&scmi_clk 141>;
> > + clock-names = "gpio";
> > + ti,ngpio = <79>;
> > + ti,davinci-gpio-unbanked = <0>;
> > + status = "disabled";
> > + };
> > +
> > + timer0: timer@2400000 {
> > + compatible = "ti,am654-timer";
>
> 64? 654? 62? You need to use proper compatibles matching the hardware
> (see writing bindings).

So most of the K3 generation of TI's SoCs will reuse the IP as long as
they can. My understanding was to pick the compatible of the platform
that introduced the driver for that particular IP. Are we suggesting we
add a compatible for the 62L?

>
> > + reg = <0x00 0x2400000 0x00 0x400>;
> > + interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&scmi_clk 47>;
> > + clock-names = "fck";
> > + power-domains = <&scmi_pds 15>;
> > + ti,timer-pwm;
> > + };
> > +
>
> ...
>
> > + chipid: chipid@14 {
> > + compatible = "ti,am654-chipid";
> > + reg = <0x14 0x4>;
> > + bootph-all;
> > + };
> > +
> > + usb0_phy_ctrl: syscon@45000 {
> > + compatible = "ti,am62-usb-phy-ctrl", "syscon";
> > + reg = <0x45000 0x4>;
> > + bootph-all;
> > + };
> > +
> > + usb1_phy_ctrl: syscon@45004 {
> > + compatible = "ti,am62-usb-phy-ctrl", "syscon";
> > + reg = <0x45004 0x4>;
>
> No, you do not get syscon per register. The entire point of syscon is to
> collect ALL registers. Your device is the syscon, not a register.
>

My understanding from [0] was that we would need to break this up into
smaller syscon nodes because the alternative would be to mark the entire
region as a syscon and every other node using it would need to use it's
base + offset which was kinda undesirable especially for the small
number of drivers that need data from this region.

a-device {
clocks = <&epwm_tbclk 0>;
};

~Bryan

[0] https://lore.kernel.org/lkml/20250122-topic-am62-dt-syscon-v6-13-v1-2-515d56edc35e@xxxxxxxxxxxx/

> Best regards,
> Krzysztof
>