[PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board
Marcel Ziswiler
marcel.ziswiler at toradex.com
Fri Apr 8 00:56:09 PDT 2022
Hi Marek
On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote:
> Hello Marek,
>
> On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote:
> > Add new board based on the Toradex Verdin iMX8M Mini SoM, the MX8Menlo.
> > The board is a compatible replacement for i.MX53 M53Menlo and features
> > USB, multiple UARTs, ethernet, LEDs, SD and eMMC.
> >
> > Signed-off-by: Marek Vasut <marex at denx.de>
> > Cc: Fabio Estevam <festevam at denx.de>
> > Cc: Marcel Ziswiler <marcel.ziswiler at toradex.com>
> > Cc: Peng Fan <peng.fan at nxp.com>
> > Cc: Shawn Guo <shawnguo at kernel.org>
> > Cc: NXP Linux Team <linux-imx at nxp.com>
> > To: linux-arm-kernel at lists.infradead.org
> > ---
> > arch/arm64/boot/dts/freescale/Makefile | 1 +
> > .../boot/dts/freescale/imx8mm-mx8menlo.dts | 331 ++++++++++++++++++
> > 2 files changed, 332 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-mx8menlo.dts
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 52ce0f798657b..bd0e9d37d5eb2 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -56,6 +56,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-mx8menlo.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-nitrogen-r2.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-tqma8mqml-mba8mx.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-mx8menlo.dts b/arch/arm64/boot/dts/freescale/imx8mm-
> > mx8menlo.dts
> > new file mode 100644
> > index 0000000000000..b2c3370a466d6
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-mx8menlo.dts
> > @@ -0,0 +1,331 @@
> > +// SPDX-License-Identifier: GPL-2.0+ OR MIT
> > +/*
> > + * Copyright 2021-2022 Marek Vasut <marex at denx.de>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "imx8mm-verdin.dtsi"
> > +
> > +/ {
> > + model = "MENLO MX8MM EMBEDDED DEVICE";
> > + compatible = "menlo,mx8menlo",
> > + "toradex,verdin-imx8mm",
> > + "fsl,imx8mm";
> > +
> > + /delete-node/ gpio-keys;
> would it be better if we had a label in the imx8mm-verdin.dtsi and we
> could just set status=disabled here?
I agree, would probably be better for future maintainability.
> > +
> > + leds {
> > + compatible = "gpio-leds";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_led>;
> > +
> > + user1 {
> > + label = "TestLed601";
> > + gpios = <&gpio4 18 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "mmc0";
> > + };
> > +
> > + user2 {
> > + label = "TestLed602";
> > + gpios = <&gpio4 10 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "heartbeat";
> > + };
> > + };
> > +
> > + beeper {
> > + compatible = "gpio-beeper";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_beeper>;
> > + gpios = <&gpio5 3 GPIO_ACTIVE_HIGH>;
> > + };
> > +};
> > +
> > +&ecspi1 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_ecspi1>;
> > + cs-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;
> > + status = "okay";
> > +
> > + /* CAN controller on the baseboard */
> > + canfd: can at 0 {
> > + compatible = "microchip,mcp2518fd";
> > + clocks = <&clk20m>;
> You are using the node defined in the verdin.dtsi here, while I guess
> this is a separate oscillator part of the carrier board.
>
> Should you define a new clock? My concern is that we had discussion to
> change the SoM to move from 20 to 40 MHz, and we would remove this entry
> from the dtsi if we would do such a change.
>
> > + gpio-controller;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
> > + reg = <0>;
> > + spi-max-frequency = <2000000>;
> > + status = "okay";
> > + };
> > +
> > +};
> > +
> > +&ecspi2 {
> > + pinctrl-0 = <&pinctrl_ecspi2 &pinctrl_gpio1>;
> > + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>, <&gpio3 4 GPIO_ACTIVE_LOW>;
> > + status = "disabled";
> > +};
> > +
> > +ðphy0 {
> > + max-speed = <100>;
> > +};
> > +
> > +&fec1 {
> > + status = "okay";
> > +};
> > +
> > +&flexspi {
> > + status = "okay";
> > +
> > + flash at 0 {
> > + reg = <0>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + compatible = "jedec,spi-nor";
> > + spi-max-frequency = <66000000>;
> > + spi-rx-bus-width = <4>;
> > + spi-tx-bus-width = <4>;
> > + };
> > +};
> > +
> > +&gpio1 {
> > + gpio-line-names =
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "";
> > +};
>
> It does not look an elegant solution to me to clean-up the
> gpio-line-names, but I guess is the only option you have ...
>
> > +
> > +&gpio2 {
> > + gpio-line-names =
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "";
> > +};
> > +
> > +&gpio3 {
> > + gpio-line-names =
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "DISP_reset", "KBD_intI",
> > + "", "", "", "",
> > + "", "", "", "";
> > +};
> > +
> > +&gpio4 {
> > + /*
> > + * CPLD_D[n] is ARM_CPLD[n] in schematic
> > + * CPLD_int is SA_INTERRUPT in schematic
> > + * CPLD_reset is RESET_SOFT in schematic
> > + */
> > + gpio-line-names =
> > + "CPLD_D[1]", "CPLD_int", "CPLD_reset", "",
> > + "", "CPLD_D[0]", "", "",
> > + "", "", "", "CPLD_D[2]",
> > + "CPLD_D[3]", "CPLD_D[4]", "CPLD_D[5]", "CPLD_D[6]",
> > + "CPLD_D[7]", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "KBD_intK",
> > + "", "", "", "";
> > +};
> > +
> > +&gpio5 {
> > + gpio-line-names =
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "",
> > + "", "", "", "";
> > +};
> > +
> > +&gpio_expander_21 {
> > + status = "okay";
> > +};
> > +
> > +&hwmon {
> > + status = "okay";
> > +};
> you delete this node in a few lines, why setting the status?
> (`hwmon: hwmon at 40`)
>
> > +
> > +&i2c1 {
> > + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */
> > + clock-frequency = <100000>;
> should this and the related changes in the other i2c nodes done in the
> verdin.dtsi? Marcel? (errata is here:
> https://www.nxp.com/docs/en/errata/IMX8MDQLQ_1N14W.pdf)
Hehe, good catch. Yeah, we probably should move this one into imx8mm-verdin.dtsi instead. On the other hand, it
won't hurt doing it twice resp. this one will always override it like that.
@Francesco: You referenced the wrong errata document. Remember, we are talking about the i.MX 8M Mini here and
not the i.MX8M. The proper one would be the following:
https://www.nxp.com/webapp/Download?colCode=IMX8MM_0N87W
Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p).
> > +};
> > +
> > +&i2c2 {
> > + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */
> > + clock-frequency = <100000>;
> > +};
> > +
> > +&i2c3 {
> > + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */
> > + clock-frequency = <100000>;
> > + status = "okay";
> > +};
> > +
> > +&i2c4 {
> > + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */
> > + clock-frequency = <100000>;
> > + /delete-node/ bridge at 2c;
> > + /delete-node/ hwmon at 40;
> > + /delete-node/ hdmi at 48;
> > + /delete-node/ touch at 4a;
> > + /delete-node/ hwmontemp at 4f;
> > + /delete-node/ eeprom at 50;
> > + /delete-node/ eeprom at 57;
> All of those are disabled in the dtsi, is it really worth deleting the
> nodes?
I am also not too big of a fan of this delete-node directive. At least it already hurt us multiple times in our
downstream device tree overlay endeavour.
> > +};
> > +
> > +&iomuxc {
Just for clarity I would probably repeat the following from the module's device tree here:
pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_gpio7>, <&pinctrl_gpio_hog1>,
> > + <&pinctrl_gpio_hog2>, <&pinctrl_gpio_hog3>;
> > +
> > + pinctrl_beeper: beepergrp {
> > + fsl,pins = <
> > + MX8MM_IOMUXC_SPDIF_TX_GPIO5_IO3 0x1c4
> > + >;
> > + };
> > +
> > + pinctrl_ecspi1: ecspi1grp {
> > + fsl,pins = <
> > + MX8MM_IOMUXC_ECSPI1_SCLK_ECSPI1_SCLK 0x4
> > + MX8MM_IOMUXC_ECSPI1_MOSI_ECSPI1_MOSI 0x4
> > + MX8MM_IOMUXC_ECSPI1_MISO_ECSPI1_MISO 0x1c4
> > + MX8MM_IOMUXC_ECSPI1_SS0_GPIO5_IO9 0x1c4
> > + >;
> > + };
> > +
> > + pinctrl_led: ledgrp {
> > + fsl,pins = <
> > + MX8MM_IOMUXC_SAI1_TXD6_GPIO4_IO18 0x1c4
> > + MX8MM_IOMUXC_SAI1_TXFS_GPIO4_IO10 0x1c4
> > + >;
> > + };
> > +
> > + pinctrl_uart4_rts: uart4rtsgrp {
> > + fsl,pins = <
> > + /* SODIMM 222 */
> > + MX8MM_IOMUXC_GPIO1_IO09_GPIO1_IO9 0x184
> > + >;
> > + };
> > +};
At least in imx8mm-verdin.dtsi we do have this iomuxc node at the very end. Not sure whether this is a hard
convention though.
> > +
> > +&pinctrl_gpio1 {
> > + fsl,pins = <
> > + /* SODIMM 206 */
> > + MX8MM_IOMUXC_NAND_CE3_B_GPIO3_IO4 0x1c4
> > + >;
> > +};
> > +
> > +&pinctrl_gpio_hog1 {
> > + fsl,pins = <
> > + /* SODIMM 88 */
> > + MX8MM_IOMUXC_SAI1_MCLK_GPIO4_IO20 0x1c4
> > + /* CPLD_int */
> > + MX8MM_IOMUXC_SAI1_RXC_GPIO4_IO1 0x1c4
> > + /* CPLD_reset */
> > + MX8MM_IOMUXC_SAI1_RXD0_GPIO4_IO2 0x1c4
> > + /* SODIMM 94 */
> > + MX8MM_IOMUXC_SAI1_RXD1_GPIO4_IO3 0x1c4
> > + /* SODIMM 96 */
> > + MX8MM_IOMUXC_SAI1_RXD2_GPIO4_IO4 0x1c4
> > + /* CPLD_D[7] */
> > + MX8MM_IOMUXC_SAI1_RXD3_GPIO4_IO5 0x1c4
> > + /* CPLD_D[6] */
> > + MX8MM_IOMUXC_SAI1_RXFS_GPIO4_IO0 0x1c4
> > + /* CPLD_D[5] */
> > + MX8MM_IOMUXC_SAI1_TXC_GPIO4_IO11 0x1c4
> > + /* CPLD_D[4] */
> > + MX8MM_IOMUXC_SAI1_TXD0_GPIO4_IO12 0x1c4
> > + /* CPLD_D[3] */
> > + MX8MM_IOMUXC_SAI1_TXD1_GPIO4_IO13 0x1c4
> > + /* CPLD_D[2] */
> > + MX8MM_IOMUXC_SAI1_TXD2_GPIO4_IO14 0x1c4
> > + /* CPLD_D[1] */
> > + MX8MM_IOMUXC_SAI1_TXD3_GPIO4_IO15 0x1c4
> > + /* CPLD_D[0] */
> > + MX8MM_IOMUXC_SAI1_TXD4_GPIO4_IO16 0x1c4
> > + /* KBD_intK */
> > + MX8MM_IOMUXC_SAI2_MCLK_GPIO4_IO27 0x1c4
> > + /* DISP_reset */
> > + MX8MM_IOMUXC_SAI5_RXD1_GPIO3_IO22 0x1c4
> > + /* KBD_intI */
> > + MX8MM_IOMUXC_SAI5_RXD2_GPIO3_IO23 0x1c4
> > + /* SODIMM 46 */
> > + MX8MM_IOMUXC_SAI5_RXD3_GPIO3_IO24 0x1c4
> > + >;
> > +};
> > +
> > +&pinctrl_uart1 {
> > + fsl,pins = <
> > + /* SODIMM 149 */
> > + MX8MM_IOMUXC_SAI2_RXFS_UART1_DCE_TX 0x1c4
> > + /* SODIMM 147 */
> > + MX8MM_IOMUXC_SAI2_RXC_UART1_DCE_RX 0x1c4
> > + /* SODIMM 210 */
> > + MX8MM_IOMUXC_UART3_RXD_UART1_DTE_RTS_B 0x1c4
> > + /* SODIMM 212 */
> > + MX8MM_IOMUXC_UART3_TXD_UART1_DTE_CTS_B 0x1c4
> > + >;
> > +};
> > +
> > +®_usb_otg1_vbus {
> > + /delete-property/ enable-active-high;
> > + gpio = <&gpio1 12 GPIO_ACTIVE_LOW>;
> > +};
> > +
> > +®_usb_otg2_vbus {
> > + /delete-property/ enable-active-high;
> > + gpio = <&gpio1 14 GPIO_ACTIVE_LOW>;
> > +};
Okay, I was on the verge of getting rid of that regulator and relying on the IP built-in USB power enable
functionality due to us having seen issues with such regulator during suspend/resume resp. power-off. However,
I guess, there might be other solutions using such regular regulators. Not sure...
> > +
> > +&sai2 {
> > + status = "disabled";
> > +};
> > +
> > +&uart1 {
> > + uart-has-rtscts;
> > + status = "okay";
> > +};
> > +
> > +&uart2 {
> > + uart-has-rtscts;
That one we already have in the module's device tree. But, yeah, I guess it won't hurt.
> > + status = "okay";
> > +};
> > +
> > +&uart4 {
> > + pinctrl-0 = <&pinctrl_uart4 &pinctrl_uart4_rts>;
> > + linux,rs485-enabled-at-boot-time;
> > + rts-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> > + status = "okay";
> > +};
> > +
> > +&usbotg1 {
> > + dr_mode = "gadget";
> > + status = "okay";
> > +};
> > +
> > +&usbotg2 {
> > + dr_mode = "host";
> > + status = "okay";
> > +};
> > +
> > +&usdhc2 {
> > + status = "okay";
> > +};
> > --
> > 2.35.1
Cheers
Marcel
More information about the linux-arm-kernel
mailing list