* [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board @ 2022-04-07 20:24 ` Marek Vasut 0 siblings, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-07 20:24 UTC (permalink / raw) To: linux-arm-kernel Cc: Marek Vasut, Fabio Estevam, Marcel Ziswiler, Peng Fan, Rob Herring, Shawn Guo, NXP Linux Team, devicetree Add DT compatible string for 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@denx.de> Cc: Fabio Estevam <festevam@denx.de> Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Shawn Guo <shawnguo@kernel.org> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: devicetree@vger.kernel.org To: linux-arm-kernel@lists.infradead.org --- Documentation/devicetree/bindings/arm/fsl.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml index 8a7ed7782e99f..a6286581fa13b 100644 --- a/Documentation/devicetree/bindings/arm/fsl.yaml +++ b/Documentation/devicetree/bindings/arm/fsl.yaml @@ -819,6 +819,7 @@ properties: - gw,imx8mm-gw7902 # i.MX8MM Gateworks Board - gw,imx8mm-gw7903 # i.MX8MM Gateworks Board - kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM + - menlo,mx8menlo # i.MX8MM Menlo board - toradex,verdin-imx8mm # Verdin iMX8M Mini Modules - toradex,verdin-imx8mm-nonwifi # Verdin iMX8M Mini Modules without Wi-Fi / BT - toradex,verdin-imx8mm-wifi # Verdin iMX8M Mini Wi-Fi / BT Modules -- 2.35.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board @ 2022-04-07 20:24 ` Marek Vasut 0 siblings, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-07 20:24 UTC (permalink / raw) To: linux-arm-kernel Cc: Marek Vasut, Fabio Estevam, Marcel Ziswiler, Peng Fan, Rob Herring, Shawn Guo, NXP Linux Team, devicetree Add DT compatible string for 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@denx.de> Cc: Fabio Estevam <festevam@denx.de> Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Shawn Guo <shawnguo@kernel.org> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: devicetree@vger.kernel.org To: linux-arm-kernel@lists.infradead.org --- Documentation/devicetree/bindings/arm/fsl.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml index 8a7ed7782e99f..a6286581fa13b 100644 --- a/Documentation/devicetree/bindings/arm/fsl.yaml +++ b/Documentation/devicetree/bindings/arm/fsl.yaml @@ -819,6 +819,7 @@ properties: - gw,imx8mm-gw7902 # i.MX8MM Gateworks Board - gw,imx8mm-gw7903 # i.MX8MM Gateworks Board - kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM + - menlo,mx8menlo # i.MX8MM Menlo board - toradex,verdin-imx8mm # Verdin iMX8M Mini Modules - toradex,verdin-imx8mm-nonwifi # Verdin iMX8M Mini Modules without Wi-Fi / BT - toradex,verdin-imx8mm-wifi # Verdin iMX8M Mini Wi-Fi / BT Modules -- 2.35.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-07 20:24 ` Marek Vasut (?) @ 2022-04-07 20:24 ` Marek Vasut 2022-04-08 6:46 ` Francesco Dolcini -1 siblings, 1 reply; 23+ messages in thread From: Marek Vasut @ 2022-04-07 20:24 UTC (permalink / raw) To: linux-arm-kernel Cc: Marek Vasut, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team 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@denx.de> Cc: Fabio Estevam <festevam@denx.de> Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Shawn Guo <shawnguo@kernel.org> Cc: NXP Linux Team <linux-imx@nxp.com> To: linux-arm-kernel@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@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; + + 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@0 { + compatible = "microchip,mcp2518fd"; + clocks = <&clk20m>; + 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@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 = + "", "", "", "", + "", "", "", "", + "", "", "", "", + "", "", "", "", + "", "", "", "", + "", "", "", "", + "", "", "", "", + "", "", "", ""; +}; + +&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"; +}; + +&i2c1 { + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */ + clock-frequency = <100000>; +}; + +&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@2c; + /delete-node/ hwmon@40; + /delete-node/ hdmi@48; + /delete-node/ touch@4a; + /delete-node/ hwmontemp@4f; + /delete-node/ eeprom@50; + /delete-node/ eeprom@57; +}; + +&iomuxc { + 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 + >; + }; +}; + +&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>; +}; + +&sai2 { + status = "disabled"; +}; + +&uart1 { + uart-has-rtscts; + status = "okay"; +}; + +&uart2 { + uart-has-rtscts; + 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-07 20:24 ` [PATCH 2/2] arm64: dts: imx8mm: " Marek Vasut @ 2022-04-08 6:46 ` Francesco Dolcini 2022-04-08 7:56 ` Marcel Ziswiler 2022-04-08 15:02 ` Marek Vasut 0 siblings, 2 replies; 23+ messages in thread From: Francesco Dolcini @ 2022-04-08 6:46 UTC (permalink / raw) To: Marek Vasut Cc: linux-arm-kernel, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team 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@denx.de> > Cc: Fabio Estevam <festevam@denx.de> > Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: NXP Linux Team <linux-imx@nxp.com> > To: linux-arm-kernel@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@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? > + > + 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@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@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@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) > +}; > + > +&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@2c; > + /delete-node/ hwmon@40; > + /delete-node/ hdmi@48; > + /delete-node/ touch@4a; > + /delete-node/ hwmontemp@4f; > + /delete-node/ eeprom@50; > + /delete-node/ eeprom@57; All of those are disabled in the dtsi, is it really worth deleting the nodes? > +}; > + > +&iomuxc { > + 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 > + >; > + }; > +}; > + > +&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>; > +}; > + > +&sai2 { > + status = "disabled"; > +}; > + > +&uart1 { > + uart-has-rtscts; > + status = "okay"; > +}; > + > +&uart2 { > + uart-has-rtscts; > + 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 6:46 ` Francesco Dolcini @ 2022-04-08 7:56 ` Marcel Ziswiler 2022-04-08 8:49 ` Francesco Dolcini 2022-04-08 15:31 ` Marek Vasut 2022-04-08 15:02 ` Marek Vasut 1 sibling, 2 replies; 23+ messages in thread From: Marcel Ziswiler @ 2022-04-08 7:56 UTC (permalink / raw) To: marex, Francesco Dolcini Cc: linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx 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@denx.de> > > Cc: Fabio Estevam <festevam@denx.de> > > Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > Cc: Peng Fan <peng.fan@nxp.com> > > Cc: Shawn Guo <shawnguo@kernel.org> > > Cc: NXP Linux Team <linux-imx@nxp.com> > > To: linux-arm-kernel@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@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@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@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@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@2c; > > + /delete-node/ hwmon@40; > > + /delete-node/ hdmi@48; > > + /delete-node/ touch@4a; > > + /delete-node/ hwmontemp@4f; > > + /delete-node/ eeprom@50; > > + /delete-node/ eeprom@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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 7:56 ` Marcel Ziswiler @ 2022-04-08 8:49 ` Francesco Dolcini 2022-04-08 15:29 ` Tim Harvey 2022-04-08 16:20 ` Marek Vasut 2022-04-08 15:31 ` Marek Vasut 1 sibling, 2 replies; 23+ messages in thread From: Francesco Dolcini @ 2022-04-08 8:49 UTC (permalink / raw) To: Marcel Ziswiler Cc: marex, Francesco Dolcini, linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: > On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: > > On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: > > > + > > > +&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. > > Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p). It looks like even i.MX7 is affected, and NXP has a quirk in the i2c-imx driver [0] [0] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 Francesco _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 8:49 ` Francesco Dolcini @ 2022-04-08 15:29 ` Tim Harvey 2022-04-08 15:54 ` Adam Ford 2022-04-08 17:21 ` Marek Vasut 2022-04-08 16:20 ` Marek Vasut 1 sibling, 2 replies; 23+ messages in thread From: Tim Harvey @ 2022-04-08 15:29 UTC (permalink / raw) To: Francesco Dolcini, marex Cc: Marcel Ziswiler, linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx, Adam Ford On Fri, Apr 8, 2022 at 2:05 AM Francesco Dolcini <francesco.dolcini@toradex.com> wrote: > > On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: > > On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: > > > On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: > > > > + > > > > +&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. > > > > Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p). > > It looks like even i.MX7 is affected, and NXP has a quirk in the > i2c-imx driver [0] > > [0] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 > > Francesco Francesco and Marek, Interesting - I've never noticed this errata. I don't see mention of it in the imx8mm-evk which sets the busses to 400khz, and all the other verdin I2C busses are set to 400khz as well. Should we address this in the i2c driver in U-Boot (and Linux) instead? Otherwise, I think all of us board maintainers should be moving the i2c clock to 100kHz. Its not clear to me what I2C slaves may be affected by this issue.... perhaps maintainers are only concerned with things that go off-board? Best Regards, Tim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 15:29 ` Tim Harvey @ 2022-04-08 15:54 ` Adam Ford 2022-04-08 16:16 ` Tim Harvey 2022-04-08 17:21 ` Marek Vasut 1 sibling, 1 reply; 23+ messages in thread From: Adam Ford @ 2022-04-08 15:54 UTC (permalink / raw) To: Tim Harvey Cc: Francesco Dolcini, marex, Marcel Ziswiler, linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx On Fri, Apr 8, 2022 at 10:29 AM Tim Harvey <tharvey@gateworks.com> wrote: > > On Fri, Apr 8, 2022 at 2:05 AM Francesco Dolcini > <francesco.dolcini@toradex.com> wrote: > > > > On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: > > > On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: > > > > On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: > > > > > + > > > > > +&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. > > > > > > Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p). > > > > It looks like even i.MX7 is affected, and NXP has a quirk in the > > i2c-imx driver [0] > > > > [0] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 > > > > Francesco > > Francesco and Marek, > > Interesting - I've never noticed this errata. > > I don't see mention of it in the imx8mm-evk which sets the busses to > 400khz, and all the other verdin I2C busses are set to 400khz as well. > > Should we address this in the i2c driver in U-Boot (and Linux) > instead? Otherwise, I think all of us board maintainers should be > moving the i2c clock to 100kHz. Its not clear to me what I2C slaves > may be affected by this issue.... perhaps maintainers are only > concerned with things that go off-board? My company has a downstream kernel and we've had no issues with 384KHz instead of 400. I am not sure that slowing all the way down to 100 is necessary. I can't imagine I2C devices care if the clocks go a bit slower. From my understanding of the errata is that it violates the rise-time or something like that. The peripherals I use don't appear to care that it's slightly out of spec at 400KHz. adam > > Best Regards, > > Tim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 15:54 ` Adam Ford @ 2022-04-08 16:16 ` Tim Harvey 2022-04-08 16:24 ` Marcel Ziswiler 0 siblings, 1 reply; 23+ messages in thread From: Tim Harvey @ 2022-04-08 16:16 UTC (permalink / raw) To: Adam Ford Cc: Francesco Dolcini, marex, Marcel Ziswiler, linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx On Fri, Apr 8, 2022 at 8:54 AM Adam Ford <aford173@gmail.com> wrote: > > On Fri, Apr 8, 2022 at 10:29 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > On Fri, Apr 8, 2022 at 2:05 AM Francesco Dolcini > > <francesco.dolcini@toradex.com> wrote: > > > > > > On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: > > > > On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: > > > > > On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: > > > > > > + > > > > > > +&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. > > > > > > > > Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p). > > > > > > It looks like even i.MX7 is affected, and NXP has a quirk in the > > > i2c-imx driver [0] > > > > > > [0] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 > > > > > > Francesco > > > > Francesco and Marek, > > > > Interesting - I've never noticed this errata. > > > > I don't see mention of it in the imx8mm-evk which sets the busses to > > 400khz, and all the other verdin I2C busses are set to 400khz as well. > > > > Should we address this in the i2c driver in U-Boot (and Linux) > > instead? Otherwise, I think all of us board maintainers should be > > moving the i2c clock to 100kHz. Its not clear to me what I2C slaves > > may be affected by this issue.... perhaps maintainers are only > > concerned with things that go off-board? > > My company has a downstream kernel and we've had no issues with 384KHz > instead of 400. I am not sure that slowing all the way down to 100 is > necessary. I can't imagine I2C devices care if the clocks go a bit > slower. From my understanding of the errata is that it violates the > rise-time or something like that. The peripherals I use don't appear > to care that it's slightly out of spec at 400KHz. > Adam, Yes good point... 384Khz is the workaround in the errata. So instead of all of us moving our I2C clocks to 384000 (which we should according to the errata) should we do this in the i2c driver based off chip type? Best Regards, Tim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 16:16 ` Tim Harvey @ 2022-04-08 16:24 ` Marcel Ziswiler 0 siblings, 0 replies; 23+ messages in thread From: Marcel Ziswiler @ 2022-04-08 16:24 UTC (permalink / raw) To: aford173, tharvey Cc: linux-arm-kernel, marex, festevam, peng.fan, Francesco Dolcini, shawnguo, linux-imx On Fri, 2022-04-08 at 09:16 -0700, Tim Harvey wrote: > On Fri, Apr 8, 2022 at 8:54 AM Adam Ford <aford173@gmail.com> wrote: > > > > On Fri, Apr 8, 2022 at 10:29 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > On Fri, Apr 8, 2022 at 2:05 AM Francesco Dolcini > > > <francesco.dolcini@toradex.com> wrote: > > > > > > > > On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: > > > > > On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: > > > > > > On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: > > > > > > > + > > > > > > > +&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. > > > > > > > > > > Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata > > > > > (;-p). > > > > > > > > It looks like even i.MX7 is affected, and NXP has a quirk in the > > > > i2c-imx driver [0] > > > > > > > > [0] > > > > https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 > > > > > > > > Francesco > > > > > > Francesco and Marek, > > > > > > Interesting - I've never noticed this errata. > > > > > > I don't see mention of it in the imx8mm-evk which sets the busses to > > > 400khz, and all the other verdin I2C busses are set to 400khz as well. > > > > > > Should we address this in the i2c driver in U-Boot (and Linux) > > > instead? Otherwise, I think all of us board maintainers should be > > > moving the i2c clock to 100kHz. Its not clear to me what I2C slaves > > > may be affected by this issue.... perhaps maintainers are only > > > concerned with things that go off-board? > > > > My company has a downstream kernel and we've had no issues with 384KHz > > instead of 400. I am not sure that slowing all the way down to 100 is > > necessary. I can't imagine I2C devices care if the clocks go a bit > > slower. From my understanding of the errata is that it violates the > > rise-time or something like that. The peripherals I use don't appear > > to care that it's slightly out of spec at 400KHz. > > > > Adam, > > Yes good point... 384Khz is the workaround in the errata. So instead > of all of us moving our I2C clocks to 384000 (which we should > according to the errata) should we do this in the i2c driver based off > chip type? How about doing something along the lines of that i.MX 7 quirk Francesco discovered in NXP's downstream? Cheers Marcel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 15:29 ` Tim Harvey 2022-04-08 15:54 ` Adam Ford @ 2022-04-08 17:21 ` Marek Vasut 1 sibling, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-08 17:21 UTC (permalink / raw) To: Tim Harvey, Francesco Dolcini Cc: Marcel Ziswiler, linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx, Adam Ford On 4/8/22 17:29, Tim Harvey wrote: > On Fri, Apr 8, 2022 at 2:05 AM Francesco Dolcini > <francesco.dolcini@toradex.com> wrote: >> >> On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: >>> On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: >>>> On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: >>>>> + >>>>> +&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. >>> >>> Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p). >> >> It looks like even i.MX7 is affected, and NXP has a quirk in the >> i2c-imx driver [0] >> >> [0] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 >> >> Francesco > > Francesco and Marek, > > Interesting - I've never noticed this errata. > > I don't see mention of it in the imx8mm-evk which sets the busses to > 400khz, and all the other verdin I2C busses are set to 400khz as well. > > Should we address this in the i2c driver in U-Boot (and Linux) > instead? Otherwise, I think all of us board maintainers should be > moving the i2c clock to 100kHz. Its not clear to me what I2C slaves > may be affected by this issue.... perhaps maintainers are only > concerned with things that go off-board? 320 kHz or 100 kHz divide down nicely, so I use either of those depending on whether I need faster I2C or not. I2C IMX Linux driver patch is posted here: https://patchwork.ozlabs.org/project/linux-i2c/patch/20220408171524.73551-1-marex@denx.de/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 8:49 ` Francesco Dolcini 2022-04-08 15:29 ` Tim Harvey @ 2022-04-08 16:20 ` Marek Vasut 2022-04-08 16:46 ` Fabio Estevam 1 sibling, 1 reply; 23+ messages in thread From: Marek Vasut @ 2022-04-08 16:20 UTC (permalink / raw) To: Francesco Dolcini, Marcel Ziswiler Cc: linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx On 4/8/22 10:49, Francesco Dolcini wrote: > On Fri, Apr 08, 2022 at 09:56:09AM +0200, Marcel Ziswiler wrote: >> On Fri, 2022-04-08 at 08:46 +0200, Francesco Dolcini wrote: >>> On Thu, Apr 07, 2022 at 10:24:56PM +0200, Marek Vasut wrote: >>>> + >>>> +&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. >> >> Anyway, looks like NXP has not fixed this and is re-using the exact same I2C IP with the same errata (;-p). > > It looks like even i.MX7 is affected, and NXP has a quirk in the > i2c-imx driver [0] > > [0] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/i2c/busses/i2c-imx.c?h=lf-5.15.y&id=493b3892ee156af529796641889ca19b971735d2 That's downstream btw, the quirk is not in mainline, but maybe we should add a quirk to the i2c-imx.c . So all of MX8M{M,N,P,Q) , MX7{S,D} , MX6{UL{,L,Z},S{,LL,X},S,D,DL,Q,DP,QP} are affected. i.MX53 is not affected, but maybe the errata wasn't found yet, the errata document rev.6 predates 2016 from when the ERR007805 / e7805 originates in MX6 errata sheets. MX7ULP, MX8Q, MX8X are not affected? What about MX8ULP ? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 16:20 ` Marek Vasut @ 2022-04-08 16:46 ` Fabio Estevam 0 siblings, 0 replies; 23+ messages in thread From: Fabio Estevam @ 2022-04-08 16:46 UTC (permalink / raw) To: Marek Vasut Cc: Francesco Dolcini, Marcel Ziswiler, linux-arm-kernel, peng.fan, shawnguo, linux-imx Hi Marek, On 08/04/2022 13:20, Marek Vasut wrote: > That's downstream btw, the quirk is not in mainline, but maybe we > should add a quirk to the i2c-imx.c . Agreed. > So all of MX8M{M,N,P,Q) , MX7{S,D} , > MX6{UL{,L,Z},S{,LL,X},S,D,DL,Q,DP,QP} are affected. > > i.MX53 is not affected, but maybe the errata wasn't found yet, the > errata document rev.6 predates 2016 from when the ERR007805 / e7805 > originates in MX6 errata sheets. > > MX7ULP, MX8Q, MX8X are not affected? What about MX8ULP ? These 4 chips use a different I2C IP. Its driver is drivers/i2c/busses/i2c-imx-lpi2c.c Regards, Fabio Estevam _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 7:56 ` Marcel Ziswiler 2022-04-08 8:49 ` Francesco Dolcini @ 2022-04-08 15:31 ` Marek Vasut 1 sibling, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-08 15:31 UTC (permalink / raw) To: Marcel Ziswiler, Francesco Dolcini Cc: linux-arm-kernel, festevam, peng.fan, shawnguo, linux-imx On 4/8/22 09:56, Marcel Ziswiler wrote: > Hi Marek Hi, I'm skipping the replies to what I commended on to Francesco already. [...] >>> +&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). It seems they are all broken, MX8M{M,N,P,Q}, same problem. Maybe adding such limit in the controller driver might be the better approach ? [...] >>> +&i2c4 { >>> + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */ >>> + clock-frequency = <100000>; >>> + /delete-node/ bridge@2c; >>> + /delete-node/ hwmon@40; >>> + /delete-node/ hdmi@48; >>> + /delete-node/ touch@4a; >>> + /delete-node/ hwmontemp@4f; >>> + /delete-node/ eeprom@50; >>> + /delete-node/ eeprom@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. Indeed, the /delete-node/ is a DTC directive, it can not be represented in DTB, so anything which is optional shouldn't be in the base DT, else the DTO cannot remove it afterward and that is a real nasty catch. That's why the /delete-node/ is a good idea in base DT, then DTOs can only fill in the extras which is no problem. And since this board does use DTOs for display pipeline, anything which shouldn't be in the base DT should not be present in the base DTB at all. [...] >>> + 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. I cannot tell, I saw it done either way in different DTs. [...] >>> +®_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... That should be doable too, on this board the regulator polarity is just inverted compared to Dahlia carrier board. So, all I would have to do is: /delete-property/ power-active-high; Or add that property to Dahlia DT and keep it out of the Verdin SoM DTSI, then I won't add it. Will you send a patch and CC me ? >>> +&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. Fixed [...] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 6:46 ` Francesco Dolcini 2022-04-08 7:56 ` Marcel Ziswiler @ 2022-04-08 15:02 ` Marek Vasut 2022-04-10 8:46 ` Francesco Dolcini 1 sibling, 1 reply; 23+ messages in thread From: Marek Vasut @ 2022-04-08 15:02 UTC (permalink / raw) To: Francesco Dolcini Cc: linux-arm-kernel, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team On 4/8/22 08:46, Francesco Dolcini wrote: Hi, >> +++ 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@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? It would be better if there was Verdin SoM dtsi and Verdin carrier board dts which includes the SoM dtsi. Right now, it seems these two things are conflated a bit. There are no GPIO buttons on the Verdin SoM, there are some on the Dahlia carrier board I think. >> + 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@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. Is the 20 MHz oscillator on your SoM or on the carrier board ? [...] >> +&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 ... Right, I need pin names which are compatible with the previous generation hardware. [...] >> +&hwmon { >> + status = "okay"; >> +}; > you delete this node in a few lines, why setting the status? > (`hwmon: hwmon@40`) This one is superfluous indeed. >> +&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) More like the imx8m{m,n,p,q}.dtsi should set this to 320000 because that is nicely divided down and well within tolerance, and then boards should be able to override it to whatever frequency they want. Maybe this should even be limited in the imx i2c controller driver itself? >> +}; >> + >> +&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@2c; >> + /delete-node/ hwmon@40; >> + /delete-node/ hdmi@48; >> + /delete-node/ touch@4a; >> + /delete-node/ hwmontemp@4f; >> + /delete-node/ eeprom@50; >> + /delete-node/ eeprom@57; > All of those are disabled in the dtsi, is it really worth deleting the > nodes? They are not present on the hardware, why should they be described in the DT ? Hence the /delete-node/ . That reminds me -- why is there lt8912 described in verdin.dtsi when this lt8912 chip is not on the SoM, not even on the carrier board, but on an optional carrier board expansion card ? [...] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 15:02 ` Marek Vasut @ 2022-04-10 8:46 ` Francesco Dolcini 2022-04-10 21:37 ` Marek Vasut 0 siblings, 1 reply; 23+ messages in thread From: Francesco Dolcini @ 2022-04-10 8:46 UTC (permalink / raw) To: Marek Vasut Cc: Francesco Dolcini, linux-arm-kernel, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team On Fri, Apr 08, 2022 at 05:02:15PM +0200, Marek Vasut wrote: > On 4/8/22 08:46, Francesco Dolcini wrote: > > > + /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? > > It would be better if there was Verdin SoM dtsi and Verdin carrier board dts > which includes the SoM dtsi. Right now, it seems these two things are > conflated a bit. > > There are no GPIO buttons on the Verdin SoM, there are some on the Dahlia > carrier board I think. The GPIO keys, for example the suspend button, are part of Verdin family SODIMM connector pinout/definition (see related datasheets). In the SoM dtsi we implement our standard family definition. Of course, you are free to redefine this in any way you prefer. In general the way we envision this is to just enable/disable in the carrier board or overlay dts, this is the reason I proposed to add a label there. I do not see any real value on deleting the node at all, just some potential for additional maintenance burden. > > > + /* CAN controller on the baseboard */ > > > + canfd: can@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. > > Is the 20 MHz oscillator on your SoM or on the carrier board ? In the SoM, not available on the SODIMM connector. Your clock if for sure not this one. > > > +&i2c4 { > > > + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */ > > > + clock-frequency = <100000>; > > > + /delete-node/ bridge@2c; > > > + /delete-node/ hwmon@40; > > > + /delete-node/ hdmi@48; > > > + /delete-node/ touch@4a; > > > + /delete-node/ hwmontemp@4f; > > > + /delete-node/ eeprom@50; > > > + /delete-node/ eeprom@57; > > All of those are disabled in the dtsi, is it really worth deleting the > > nodes? > > They are not present on the hardware, why should they be described in the DT > ? Hence the /delete-node/ . Not 100% correct (see the SoM datasheet), anyway is this causing you any real trouble having those nodes disabled instead of removed from the dts? I assume nor the file size nor the parsing time are an issue there. Of course, not a big deal for me you removing this nodes in the menlo dts, but in general I'm happy to make your life easier when using our som dtsi file, this is the reason for asking. Francesco _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-10 8:46 ` Francesco Dolcini @ 2022-04-10 21:37 ` Marek Vasut 2022-04-13 9:44 ` Francesco Dolcini 0 siblings, 1 reply; 23+ messages in thread From: Marek Vasut @ 2022-04-10 21:37 UTC (permalink / raw) To: Francesco Dolcini Cc: linux-arm-kernel, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team On 4/10/22 10:46, Francesco Dolcini wrote: > On Fri, Apr 08, 2022 at 05:02:15PM +0200, Marek Vasut wrote: >> On 4/8/22 08:46, Francesco Dolcini wrote: >>>> + /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? >> >> It would be better if there was Verdin SoM dtsi and Verdin carrier board dts >> which includes the SoM dtsi. Right now, it seems these two things are >> conflated a bit. >> >> There are no GPIO buttons on the Verdin SoM, there are some on the Dahlia >> carrier board I think. > > The GPIO keys, for example the suspend button, are part of Verdin family > SODIMM connector pinout/definition (see related datasheets). In the SoM > dtsi we implement our standard family definition. > > Of course, you are free to redefine this in any way you prefer. In > general the way we envision this is to just enable/disable in the > carrier board or overlay dts, this is the reason I proposed to add > a label there. I do not see any real value on deleting the node at all, > just some potential for additional maintenance burden. There are two reasons for not adding DT nodes for hardware which is not populated: - You are polluting the DT with unused nodes representing hardware which can never be present on the system, that makes the DT bigger and more complex, for no reason. - Once DTOs enter the picture, these so far only useless nodes and properties actively become a problem. You cannot delete either node or property from a base DT using a DTO, because neither /delete-node/ nor /delete-property/ can be encoded into the DTO blob . So, speaking about this GPIO button which is not physically on the SoM, I would propose the following: - Keep some sort of gpio-line-names property on the SoM level, to have mapping between GPIOs and SoM edge connector pins - Move the "gpio-button" node into the carrier board DT If someone designs a carrier board with this button populated, then they should describe it in the DT. If someone designs a carrier board without any physical button, then they won't describe it in the DT. And as a bit of an encore, if you have optional components on a SoM (e.g. you have different SoM variants, some of which may contain extra components and some of them may not, e.g. to save cost or whatever), then you should describe the components in SoM DT, but set it to status="disabled" by default. And then when a carrier board is populated with specific SoM model, it can enable those extra populated SoM components on carrier board DT level (or in some proxy SoM-variant DTSI if you really want to be very precise about the separation, but that leads to combinatorial explosion of SoM-variant DTSIs over time). >>>> + /* CAN controller on the baseboard */ >>>> + canfd: can@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. >> >> Is the 20 MHz oscillator on your SoM or on the carrier board ? > In the SoM, not available on the SODIMM connector. Your clock if for > sure not this one. Right, lemme switch this one to dedicated clock node. >>>> +&i2c4 { >>>> + /* IMX8MM ERRATA e7805 -- I2C is limited to 384 kHz due to SoC bug */ >>>> + clock-frequency = <100000>; >>>> + /delete-node/ bridge@2c; >>>> + /delete-node/ hwmon@40; >>>> + /delete-node/ hdmi@48; >>>> + /delete-node/ touch@4a; >>>> + /delete-node/ hwmontemp@4f; >>>> + /delete-node/ eeprom@50; >>>> + /delete-node/ eeprom@57; >>> All of those are disabled in the dtsi, is it really worth deleting the >>> nodes? >> >> They are not present on the hardware, why should they be described in the DT >> ? Hence the /delete-node/ . > Not 100% correct (see the SoM datasheet) I can't find e.g. the hdmi@48 LT8912 DSI-to-HDMI bridge, nor the bridge@2c SN65DSI83 DSI-to-LVDS bridge in the datasheet, and visual inspection of the SoM also leads to nothing. I also cannot find them on the Dahlia carrier board. But I can find them on the optional expansion modules for the Dahlia carrier board. So, why are they described in the SoM DT (they shouldn't be) ? Maybe the other components are actually on the SoM, but then, why are those not enabled by default ? Maybe because not all SoM variants have them populated ? > , anyway is this causing you any > real trouble having those nodes disabled instead of removed from the > dts? I assume nor the file size nor the parsing time are an issue there. See above, it is the same argument as regarding the gpio-button . The SoM DTSI shouldn't become a dumping ground for nodes describing any random hardware which might be attached to arbitrary carrier board. > Of course, not a big deal for me you removing this nodes in the menlo > dts, but in general I'm happy to make your life easier when using our > som dtsi file, this is the reason for asking. See above, I hope that clarifies it. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-10 21:37 ` Marek Vasut @ 2022-04-13 9:44 ` Francesco Dolcini 2022-04-13 14:14 ` Marek Vasut 0 siblings, 1 reply; 23+ messages in thread From: Francesco Dolcini @ 2022-04-13 9:44 UTC (permalink / raw) To: Marek Vasut Cc: Francesco Dolcini, linux-arm-kernel, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team Hello Marek On Sun, Apr 10, 2022 at 11:37:52PM +0200, Marek Vasut wrote: > On 4/10/22 10:46, Francesco Dolcini wrote: > > On Fri, Apr 08, 2022 at 05:02:15PM +0200, Marek Vasut wrote: > > > On 4/8/22 08:46, Francesco Dolcini wrote: > > > > > + /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? > > > > > > It would be better if there was Verdin SoM dtsi and Verdin carrier board dts > > > which includes the SoM dtsi. Right now, it seems these two things are > > > conflated a bit. > > > > > > There are no GPIO buttons on the Verdin SoM, there are some on the Dahlia > > > carrier board I think. > > > > The GPIO keys, for example the suspend button, are part of Verdin family > > SODIMM connector pinout/definition (see related datasheets). In the SoM > > dtsi we implement our standard family definition. > > > > Of course, you are free to redefine this in any way you prefer. In > > general the way we envision this is to just enable/disable in the > > carrier board or overlay dts, this is the reason I proposed to add > > a label there. I do not see any real value on deleting the node at all, > > just some potential for additional maintenance burden. > > There are two reasons for not adding DT nodes for hardware which is not > populated: > - You are polluting the DT with unused nodes representing hardware which > can never be present on the system, that makes the DT bigger and more > complex, for no reason. > - Once DTOs enter the picture, these so far only useless nodes and > properties actively become a problem. You cannot delete either node or > property from a base DT using a DTO, because neither /delete-node/ nor > /delete-property/ can be encoded into the DTO blob . Ok, I understand your arguments and I agree they are fully valid. We (Toradex) had some discussion about in the past and we still see benefit on the way we are doing it never the less. - The SoM dtsi representing not only the functionality implemented into the SoM, but the whole connector pinout to the carrier makes very easy to just include a different som.dtsi in the carrier board dts and just switch SoM, for example from a colibri-imx6 to a colibri-imx7. - We avoid code duplication - Even if the DTO cannot `delete`, it can disable a node. We do our best to have label and keep nodes disabled when functionality is not self-contained in the SoM. This is working for us pretty well so far and the majority of the users of ours modules rely on that, changing it now would just be too disruptive. I would propose that you go with the delete-node as you are doing and we keep the verdin.dtsi as it is. Thanks once more for the very good discussion, I hope that my proposal works fine for you. Francesco _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-13 9:44 ` Francesco Dolcini @ 2022-04-13 14:14 ` Marek Vasut 0 siblings, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-13 14:14 UTC (permalink / raw) To: Francesco Dolcini Cc: linux-arm-kernel, Fabio Estevam, Marcel Ziswiler, Peng Fan, Shawn Guo, NXP Linux Team On 4/13/22 11:44, Francesco Dolcini wrote: > Hello Marek Hi, > On Sun, Apr 10, 2022 at 11:37:52PM +0200, Marek Vasut wrote: >> On 4/10/22 10:46, Francesco Dolcini wrote: >>> On Fri, Apr 08, 2022 at 05:02:15PM +0200, Marek Vasut wrote: >>>> On 4/8/22 08:46, Francesco Dolcini wrote: >>>>>> + /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? >>>> >>>> It would be better if there was Verdin SoM dtsi and Verdin carrier board dts >>>> which includes the SoM dtsi. Right now, it seems these two things are >>>> conflated a bit. >>>> >>>> There are no GPIO buttons on the Verdin SoM, there are some on the Dahlia >>>> carrier board I think. >>> >>> The GPIO keys, for example the suspend button, are part of Verdin family >>> SODIMM connector pinout/definition (see related datasheets). In the SoM >>> dtsi we implement our standard family definition. >>> >>> Of course, you are free to redefine this in any way you prefer. In >>> general the way we envision this is to just enable/disable in the >>> carrier board or overlay dts, this is the reason I proposed to add >>> a label there. I do not see any real value on deleting the node at all, >>> just some potential for additional maintenance burden. >> >> There are two reasons for not adding DT nodes for hardware which is not >> populated: >> - You are polluting the DT with unused nodes representing hardware which >> can never be present on the system, that makes the DT bigger and more >> complex, for no reason. >> - Once DTOs enter the picture, these so far only useless nodes and >> properties actively become a problem. You cannot delete either node or >> property from a base DT using a DTO, because neither /delete-node/ nor >> /delete-property/ can be encoded into the DTO blob . > > Ok, I understand your arguments and I agree they are fully valid. > We (Toradex) had some discussion about in the past and we still see > benefit on the way we are doing it never the less. > > - The SoM dtsi representing not only the functionality implemented into > the SoM, but the whole connector pinout to the carrier makes very easy > to just include a different som.dtsi in the carrier board dts and just > switch SoM, for example from a colibri-imx6 to a colibri-imx7. > - We avoid code duplication > - Even if the DTO cannot `delete`, it can disable a node. We do our > best to have label and keep nodes disabled when functionality is not > self-contained in the SoM. You missed one important problem here, superfluous properties in the SoM DT: ®_usb_otg2_vbus { /delete-property/ enable-active-high; // <----- HERE }; You cannot get rid of those using DTOs. > This is working for us pretty well so far and the majority of the users > of ours modules rely on that, changing it now would just be too > disruptive. > > I would propose that you go with the delete-node as you are doing and we > keep the verdin.dtsi as it is. Let's do that, thanks. [...] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-07 20:24 ` Marek Vasut @ 2022-04-08 7:55 ` Marcel Ziswiler -1 siblings, 0 replies; 23+ messages in thread From: Marcel Ziswiler @ 2022-04-08 7:55 UTC (permalink / raw) To: linux-arm-kernel, marex Cc: peng.fan, festevam, linux-imx, devicetree, shawnguo, robh+dt Hi Marek Nice one (;-p). On Thu, 2022-04-07 at 22:24 +0200, Marek Vasut wrote: > Add DT compatible string for 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@denx.de> > Cc: Fabio Estevam <festevam@denx.de> > Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: devicetree@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml > index 8a7ed7782e99f..a6286581fa13b 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -819,6 +819,7 @@ properties: > - gw,imx8mm-gw7902 # i.MX8MM Gateworks Board > - gw,imx8mm-gw7903 # i.MX8MM Gateworks Board > - kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM > + - menlo,mx8menlo # i.MX8MM Menlo board Would it make sense mentioning that it is a carrier board for Verdin modules? > - toradex,verdin-imx8mm # Verdin iMX8M Mini Modules > - toradex,verdin-imx8mm-nonwifi # Verdin iMX8M Mini Modules without Wi-Fi / BT > - toradex,verdin-imx8mm-wifi # Verdin iMX8M Mini Wi-Fi / BT Modules Cheers Marcel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board @ 2022-04-08 7:55 ` Marcel Ziswiler 0 siblings, 0 replies; 23+ messages in thread From: Marcel Ziswiler @ 2022-04-08 7:55 UTC (permalink / raw) To: linux-arm-kernel, marex Cc: peng.fan, festevam, linux-imx, devicetree, shawnguo, robh+dt Hi Marek Nice one (;-p). On Thu, 2022-04-07 at 22:24 +0200, Marek Vasut wrote: > Add DT compatible string for 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@denx.de> > Cc: Fabio Estevam <festevam@denx.de> > Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: devicetree@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml > index 8a7ed7782e99f..a6286581fa13b 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -819,6 +819,7 @@ properties: > - gw,imx8mm-gw7902 # i.MX8MM Gateworks Board > - gw,imx8mm-gw7903 # i.MX8MM Gateworks Board > - kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM > + - menlo,mx8menlo # i.MX8MM Menlo board Would it make sense mentioning that it is a carrier board for Verdin modules? > - toradex,verdin-imx8mm # Verdin iMX8M Mini Modules > - toradex,verdin-imx8mm-nonwifi # Verdin iMX8M Mini Modules without Wi-Fi / BT > - toradex,verdin-imx8mm-wifi # Verdin iMX8M Mini Wi-Fi / BT Modules Cheers Marcel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board 2022-04-08 7:55 ` Marcel Ziswiler @ 2022-04-08 16:25 ` Marek Vasut -1 siblings, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-08 16:25 UTC (permalink / raw) To: Marcel Ziswiler, linux-arm-kernel Cc: peng.fan, festevam, linux-imx, devicetree, shawnguo, robh+dt On 4/8/22 09:55, Marcel Ziswiler wrote: [...] >> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml >> index 8a7ed7782e99f..a6286581fa13b 100644 >> --- a/Documentation/devicetree/bindings/arm/fsl.yaml >> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml >> @@ -819,6 +819,7 @@ properties: >> - gw,imx8mm-gw7902 # i.MX8MM Gateworks Board >> - gw,imx8mm-gw7903 # i.MX8MM Gateworks Board >> - kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM >> + - menlo,mx8menlo # i.MX8MM Menlo board > > Would it make sense mentioning that it is a carrier board for Verdin modules? I will add it in V2. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board @ 2022-04-08 16:25 ` Marek Vasut 0 siblings, 0 replies; 23+ messages in thread From: Marek Vasut @ 2022-04-08 16:25 UTC (permalink / raw) To: Marcel Ziswiler, linux-arm-kernel Cc: peng.fan, festevam, linux-imx, devicetree, shawnguo, robh+dt On 4/8/22 09:55, Marcel Ziswiler wrote: [...] >> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml >> index 8a7ed7782e99f..a6286581fa13b 100644 >> --- a/Documentation/devicetree/bindings/arm/fsl.yaml >> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml >> @@ -819,6 +819,7 @@ properties: >> - gw,imx8mm-gw7902 # i.MX8MM Gateworks Board >> - gw,imx8mm-gw7903 # i.MX8MM Gateworks Board >> - kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM >> + - menlo,mx8menlo # i.MX8MM Menlo board > > Would it make sense mentioning that it is a carrier board for Verdin modules? I will add it in V2. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-04-13 14:15 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-07 20:24 [PATCH 1/2] dt-bindings: arm: Add i.MX8M Mini Toradex Verdin based Menlo board Marek Vasut 2022-04-07 20:24 ` Marek Vasut 2022-04-07 20:24 ` [PATCH 2/2] arm64: dts: imx8mm: " Marek Vasut 2022-04-08 6:46 ` Francesco Dolcini 2022-04-08 7:56 ` Marcel Ziswiler 2022-04-08 8:49 ` Francesco Dolcini 2022-04-08 15:29 ` Tim Harvey 2022-04-08 15:54 ` Adam Ford 2022-04-08 16:16 ` Tim Harvey 2022-04-08 16:24 ` Marcel Ziswiler 2022-04-08 17:21 ` Marek Vasut 2022-04-08 16:20 ` Marek Vasut 2022-04-08 16:46 ` Fabio Estevam 2022-04-08 15:31 ` Marek Vasut 2022-04-08 15:02 ` Marek Vasut 2022-04-10 8:46 ` Francesco Dolcini 2022-04-10 21:37 ` Marek Vasut 2022-04-13 9:44 ` Francesco Dolcini 2022-04-13 14:14 ` Marek Vasut 2022-04-08 7:55 ` [PATCH 1/2] dt-bindings: arm: " Marcel Ziswiler 2022-04-08 7:55 ` Marcel Ziswiler 2022-04-08 16:25 ` Marek Vasut 2022-04-08 16:25 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.