Re: [PATCH v3 2/2] arm64: dts: ti: Add k3-am67a-beagley-ai

[Nishanth Menon <nm@ti.com>]

On 12:04-20240822, Robert Nelson wrote:
Minor nmits below:

> BeagleBoard.org BeagleY-AI is an easy to use, affordable open source
> hardware single board computer based on the Texas Instruments AM67A,
> which features a quad-core 64-bit Arm CPU subsystem, 2 general-purpose
> digital-signal-processors (DSP) and matrix-multiply-accelerators (MMA),
> GPU, vision and deep learning accelerators, and multiple Arm Cortex-R5
> cores for low-power, low-latency GPIO control.
> 
[...]
> +
> +	vdd_3v3: regulator-2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vsys_5v0>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +
> +	vdd_mmc1: regulator-mmc1 {

	Also responding:

> Okay, i'll change these... I already see a problem, as I've got two
> 3v3... can we use 'regulator-[0-9]v[0-9]-X' ?


let us just call them regulator-3 and so on incrementally. You can
already name with regulator-name property. We really don't need the "v'
specification here.

[...]
> +&wkup_i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&wkup_i2c0_pins_default>;
> +	clock-frequency = <100000>;
> +	bootph-all;
> +	status = "okay";
> +
> +	tps65219: pmic@30 {
> +		compatible = "ti,tps65219";
> +		reg = <0x30>;
> +		buck1-supply = <&vsys_5v0>;
> +		buck2-supply = <&vsys_5v0>;
> +		buck3-supply = <&vsys_5v0>;
> +		ldo1-supply = <&vdd_3v3>;
> +		ldo3-supply = <&vdd_3v3>;
> +		ldo4-supply = <&vdd_3v3>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_irq_pins_default>;
> +		interrupt-parent = <&gic500>;
> +		interrupts = <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +
> +		system-power-controller;
> +		ti,power-button;
> +		bootph-all;

Flip the bootph-all above the system-power-controller to stay within the
coding style boundaries?

> +
> +		regulators {
	[...]
> +};
> +
> +&sdhci1 {
> +	/* SD/MMC */
> +	vmmc-supply = <&vdd_mmc1>;
> +	vqmmc-supply = <&vdd_sd_dv>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mmc1_pins_default>;
> +	disable-wp;
> +	cd-gpios = <&main_gpio1 48 GPIO_ACTIVE_LOW>;
> +	cd-debounce-delay-ms = <100>;
> +	ti,fails-without-test-cd;
> +	bootph-all;

Nit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n117
keep the  bootph property above the vendor prefixed ones..

> +	status = "okay";
> +};
> -- 
> 2.39.2
> 


Additionally, with dtbs_check on next-20240823, I get:

/workdir/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dtb: leds: led-0: Unevaluated properties are not allowed ('linux,default-trigger' was unexpected)
/workdir/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dtb: leds: led-0:linux,default-trigger: 'oneOf' conditional failed, one must be fixed:

Please fix appropriately.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D