Re: [PATCH v3 2/2] arm64: dts: imx8mp: Add TechNexion EDM-G-IMX8M-PLUS SOM on WB-EDM-G carrier board

[Richard Hu <richard.hu@technexion.com>]

On Fri, Jul 11, 2025 at 04:19:48PM +0800, Shawn Guo wrote:
> On Wed, Jul 09, 2025 at 01:47:45PM +0800, Richard Hu wrote:
> > Add support for TechNexion EDM-G-IMX8M-PLUS SOM and WB-EDM-G carrier board.
> > Key interfaces include:
> > - Gigabit Ethernet
> > - USB 3.0
> > - I2S, UART, SPI, I2C, PWM, GPIO
> > 
> > Signed-off-by: Richard Hu <richard.hu@technexion.com>
> > Signed-off-by: Ray Chang <ray.chang@technexion.com>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile            |   1 +
> >  arch/arm64/boot/dts/freescale/imx8mp-edm-g-wb.dts | 373 ++++++++++
> >  arch/arm64/boot/dts/freescale/imx8mp-edm-g.dtsi   | 806 ++++++++++++++++++++++
> >  3 files changed, 1180 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 0b473a23d120..b56c866d4a9d 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-drc02.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk3.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-picoitx.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-edm-g-wb.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-iota2-lumpy.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-edm-g-wb.dts b/arch/arm64/boot/dts/freescale/imx8mp-edm-g-wb.dts
> > new file mode 100644
> > index 000000000000..0e5c7bf219b0
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-edm-g-wb.dts
> > @@ -0,0 +1,373 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2024 TechNexion Ltd.
> > + *
> > + * Author: Ray Chang <ray.chang@technexion.com>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "imx8mp-edm-g.dtsi"
> > +
> > +/ {
> > +	compatible = "technexion,edm-g-imx8mp-wb", "technexion,edm-g-imx8mp", "fsl,imx8mp";
> > +	model = "TechNexion EDM-G-IMX8MP SOM on WB-EDM-G";
> > +
> > +	connector {
> > +		compatible = "usb-c-connector";
> > +		data-role = "dual";
> > +		label = "USB-C";
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> 
> Have a newline between properties and child node.

Thanks for pointing this out. You're right, I'll add the newline to fix this.

> > +				hs_ep: endpoint {
> > +					remote-endpoint = <&usb3_hs_ep>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				ss_ep: endpoint {
> > +					remote-endpoint = <&hd3ss3220_in_ep>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> > +	gpio-leds {
> > +		compatible = "gpio-leds";
> > +
> > +		led {
> > +			default-state = "on";
> > +			gpios = <&expander2 1 GPIO_ACTIVE_HIGH>;
> > +			label = "gpio-led";
> > +		};
> > +	};
> > +
> > +	lvds_backlight: lvds-backlight {
> > +		compatible = "pwm-backlight";
> > +		brightness-levels = <0 36 72 108 144 180 216 255>;
> > +		default-brightness-level = <6>;
> > +		pwms = <&pwm2 0 50000 0>;
> > +		status = "disabled";
> 
> Just out of curiosity, why is it disabled?

That's a good question. We disabled the backlight because the LVDS panel is an optional accessory.
We were planning to enable this lvds_backlight node in the overlay,
but would you recommend we move the node definition itself entirely into the overlay file?
We're happy to follow the best practice here.

> > +	};
> > +
> > +	native-hdmi-connector {
> > +		compatible = "hdmi-connector";
> > +		label = "HDMI OUT";
> > +		type = "a";
> > +
> > +		port {
> > +			hdmi_in: endpoint {
> > +				remote-endpoint = <&hdmi_tx_out>;
> > +			};
> > +		};
> > +	};
> > +
> > +	pcie0_refclk: pcie0-refclk {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <100000000>;
> > +	};
> > +
> > +	reg_pcie0: regulator-pcie {
> > +		compatible = "regulator-fixed";
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-name = "PCIE_CLKREQ_N";
> > +		gpio = <&gpio1 13 GPIO_ACTIVE_LOW>;
> > +	};
> > +
> > +	reg_pwr_3v3: regulator-pwr-3v3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-name = "pwr-3v3";
> > +	};
> > +
> > +	reg_pwr_5v: regulator-pwr-5v {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-name = "pwr-5v";
> > +	};
> > +
> > +	sound-hdmi {
> > +		compatible = "fsl,imx-audio-hdmi";
> > +		audio-cpu = <&aud2htx>;
> > +		hdmi-out;
> > +		model = "audio-hdmi";
> > +	};
> > +
> > +	sound-wm8960 {
> > +		compatible = "simple-audio-card";
> > +		simple-audio-card,bitclock-master = <&cpudai>;
> > +		simple-audio-card,format = "i2s";
> > +		simple-audio-card,frame-master = <&cpudai>;
> > +		simple-audio-card,name = "wm8960-audio";
> > +		simple-audio-card,routing = "Headphone Jack", "HP_L",
> > +			"Headphone Jack", "HP_R",
> > +			"External Speaker", "SPK_LP",
> > +			"External Speaker", "SPK_LN",
> > +			"External Speaker", "SPK_RP",
> > +			"External Speaker", "SPK_RN",
> > +			"LINPUT1", "Mic Jack",
> > +			"RINPUT1", "Mic Jack",
> > +			"Mic Jack", "MICB";
> > +		simple-audio-card,widgets = "Headphone", "Headphone Jack",
> > +			"Speaker", "External Speaker",
> > +			"Microphone", "Mic Jack";
> > +
> > +		simple-audio-card,codec {
> > +			sound-dai = <&wm8960>;
> > +		};
> > +
> > +		cpudai: simple-audio-card,cpu {
> > +			sound-dai = <&sai3>;
> > +		};
> > +	};
> > +};
> > +
> > +&aud2htx {
> > +	status = "okay";
> > +};
> > +
> > +&flexcan1 {
> > +	status = "okay";
> > +};
> > +
> > +&gpio1 {
> > +	gpio-line-names = \
> 
> Hmm, not sure "\" is necessary.

You're right, it's not needed. I'll remove the unnecessary backslashes from the gpio-line-names properties.

> > +		"", "", "", "", "", "", "DSI_RST", "",	\
> > +		"", "", "", "", "", "", "", "",	\
> > +		"", "", "", "", "", "", "", "",	\
> > +		"", "", "", "", "", "", "", "";
> > +	pinctrl-0 = <&pinctrl_gpio1>;
> > +};
> > +
> > +&gpio4 {
> > +	gpio-line-names = \
> > +		"", "", "", "", "", "", "GPIO_P249", "GPIO_P251",	\
> > +		"", "GPIO_P255", "", "", "", "", "", "",	\
> > +		"DSI_BL_EN", "DSI_VDDEN", "", "", "", "", "", "",	\
> > +		"", "", "", "", "", "", "", "";
> > +	pinctrl-0 = <&pinctrl_gpio4>;
> > +};
> > +
> > +&hdmi_pvi {
> > +	status = "okay";
> > +};
> > +
> > +&hdmi_tx {
> > +	pinctrl-0 = <&pinctrl_hdmi>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +
> > +	ports {
> > +		port@1 {
> > +			hdmi_tx_out: endpoint {
> > +				remote-endpoint = <&hdmi_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&hdmi_tx_phy {
> > +	status = "okay";
> > +};
> > +
> > +&i2c2 {
> > +	status = "okay";
> > +
> > +	wm8960: codec@1a {
> 
> audio-codec for the node name.

Got it. I will rename the node from codec@1a to audio-codec@1a to follow the standard convention.

> > +		compatible = "wlf,wm8960";
> > +		reg = <0x1a>;
> > +		clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_SAI3_MCLK1>;
> > +		clock-names = "mclk";
> > +		#sound-dai-cells = <0>;
> > +		AVDD-supply = <&reg_pwr_3v3>;
> > +		DBVDD-supply = <&reg_pwr_3v3>;
> > +		DCVDD-supply = <&reg_pwr_3v3>;
> > +		SPKVDD1-supply = <&reg_pwr_5v>;
> > +		SPKVDD2-supply = <&reg_pwr_5v>;
> > +		wlf,hp-cfg = <2 2 3>;
> > +		wlf,shared-lrclk;
> > +	};
> > +
> > +	expander1: gpio@21 {
> > +		compatible = "nxp,pca9555";
> > +		reg = <0x21>;
> > +		#gpio-cells = <2>;
> > +		gpio-controller;
> > +		gpio-line-names = "EXPOSURE_TRIG_IN1", "FLASH_OUT1",
> > +				  "INFO_TRIG_IN1", "CAM_SHUTTER1", "XVS1",
> > +				  "PWR1_TIME0", "PWR1_TIME1", "PWR1_TIME2",
> > +				  "EXPOSURE_TRIG_IN2", "FLASH_OUT2",
> > +				  "INFO_TRIG_IN2", "CAM_SHUTTER2", "XVS2",
> > +				  "PWR2_TIME0", "PWR2_TIME1", "PWR2_TIME2";
> > +	};
> > +
> > +	expander2: gpio@23 {
> > +		compatible = "nxp,pca9555";
> > +		reg = <0x23>;
> > +		#interrupt-cells = <2>;
> > +		interrupt-controller;
> > +		interrupt-parent = <&gpio4>;
> > +		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > +		#gpio-cells = <2>;
> > +		gpio-controller;
> > +		gpio-line-names = "M2_DISABLE_N", "LED_EN", "", "",
> > +				  "", "", "", "USB_OTG_OC",
> > +				  "EXT_GPIO8", "EXT_GPIO9", "", "",
> > +				  "", "CSI1_PDB", "CSI2_PDB", "PD_FAULT";
> > +		pinctrl-0 = <&pinctrl_expander2_irq>;
> > +		pinctrl-names = "default";
> > +	};
> > +
> > +	usb_typec: usb-typec@67 {
> > +		compatible = "ti,hd3ss3220";
> > +		reg = <0x67>;
> > +		interrupt-parent = <&gpio4>;
> > +		interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> > +		pinctrl-0 = <&pinctrl_hd3ss3220_irq>;
> > +		pinctrl-names = "default";
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> 
> Have a newline between properties and child node.

Okay, I'll add a newline for better readability as suggested.

> Shawn


Thank you!


Best regards

Richard