[2/3] arm64: dts: ti: Introduce J722S family of SoCs

  The J722S is a family of  application processors built for Automotive and
Linux Application development. Scalable Arm Cortex-A53 performance and
embedded features, such as multi high-definition display support,
3D-graphics acceleration, 4K video acceleration, and extensive peripherals
make the J722S well-suited for a broad range of automation and industrial
application, including automotive digital instrumentation, automotive
displays, industrial HMI, and more.

Some highlights of J722S SoC are:
    * Quad-Cortex-A53s (running up to 1.4GHz) in a single cluster.
    * One Device manager Cortext-R5F for system power and resource
      management, two Cortex-R5F for Functional Safety or
      general-purpose usage and two C7x floating point vector DSPs
      with Matrix Multiply Accelerator(MMA) for deep learning.
    * One 3D GPU up to 50 GLFOPS
    * H.264/H.265, JPEG Video Encode/Decode.
    * Display support: 3x display support over OLDI/LVDS (1x OLDI-DL, 1x or
      2x OLDI-SL), DSI, or DPI. Up to 3840x1080@60fps resolution
    * Integrated Giga-bit Ethernet switch supporting up to a total of two
      external ports (TSN capable).
    * Vision Processing Accelerator (VPAC) with image signal processor
      and Depth and Motion Processing Accelerator (DMPAC).
    * 7xUARTs, 3xSPI, 5xI2C, 2xUSB2, 2xCAN-FD, 3xMMC and SD, GPMC for
      NAND/FPGA connection, OSPI memory controller, 5xMcASP for audio,
      4xCSI-RX for Camera, 1 PCIe Gen3 controller, eCAP/eQEP,
      ePWM, among other peripherals.
    * Dedicated Centralized Hardware Security Module with support for
      secure boot, debug security and crypto acceleration and trusted
      execution environment.
    * One 32-bit DDR Subsystem that supports LPDDR4 memory type.

J722S family of SoCs is a superset of the AM62P SoC family and shares
similar memory map, thus the nodes are being reused from AM62P includes
instead of duplicating the definitions. Unused nodes with dependencies
are kept disabled and will be enabled later when the dependencies are
enabled.

For those interested, more details about this SoC can be found in the
Technical Reference Manual here:
	https://www.ti.com/lit/zip/sprujb3

Co-developed-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---

checkpatch error is ignored for arch/arm64/boot/dts/ti/k3-pinctrl.h:
ERROR: Macros with complex values should be enclosed in parentheses

However, we do not need parentheses enclosing the values for this
macro as we do intend it to generate two separate values as has been
done for other similar platforms.

Depends on:
https://lore.kernel.org/all/20231211132600.25289-1-vaishnav.a@ti.com/
https://lore.kernel.org/all/20231213081318.26203-1-vaishnav.a@ti.com/

 arch/arm64/boot/dts/ti/k3-j722s.dtsi | 275 +++++++++++++++++++++++++++
 arch/arm64/boot/dts/ti/k3-pinctrl.h  |   3 +
 2 files changed, 278 insertions(+)
 create mode 100644 arch/arm64/boot/dts/ti/k3-j722s.dtsi
  

On 18:19-20231213, Vaishnav Achath wrote:
> The J722S is a family of  application processors built for Automotive and
> Linux Application development. Scalable Arm Cortex-A53 performance and
> embedded features, such as multi high-definition display support,
> 3D-graphics acceleration, 4K video acceleration, and extensive peripherals
> make the J722S well-suited for a broad range of automation and industrial
> application, including automotive digital instrumentation, automotive
> displays, industrial HMI, and more.

Trim this down please
> 
> Some highlights of J722S SoC are:
>     * Quad-Cortex-A53s (running up to 1.4GHz) in a single cluster.
>     * One Device manager Cortext-R5F for system power and resource
>       management, two Cortex-R5F for Functional Safety or
>       general-purpose usage and two C7x floating point vector DSPs
>       with Matrix Multiply Accelerator(MMA) for deep learning.
>     * One 3D GPU up to 50 GLFOPS
>     * H.264/H.265, JPEG Video Encode/Decode.
>     * Display support: 3x display support over OLDI/LVDS (1x OLDI-DL, 1x or
>       2x OLDI-SL), DSI, or DPI. Up to 3840x1080@60fps resolution
>     * Integrated Giga-bit Ethernet switch supporting up to a total of two
>       external ports (TSN capable).
>     * Vision Processing Accelerator (VPAC) with image signal processor
>       and Depth and Motion Processing Accelerator (DMPAC).
>     * 7xUARTs, 3xSPI, 5xI2C, 2xUSB2, 2xCAN-FD, 3xMMC and SD, GPMC for
>       NAND/FPGA connection, OSPI memory controller, 5xMcASP for audio,
>       4xCSI-RX for Camera, 1 PCIe Gen3 controller, eCAP/eQEP,
>       ePWM, among other peripherals.
>     * Dedicated Centralized Hardware Security Module with support for
>       secure boot, debug security and crypto acceleration and trusted
>       execution environment.
>     * One 32-bit DDR Subsystem that supports LPDDR4 memory type.

Trim this down to what is different from AM62P?

> 
> J722S family of SoCs is a superset of the AM62P SoC family and shares
> similar memory map, thus the nodes are being reused from AM62P includes
> instead of duplicating the definitions. Unused nodes with dependencies
> are kept disabled and will be enabled later when the dependencies are
> enabled.
> 
> For those interested, more details about this SoC can be found in the
> Technical Reference Manual here:
> 	https://www.ti.com/lit/zip/sprujb3
> 
> Co-developed-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> ---
> 
> checkpatch error is ignored for arch/arm64/boot/dts/ti/k3-pinctrl.h:
> ERROR: Macros with complex values should be enclosed in parentheses
> 
> However, we do not need parentheses enclosing the values for this
> macro as we do intend it to generate two separate values as has been
> done for other similar platforms.
> 
> Depends on:
> https://lore.kernel.org/all/20231211132600.25289-1-vaishnav.a@ti.com/
> https://lore.kernel.org/all/20231213081318.26203-1-vaishnav.a@ti.com/
> 
>  arch/arm64/boot/dts/ti/k3-j722s.dtsi | 275 +++++++++++++++++++++++++++
>  arch/arm64/boot/dts/ti/k3-pinctrl.h  |   3 +
>  2 files changed, 278 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/ti/k3-j722s.dtsi
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s.dtsi b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> new file mode 100644
> index 000000000000..dbd762bfd779
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for J722S SoC Family
> + *
> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +
> +#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 J722S SoC";
> +	compatible = "ti,j722s";
> +	interrupt-parent = <&gic500>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu-map {
> +			cluster0: cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
> +
> +				core1 {
> +					cpu = <&cpu1>;
> +				};
> +
> +				core2 {
> +					cpu = <&cpu2>;
> +				};
> +
> +				core3 {
> +					cpu = <&cpu3>;
> +				};
> +			};
> +		};
> +
> +		cpu0: cpu@0 {
> +			compatible = "arm,cortex-a53";
> +			reg = <0x000>;
> +			device_type = "cpu";
> +			enable-method = "psci";
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			next-level-cache = <&l2_0>;
> +			clocks = <&k3_clks 135 0>;
> +		};
> +
> +		cpu1: cpu@1 {
> +			compatible = "arm,cortex-a53";
> +			reg = <0x001>;
> +			device_type = "cpu";
> +			enable-method = "psci";
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			next-level-cache = <&l2_0>;
> +			clocks = <&k3_clks 136 0>;
> +		};
> +
> +		cpu2: cpu@2 {
> +			compatible = "arm,cortex-a53";
> +			reg = <0x002>;
> +			device_type = "cpu";
> +			enable-method = "psci";
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			next-level-cache = <&l2_0>;
> +			clocks = <&k3_clks 137 0>;
> +		};
> +
> +		cpu3: cpu@3 {
> +			compatible = "arm,cortex-a53";
> +			reg = <0x003>;
> +			device_type = "cpu";
> +			enable-method = "psci";
> +			i-cache-size = <0x8000>;
> +			i-cache-line-size = <64>;
> +			i-cache-sets = <256>;
> +			d-cache-size = <0x8000>;
> +			d-cache-line-size = <64>;
> +			d-cache-sets = <128>;
> +			next-level-cache = <&l2_0>;
> +			clocks = <&k3_clks 138 0>;
> +		};
> +	};
> +
> +	l2_0: l2-cache0 {
> +		compatible = "cache";
> +		cache-unified;
> +		cache-level = <2>;
> +		cache-size = <0x80000>;
> +		cache-line-size = <64>;
> +		cache-sets = <512>;
> +	};

