| Message ID | 20220208131827.1430086-1-vigneshr@ti.com |
|---|---|
| Headers | show |
| Series | arm64: Initial support for Texas Instruments AM62 Platform | expand |
On 09/02/2022 20:04, Vignesh Raghavendra wrote: > > > On 08/02/22 10:31 pm, Krzysztof Kozlowski wrote: >> On 08/02/2022 14:18, Vignesh Raghavendra wrote: >>> From: Nishanth Menon <nm@ti.com> >>> >>> The AM62 SoC family is the follow on AM335x built on K3 Multicore SoC >>> architecture platform, providing ultra-low-power modes, dual display, >>> multi-sensor edge compute, security and other BOM-saving integration. >>> The AM62 SoC targets broad market to enable applications such as >>> Industrial HMI, PLC/CNC/Robot control, Medical Equipment, Building >>> Automation, Appliances and more. >>> >>> Some highlights of this SoC are: >>> >>> * Quad-Cortex-A53s (running up to 1.4GHz) in a single cluster. >>> Pin-to-pin compatible options for single and quad core are available. >>> * Cortex-M4F for general-purpose or safety usage. >>> * Dual display support, providing 24-bit RBG parallel interface and >>> OLDI/LVDS-4 Lane x2, up to 200MHz pixel clock support for 2K display >>> resolution. >>> * Selectable GPUsupport, up to 8GFLOPS, providing better user experience >>> in 3D graphic display case and Android. >>> * PRU(Programmable Realtime Unit) support for customized programmable >>> interfaces/IOs. >>> * Integrated Giga-bit Ethernet switch supporting up to a total of two >>> external ports (TSN capable). >>> * 9xUARTs, 5xSPI, 6xI2C, 2xUSB2, 3xCAN-FD, 3x eMMC and SD, GPMC for >>> NAND/FPGA connection, OSPI memory controller, 3xMcASP for audio, >>> 1x CSI-RX-4L for Camera, eCAP/eQEP, ePWM, among other peripherals. >>> * Dedicated Centralized System Controller for Security, Power, and >>> Resource Management. >>> * Multiple low power modes support, ex: Deep sleep,Standby, MCU-only, >>> enabling battery powered system design. >>> >>> AM625 is the first device of the family. Add DT bindings for the same. >> >> Don't paste the same huge commit description in several commits. > > Sorry, I think this is the first commit with full description. I will > probably trim 4/5 at bit > >> >>> >>> More details can be found in the Technical Reference Manual: >>> https://www.ti.com/lit/pdf/spruiv7 >>> >>> Signed-off-by: Nishanth Menon <nm@ti.com> >>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >>> --- >>> Documentation/devicetree/bindings/arm/ti/k3.yaml | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/arm/ti/k3.yaml b/Documentation/devicetree/bindings/arm/ti/k3.yaml >>> index b03c10fa2e7a..64f3db3ea9dd 100644 >>> --- a/Documentation/devicetree/bindings/arm/ti/k3.yaml >>> +++ b/Documentation/devicetree/bindings/arm/ti/k3.yaml >>> @@ -53,6 +53,12 @@ properties: >>> - ti,am642-sk >>> - const: ti,am642 >>> >>> + - description: K3 AM625 SoC >>> + items: >>> + - enum: >>> + - ti,am625-sk >>> + - const: ti,am625 >> >> Why keeping it not alphabetically sorted? What sorting did you choose? >> > > Above list is not sorted alphabetically, I tried to keep similar SoCs > bunched together. AM625 and AM642 are of same family, hence chose to add > the new entry here. Then maybe it should be before AM642? > One alternative is to add it to end of the list (chronologically)? > Or I can add a patch to sort the list alphabetically first and then > introduce new compatible. Please let me know your preference? It's not that important, just wondering. I propose to avoid putting at the end, because this causes conflicts in case of concurrent work. If I had to choose, I would propose to sort SoCs by name. Either way is fine - with trimmed commit msg in patch 4 or here: Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
On 19:10-20220209, Marc Zyngier wrote: [...] > > +&cbass_main { > > + gic500: interrupt-controller@1800000 { > > + compatible = "arm,gic-v3"; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + #interrupt-cells = <3>; > > + interrupt-controller; > > + reg = <0x00 0x01800000 0x00 0x10000>, /* GICD */ > > + <0x00 0x01880000 0x00 0xC0000>; /* GICR */ > > Usual rant: you are missing the GICC, GICH and GICV regions > that are implemented by the CPU. Cortex-A53 implements them > (they are not optional), so please describe them. > -ECONFUSED. TRM for GIC500 refers to just GICD, GICR and ITS range[1]. Same thing is indicated by Generic Interrupt Controller Architecture Specification[2] See table 1-1 (page 23). I think you are expecting GICV3's backward compatibility mode (Table 1-2 in page 24), But in K3 architecture, are_option meant for backward compatibility is set to true (aka no backward compatibility). I think this did popup sometime back as well (first k3 SoC)[3]. I think the more clearer description is available in [4]. I believe the argumentation that GICC/H/V is mandatory for A53 if GIC500 is used is not accurate. Please correct me if I am mistaken. [1] https://developer.arm.com/documentation/ddi0516/e/programmers-model/the-gic-500-register-map?lang=en [2] https://developer.arm.com/documentation/ihi0069/d [3] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20180607233853.p7iw7nlxxuyi66og@kahuna/ [4] https://developer.arm.com/documentation/ddi0516/e/functional-description/operation/backwards-compatibility?lang=en
On 11:33-20220211, Marc Zyngier wrote: > On Thu, 10 Feb 2022 19:34:59 +0000, > Nishanth Menon <nm@ti.com> wrote: > > > > On 19:10-20220209, Marc Zyngier wrote: > > [...] > > > > > > +&cbass_main { > > > > + gic500: interrupt-controller@1800000 { > > > > + compatible = "arm,gic-v3"; > > > > + #address-cells = <2>; > > > > + #size-cells = <2>; > > > > + ranges; > > > > + #interrupt-cells = <3>; > > > > + interrupt-controller; > > > > + reg = <0x00 0x01800000 0x00 0x10000>, /* GICD */ > > > > + <0x00 0x01880000 0x00 0xC0000>; /* GICR */ > > > > > > Usual rant: you are missing the GICC, GICH and GICV regions > > > that are implemented by the CPU. Cortex-A53 implements them > > > (they are not optional), so please describe them. > > > > > > > > > -ECONFUSED. TRM for GIC500 refers to just GICD, GICR and ITS range[1]. > > And I'm not talking about the GIC, but of the CPU interface. The fact > that we describe both in the GIC binding doesn't mean they are > implemented by the same IP block (and the architecture is quite clear > about that). > > > Same thing is indicated by Generic Interrupt Controller Architecture > > Specification[2] See table 1-1 (page 23). > > > > I think you are expecting GICV3's backward compatibility mode (Table 1-2 > > in page 24), But in K3 architecture, are_option meant for backward > > compatibility is set to true (aka no backward compatibility). I think > > this did popup sometime back as well (first k3 SoC)[3]. I think the more > > clearer description is available in [4]. > > No, this description is for the architecture as a whole. ARE being > disabled *int the GIC* doesn't mean it is disabled overall, and the > CPU is free to implement the CPU interface by any mean it wants as > long as it communicates with the GIC using the Stream Protocol. > Cortex-A32, A34, 35, A53, A57, A72 and A73 all implement both the > sysreg and MMIO CPU interfaces. Later ARM CPUs don't. Both can work > with GIC500. > > > I believe the argumentation that GICC/H/V is mandatory for A53 if GIC500 > > is used is not accurate. Please correct me if I am mistaken. > > GIC500 is not involved at all, and A53 always implements both the > system register and MMIO interfaces. See the A53 TRM, chapter 9. The > only way to disable this interface is to assert GICCDISABLE, which > disables the whole of the CPU interface. Given that you have a (more > or less) functional system, it probably isn't the case. > > See Table 9-1, which tells you where these registers are as an offset > from PERIPHBASE. Dumping these registers should show you that they are > indeed implemented and not solely a figment of my own imagination. Thanks for explaining.. I don't see this is working in practise.. Let me know if I am making a mistake in my interpretation. Quote from our internal integration spec (yep it leaves it to ARM cluster's use): "" Note: GIC periphery base tieoff to ARM corepacs for GIC v2 compatibility requires a dedicated unallocated space to be passed as input to ARM corepac. The CC internal region 0F00_0000-0x0F03_FFFF is assigned as GIC periphery base tieoff to the corepac. When GIC-500 is in v3 mode, and A72 with GICCDISABLE=0 and PERIPHBASE set: - the CPU interface registers are accessed via ICC* system register. - the GICC* regions (PERIPHBASE - PERIPHBASE+0x3FFFF) are reserved and access will be Read as Zero / Write Ignored. So any writes/reads to this region would be trapped by ARM corepacs. "" Anyways, Here is my report. I checked across all K3 devices (a72 and a53) AM65x: PERIPH_BASE = 0x6f000000 (a53) j721e: PERIPH_BASE = 0x6f000000 (a72) J7200: PERIPH_BASE = 0x6f000000 (a72) j721s2: PERIPH_BASE = 0x6f000000 (a72) AM64: PERIPH_BASE = 0x100000000 (a53) AM62: PERIPH_BASE = 0x100000000 (a53) (side note: am64/62 needed the 0x6f.. address space for PCIe stuff.. but the address chosen has nothing in SoC fabric) Tested at u-boot shell prompt (running at EL2): If I understood the expectation correctly..I should be seeing offsets off [1]. Taking 'CPU Interface'/GICC as an example, [2] should be the registers I should be seeing. aka, at offset 0xfc from PERIPHBASE, i should see 0x0034443B. Note: on K3 devices (in the 32bit address space), as in the description above, we have a null endpoint handler in the bus fabric that responds with 0x0 for read requests for invalid/reserved addresses. What I see is 0x0 (and not IIDR) in all the address ranges - which matches ARM sending that region requests straight down to SoC level and SoC returning "ignore".. On AM62, I attached Lauterbach. and tried to look at the addresses: [3] from cpu view and from bus view. I also checked from kernel side with devmem to make sure to dump while kernel GICV3 is active.. I see the same thing as well.. Is there something TFA or someone has to do to "enable" this? I tried re-reading porting-guide.rst yet again to make sure we have'nt missed anything. [1] https://developer.arm.com/documentation/ddi0500/j/Generic-Interrupt-Controller-CPU-Interface/GIC-programmers-model/Memory-map?lang=en [2] https://developer.arm.com/documentation/ddi0500/j/Generic-Interrupt-Controller-CPU-Interface/GIC-programmers-model/CPU-interface-register-summary [3] https://pasteboard.co/3O44PAwLeAXz.png