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

[Matthias Schiffer <matthias.schiffer@ew.tq-group.com>]

On Fri, 2026-01-09 at 13:59 -0600, Nishanth Menon wrote:
> 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?

Will be enabled by DTSO for different panels. This will be added in a follow-up
series, so I'll add a comment.

> 
> [...]
> > +
> > +	panel: panel {
> > +		pinctrl-names = "default";
> 
> Missing compatible? thinking again.. Should'nt these be part of some
> panel dtsi?

We tried to leave as much common configuration for different panels as possible
in the base DTS, so the DTBOs aren't bigger than necessary. I think it makes
sense to have this in the board DTS, as the signals used by the panels are part
of the board design.


> 
> > +		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?

Will add a comment (disabled as our starterkits don't include a fan, and
different fans may need different PWM frequencies).


> 
> [...]
> > +
> > +	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?

This is a standard 4-pin fan connector as is common on x86 mainboards, which
includes a 12V supply.


> 
> > +		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) ... */

I tried to avoid making the lines overly long an adhered to the regular 100
column limit where reasonable. I agree that the current inconsistent style is
not so nice... I see two options here:

- Consistently move the comment to a separate line
- Break long comments including pin name, pin function and board signal function
  into two lines


> 
> btw, panel node uses it as:
> enable-gpios = <&main_gpio0 36 GPIO_ACTIVE_HIGH>;
> 
> isn't RESET# meant to be active low?

As an "enable" pin enables when active and a "reset" pin disables when active,
an active-low reset is the same as an active-high enable.


> 
> > +		>;
> > +	};
> > +
> 
> [...]
> 
> 
> > 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?

Correct, the bootloader patches both PMIC config and OPP table.

Best,
Matthias


> 
> > +	};
> > +};
> > +
> 

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/