^^ this is a duplication of am62p5.dtsi? what about the spins with
different CPUs enabled?

> +
> +	firmware {
> +		optee {
> +			compatible = "linaro,optee-tz";
> +			method = "smc";
> +		};
> +
> +		psci: psci {
> +			compatible = "arm,psci-1.0";
> +			method = "smc";
> +		};
> +	};
> +
> +	a53_timer0: timer-cl0-cpu0 {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>, /* cntpsirq */
> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, /* cntpnsirq */
> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, /* cntvirq */
> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; /* cnthpirq */
> +	};
> +
> +	pmu: pmu {
> +		compatible = "arm,cortex-a53-pmu";
> +		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +	};
> +
> +	cbass_main: bus@f0000 {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		bootph-all;

Nope.

> +
> +		ranges = <0x00 0x000f0000 0x00 0x000f0000 0x00 0x00030000>, /* Main MMRs */
> +			 <0x00 0x00420000 0x00 0x00420000 0x00 0x00001000>, /* ESM0 */
> +			 <0x00 0x00600000 0x00 0x00600000 0x00 0x00001100>, /* GPIO */
> +			 <0x00 0x00703000 0x00 0x00703000 0x00 0x00000200>, /* USB0 debug trace */
> +			 <0x00 0x0070c000 0x00 0x0070c000 0x00 0x00000200>, /* USB1 debug trace */
> +			 <0x00 0x00a40000 0x00 0x00a40000 0x00 0x00000800>, /* Timesync router */
> +			 <0x00 0x01000000 0x00 0x01000000 0x00 0x01b28400>, /* First peripheral window */
> +			 <0x00 0x08000000 0x00 0x08000000 0x00 0x00200000>, /* Main CPSW */
> +			 <0x00 0x0e000000 0x00 0x0e000000 0x00 0x01d20000>, /* Second peripheral window */
> +			 <0x00 0x0fd80000 0x00 0x0fd80000 0x00 0x00080000>, /* GPU */
> +			 <0x00 0x0fd20000 0x00 0x0fd20000 0x00 0x00000100>, /* JPEGENC0_CORE */
> +			 <0x00 0x0fd20200 0x00 0x0fd20200 0x00 0x00000200>, /* JPEGENC0_CORE_MMU */
> +			 <0x00 0x20000000 0x00 0x20000000 0x00 0x0a008000>, /* Third peripheral window */
> +			 <0x00 0x30040000 0x00 0x30040000 0x00 0x00080000>, /* PRUSS-M */
> +			 <0x00 0x301C0000 0x00 0x301C0000 0x00 0x00001000>, /* DPHY-TX */
> +			 <0x00 0x30101000 0x00 0x30101000 0x00 0x00080100>, /* CSI window */
> +			 <0x00 0x30200000 0x00 0x30200000 0x00 0x00010000>, /* DSS */
> +			 <0x00 0x30210000 0x00 0x30210000 0x00 0x00010000>, /* VPU */
> +			 <0x00 0x30220000 0x00 0x30220000 0x00 0x00010000>, /* DSS1 */
> +			 <0x00 0x30270000 0x00 0x30270000 0x00 0x00010000>, /* DSI-base1 */
> +			 <0x00 0x30500000 0x00 0x30500000 0x00 0x00100000>, /* DSI-base2 */
> +			 <0x00 0x31000000 0x00 0x31000000 0x00 0x00050000>, /* USB0 DWC3 Core window */
> +			 <0x00 0x31200000 0x00 0x31200000 0x00 0x00040000>, /* USB1 DWC3 Core window */
> +			 <0x00 0x40900000 0x00 0x40900000 0x00 0x00030000>, /* SA3UL */
> +			 <0x00 0x43600000 0x00 0x43600000 0x00 0x00010000>, /* SA3 sproxy data */
> +			 <0x00 0x44043000 0x00 0x44043000 0x00 0x00000fe0>, /* TI SCI DEBUG */
> +			 <0x00 0x44860000 0x00 0x44860000 0x00 0x00040000>, /* SA3 sproxy config */
> +			 <0x00 0x48000000 0x00 0x48000000 0x00 0x06408000>, /* DMSS */
> +			 <0x00 0x60000000 0x00 0x60000000 0x00 0x08000000>, /* FSS0 DAT1 */
> +			 <0x00 0x70000000 0x00 0x70000000 0x00 0x00040000>, /* OCSRAM */
> +			 <0x00 0x78400000 0x00 0x78400000 0x00 0x00008000>, /* MAIN R5FSS0 ATCM */
> +			 <0x00 0x78500000 0x00 0x78500000 0x00 0x00008000>, /* MAIN R5FSS0 BTCM */
> +			 <0x00 0x7e000000 0x00 0x7e000000 0x00 0x00200000>, /* C7X_0 L2SRAM */
> +			 <0x00 0x7e200000 0x00 0x7e200000 0x00 0x00200000>, /* C7X_1 L2SRAM */
> +			 <0x01 0x00000000 0x01 0x00000000 0x00 0x00310000>, /* A53 PERIPHBASE */
> +			 <0x05 0x00000000 0x05 0x00000000 0x01 0x00000000>, /* FSS0 DAT3 */
> +
> +			 /* MCU Domain Range */
> +			 <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>,
> +			 <0x00 0x79000000 0x00 0x79000000 0x00 0x00008000>,
> +			 <0x00 0x79020000 0x00 0x79020000 0x00 0x00008000>,
> +			 <0x00 0x79100000 0x00 0x79100000 0x00 0x00040000>,
> +			 <0x00 0x79140000 0x00 0x79140000 0x00 0x00040000>,
> +
> +			 /* Wakeup Domain Range */
> +			 <0x00 0x00b00000 0x00 0x00b00000 0x00 0x00002400>,
> +			 <0x00 0x2b000000 0x00 0x2b000000 0x00 0x00300400>,
> +			 <0x00 0x43000000 0x00 0x43000000 0x00 0x00020000>,
> +			 <0x00 0x78000000 0x00 0x78000000 0x00 0x00008000>,
> +			 <0x00 0x78100000 0x00 0x78100000 0x00 0x00008000>;
> +
> +		cbass_mcu: bus@4000000 {
> +			compatible = "simple-bus";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>, /* Peripheral window */
> +				 <0x00 0x79000000 0x00 0x79000000 0x00 0x00008000>, /* MCU R5 ATCM */
> +				 <0x00 0x79020000 0x00 0x79020000 0x00 0x00008000>, /* MCU R5 BTCM */
> +				 <0x00 0x79100000 0x00 0x79100000 0x00 0x00040000>, /* MCU IRAM0 */
> +				 <0x00 0x79140000 0x00 0x79140000 0x00 0x00040000>; /* MCU IRAM1 */
> +			bootph-all;
> +		};
> +
> +		cbass_wakeup: bus@b00000 {
> +			compatible = "simple-bus";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges = <0x00 0x00b00000 0x00 0x00b00000 0x00 0x00002400>, /* VTM */
> +				 <0x00 0x2b000000 0x00 0x2b000000 0x00 0x00300400>, /* Peripheral Window */
> +				 <0x00 0x43000000 0x00 0x43000000 0x00 0x00020000>, /* WKUP CTRL MMR */
> +				 <0x00 0x78000000 0x00 0x78000000 0x00 0x00008000>, /* DM R5 ATCM*/
> +				 <0x00 0x78100000 0x00 0x78100000 0x00 0x00008000>; /* DM R5 BTCM*/
> +			bootph-all;

Not in the bus nodes.. only in the leaf nodes please.


Also what is wrong with expanding the am62p to include the
missing bus segments instead of duplicating all of them?

> +		};
> +	};
> +
> +	#include "k3-am62p-thermal.dtsi"

