Re: [PATCH v5 2/2] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees

[Nishanth Menon <nm@ti.com>]

On 09:14-20260107, Matthias Schiffer wrote:
> The TQMa62xx is a SoM family with a pluggable board connector based on the
> TI AM62x SoCs. Add DTS(I) for the AM625 (2x Cortex-A53) variant and its
> combination with our MBa62xx carrier board.
> 
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>

Few clarifications below..

[...]
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts b/arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts
> new file mode 100644
> index 0000000000000..cca8d0018504d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts
> @@ -0,0 +1,930 @@

[...]

> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&backlight_pins>;
> +		enable-gpios = <&main_gpio0 38 GPIO_ACTIVE_HIGH>;
> +		status = "disabled";

Document why disabled?

[...]
> +
> +	panel: panel {
> +		pinctrl-names = "default";

Missing compatible? thinking again.. Should'nt these be part of some
panel dtsi?

> +		pinctrl-0 = <&lvds_panel_pins>;
> +		enable-gpios = <&main_gpio0 36 GPIO_ACTIVE_HIGH>;
> +		power-supply = <&reg_lvds_pwr>;
> +	};
> +
> +	fan0: pwm-fan {
> +		compatible = "pwm-fan";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pwm_fan_pins>;
> +		fan-supply = <&reg_pwm_fan>;
> +		#cooling-cells = <2>;
> +		/* typical 25 kHz -> 40.000 nsec */
> +		pwms = <&epwm0 1 40000 PWM_POLARITY_INVERTED>;
> +		cooling-levels = <0 32 64 128 196 240>;
> +		pulses-per-revolution = <2>;
> +		interrupt-parent = <&main_gpio1>;
> +		interrupts = <30 IRQ_TYPE_EDGE_FALLING>;
> +		status = "disabled";

please document why disabled?

[...]
> +
> +	reg_pwm_fan: regulator-pwm-fan {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&reg_pwm_fan_pins>;
> +		regulator-name = "FAN_PWR";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;

Just checking.. Did you intent a 12v supply for fan?

> +		gpio = <&main_gpio0 62 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +

[...]

> +
> +	lvds_panel_pins: lvds-panel-pins {
> +		pinctrl-single,pins = <
> +			/* (N20) GPMC0_BE1n.GPIO0_36 - LVDS0_RESET# */
> +			AM62X_IOPAD(0x0094, PIN_OUTPUT, 7)

Could you follow the comment convention you have elsewhere? please follow the
same in gpio below etc..
AM62X_IOPAD(....) /* (N20) ... */

btw, panel node uses it as:
enable-gpios = <&main_gpio0 36 GPIO_ACTIVE_HIGH>;

isn't RESET# meant to be active low?

> +		>;
> +	};
> +

[...]


> diff --git a/arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi b/arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi
> new file mode 100644
> index 0000000000000..1b7d58cb7b936
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi
> @@ -0,0 +1,331 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Copyright (C) 2021-2022 Texas Instruments Incorporated - https://www.ti.com/
> + * Copyright (c) 2023-2025 TQ-Systems GmbH <linux@ew.tq-group.com>, D-82229 Seefeld, Germany.
> + * Author: Matthias Schiffer
> + */

[...]

> +		regulators {
> +			reg_buck1: buck1 {
> +				regulator-name = "V_VDD_CORE";
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <750000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
[...]

> +
> +&a53_opp_table {
> +	opp-1400000000 {
> +		opp-hz = /bits/ 64 <1400000000>;
> +		opp-supported-hw = <0x01 0x0004>;
> +		opp-suspend;
> +		clock-latency-ns = <6000000>;
> +		/* Enabled by bootloader if supported */
> +		status = "disabled";

This (bootloader enabling just this node) wont work unless vdd_core
voltage is configured to 0.8v as well, no?

> +	};
> +};
> +

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