* [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 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 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 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 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 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 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 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: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 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
* 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 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 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
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.