Is this correct?

> +};
> +
> +/*
> + * Include peripherals for each bus segment derived
> + * from AM62P and overrides specific to J722S.
> + */
> +#include "k3-am62p-main.dtsi"
> +#include "k3-am62p-mcu.dtsi"
> +#include "k3-am62p-wakeup.dtsi"
> +
> +/* Main domain overrides */
> +
> +&cpsw3g {
> +	status = "disabled";

	here and rest: Why disabled?
> +};
> +
> +&inta_main_dmss {
> +	ti,interrupt-ranges = <5 69 35>;
> +};
> +
> +&mailbox0_cluster0 {
> +	status = "disabled";
> +};
> +
> +&mailbox0_cluster1 {
> +	status = "disabled";
> +};
> +
> +&mailbox0_cluster2 {
> +	status = "disabled";
> +};
> +
> +&mailbox0_cluster3 {
> +	status = "disabled";
> +};
> +
> +&oc_sram {
> +	reg = <0x00 0x70000000 0x00 0x40000>;
> +	ranges = <0x00 0x00 0x70000000 0x40000>;
> +};
> +
> +/* MCU domain overrides */
> +
> +&mcu_r5fss0 {
> +	status = "disabled";
> +};
> +
> +/* wakeup domain overrides */
> +
> +&wkup_r5fss0 {
> +	status = "disabled";
> +};
> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> index 2a4e0e084d69..591be4489f37 100644
> --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
> +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> @@ -59,6 +59,9 @@
>  #define J721S2_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>  #define J721S2_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>  
> +#define J722S_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
> +#define J722S_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
> +
>  #define J784S4_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>  #define J784S4_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>  
> -- 
> 2.17.1
>
  

Hi Nishanth,

On 14/12/23 01:57, Nishanth Menon wrote:
> On 18:19-20231213, Vaishnav Achath wrote:
>> The J722S is a family of  application processors built for Automotive and
>> Linux Application development. Scalable Arm Cortex-A53 performance and
>> embedded features, such as multi high-definition display support,
>> 3D-graphics acceleration, 4K video acceleration, and extensive peripherals
>> make the J722S well-suited for a broad range of automation and industrial
>> application, including automotive digital instrumentation, automotive
>> displays, industrial HMI, and more.
> 
> Trim this down please
>>
>> Some highlights of J722S SoC are:
>>     * Quad-Cortex-A53s (running up to 1.4GHz) in a single cluster.
>>     * One Device manager Cortext-R5F for system power and resource
>>       management, two Cortex-R5F for Functional Safety or
>>       general-purpose usage and two C7x floating point vector DSPs
>>       with Matrix Multiply Accelerator(MMA) for deep learning.
>>     * One 3D GPU up to 50 GLFOPS
>>     * H.264/H.265, JPEG Video Encode/Decode.
>>     * Display support: 3x display support over OLDI/LVDS (1x OLDI-DL, 1x or
>>       2x OLDI-SL), DSI, or DPI. Up to 3840x1080@60fps resolution
>>     * Integrated Giga-bit Ethernet switch supporting up to a total of two
>>       external ports (TSN capable).
>>     * Vision Processing Accelerator (VPAC) with image signal processor
>>       and Depth and Motion Processing Accelerator (DMPAC).
>>     * 7xUARTs, 3xSPI, 5xI2C, 2xUSB2, 2xCAN-FD, 3xMMC and SD, GPMC for
>>       NAND/FPGA connection, OSPI memory controller, 5xMcASP for audio,
>>       4xCSI-RX for Camera, 1 PCIe Gen3 controller, eCAP/eQEP,
>>       ePWM, among other peripherals.
>>     * Dedicated Centralized Hardware Security Module with support for
>>       secure boot, debug security and crypto acceleration and trusted
>>       execution environment.
>>     * One 32-bit DDR Subsystem that supports LPDDR4 memory type.
> 
> Trim this down to what is different from AM62P?
> 

Thanks for the review, I will trim this down in next revision, but the above is
just a summary of the main features of this SoC, pointing to AM62P feature set
here seems confusing to me. why does a new user/developer using J722S need to be
aware of the existence of AM62P to just understand a high level summary about
this device?

