Re: [PATCH v2 2/2] arm64: dts: ti: add k3-j721e-beagleboneai64

[Vignesh Raghavendra <vigneshr@ti.com>]

Hi Robert,

On 23/08/22 04:33, Robert Nelson wrote:
> BeagleBoard.org BeagleBone AI-64 is an open source hardware single
> board computer based on the Texas Instruments TDA4VM SoC featuring
> dual-core 2.0GHz Arm Cortex-A72 processor, C7x+MMA and 2 C66x
> floating-point VLIW DSPs, 3x dual Arm Cortex-R5 co-processors,
> 2x 6-core Programmable Real-Time Unit and Industrial Communication
> SubSystem, PowerVR Rogue 8XE GE8430 3D GPU. The board features 4GB
> DDR4, USB3.0 Type-C, USB HS Type-A, miniDisplayPort, 2x 4-lane CSI,
> DSI, 16GB eMMC flash, 1G Ethernet, M.2 E-key for WiFi/BT, and
> BeagleBone expansion headers.

Thanks for the patch, few comments

> 
> This board family can be indentified by the BBONEAI-64-B0 in the at24 eeprom:

s/indentified/identified

> 
> [aa 55 33 ee 01 37 00 10  2e 00 42 42 4f 4e 45 41 |.U3..7....BBONEA|]
> [49 2d 36 34 2d 42 30 2d  00 00 42 30 30 30 37 38 |I-64-B0-..B00078|]
> 
> https://beagleboard.org/ai-64
> https://git.beagleboard.org/beagleboard/beaglebone-ai-64
> 
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> CC: Nishanth Menon <nm@ti.com>
> CC: Vignesh Raghavendra <vigneshr@ti.com>
> CC: Tero Kristo <kristo@kernel.org>
> CC: Jason Kridner <jkridner@beagleboard.org>
> CC: Drew Fustini <drew@beagleboard.org>
> ---

[...]

> +	chosen {
> +		stdout-path = "serial2:115200n8";
> +		bootargs = "console=ttyS2,115200n8 earlycon=ns16550a,mmio32,0x02800000";

bootargs are no longer to be part of DT file and to be supplied by
bootloader as needed [1]

[1]
https://lore.kernel.org/all/81134eb9-2b7d-05bc-3035-a47f020861a8@linaro.org/

[...]

> +	evm_12v0: fixedregulator-evm12v0 {

please use generic node name:
		regulator-X  X=1,2,3...


> +		/* main supply */
> +		compatible = "regulator-fixed";
> +		regulator-name = "evm_12v0";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +
> +	vsys_3v3: fixedregulator-vsys3v3 {
> +		/* Output of LMS140 */
> +		compatible = "regulator-fixed";
> +		regulator-name = "vsys_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&evm_12v0>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +
> +	vsys_5v0: fixedregulator-vsys5v0 {
> +		/* Output of LM5140 */
> +		compatible = "regulator-fixed";
> +		regulator-name = "vsys_5v0";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&evm_12v0>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +
> +	vdd_mmc1: fixedregulator-sd {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sd_pwr_en_pins_default>;
> +		regulator-name = "vdd_mmc1";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		enable-active-high;
> +		vin-supply = <&vsys_3v3>;
> +		gpios = <&main_gpio0 82 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	vdd_sd_dv_alt: gpio-regulator-TLV71033 {
> +		compatible = "regulator-gpio";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&vdd_sd_dv_alt_pins_default>;
> +		regulator-name = "tlv71033";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		vin-supply = <&vsys_5v0>;
> +		gpios = <&main_gpio0 117 GPIO_ACTIVE_HIGH>;
> +		states = <1800000 0x0>,
> +			 <3300000 0x1>;
> +	};
> +
> +	dp_pwr_3v3: fixedregulator-dp-prw {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&dp0_3v3_en_pins_default>;
> +		regulator-name = "dp-pwr";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpios = <&main_gpio0 49 GPIO_ACTIVE_HIGH>; /* DP0_PWR_SW_EN */
> +		enable-active-high;
> +	};
> +
> +	dp0: connector {
> +		compatible = "dp-connector";
> +		label = "DP0";
> +		type = "full-size";
> +		dp-pwr-supply = <&dp_pwr_3v3>;
> +
> +		port {
> +			dp_connector_in: endpoint {
> +				remote-endpoint = <&dp0_out>;
> +			};
> +		};
> +	};
> +};
> +

[...]

-- 
Regards
Vignesh