| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aHSvJv6zbndPx72h@vm>
Date: Mon, 14 Jul 2025 15:17:58 +0800
From: Richard Hu <richard.hu@...hnexion.com>
To: Shawn Guo <shawnguo2@...h.net>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org,
Ray Chang <ray.chang@...hnexion.com>
Subject: Re: [PATCH v3 2/2] arm64: dts: imx8mp: Add TechNexion
EDM-G-IMX8M-PLUS SOM on WB-EDM-G carrier board
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@...hnexion.com>
> > Signed-off-by: Ray Chang <ray.chang@...hnexion.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@...hnexion.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 = <®_pwr_3v3>;
> > + DBVDD-supply = <®_pwr_3v3>;
> > + DCVDD-supply = <®_pwr_3v3>;
> > + SPKVDD1-supply = <®_pwr_5v>;
> > + SPKVDD2-supply = <®_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
Powered by blists - more mailing lists