>>
>> J722S family of SoCs is a superset of the AM62P SoC family and shares
>> similar memory map, thus the nodes are being reused from AM62P includes
>> instead of duplicating the definitions. Unused nodes with dependencies
>> are kept disabled and will be enabled later when the dependencies are
>> enabled.
>>
>> For those interested, more details about this SoC can be found in the
>> Technical Reference Manual here:
>> 	https://www.ti.com/lit/zip/sprujb3
>>
>> Co-developed-by: Jayesh Choudhary <j-choudhary@ti.com>
>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>> ---
>>
>> checkpatch error is ignored for arch/arm64/boot/dts/ti/k3-pinctrl.h:
>> ERROR: Macros with complex values should be enclosed in parentheses
>>
>> However, we do not need parentheses enclosing the values for this
>> macro as we do intend it to generate two separate values as has been
>> done for other similar platforms.
>>
>> Depends on:
>> https://lore.kernel.org/all/20231211132600.25289-1-vaishnav.a@ti.com/
>> https://lore.kernel.org/all/20231213081318.26203-1-vaishnav.a@ti.com/
>>
>>  arch/arm64/boot/dts/ti/k3-j722s.dtsi | 275 +++++++++++++++++++++++++++
>>  arch/arm64/boot/dts/ti/k3-pinctrl.h  |   3 +
>>  2 files changed, 278 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/ti/k3-j722s.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j722s.dtsi b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
>> new file mode 100644
>> index 000000000000..dbd762bfd779
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
>> @@ -0,0 +1,275 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Device Tree Source for J722S SoC Family
>> + *
>> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
>> + */
>> +
>> +#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 J722S SoC";
>> +	compatible = "ti,j722s";
>> +	interrupt-parent = <&gic500>;
>> +	#address-cells = <2>;
>> +	#size-cells = <2>;
>> +
>> +	cpus {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		cpu-map {
>> +			cluster0: cluster0 {
>> +				core0 {
>> +					cpu = <&cpu0>;
>> +				};
>> +
>> +				core1 {
>> +					cpu = <&cpu1>;
>> +				};
>> +
>> +				core2 {
>> +					cpu = <&cpu2>;
>> +				};
>> +
>> +				core3 {
>> +					cpu = <&cpu3>;
>> +				};
>> +			};
>> +		};
>> +
>> +		cpu0: cpu@0 {
>> +			compatible = "arm,cortex-a53";
>> +			reg = <0x000>;
>> +			device_type = "cpu";
>> +			enable-method = "psci";
>> +			i-cache-size = <0x8000>;
>> +			i-cache-line-size = <64>;
>> +			i-cache-sets = <256>;
>> +			d-cache-size = <0x8000>;
>> +			d-cache-line-size = <64>;
>> +			d-cache-sets = <128>;
>> +			next-level-cache = <&l2_0>;
>> +			clocks = <&k3_clks 135 0>;
>> +		};
>> +
>> +		cpu1: cpu@1 {
>> +			compatible = "arm,cortex-a53";
>> +			reg = <0x001>;
>> +			device_type = "cpu";
>> +			enable-method = "psci";
>> +			i-cache-size = <0x8000>;
>> +			i-cache-line-size = <64>;
>> +			i-cache-sets = <256>;
>> +			d-cache-size = <0x8000>;
>> +			d-cache-line-size = <64>;
>> +			d-cache-sets = <128>;
>> +			next-level-cache = <&l2_0>;
>> +			clocks = <&k3_clks 136 0>;
>> +		};
>> +
>> +		cpu2: cpu@2 {
>> +			compatible = "arm,cortex-a53";
>> +			reg = <0x002>;
>> +			device_type = "cpu";
>> +			enable-method = "psci";
>> +			i-cache-size = <0x8000>;
>> +			i-cache-line-size = <64>;
>> +			i-cache-sets = <256>;
>> +			d-cache-size = <0x8000>;
>> +			d-cache-line-size = <64>;
>> +			d-cache-sets = <128>;
>> +			next-level-cache = <&l2_0>;
>> +			clocks = <&k3_clks 137 0>;
>> +		};
>> +
>> +		cpu3: cpu@3 {
>> +			compatible = "arm,cortex-a53";
>> +			reg = <0x003>;
>> +			device_type = "cpu";
>> +			enable-method = "psci";
>> +			i-cache-size = <0x8000>;
>> +			i-cache-line-size = <64>;
>> +			i-cache-sets = <256>;
>> +			d-cache-size = <0x8000>;
>> +			d-cache-line-size = <64>;
>> +			d-cache-sets = <128>;
>> +			next-level-cache = <&l2_0>;
>> +			clocks = <&k3_clks 138 0>;
>> +		};
>> +	};
>> +
>> +	l2_0: l2-cache0 {
>> +		compatible = "cache";
>> +		cache-unified;
>> +		cache-level = <2>;
>> +		cache-size = <0x80000>;
>> +		cache-line-size = <64>;
>> +		cache-sets = <512>;
>> +	};
> 
> ^^ this is a duplication of am62p5.dtsi? what about the spins with
> different CPUs enabled?
> 

Yes it is a duplicate, as of now we are not aware of plan for spins with cores
disabled, so just followed the pattern followed for other Jacinto devices
(J721e, J7200, J721s2, J784s4).

