Hi Laurent On Wed, 2022-03-23 at 01:02 +0200, Laurent Pinchart wrote: > Hi Marcel, > > Thank you for the patch. Thank you for reviewing/testing. > On Thu, Mar 17, 2022 at 05:01:22PM +0100, Marcel Ziswiler wrote: > > From: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx> > > > > This patch adds the device tree to support Toradex Verdin iMX8M Plus [1] > > a computer on module which can be used on different carrier boards. > > > > The module consists of an NXP i.MX 8M Plus family SoC (either i.MX 8M > > Plus Quad or 8M Plus QuadLite), a PCA9450C PMIC, a Gigabit Ethernet PHY, > > 1, 2, 4 or 8 GB of LPDDR4 RAM, an eMMC, a TLA2024 ADC, an I2C EEPROM, an > > RX8130 RTC, an optional I2C temperature sensor plus an optional > > Bluetooth/Wi-Fi module. > > > > Anything that is not self-contained on the module is disabled by > > default. > > > > The device tree for the Dahlia includes the module's device tree and > > enables the supported peripherals of the carrier board. > > > > The device tree for the Verdin Development Board includes the module's > > device tree as well as the Dahlia one as it is a superset and supports > > almost all peripherals available. > > > > So far there is no display functionality supported at all but basic > > console UART, USB host, eMMC and Ethernet functionality work fine. > > > > [1] https://www.toradex.com/computer-on-modules/verdin-arm-family/nxp-imx-8m-plus > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx> > > > > --- > > > > arch/arm64/boot/dts/freescale/Makefile | 4 + > > .../dts/freescale/imx8mp-verdin-dahlia.dtsi | 125 ++ > > .../boot/dts/freescale/imx8mp-verdin-dev.dtsi | 44 + > > .../imx8mp-verdin-nonwifi-dahlia.dts | 18 + > > .../freescale/imx8mp-verdin-nonwifi-dev.dts | 18 + > > .../dts/freescale/imx8mp-verdin-nonwifi.dtsi | 54 + > > .../freescale/imx8mp-verdin-wifi-dahlia.dts | 18 + > > .../dts/freescale/imx8mp-verdin-wifi-dev.dts | 18 + > > .../dts/freescale/imx8mp-verdin-wifi.dtsi | 82 + > > .../boot/dts/freescale/imx8mp-verdin.dtsi | 1373 +++++++++++++++++ > > 10 files changed, 1754 insertions(+) > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi-dahlia.dts > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi-dev.dts > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi.dtsi > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi-dahlia.dts > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi-dev.dts > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi.dtsi > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi > > [snip] > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > > new file mode 100644 > > index 000000000000..103d97a3b029 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > > @@ -0,0 +1,125 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > > +/* > > + * Copyright 2022 Toradex > > + */ > > + > > +&backlight { > > + power-supply = <®_3p3v>; > > +}; > > + > > +/* Verdin SPI_1 */ > > +&ecspi1 { > > + status = "okay"; > > +}; > > + > > +/* EEPROM on display adapter boards */ > > +&eeprom_display_adapter { > > + status = "okay"; > > +}; > > + > > +/* EEPROM on Verdin Development board */ > > +&eeprom_carrier_board { > > + status = "okay"; > > +}; > > + > > +&eqos { > > + status = "okay"; > > +}; > > + > > +&flexcan1 { > > + status = "okay"; > > +}; > > + > > +&flexcan2 { > > + status = "okay"; > > +}; > > + > > +/* Verdin QSPI_1 */ > > +&flexspi { > > + status = "okay"; > > +}; > > + > > +/* Current measurement into module VCC */ > > +&hwmon { > > + status = "okay"; > > +}; > > + > > +&hwmon_temp { > > + vs-supply = <®_1p8v>; > > + status = "okay"; > > +}; > > + > > +/* Verdin I2C_2_DSI */ > > +&i2c2 { > > + status = "okay"; > > +}; > > + > > +&i2c3 { > > + status = "okay"; > > +}; > > + > > +/* Verdin I2C_1 */ > > +&i2c4 { > > + status = "okay"; > > +}; > > + > > +/* Verdin PCIE_1 */ > > Is this comment needed ? Is it a placeholder for future PCIe support ? > If so I'd write > > /* TODO: Verdin PCIE_1 */ Yes, exactly, let me do as you suggest. > > + > > +/* Verdin PWM_1 */ > > +&pwm1 { > > + status = "okay"; > > +}; > > + > > +/* Verdin PWM_2 */ > > +&pwm2 { > > + status = "okay"; > > +}; > > + > > +/* Verdin PWM_3_DSI */ > > +&pwm3 { > > + status = "okay"; > > +}; > > + > > +®_usdhc2_vmmc { > > + vin-supply = <®_3p3v>; > > +}; > > + > > +/* VERDIN I2S_1 */ > > Same here. By the way, you may want to standardize on Verdin or VERDIN > and not mix both. These comments apply to the other files too. Yes, let me also add a TODO in front here. Plus yeah, this capitalized variant just slipped our eyes and I will change them all to regular Verdin. Thanks for spotting this. > > + > > +/* Verdin UART_1 */ > > +&uart1 { > > + status = "okay"; > > +}; > > + > > +/* Verdin UART_2 */ > > +&uart2 { > > + status = "okay"; > > +}; > > + > > +/* Verdin UART_3, used as the Linux Console */ > > +&uart3 { > > + status = "okay"; > > +}; > > + > > +/* Verdin USB_1 */ > > +&usb3_0 { > > + status = "okay"; > > +}; > > + > > +&usb3_phy0 { > > + status = "okay"; > > +}; > > + > > +/* Verdin USB_2 */ > > +&usb3_1 { > > + status = "okay"; > > +}; > > + > > +&usb3_phy1 { > > + status = "okay"; > > +}; > > + > > +/* Verdin SD_1 */ > > +&usdhc2 { > > + status = "okay"; > > +}; > > [snip] > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp- > > verdin.dtsi > > new file mode 100644 > > index 000000000000..26d6c2819ee8 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi > > @@ -0,0 +1,1373 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > > +/* > > + * Copyright 2022 Toradex > > + */ > > + > > +#include "dt-bindings/pwm/pwm.h" > > +#include "imx8mp.dtsi" > > + > > +/ { > > + chosen { > > + stdout-path = &uart3; > > + }; > > + > > + aliases { > > + /* Ethernet aliases to ensure correct MAC addresses */ > > + ethernet0 = &eqos; > > + ethernet1 = &fec; > > On Dahlia the ethernet connector is routed to the eqos if I'm not > mistaken. Yes, actually the on-module PHY which is what is routed to the RJ-45 on Dahlia uses the EQOS MAC, correct. > On my board U-Boot considers this to be the second ethernet > controller, with the fec being the first one. Yes, however, U-Boot does use the EQOS one as primary MAC as well. This is just how U-Boot does things. There is no easy way to change the actual instance numbering. Actually just like on Linux really with the one difference that U-Boot will always show both independent whether or not your carrier board even has the second PHY or not. This is due to U-Boot doing lazy loading aka not touch any Ethernet hardware unless you actually run some network commands. > The mismatch results in > the MAC addresses being swapped between eth0 and eth1 when comparing > U-Boot and Linux. No, this alias should really be what ensures the correct MAC addresses. > Am I using a too old boot loader, or should the two > ethernet controlls be swapped here ? Yes, I guess you might not be using latest mainline U-Boot. I just verified this on my table again having both the Dahlia and Verdin Development carrier boards available. Please note that on the Verdin Development Board the primary Ethernet is also called eth1. Unfortunately, changing this naming is also not entirely trivial. But it really should have the proper MAC address. Dahlia U-Boot 2022.04-rc4-00068-g5f68470d69 (Mar 23 2022 - 10:15:46 +0100) Verdin iMX8MP # echo $ethaddr 00:14:2d:6c:71:64 => verified with wireshark root@verdin-imx8mp-07106916:~# ip addr ... 4: eth0: <BROADCAST,MULTICAST,DYNAMIC,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000 link/ether 00:14:2d:6c:71:64 brd ff:ff:ff:ff:ff:ff inet 192.168.10.239/24 brd 192.168.10.255 scope global eth0 valid_lft forever preferred_lft forever inet6 fe80::214:2dff:fe6c:7164/64 scope link valid_lft forever preferred_lft forever ... Verdin Developer Board U-Boot 2022.04-rc4-00068-g5f68470d69 (Mar 23 2022 - 10:15:46 +0100) Verdin iMX8MP # echo $ethaddr 00:14:2d:6c:71:64 => verified with wireshark root@verdin-imx8mp-07106916:~# uname -a Linux verdin-imx8mp-07106916 5.17.0-rc8-next-20220317-00003-g695dc7d13c2a #12 SMP PREEMPT Thu Mar 17 16:07:49 CET 2022 aarch64 aarch64 aarch64 GNU/Linux root@verdin-imx8mp-07106916:~# ip addr ... 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,DYNAMIC,UP> mtu 1500 qdisc mq state DOWN group default qlen 1000 link/ether 00:14:2d:7c:71:64 brd ff:ff:ff:ff:ff:ff ... 5: eth1: <BROADCAST,MULTICAST,DYNAMIC,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000 link/ether 00:14:2d:6c:71:64 brd ff:ff:ff:ff:ff:ff inet 192.168.10.239/24 brd 192.168.10.255 scope global eth1 valid_lft forever preferred_lft forever inet6 fe80::214:2dff:fe6c:7164/64 scope link valid_lft forever preferred_lft forever ... > > + rtc0 = &rtc_i2c; > > + rtc1 = &snvs_rtc; > > + }; > > [snip] > > With these issues addressed, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Tested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> Thanks again! Cheers Marcel