>> +
>> +	firmware {
>> +		optee {
>> +			compatible = "linaro,optee-tz";
>> +			method = "smc";
>> +		};
>> +
>> +		psci: psci {
>> +			compatible = "arm,psci-1.0";
>> +			method = "smc";
>> +		};
>> +	};
>> +
>> +	a53_timer0: timer-cl0-cpu0 {
>> +		compatible = "arm,armv8-timer";
>> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>, /* cntpsirq */
>> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, /* cntpnsirq */
>> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, /* cntvirq */
>> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; /* cnthpirq */
>> +	};
>> +
>> +	pmu: pmu {
>> +		compatible = "arm,cortex-a53-pmu";
>> +		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
>> +	};
>> +
>> +	cbass_main: bus@f0000 {
>> +		compatible = "simple-bus";
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		bootph-all;
> 
> Nope.
> 

Sorry I missed this, will fix here and in all other places (including comments
in patch 3/3).

>> +
>> +		ranges = <0x00 0x000f0000 0x00 0x000f0000 0x00 0x00030000>, /* Main MMRs */
>> +			 <0x00 0x00420000 0x00 0x00420000 0x00 0x00001000>, /* ESM0 */
>> +			 <0x00 0x00600000 0x00 0x00600000 0x00 0x00001100>, /* GPIO */
>> +			 <0x00 0x00703000 0x00 0x00703000 0x00 0x00000200>, /* USB0 debug trace */
>> +			 <0x00 0x0070c000 0x00 0x0070c000 0x00 0x00000200>, /* USB1 debug trace */
>> +			 <0x00 0x00a40000 0x00 0x00a40000 0x00 0x00000800>, /* Timesync router */
>> +			 <0x00 0x01000000 0x00 0x01000000 0x00 0x01b28400>, /* First peripheral window */
>> +			 <0x00 0x08000000 0x00 0x08000000 0x00 0x00200000>, /* Main CPSW */
>> +			 <0x00 0x0e000000 0x00 0x0e000000 0x00 0x01d20000>, /* Second peripheral window */
>> +			 <0x00 0x0fd80000 0x00 0x0fd80000 0x00 0x00080000>, /* GPU */
>> +			 <0x00 0x0fd20000 0x00 0x0fd20000 0x00 0x00000100>, /* JPEGENC0_CORE */
>> +			 <0x00 0x0fd20200 0x00 0x0fd20200 0x00 0x00000200>, /* JPEGENC0_CORE_MMU */
>> +			 <0x00 0x20000000 0x00 0x20000000 0x00 0x0a008000>, /* Third peripheral window */
>> +			 <0x00 0x30040000 0x00 0x30040000 0x00 0x00080000>, /* PRUSS-M */
>> +			 <0x00 0x301C0000 0x00 0x301C0000 0x00 0x00001000>, /* DPHY-TX */
>> +			 <0x00 0x30101000 0x00 0x30101000 0x00 0x00080100>, /* CSI window */
>> +			 <0x00 0x30200000 0x00 0x30200000 0x00 0x00010000>, /* DSS */
>> +			 <0x00 0x30210000 0x00 0x30210000 0x00 0x00010000>, /* VPU */
>> +			 <0x00 0x30220000 0x00 0x30220000 0x00 0x00010000>, /* DSS1 */
>> +			 <0x00 0x30270000 0x00 0x30270000 0x00 0x00010000>, /* DSI-base1 */
>> +			 <0x00 0x30500000 0x00 0x30500000 0x00 0x00100000>, /* DSI-base2 */
>> +			 <0x00 0x31000000 0x00 0x31000000 0x00 0x00050000>, /* USB0 DWC3 Core window */
>> +			 <0x00 0x31200000 0x00 0x31200000 0x00 0x00040000>, /* USB1 DWC3 Core window */
>> +			 <0x00 0x40900000 0x00 0x40900000 0x00 0x00030000>, /* SA3UL */
>> +			 <0x00 0x43600000 0x00 0x43600000 0x00 0x00010000>, /* SA3 sproxy data */
>> +			 <0x00 0x44043000 0x00 0x44043000 0x00 0x00000fe0>, /* TI SCI DEBUG */
>> +			 <0x00 0x44860000 0x00 0x44860000 0x00 0x00040000>, /* SA3 sproxy config */
>> +			 <0x00 0x48000000 0x00 0x48000000 0x00 0x06408000>, /* DMSS */
>> +			 <0x00 0x60000000 0x00 0x60000000 0x00 0x08000000>, /* FSS0 DAT1 */
>> +			 <0x00 0x70000000 0x00 0x70000000 0x00 0x00040000>, /* OCSRAM */
>> +			 <0x00 0x78400000 0x00 0x78400000 0x00 0x00008000>, /* MAIN R5FSS0 ATCM */
>> +			 <0x00 0x78500000 0x00 0x78500000 0x00 0x00008000>, /* MAIN R5FSS0 BTCM */
>> +			 <0x00 0x7e000000 0x00 0x7e000000 0x00 0x00200000>, /* C7X_0 L2SRAM */
>> +			 <0x00 0x7e200000 0x00 0x7e200000 0x00 0x00200000>, /* C7X_1 L2SRAM */
>> +			 <0x01 0x00000000 0x01 0x00000000 0x00 0x00310000>, /* A53 PERIPHBASE */
>> +			 <0x05 0x00000000 0x05 0x00000000 0x01 0x00000000>, /* FSS0 DAT3 */
>> +
>> +			 /* MCU Domain Range */
>> +			 <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>,
>> +			 <0x00 0x79000000 0x00 0x79000000 0x00 0x00008000>,
>> +			 <0x00 0x79020000 0x00 0x79020000 0x00 0x00008000>,
>> +			 <0x00 0x79100000 0x00 0x79100000 0x00 0x00040000>,
>> +			 <0x00 0x79140000 0x00 0x79140000 0x00 0x00040000>,
>> +
>> +			 /* Wakeup Domain Range */
>> +			 <0x00 0x00b00000 0x00 0x00b00000 0x00 0x00002400>,
>> +			 <0x00 0x2b000000 0x00 0x2b000000 0x00 0x00300400>,
>> +			 <0x00 0x43000000 0x00 0x43000000 0x00 0x00020000>,
>> +			 <0x00 0x78000000 0x00 0x78000000 0x00 0x00008000>,
>> +			 <0x00 0x78100000 0x00 0x78100000 0x00 0x00008000>;
>> +
>> +		cbass_mcu: bus@4000000 {
>> +			compatible = "simple-bus";
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges = <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>, /* Peripheral window */
>> +				 <0x00 0x79000000 0x00 0x79000000 0x00 0x00008000>, /* MCU R5 ATCM */
>> +				 <0x00 0x79020000 0x00 0x79020000 0x00 0x00008000>, /* MCU R5 BTCM */
>> +				 <0x00 0x79100000 0x00 0x79100000 0x00 0x00040000>, /* MCU IRAM0 */
>> +				 <0x00 0x79140000 0x00 0x79140000 0x00 0x00040000>; /* MCU IRAM1 */
>> +			bootph-all;
>> +		};
>> +
>> +		cbass_wakeup: bus@b00000 {
>> +			compatible = "simple-bus";
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges = <0x00 0x00b00000 0x00 0x00b00000 0x00 0x00002400>, /* VTM */
>> +				 <0x00 0x2b000000 0x00 0x2b000000 0x00 0x00300400>, /* Peripheral Window */
>> +				 <0x00 0x43000000 0x00 0x43000000 0x00 0x00020000>, /* WKUP CTRL MMR */
>> +				 <0x00 0x78000000 0x00 0x78000000 0x00 0x00008000>, /* DM R5 ATCM*/
>> +				 <0x00 0x78100000 0x00 0x78100000 0x00 0x00008000>; /* DM R5 BTCM*/
>> +			bootph-all;
> 
> Not in the bus nodes.. only in the leaf nodes please.
> 
> 
> Also what is wrong with expanding the am62p to include the
> missing bus segments instead of duplicating all of them?
> 

We can do that, but the same would be true for AM625, AM62A and AM62P families
and also for (J721E, J7200), since all these SoCs introduced new dtsi instead of
resuing existing ones, I thought there would be some valid reason to have
separate dtsi and just followed the same pattern, please let know if we need to
reuse from AM62P here also.

>> +		};
>> +	};
>> +
>> +	#include "k3-am62p-thermal.dtsi"
> 
> Is this correct?
> 

I think it would be preferred to have a duplicate for J722S here as users may
want to customize the trip points as per system and doing that without affecting
AM62P would be preferred, but the initial information here would be same for J722S.

>> +};
>> +
>> +/*
>> + * Include peripherals for each bus segment derived
>> + * from AM62P and overrides specific to J722S.
>> + */
>> +#include "k3-am62p-main.dtsi"
>> +#include "k3-am62p-mcu.dtsi"
>> +#include "k3-am62p-wakeup.dtsi"
>> +
>> +/* Main domain overrides */
>> +
>> +&cpsw3g {
>> +	status = "disabled";
> 
> 	here and rest: Why disabled?

These nodes seems to have dependencies that are not merged already and would
throw errors during boot, AM62P SoC dtsi seems to have these enabled by default
without these dependencies met and have errors like below during boot (seen on
AM62P with 6.7.0-rc4-next-20231211).

[   17.869092] platform 8000000.ethernet: deferred probe pending:
am65-cpsw-nuss: Failed to request tx dma channel

>> +};
>> +
>> +&inta_main_dmss {
>> +	ti,interrupt-ranges = <5 69 35>;
>> +};
>> +
>> +&mailbox0_cluster0 {
>> +	status = "disabled";
>> +};
>> +
>> +&mailbox0_cluster1 {
>> +	status = "disabled";
>> +};
>> +
>> +&mailbox0_cluster2 {
>> +	status = "disabled";
>> +};
>> +
>> +&mailbox0_cluster3 {
>> +	status = "disabled";
>> +};
>> +

[    7.198470] omap-mailbox 29020000.mailbox: no available mbox devices found
[    7.210411] omap-mailbox 29030000.mailbox: no available mbox devices found

>> +&oc_sram {
>> +	reg = <0x00 0x70000000 0x00 0x40000>;
>> +	ranges = <0x00 0x00 0x70000000 0x40000>;
>> +};
>> +
>> +/* MCU domain overrides */
>> +
>> +&mcu_r5fss0 {
>> +	status = "disabled";
>> +};

[    7.492406] platform 79000000.r5f: configured R5F for remoteproc mode
[    7.499887] platform 79000000.r5f: device does not have reserved memory
regions, ret = -22
[    7.508271] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000: reserved memory
init failed, ret = -22
[    7.517549] remoteproc remoteproc0: releasing 79000000.r5f
[    7.523338] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000:
k3_r5_cluster_rproc_init failed, ret = -22
[    7.532993] k3_r5_rproc: probe of bus@f0000:bus@4000000:r5fss@79000000 failed
with error -22

>> +
>> +/* wakeup domain overrides */
>> +
>> +&wkup_r5fss0 {
>> +	status = "disabled";
>> +};

[    7.576576] platform 78000000.r5f: configured R5F for IPC-only mode
[    7.605535] platform 78000000.r5f: device does not have reserved memory
regions, ret = -22
[    7.613942] k3_r5_rproc bus@f0000:bus@b00000:r5fss@78000000: reserved memory
init failed, ret = -22
[    7.635990] remoteproc remoteproc0: releasing 78000000.r5f
[    7.648639] k3_r5_rproc bus@f0000:bus@b00000:r5fss@78000000:
k3_r5_cluster_rproc_init failed, ret = -22
[    7.664043] k3_r5_rproc: probe of bus@f0000:bus@b00000:r5fss@78000000 failed
with error -22

>> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
>> index 2a4e0e084d69..591be4489f37 100644
>> --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
>> +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
>> @@ -59,6 +59,9 @@
>>  #define J721S2_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>>  #define J721S2_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>>  
>> +#define J722S_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>> +#define J722S_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>> +
>>  #define J784S4_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>>  #define J784S4_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>>  
>> -- 
>> 2.17.1
>>

0 - AM62P Boot logs (next-20231211) -
https://gist.github.com/vaishnavachath/7143da253bc708a1e60a13fe081d3914

To avoid these errors I am keeping these disabled, can we add support for these
also as part of the initial support addition series? then there is no need for
disabling these.

Thanks and Regards,
Vaishnav

>
  

On 14:07-20231214, Vaishnav Achath wrote:
[..]
> > Trim this down to what is different from AM62P?
> > 
> 
> Thanks for the review, I will trim this down in next revision, but the above is
> just a summary of the main features of this SoC, pointing to AM62P feature set
> here seems confusing to me. why does a new user/developer using J722S need to be
> aware of the existence of AM62P to just understand a high level summary about
> this device?

Since this is a reuse device. Helps with review and focus on deltas.

[...]

> >> +	l2_0: l2-cache0 {
> >> +		compatible = "cache";
> >> +		cache-unified;
> >> +		cache-level = <2>;
> >> +		cache-size = <0x80000>;
> >> +		cache-line-size = <64>;
> >> +		cache-sets = <512>;
> >> +	};
> > 
> > ^^ this is a duplication of am62p5.dtsi? what about the spins with
> > different CPUs enabled?
> > 
> 
> Yes it is a duplicate, as of now we are not aware of plan for spins with cores
> disabled, so just followed the pattern followed for other Jacinto devices
> (J721e, J7200, J721s2, J784s4).

None of the devices have been as close a reuse device as this has been.
errata in one carries over to the other etc.. There are definitely some
differences - for example: itap/otap delays for mmc those would be
unique on this device, but overrides with documentation will make
perfect sense there.

[..]
> >> +		cbass_wakeup: bus@b00000 {
> >> +			compatible = "simple-bus";
> >> +			#address-cells = <2>;
> >> +			#size-cells = <2>;
> >> +			ranges = <0x00 0x00b00000 0x00 0x00b00000 0x00 0x00002400>, /* VTM */
> >> +				 <0x00 0x2b000000 0x00 0x2b000000 0x00 0x00300400>, /* Peripheral Window */
> >> +				 <0x00 0x43000000 0x00 0x43000000 0x00 0x00020000>, /* WKUP CTRL MMR */
> >> +				 <0x00 0x78000000 0x00 0x78000000 0x00 0x00008000>, /* DM R5 ATCM*/
> >> +				 <0x00 0x78100000 0x00 0x78100000 0x00 0x00008000>; /* DM R5 BTCM*/
> >> +			bootph-all;
> > 
> > Not in the bus nodes.. only in the leaf nodes please.
> > 
> > 
> > Also what is wrong with expanding the am62p to include the
> > missing bus segments instead of duplicating all of them?
> > 
> 
> We can do that, but the same would be true for AM625, AM62A and AM62P families
> and also for (J721E, J7200), since all these SoCs introduced new dtsi instead of
> resuing existing ones, I thought there would be some valid reason to have
> separate dtsi and just followed the same pattern, please let know if we need to
> reuse from AM62P here also.

See above.

[...]

> 
> >> +		};
> >> +	};
> >> +
> >> +	#include "k3-am62p-thermal.dtsi"
> > 
> > Is this correct?
> > 
> 
> I think it would be preferred to have a duplicate for J722S here as users may
> want to customize the trip points as per system and doing that without affecting
> AM62P would be preferred, but the initial information here would be same for J722S.

Please check with your hardware teams if the am62p SoC level thermal
behavior will be similar to j722s or will it be different. If it is
estimated to be different, then you'd prefer to hold off introducing
the trips till the characterization provides you with the data.

> 
> >> +};
> >> +
> >> +/*
> >> + * Include peripherals for each bus segment derived
> >> + * from AM62P and overrides specific to J722S.
> >> + */
> >> +#include "k3-am62p-main.dtsi"
> >> +#include "k3-am62p-mcu.dtsi"
> >> +#include "k3-am62p-wakeup.dtsi"
> >> +
> >> +/* Main domain overrides */
> >> +
> >> +&cpsw3g {
> >> +	status = "disabled";
> > 
> > 	here and rest: Why disabled?
> 
> These nodes seems to have dependencies that are not merged already and would
> throw errors during boot, AM62P SoC dtsi seems to have these enabled by default
> without these dependencies met and have errors like below during boot (seen on
> AM62P with 6.7.0-rc4-next-20231211).
> 
> [   17.869092] platform 8000000.ethernet: deferred probe pending:
> am65-cpsw-nuss: Failed to request tx dma channel

OK - lets get the dependencies sorted out before merging then. I though
all the dependencies were indicated in the cover letter.

> 
> >> +};
> >> +
> >> +&inta_main_dmss {
> >> +	ti,interrupt-ranges = <5 69 35>;
> >> +};
> >> +
> >> +&mailbox0_cluster0 {
> >> +	status = "disabled";
> >> +};
> >> +
> >> +&mailbox0_cluster1 {
> >> +	status = "disabled";
> >> +};
> >> +
> >> +&mailbox0_cluster2 {
> >> +	status = "disabled";
> >> +};
> >> +
> >> +&mailbox0_cluster3 {
> >> +	status = "disabled";
> >> +};
> >> +
> 
> [    7.198470] omap-mailbox 29020000.mailbox: no available mbox devices found
> [    7.210411] omap-mailbox 29030000.mailbox: no available mbox devices found

You might want to dig into why..

> 
> >> +&oc_sram {
> >> +	reg = <0x00 0x70000000 0x00 0x40000>;
> >> +	ranges = <0x00 0x00 0x70000000 0x40000>;
> >> +};
> >> +
> >> +/* MCU domain overrides */
> >> +
> >> +&mcu_r5fss0 {
> >> +	status = "disabled";
> >> +};
> 
> [    7.492406] platform 79000000.r5f: configured R5F for remoteproc mode
> [    7.499887] platform 79000000.r5f: device does not have reserved memory
> regions, ret = -22
> [    7.508271] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000: reserved memory
> init failed, ret = -22
> [    7.517549] remoteproc remoteproc0: releasing 79000000.r5f
> [    7.523338] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000:
> k3_r5_cluster_rproc_init failed, ret = -22
> [    7.532993] k3_r5_rproc: probe of bus@f0000:bus@4000000:r5fss@79000000 failed
> with error -22

Yes, and the approach should rather be to disable the remote procs in
the board or at the SoC dtsi in a consistent manner. I had previously
suggested to do that SoC level (which means at am62p dtsi) since the remoteprocs have direct
dependency on how the memory layouts are partitioned in board.dts - but
i had asked folks working on remote procs to do that consistently across
SoCs. I don't see that having been done so far.

> 
> >> +
> >> +/* wakeup domain overrides */
> >> +
> >> +&wkup_r5fss0 {
> >> +	status = "disabled";
> >> +};
> 
> [    7.576576] platform 78000000.r5f: configured R5F for IPC-only mode
> [    7.605535] platform 78000000.r5f: device does not have reserved memory
> regions, ret = -22
> [    7.613942] k3_r5_rproc bus@f0000:bus@b00000:r5fss@78000000: reserved memory
> init failed, ret = -22
> [    7.635990] remoteproc remoteproc0: releasing 78000000.r5f
> [    7.648639] k3_r5_rproc bus@f0000:bus@b00000:r5fss@78000000:
> k3_r5_cluster_rproc_init failed, ret = -22
> [    7.664043] k3_r5_rproc: probe of bus@f0000:bus@b00000:r5fss@78000000 failed
> with error -22

See above.

> 
> >> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> >> index 2a4e0e084d69..591be4489f37 100644
> >> --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
> >> +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> >> @@ -59,6 +59,9 @@
> >>  #define J721S2_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
> >>  #define J721S2_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
> >>  
> >> +#define J722S_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
> >> +#define J722S_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
> >> +
> >>  #define J784S4_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
> >>  #define J784S4_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
> >>  
> >> -- 
> >> 2.17.1
> >>
> 
> 0 - AM62P Boot logs (next-20231211) -
> https://gist.github.com/vaishnavachath/7143da253bc708a1e60a13fe081d3914
> 
> To avoid these errors I am keeping these disabled, can we add support for these
> also as part of the initial support addition series? then there is no need for
> disabling these.

Fix them, then introduce j722s.
  

On 12/14/23 6:27 AM, Nishanth Menon wrote:
> On 14:07-20231214, Vaishnav Achath wrote:
> [..]
>>> Trim this down to what is different from AM62P?
>>>
>>
>> Thanks for the review, I will trim this down in next revision, but the above is
>> just a summary of the main features of this SoC, pointing to AM62P feature set
>> here seems confusing to me. why does a new user/developer using J722S need to be
>> aware of the existence of AM62P to just understand a high level summary about
>> this device?
> 
> Since this is a reuse device. Helps with review and focus on deltas.
> 
> [...]
> 
>>>> +	l2_0: l2-cache0 {
>>>> +		compatible = "cache";
>>>> +		cache-unified;
>>>> +		cache-level = <2>;
>>>> +		cache-size = <0x80000>;
>>>> +		cache-line-size = <64>;
>>>> +		cache-sets = <512>;
>>>> +	};
>>>
>>> ^^ this is a duplication of am62p5.dtsi? what about the spins with
>>> different CPUs enabled?
>>>
>>
>> Yes it is a duplicate, as of now we are not aware of plan for spins with cores
>> disabled, so just followed the pattern followed for other Jacinto devices
>> (J721e, J7200, J721s2, J784s4).
> 
> None of the devices have been as close a reuse device as this has been.

I'd argue J721e and J7200 are more similar in terms of reuse. It was a
mistake to model them as simple super/subsets of each other, only causes
confusion later. Let's keep at least this top level file, we will end up
using it more as more features/deltas are enabled/found.

> errata in one carries over to the other etc.. There are definitely some
> differences - for example: itap/otap delays for mmc those would be
> unique on this device, but overrides with documentation will make
> perfect sense there.
> 
> [..]
>>>> +		cbass_wakeup: bus@b00000 {
>>>> +			compatible = "simple-bus";
>>>> +			#address-cells = <2>;
>>>> +			#size-cells = <2>;
>>>> +			ranges = <0x00 0x00b00000 0x00 0x00b00000 0x00 0x00002400>, /* VTM */
>>>> +				 <0x00 0x2b000000 0x00 0x2b000000 0x00 0x00300400>, /* Peripheral Window */
>>>> +				 <0x00 0x43000000 0x00 0x43000000 0x00 0x00020000>, /* WKUP CTRL MMR */
>>>> +				 <0x00 0x78000000 0x00 0x78000000 0x00 0x00008000>, /* DM R5 ATCM*/
>>>> +				 <0x00 0x78100000 0x00 0x78100000 0x00 0x00008000>; /* DM R5 BTCM*/
>>>> +			bootph-all;
>>>
>>> Not in the bus nodes.. only in the leaf nodes please.
>>>
>>>
>>> Also what is wrong with expanding the am62p to include the
>>> missing bus segments instead of duplicating all of them?
>>>
>>
>> We can do that, but the same would be true for AM625, AM62A and AM62P families
>> and also for (J721E, J7200), since all these SoCs introduced new dtsi instead of
>> resuing existing ones, I thought there would be some valid reason to have
>> separate dtsi and just followed the same pattern, please let know if we need to
>> reuse from AM62P here also.
> 
> See above.
> 
> [...]
> 
>>
>>>> +		};
>>>> +	};
>>>> +
>>>> +	#include "k3-am62p-thermal.dtsi"
>>>
>>> Is this correct?
>>>
>>
>> I think it would be preferred to have a duplicate for J722S here as users may
>> want to customize the trip points as per system and doing that without affecting
>> AM62P would be preferred, but the initial information here would be same for J722S.
> 
> Please check with your hardware teams if the am62p SoC level thermal
> behavior will be similar to j722s or will it be different. If it is
> estimated to be different, then you'd prefer to hold off introducing
> the trips till the characterization provides you with the data.
> 
>>
>>>> +};
>>>> +
>>>> +/*
>>>> + * Include peripherals for each bus segment derived
>>>> + * from AM62P and overrides specific to J722S.
>>>> + */
>>>> +#include "k3-am62p-main.dtsi"
>>>> +#include "k3-am62p-mcu.dtsi"
>>>> +#include "k3-am62p-wakeup.dtsi"
>>>> +
>>>> +/* Main domain overrides */
>>>> +
>>>> +&cpsw3g {
>>>> +	status = "disabled";
>>>
>>> 	here and rest: Why disabled?
>>
>> These nodes seems to have dependencies that are not merged already and would
>> throw errors during boot, AM62P SoC dtsi seems to have these enabled by default
>> without these dependencies met and have errors like below during boot (seen on
>> AM62P with 6.7.0-rc4-next-20231211).
>>
>> [   17.869092] platform 8000000.ethernet: deferred probe pending:
>> am65-cpsw-nuss: Failed to request tx dma channel
> 
> OK - lets get the dependencies sorted out before merging then. I though
> all the dependencies were indicated in the cover letter.
> 
>>
>>>> +};
>>>> +
>>>> +&inta_main_dmss {
>>>> +	ti,interrupt-ranges = <5 69 35>;
>>>> +};
>>>> +
>>>> +&mailbox0_cluster0 {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&mailbox0_cluster1 {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&mailbox0_cluster2 {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>>> +&mailbox0_cluster3 {
>>>> +	status = "disabled";
>>>> +};
>>>> +
>>
>> [    7.198470] omap-mailbox 29020000.mailbox: no available mbox devices found
>> [    7.210411] omap-mailbox 29030000.mailbox: no available mbox devices found
> 
> You might want to dig into why..
> 
>>
>>>> +&oc_sram {
>>>> +	reg = <0x00 0x70000000 0x00 0x40000>;
>>>> +	ranges = <0x00 0x00 0x70000000 0x40000>;
>>>> +};
>>>> +
>>>> +/* MCU domain overrides */
>>>> +
>>>> +&mcu_r5fss0 {
>>>> +	status = "disabled";
>>>> +};
>>
>> [    7.492406] platform 79000000.r5f: configured R5F for remoteproc mode
>> [    7.499887] platform 79000000.r5f: device does not have reserved memory
>> regions, ret = -22
>> [    7.508271] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000: reserved memory
>> init failed, ret = -22
>> [    7.517549] remoteproc remoteproc0: releasing 79000000.r5f
>> [    7.523338] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000:
>> k3_r5_cluster_rproc_init failed, ret = -22
>> [    7.532993] k3_r5_rproc: probe of bus@f0000:bus@4000000:r5fss@79000000 failed
>> with error -22
> 
> Yes, and the approach should rather be to disable the remote procs in
> the board or at the SoC dtsi in a consistent manner. I had previously
> suggested to do that SoC level (which means at am62p dtsi) since the remoteprocs have direct
> dependency on how the memory layouts are partitioned in board.dts - but
> i had asked folks working on remote procs to do that consistently across
> SoCs. I don't see that having been done so far.
> 

I fixed this for a couple SoCs way back last year (7e48b665100ee), seems
folks kept adding mailboxes/rprocs un-disabled in the base .dtbi for
new SoCs anyway :( This needs fixed in AM62p .dtsi first, then these
disables can be removed from here.

Andrew

>>
>>>> +
>>>> +/* wakeup domain overrides */
>>>> +
>>>> +&wkup_r5fss0 {
>>>> +	status = "disabled";
>>>> +};
>>
>> [    7.576576] platform 78000000.r5f: configured R5F for IPC-only mode
>> [    7.605535] platform 78000000.r5f: device does not have reserved memory
>> regions, ret = -22
>> [    7.613942] k3_r5_rproc bus@f0000:bus@b00000:r5fss@78000000: reserved memory
>> init failed, ret = -22
>> [    7.635990] remoteproc remoteproc0: releasing 78000000.r5f
>> [    7.648639] k3_r5_rproc bus@f0000:bus@b00000:r5fss@78000000:
>> k3_r5_cluster_rproc_init failed, ret = -22
>> [    7.664043] k3_r5_rproc: probe of bus@f0000:bus@b00000:r5fss@78000000 failed
>> with error -22
> 
> See above.
> 
>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
>>>> index 2a4e0e084d69..591be4489f37 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
>>>> +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
>>>> @@ -59,6 +59,9 @@
>>>>   #define J721S2_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>>>>   #define J721S2_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>>>>   
>>>> +#define J722S_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>>>> +#define J722S_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>>>> +
>>>>   #define J784S4_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>>>>   #define J784S4_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>>>>   
>>>> -- 
>>>> 2.17.1
>>>>
>>
>> 0 - AM62P Boot logs (next-20231211) -
>> https://gist.github.com/vaishnavachath/7143da253bc708a1e60a13fe081d3914
>>
>> To avoid these errors I am keeping these disabled, can we add support for these
>> also as part of the initial support addition series? then there is no need for
>> disabling these.
> 
> Fix them, then introduce j722s.
>
  

On 09:43-20231214, Andrew Davis wrote:
> On 12/14/23 6:27 AM, Nishanth Menon wrote:
> > On 14:07-20231214, Vaishnav Achath wrote:
> > [..]
> > > > Trim this down to what is different from AM62P?
> > > > 
> > > 
> > > Thanks for the review, I will trim this down in next revision, but the above is
> > > just a summary of the main features of this SoC, pointing to AM62P feature set
> > > here seems confusing to me. why does a new user/developer using J722S need to be
> > > aware of the existence of AM62P to just understand a high level summary about
> > > this device?
> > 
> > Since this is a reuse device. Helps with review and focus on deltas.
> > 
> > [...]
> > 
> > > > > +	l2_0: l2-cache0 {
> > > > > +		compatible = "cache";
> > > > > +		cache-unified;
> > > > > +		cache-level = <2>;
> > > > > +		cache-size = <0x80000>;
> > > > > +		cache-line-size = <64>;
> > > > > +		cache-sets = <512>;
> > > > > +	};
> > > > 
> > > > ^^ this is a duplication of am62p5.dtsi? what about the spins with
> > > > different CPUs enabled?
> > > > 
> > > 
> > > Yes it is a duplicate, as of now we are not aware of plan for spins with cores
> > > disabled, so just followed the pattern followed for other Jacinto devices
> > > (J721e, J7200, J721s2, J784s4).
> > 
> > None of the devices have been as close a reuse device as this has been.
> 
> I'd argue J721e and J7200 are more similar in terms of reuse. It was a
> mistake to model them as simple super/subsets of each other, only causes
> confusion later. Let's keep at least this top level file, we will end up
> using it more as more features/deltas are enabled/found.
> 

yes, we do need a top level dtsi for the SoC. just minimize the amount
of duplication.

[...]
> > > 
> > > [    7.492406] platform 79000000.r5f: configured R5F for remoteproc mode
> > > [    7.499887] platform 79000000.r5f: device does not have reserved memory
> > > regions, ret = -22
> > > [    7.508271] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000: reserved memory
> > > init failed, ret = -22
> > > [    7.517549] remoteproc remoteproc0: releasing 79000000.r5f
> > > [    7.523338] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000:
> > > k3_r5_cluster_rproc_init failed, ret = -22
> > > [    7.532993] k3_r5_rproc: probe of bus@f0000:bus@4000000:r5fss@79000000 failed
> > > with error -22
> > 
> > Yes, and the approach should rather be to disable the remote procs in
> > the board or at the SoC dtsi in a consistent manner. I had previously
> > suggested to do that SoC level (which means at am62p dtsi) since the remoteprocs have direct
> > dependency on how the memory layouts are partitioned in board.dts - but
> > i had asked folks working on remote procs to do that consistently across
> > SoCs. I don't see that having been done so far.
> > 
> 
> I fixed this for a couple SoCs way back last year (7e48b665100ee), seems
> folks kept adding mailboxes/rprocs un-disabled in the base .dtbi for
> new SoCs anyway :( This needs fixed in AM62p .dtsi first, then these
> disables can be removed from here.
> 

Yes, hence blocking it from here on. Cleanup, then add.