2024-01-22 09:54:39

by Mathieu Othacehe

[permalink] [raw]
Subject: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Add basic support for phyBOARD-Segin-i.MX93.
Main features are:
* eMMC
* Ethernet
* SD-Card
* UART

Signed-off-by: Mathieu Othacehe <[email protected]>
---
arch/arm64/boot/dts/freescale/Makefile | 1 +
.../dts/freescale/imx93-phyboard-segin.dts | 141 ++++++++++++++++++
.../boot/dts/freescale/imx93-phycore-som.dtsi | 127 ++++++++++++++++
3 files changed, 269 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
create mode 100644 arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 2e027675d7bb..65db918c821c 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-iris-v2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb

diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
new file mode 100644
index 000000000000..5433c33d1322
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 PHYTEC Messtechnik GmbH
+ * Author: Wadim Egorov <[email protected]>, Christoph Stoidner <[email protected]>
+ * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
+ *
+ * Product homepage:
+ * phyBOARD-Segin carrier board is reused for the i.MX93 design.
+ * https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
+ */
+
+#include "imx93-phycore-som.dtsi"
+
+/{
+ model = "PHYTEC phyBOARD-Segin-i.MX93";
+ compatible = "phytec,imx93-phyboard-segin", "phytec,imx93-phycore-som",
+ "fsl,imx93";
+
+ chosen {
+ stdout-path = &lpuart1;
+ };
+
+ reg_usdhc2_vmmc: regulator-usdhc2 {
+ compatible = "regulator-fixed";
+ enable-active-high;
+ gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "VCC_SD";
+ };
+};
+
+/* GPIOs */
+&gpio1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_gpio1>;
+ gpio-line-names = "X_GPIO1_3";
+};
+
+&gpio4 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_gpio4>;
+ gpio-line-names = "", "", "", "", "", "", "", "X_GPIO5_9";
+};
+
+/* Console */
+&lpuart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart1>;
+ status = "okay";
+};
+
+/* eMMC */
+&usdhc1 {
+ no-1-8-v;
+};
+
+/* SD-Card */
+&usdhc2 {
+ pinctrl-names = "default", "state_100mhz", "state_200mhz";
+ pinctrl-0 = <&pinctrl_usdhc2_default>, <&pinctrl_usdhc2_cd>;
+ pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_cd>;
+ pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_cd>;
+ bus-width = <4>;
+ cd-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
+ no-mmc;
+ no-sdio;
+ vmmc-supply = <&reg_usdhc2_vmmc>;
+ status = "okay";
+};
+
+&iomuxc {
+ pinctrl_gpio1: gpio1grp {
+ fsl,pins = <
+ MX93_PAD_I2C1_SCL__GPIO1_IO00 0x10
+ >;
+ };
+
+ pinctrl_gpio4: gpio4grp {
+ fsl,pins = <
+ MX93_PAD_ENET1_TXC__GPIO4_IO07 0x10
+ >;
+ };
+
+ pinctrl_uart1: uart1grp {
+ fsl,pins = <
+ MX93_PAD_UART1_RXD__LPUART1_RX 0x31e
+ MX93_PAD_UART1_TXD__LPUART1_TX 0x30e
+ >;
+ };
+
+ pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
+ fsl,pins = <
+ MX93_PAD_SD2_RESET_B__GPIO3_IO07 0x31e
+ >;
+ };
+
+ pinctrl_usdhc2_cd: usdhc2cdgrp {
+ fsl,pins = <
+ MX93_PAD_SD2_CD_B__GPIO3_IO00 0x31e
+ >;
+ };
+
+ pinctrl_usdhc2_default: usdhc2grp {
+ fsl,pins = <
+ MX93_PAD_SD2_CLK__USDHC2_CLK 0x179e
+ MX93_PAD_SD2_CMD__USDHC2_CMD 0x139e
+ MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x138e
+ MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x138e
+ MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x138e
+ MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x139e
+ MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
+ >;
+ };
+
+ pinctrl_usdhc2_100mhz: usdhc2grp {
+ fsl,pins = <
+ MX93_PAD_SD2_CLK__USDHC2_CLK 0x179e
+ MX93_PAD_SD2_CMD__USDHC2_CMD 0x139e
+ MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x138e
+ MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x138e
+ MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x139e
+ MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x139e
+ MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
+ >;
+ };
+
+ pinctrl_usdhc2_200mhz: usdhc2grp {
+ fsl,pins = <
+ MX93_PAD_SD2_CLK__USDHC2_CLK 0x178e
+ MX93_PAD_SD2_CMD__USDHC2_CMD 0x139e
+ MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x139e
+ MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x139e
+ MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x139e
+ MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x139e
+ MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
+ >;
+ };
+};
diff --git a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
new file mode 100644
index 000000000000..439ea4176f8c
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 PHYTEC Messtechnik GmbH
+ * Author: Wadim Egorov <[email protected]>, Christoph Stoidner <[email protected]>
+ * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
+ *
+ * Product homepage:
+ * https://www.phytec.de/produkte/system-on-modules/phycore-imx-91-93/
+ */
+/dts-v1/;
+
+#include <dt-bindings/leds/common.h>
+
+#include "imx93.dtsi"
+
+/{
+ model = "PHYTEC phyCORE-i.MX93";
+ compatible = "phytec,imx93-phycore-som", "fsl,imx93";
+
+ reserved-memory {
+ ranges;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ linux,cma {
+ compatible = "shared-dma-pool";
+ reusable;
+ alloc-ranges = <0 0x80000000 0 0x40000000>;
+ size = <0 0x10000000>;
+ linux,cma-default;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_leds>;
+
+ led-0 {
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_HEARTBEAT;
+ gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+};
+
+/* Ethernet */
+&fec {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_fec>;
+ phy-mode = "rmii";
+ phy-handle = <&ethphy1>;
+ fsl,magic-packet;
+ assigned-clocks = <&clk IMX93_CLK_ENET_TIMER1>,
+ <&clk IMX93_CLK_ENET_REF>,
+ <&clk IMX93_CLK_ENET_REF_PHY>;
+ assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
+ <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
+ <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
+ assigned-clock-rates = <100000000>, <50000000>, <50000000>;
+ status = "okay";
+
+ mdio: mdio {
+ clock-frequency = <5000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethphy1: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+ };
+};
+
+/* eMMC */
+&usdhc1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usdhc1>;
+ bus-width = <8>;
+ non-removable;
+ status = "okay";
+};
+
+/* Watchdog */
+&wdog3 {
+ status = "okay";
+};
+
+&iomuxc {
+ pinctrl_fec: fecgrp {
+ fsl,pins = <
+ MX93_PAD_ENET2_MDC__ENET1_MDC 0x50e
+ MX93_PAD_ENET2_MDIO__ENET1_MDIO 0x502
+ MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0 0x57e
+ MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1 0x57e
+ MX93_PAD_ENET2_RXC__ENET1_RX_ER 0x5fe
+ MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL 0x57e
+ MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0 0x50e
+ MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1 0x50e
+ MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL 0x50e
+ MX93_PAD_ENET2_TD2__ENET1_TX_CLK 0x4000050e
+ >;
+ };
+
+ pinctrl_leds: ledsgrp {
+ fsl,pins = <
+ MX93_PAD_I2C1_SDA__GPIO1_IO01 0x31e
+ >;
+ };
+
+ pinctrl_usdhc1: usdhc1grp {
+ fsl,pins = <
+ MX93_PAD_SD1_CLK__USDHC1_CLK 0x179e
+ MX93_PAD_SD1_CMD__USDHC1_CMD 0x1386
+ MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x138e
+ MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x1386
+ MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x138e
+ MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x1386
+ MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x1386
+ MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x1386
+ MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x1386
+ MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x1386
+ MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x179e
+ >;
+ };
+};
--
2.41.0



2024-01-23 06:12:10

by Wadim Egorov

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Hey Mathieu,

Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:
> Add basic support for phyBOARD-Segin-i.MX93.
> Main features are:
> * eMMC
> * Ethernet
> * SD-Card
> * UART
>
> Signed-off-by: Mathieu Othacehe <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 1 +
> .../dts/freescale/imx93-phyboard-segin.dts | 141 ++++++++++++++++++
> .../boot/dts/freescale/imx93-phycore-som.dtsi | 127 ++++++++++++++++
> 3 files changed, 269 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 2e027675d7bb..65db918c821c 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-iris-v2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb
>
> diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
> new file mode 100644
> index 000000000000..5433c33d1322
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner <[email protected]>
> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
> + *
> + * Product homepage:
> + * phyBOARD-Segin carrier board is reused for the i.MX93 design.
> + * https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
> + */
> +
> +#include "imx93-phycore-som.dtsi"
> +
> +/{
> + model = "PHYTEC phyBOARD-Segin-i.MX93";
> + compatible = "phytec,imx93-phyboard-segin", "phytec,imx93-phycore-som",
> + "fsl,imx93";
> +
> + chosen {
> + stdout-path = &lpuart1;
> + };
> +
> + reg_usdhc2_vmmc: regulator-usdhc2 {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "VCC_SD";
> + };
> +};
> +
> +/* GPIOs */
> +&gpio1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_gpio1>;

You are doing more than you describing in your changes log.
Here you are forcing a gpio-only functionality for the X16 header. But
the pins we route down to the X16 expansion connector can be also used
differently.
Typically we provide device tree overlays for different use cases on
this expansion connectors.

Please drop the muxing.

Same applies for the gpio names.

> + gpio-line-names = "X_GPIO1_3";
> +};
> +
> +&gpio4 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_gpio4>;
> + gpio-line-names = "", "", "", "", "", "", "", "X_GPIO5_9";

Same for gpio4.


> +};
> +
> +/* Console */
> +&lpuart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart1>;
> + status = "okay";
> +};
> +
> +/* eMMC */
> +&usdhc1 {
> + no-1-8-v;
> +};
> +
> +/* SD-Card */
> +&usdhc2 {
> + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> + pinctrl-0 = <&pinctrl_usdhc2_default>, <&pinctrl_usdhc2_cd>;
> + pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_cd>;
> + pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_cd>;
> + bus-width = <4>;
> + cd-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
> + no-mmc;
> + no-sdio;
> + vmmc-supply = <&reg_usdhc2_vmmc>;
> + status = "okay";
> +};
> +
> +&iomuxc {
> + pinctrl_gpio1: gpio1grp {
> + fsl,pins = <
> + MX93_PAD_I2C1_SCL__GPIO1_IO00 0x10
> + >;
> + };
> +
> + pinctrl_gpio4: gpio4grp {
> + fsl,pins = <
> + MX93_PAD_ENET1_TXC__GPIO4_IO07 0x10
> + >;
> + };
> +
> + pinctrl_uart1: uart1grp {
> + fsl,pins = <
> + MX93_PAD_UART1_RXD__LPUART1_RX 0x31e
> + MX93_PAD_UART1_TXD__LPUART1_TX 0x30e
> + >;
> + };
> +
> + pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_RESET_B__GPIO3_IO07 0x31e
> + >;
> + };
> +
> + pinctrl_usdhc2_cd: usdhc2cdgrp {
> + fsl,pins = <
> + MX93_PAD_SD2_CD_B__GPIO3_IO00 0x31e
> + >;
> + };
> +
> + pinctrl_usdhc2_default: usdhc2grp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__USDHC2_CLK 0x179e
> + MX93_PAD_SD2_CMD__USDHC2_CMD 0x139e
> + MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x138e
> + MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x138e
> + MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x138e
> + MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x139e
> + MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> + >;
> + };
> +
> + pinctrl_usdhc2_100mhz: usdhc2grp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__USDHC2_CLK 0x179e
> + MX93_PAD_SD2_CMD__USDHC2_CMD 0x139e
> + MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x138e
> + MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x138e
> + MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x139e
> + MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x139e
> + MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> + >;
> + };
> +
> + pinctrl_usdhc2_200mhz: usdhc2grp {
> + fsl,pins = <
> + MX93_PAD_SD2_CLK__USDHC2_CLK 0x178e
> + MX93_PAD_SD2_CMD__USDHC2_CMD 0x139e
> + MX93_PAD_SD2_DATA0__USDHC2_DATA0 0x139e
> + MX93_PAD_SD2_DATA1__USDHC2_DATA1 0x139e
> + MX93_PAD_SD2_DATA2__USDHC2_DATA2 0x139e
> + MX93_PAD_SD2_DATA3__USDHC2_DATA3 0x139e
> + MX93_PAD_SD2_VSELECT__USDHC2_VSELECT 0x51e
> + >;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> new file mode 100644
> index 000000000000..439ea4176f8c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner <[email protected]>
> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
> + *
> + * Product homepage:
> + * https://www.phytec.de/produkte/system-on-modules/phycore-imx-91-93/
> + */
> +/dts-v1/;

Maybe this comment was lost, but I asked to move the dts-v1 tag into the
carrier board device tree.

Regards,
Wadim


> +
> +#include <dt-bindings/leds/common.h>
> +
> +#include "imx93.dtsi"
> +
> +/{
> + model = "PHYTEC phyCORE-i.MX93";
> + compatible = "phytec,imx93-phycore-som", "fsl,imx93";
> +
> + reserved-memory {
> + ranges;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + linux,cma {
> + compatible = "shared-dma-pool";
> + reusable;
> + alloc-ranges = <0 0x80000000 0 0x40000000>;
> + size = <0 0x10000000>;
> + linux,cma-default;
> + };
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_leds>;
> +
> + led-0 {
> + color = <LED_COLOR_ID_GREEN>;
> + function = LED_FUNCTION_HEARTBEAT;
> + gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "heartbeat";
> + };
> + };
> +};
> +
> +/* Ethernet */
> +&fec {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_fec>;
> + phy-mode = "rmii";
> + phy-handle = <&ethphy1>;
> + fsl,magic-packet;
> + assigned-clocks = <&clk IMX93_CLK_ENET_TIMER1>,
> + <&clk IMX93_CLK_ENET_REF>,
> + <&clk IMX93_CLK_ENET_REF_PHY>;
> + assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
> + <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
> + <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
> + assigned-clock-rates = <100000000>, <50000000>, <50000000>;
> + status = "okay";
> +
> + mdio: mdio {
> + clock-frequency = <5000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy1: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + };
> + };
> +};
> +
> +/* eMMC */
> +&usdhc1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usdhc1>;
> + bus-width = <8>;
> + non-removable;
> + status = "okay";
> +};
> +
> +/* Watchdog */
> +&wdog3 {
> + status = "okay";
> +};
> +
> +&iomuxc {
> + pinctrl_fec: fecgrp {
> + fsl,pins = <
> + MX93_PAD_ENET2_MDC__ENET1_MDC 0x50e
> + MX93_PAD_ENET2_MDIO__ENET1_MDIO 0x502
> + MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0 0x57e
> + MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1 0x57e
> + MX93_PAD_ENET2_RXC__ENET1_RX_ER 0x5fe
> + MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL 0x57e
> + MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0 0x50e
> + MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1 0x50e
> + MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL 0x50e
> + MX93_PAD_ENET2_TD2__ENET1_TX_CLK 0x4000050e
> + >;
> + };
> +
> + pinctrl_leds: ledsgrp {
> + fsl,pins = <
> + MX93_PAD_I2C1_SDA__GPIO1_IO01 0x31e
> + >;
> + };
> +
> + pinctrl_usdhc1: usdhc1grp {
> + fsl,pins = <
> + MX93_PAD_SD1_CLK__USDHC1_CLK 0x179e
> + MX93_PAD_SD1_CMD__USDHC1_CMD 0x1386
> + MX93_PAD_SD1_DATA0__USDHC1_DATA0 0x138e
> + MX93_PAD_SD1_DATA1__USDHC1_DATA1 0x1386
> + MX93_PAD_SD1_DATA2__USDHC1_DATA2 0x138e
> + MX93_PAD_SD1_DATA3__USDHC1_DATA3 0x1386
> + MX93_PAD_SD1_DATA4__USDHC1_DATA4 0x1386
> + MX93_PAD_SD1_DATA5__USDHC1_DATA5 0x1386
> + MX93_PAD_SD1_DATA6__USDHC1_DATA6 0x1386
> + MX93_PAD_SD1_DATA7__USDHC1_DATA7 0x1386
> + MX93_PAD_SD1_STROBE__USDHC1_STROBE 0x179e
> + >;
> + };
> +};

2024-01-23 07:42:47

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Hi Wadim,

Am 23.01.24 um 07:11 schrieb Wadim Egorov:
> Hey Mathieu,
>
> Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:
>> Add basic support for phyBOARD-Segin-i.MX93.
>> Main features are:
>> * eMMC
>> * Ethernet
>> * SD-Card
>> * UART
>>
>> Signed-off-by: Mathieu Othacehe <[email protected]>
>> ---
>>   arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>   .../dts/freescale/imx93-phyboard-segin.dts    | 141 ++++++++++++++++++
>>   .../boot/dts/freescale/imx93-phycore-som.dtsi | 127 ++++++++++++++++
>>   3 files changed, 269 insertions(+)
>>   create mode 100644
>> arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>   create mode 100644
>> arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>> b/arch/arm64/boot/dts/freescale/Makefile
>> index 2e027675d7bb..65db918c821c 100644
>> --- a/arch/arm64/boot/dts/freescale/Makefile
>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>> @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) +=
>> imx8qxp-colibri-iris-v2.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
>> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb
>>   diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>> b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>> new file mode 100644
>> index 000000000000..5433c33d1322
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>> @@ -0,0 +1,141 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner
>> <[email protected]>
>> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
>> + *
>> + * Product homepage:
>> + * phyBOARD-Segin carrier board is reused for the i.MX93 design.
>> + *
>> https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
>> + */
>> +
>> +#include "imx93-phycore-som.dtsi"
>> +
>> +/{
>> +    model = "PHYTEC phyBOARD-Segin-i.MX93";
>> +    compatible = "phytec,imx93-phyboard-segin",
>> "phytec,imx93-phycore-som",
>> +             "fsl,imx93";
>> +
>> +    chosen {
>> +        stdout-path = &lpuart1;
>> +    };
>> +
>> +    reg_usdhc2_vmmc: regulator-usdhc2 {
>> +        compatible = "regulator-fixed";
>> +        enable-active-high;
>> +        gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>> +        regulator-min-microvolt = <3300000>;
>> +        regulator-max-microvolt = <3300000>;
>> +        regulator-name = "VCC_SD";
>> +    };
>> +};
>> +
>> +/* GPIOs */
>> +&gpio1 {
>> +    pinctrl-names = "default";
>> +    pinctrl-0 = <&pinctrl_gpio1>;
>
> You are doing more than you describing in your changes log.
> Here you are forcing a gpio-only functionality for the X16 header. But
> the pins we route down to the X16 expansion connector can be also used
> differently.

i think the word "forcing" is little bit hard in this case. It doesn't
define a gpio-hog.

> Typically we provide device tree overlays for different use cases on
> this expansion connectors.

Can you please explain why the device tree overlays cannot overwrite the
pinmuxing?

>
> Please drop the muxing.
>
> Same applies for the gpio names.
What's the problem with defining gpio line names for user friendliness?
The Raspberry Pi has also an expansion header, all the pins can be muxed
to different functions but still have gpio line names.

Best regards

2024-01-23 07:45:29

by Mathieu Othacehe

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support


Hey Wadim,

> You are doing more than you describing in your changes log.
> Here you are forcing a gpio-only functionality for the X16 header. But the
> pins we route down to the X16 expansion connector can be also used
> differently.
> Typically we provide device tree overlays for different use cases on this
> expansion connectors.

Adding those GPIOs on X16 was a suggestion from Stefan back in v3. I
must say that it is quite convenient as a user to be able to interact
with gpiolib on some GPIOs on the eval board.

Would it be a possibility to change the muxing of those two pins in the
overlays if the need arises?

> Maybe this comment was lost, but I asked to move the dts-v1 tag into the
> carrier board device tree.

Missed that, sorry.

Thanks,

Mathieu

2024-01-23 08:29:53

by Wadim Egorov

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support


Am 23.01.24 um 08:42 schrieb Stefan Wahren:
> Hi Wadim,
>
> Am 23.01.24 um 07:11 schrieb Wadim Egorov:
>> Hey Mathieu,
>>
>> Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:
>>> Add basic support for phyBOARD-Segin-i.MX93.
>>> Main features are:
>>> * eMMC
>>> * Ethernet
>>> * SD-Card
>>> * UART
>>>
>>> Signed-off-by: Mathieu Othacehe <[email protected]>
>>> ---
>>>   arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>>   .../dts/freescale/imx93-phyboard-segin.dts    | 141
>>> ++++++++++++++++++
>>>   .../boot/dts/freescale/imx93-phycore-som.dtsi | 127 ++++++++++++++++
>>>   3 files changed, 269 insertions(+)
>>>   create mode 100644
>>> arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>   create mode 100644
>>> arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>>> b/arch/arm64/boot/dts/freescale/Makefile
>>> index 2e027675d7bb..65db918c821c 100644
>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>> @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) +=
>>> imx8qxp-colibri-iris-v2.dtb
>>>   dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>>>   dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
>>> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb
>>>   diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>> b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>> new file mode 100644
>>> index 000000000000..5433c33d1322
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>> @@ -0,0 +1,141 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>>> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner
>>> <[email protected]>
>>> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
>>> + *
>>> + * Product homepage:
>>> + * phyBOARD-Segin carrier board is reused for the i.MX93 design.
>>> + *
>>> https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
>>>
>>> + */
>>> +
>>> +#include "imx93-phycore-som.dtsi"
>>> +
>>> +/{
>>> +    model = "PHYTEC phyBOARD-Segin-i.MX93";
>>> +    compatible = "phytec,imx93-phyboard-segin",
>>> "phytec,imx93-phycore-som",
>>> +             "fsl,imx93";
>>> +
>>> +    chosen {
>>> +        stdout-path = &lpuart1;
>>> +    };
>>> +
>>> +    reg_usdhc2_vmmc: regulator-usdhc2 {
>>> +        compatible = "regulator-fixed";
>>> +        enable-active-high;
>>> +        gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
>>> +        pinctrl-names = "default";
>>> +        pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>>> +        regulator-min-microvolt = <3300000>;
>>> +        regulator-max-microvolt = <3300000>;
>>> +        regulator-name = "VCC_SD";
>>> +    };
>>> +};
>>> +
>>> +/* GPIOs */
>>> +&gpio1 {
>>> +    pinctrl-names = "default";
>>> +    pinctrl-0 = <&pinctrl_gpio1>;
>>
>> You are doing more than you describing in your changes log.
>> Here you are forcing a gpio-only functionality for the X16 header. But
>> the pins we route down to the X16 expansion connector can be also used
>> differently.
>
> i think the word "forcing" is little bit hard in this case. It doesn't
> define a gpio-hog.

You are defaulting it to be a GPIO.

>
>> Typically we provide device tree overlays for different use cases on
>> this expansion connectors.
>
> Can you please explain why the device tree overlays cannot overwrite the
> pinmuxing?

It can, and it should. Thats why I mentioned to use different overlays
for different use cases.
I think it is nicer to have a board only defining it's static components.
At this point we do not know what users will use the expansion connector
for.
Adding this kind of functionality with overlays follows the idea of
defining components where they are actually used/implemented: soc,
som/board level.
You can find a few of the adapters we provide as dtsi files in
  arch/arm/boot/dts/nxp/imx/*peb*
Nowadays we have overlays and can use them instead.


>
>>
>> Please drop the muxing.
>>
>> Same applies for the gpio names.
> What's the problem with defining gpio line names for user friendliness?
> The Raspberry Pi has also an expansion header, all the pins can be muxed
> to different functions but still have gpio line names.

This may cause confusion if you use overlays defining other
functionalities as the names you define.

Regards,
Wadim


>
> Best regards

2024-01-23 10:23:08

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Hi Wadim,

Am 23.01.24 um 09:25 schrieb Wadim Egorov:
>
> Am 23.01.24 um 08:42 schrieb Stefan Wahren:
>> Hi Wadim,
>>
>> Am 23.01.24 um 07:11 schrieb Wadim Egorov:
>>> Hey Mathieu,
>>>
>>> Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:
>>>> Add basic support for phyBOARD-Segin-i.MX93.
>>>> Main features are:
>>>> * eMMC
>>>> * Ethernet
>>>> * SD-Card
>>>> * UART
>>>>
>>>> Signed-off-by: Mathieu Othacehe <[email protected]>
>>>> ---
>>>>   arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>>>   .../dts/freescale/imx93-phyboard-segin.dts    | 141
>>>> ++++++++++++++++++
>>>>   .../boot/dts/freescale/imx93-phycore-som.dtsi | 127 ++++++++++++++++
>>>>   3 files changed, 269 insertions(+)
>>>>   create mode 100644
>>>> arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>   create mode 100644
>>>> arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>>>> b/arch/arm64/boot/dts/freescale/Makefile
>>>> index 2e027675d7bb..65db918c821c 100644
>>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>>> @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) +=
>>>> imx8qxp-colibri-iris-v2.dtb
>>>>   dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>>>>   dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
>>>> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb
>>>>   diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>> b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>> new file mode 100644
>>>> index 000000000000..5433c33d1322
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>> @@ -0,0 +1,141 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>>>> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner
>>>> <[email protected]>
>>>> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
>>>> + *
>>>> + * Product homepage:
>>>> + * phyBOARD-Segin carrier board is reused for the i.MX93 design.
>>>> + *
>>>> https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
>>>>
>>>> + */
>>>> +
>>>> +#include "imx93-phycore-som.dtsi"
>>>> +
>>>> +/{
>>>> +    model = "PHYTEC phyBOARD-Segin-i.MX93";
>>>> +    compatible = "phytec,imx93-phyboard-segin",
>>>> "phytec,imx93-phycore-som",
>>>> +             "fsl,imx93";
>>>> +
>>>> +    chosen {
>>>> +        stdout-path = &lpuart1;
>>>> +    };
>>>> +
>>>> +    reg_usdhc2_vmmc: regulator-usdhc2 {
>>>> +        compatible = "regulator-fixed";
>>>> +        enable-active-high;
>>>> +        gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
>>>> +        pinctrl-names = "default";
>>>> +        pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>>>> +        regulator-min-microvolt = <3300000>;
>>>> +        regulator-max-microvolt = <3300000>;
>>>> +        regulator-name = "VCC_SD";
>>>> +    };
>>>> +};
>>>> +
>>>> +/* GPIOs */
>>>> +&gpio1 {
>>>> +    pinctrl-names = "default";
>>>> +    pinctrl-0 = <&pinctrl_gpio1>;
>>>
>>> You are doing more than you describing in your changes log.
>>> Here you are forcing a gpio-only functionality for the X16 header. But
>>> the pins we route down to the X16 expansion connector can be also used
>>> differently.
>>
>> i think the word "forcing" is little bit hard in this case. It doesn't
>> define a gpio-hog.
>
> You are defaulting it to be a GPIO.
Sure, but i still cannot see the problem. Are you concerned about
hardware damage, different behavior in comparison to your downstream BSP
or overwriting the bootloader defaults?
>
>>
>>> Typically we provide device tree overlays for different use cases on
>>> this expansion connectors.
>>
>> Can you please explain why the device tree overlays cannot overwrite the
>> pinmuxing?
>
> It can, and it should. Thats why I mentioned to use different overlays
> for different use cases.
> I think it is nicer to have a board only defining it's static components.
Yes and i would consider the line names as static and board specific.
> At this point we do not know what users will use the expansion
> connector for.
> Adding this kind of functionality with overlays follows the idea of
> defining components where they are actually used/implemented: soc,
> som/board level.
> You can find a few of the adapters we provide as dtsi files in
>   arch/arm/boot/dts/nxp/imx/*peb*
> Nowadays we have overlays and can use them instead.
>
>
>>
>>>
>>> Please drop the muxing.
>>>
>>> Same applies for the gpio names.
>> What's the problem with defining gpio line names for user friendliness?
>> The Raspberry Pi has also an expansion header, all the pins can be muxed
>> to different functions but still have gpio line names.
>
> This may cause confusion if you use overlays defining other
> functionalities as the names you define.
I agree most of the line names on the Raspberry Pi contains a function,
which wasn't the best idea for an expansion header. But this doesn't
mean we must do this here, too.

I just want to give you feedback from my point of view as a user. I
would expect that the gpio line names are defined regardless of the used
overlay.

But at the end it's your product.
>
> Regards,
> Wadim
>
>
>>
>> Best regards


2024-01-24 05:15:07

by Wadim Egorov

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Hi,

Am 23.01.24 um 11:21 schrieb Stefan Wahren:
> Hi Wadim,
>
> Am 23.01.24 um 09:25 schrieb Wadim Egorov:
>>
>> Am 23.01.24 um 08:42 schrieb Stefan Wahren:
>>> Hi Wadim,
>>>
>>> Am 23.01.24 um 07:11 schrieb Wadim Egorov:
>>>> Hey Mathieu,
>>>>
>>>> Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:
>>>>> Add basic support for phyBOARD-Segin-i.MX93.
>>>>> Main features are:
>>>>> * eMMC
>>>>> * Ethernet
>>>>> * SD-Card
>>>>> * UART
>>>>>
>>>>> Signed-off-by: Mathieu Othacehe <[email protected]>
>>>>> ---
>>>>>   arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>>>>   .../dts/freescale/imx93-phyboard-segin.dts    | 141
>>>>> ++++++++++++++++++
>>>>>   .../boot/dts/freescale/imx93-phycore-som.dtsi | 127
>>>>> ++++++++++++++++
>>>>>   3 files changed, 269 insertions(+)
>>>>>   create mode 100644
>>>>> arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>>   create mode 100644
>>>>> arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>>>>> b/arch/arm64/boot/dts/freescale/Makefile
>>>>> index 2e027675d7bb..65db918c821c 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>>>> @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) +=
>>>>> imx8qxp-colibri-iris-v2.dtb
>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
>>>>> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb
>>>>>   diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>> b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>> new file mode 100644
>>>>> index 000000000000..5433c33d1322
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>> @@ -0,0 +1,141 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +/*
>>>>> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>>>>> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner
>>>>> <[email protected]>
>>>>> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
>>>>> + *
>>>>> + * Product homepage:
>>>>> + * phyBOARD-Segin carrier board is reused for the i.MX93 design.
>>>>> + *
>>>>> https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
>>>>>
>>>>>
>>>>> + */
>>>>> +
>>>>> +#include "imx93-phycore-som.dtsi"
>>>>> +
>>>>> +/{
>>>>> +    model = "PHYTEC phyBOARD-Segin-i.MX93";
>>>>> +    compatible = "phytec,imx93-phyboard-segin",
>>>>> "phytec,imx93-phycore-som",
>>>>> +             "fsl,imx93";
>>>>> +
>>>>> +    chosen {
>>>>> +        stdout-path = &lpuart1;
>>>>> +    };
>>>>> +
>>>>> +    reg_usdhc2_vmmc: regulator-usdhc2 {
>>>>> +        compatible = "regulator-fixed";
>>>>> +        enable-active-high;
>>>>> +        gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
>>>>> +        pinctrl-names = "default";
>>>>> +        pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>>>>> +        regulator-min-microvolt = <3300000>;
>>>>> +        regulator-max-microvolt = <3300000>;
>>>>> +        regulator-name = "VCC_SD";
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +/* GPIOs */
>>>>> +&gpio1 {
>>>>> +    pinctrl-names = "default";
>>>>> +    pinctrl-0 = <&pinctrl_gpio1>;
>>>>
>>>> You are doing more than you describing in your changes log.
>>>> Here you are forcing a gpio-only functionality for the X16 header. But
>>>> the pins we route down to the X16 expansion connector can be also used
>>>> differently.
>>>
>>> i think the word "forcing" is little bit hard in this case. It doesn't
>>> define a gpio-hog.
>>
>> You are defaulting it to be a GPIO.
> Sure, but i still cannot see the problem. Are you concerned about
> hardware damage, different behavior in comparison to your downstream BSP
> or overwriting the bootloader defaults?
>>
>>>
>>>> Typically we provide device tree overlays for different use cases on
>>>> this expansion connectors.
>>>
>>> Can you please explain why the device tree overlays cannot overwrite
>>> the
>>> pinmuxing?
>>
>> It can, and it should. Thats why I mentioned to use different overlays
>> for different use cases.
>> I think it is nicer to have a board only defining it's static
>> components.
> Yes and i would consider the line names as static and board specific.
>> At this point we do not know what users will use the expansion
>> connector for.
>> Adding this kind of functionality with overlays follows the idea of
>> defining components where they are actually used/implemented: soc,
>> som/board level.
>> You can find a few of the adapters we provide as dtsi files in
>>   arch/arm/boot/dts/nxp/imx/*peb*
>> Nowadays we have overlays and can use them instead.
>>
>>
>>>
>>>>
>>>> Please drop the muxing.
>>>>
>>>> Same applies for the gpio names.
>>> What's the problem with defining gpio line names for user friendliness?
>>> The Raspberry Pi has also an expansion header, all the pins can be
>>> muxed
>>> to different functions but still have gpio line names.
>>
>> This may cause confusion if you use overlays defining other
>> functionalities as the names you define.
> I agree most of the line names on the Raspberry Pi contains a function,
> which wasn't the best idea for an expansion header. But this doesn't
> mean we must do this here, too.
>
> I just want to give you feedback from my point of view as a user. I
> would expect that the gpio line names are defined regardless of the used
> overlay.

I appreciate the feedback :)
Defining line names should be fine. But I would still prefer to have the
muxing in an overlay bound to a specific use case.

Regards,
Wadim

>
> But at the end it's your product.
>>
>> Regards,
>> Wadim
>>
>>
>>>
>>> Best regards
>

2024-01-24 11:44:35

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Am 24.01.24 um 06:13 schrieb Wadim Egorov:
> Hi,
>
> Am 23.01.24 um 11:21 schrieb Stefan Wahren:
>> Hi Wadim,
>>
>> Am 23.01.24 um 09:25 schrieb Wadim Egorov:
>>>
>>> Am 23.01.24 um 08:42 schrieb Stefan Wahren:
>>>> Hi Wadim,
>>>>
>>>> Am 23.01.24 um 07:11 schrieb Wadim Egorov:
>>>>> Hey Mathieu,
>>>>>
>>>>> Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:
>>>>>> Add basic support for phyBOARD-Segin-i.MX93.
>>>>>> Main features are:
>>>>>> * eMMC
>>>>>> * Ethernet
>>>>>> * SD-Card
>>>>>> * UART
>>>>>>
>>>>>> Signed-off-by: Mathieu Othacehe <[email protected]>
>>>>>> ---
>>>>>>   arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>>>>>   .../dts/freescale/imx93-phyboard-segin.dts    | 141
>>>>>> ++++++++++++++++++
>>>>>>   .../boot/dts/freescale/imx93-phycore-som.dtsi | 127
>>>>>> ++++++++++++++++
>>>>>>   3 files changed, 269 insertions(+)
>>>>>>   create mode 100644
>>>>>> arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>>>   create mode 100644
>>>>>> arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>>>>>> b/arch/arm64/boot/dts/freescale/Makefile
>>>>>> index 2e027675d7bb..65db918c821c 100644
>>>>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>>>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>>>>> @@ -201,6 +201,7 @@ dtb-$(CONFIG_ARCH_MXC) +=
>>>>>> imx8qxp-colibri-iris-v2.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
>>>>>> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxca.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MXC) += imx93-tqma9352-mba93xxla.dtb
>>>>>>   diff --git
>>>>>> a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>>> b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>>> new file mode 100644
>>>>>> index 000000000000..5433c33d1322
>>>>>> --- /dev/null
>>>>>> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin.dts
>>>>>> @@ -0,0 +1,141 @@
>>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>>> +/*
>>>>>> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>>>>>> + * Author: Wadim Egorov <[email protected]>, Christoph Stoidner
>>>>>> <[email protected]>
>>>>>> + * Copyright (C) 2024 Mathieu Othacehe <[email protected]>
>>>>>> + *
>>>>>> + * Product homepage:
>>>>>> + * phyBOARD-Segin carrier board is reused for the i.MX93 design.
>>>>>> + *
>>>>>> https://www.phytec.de/produkte/single-board-computer/phyboard-segin-imx6ul/
>>>>>>
>>>>>>
>>>>>> + */
>>>>>> +
>>>>>> +#include "imx93-phycore-som.dtsi"
>>>>>> +
>>>>>> +/{
>>>>>> +    model = "PHYTEC phyBOARD-Segin-i.MX93";
>>>>>> +    compatible = "phytec,imx93-phyboard-segin",
>>>>>> "phytec,imx93-phycore-som",
>>>>>> +             "fsl,imx93";
>>>>>> +
>>>>>> +    chosen {
>>>>>> +        stdout-path = &lpuart1;
>>>>>> +    };
>>>>>> +
>>>>>> +    reg_usdhc2_vmmc: regulator-usdhc2 {
>>>>>> +        compatible = "regulator-fixed";
>>>>>> +        enable-active-high;
>>>>>> +        gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
>>>>>> +        pinctrl-names = "default";
>>>>>> +        pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
>>>>>> +        regulator-min-microvolt = <3300000>;
>>>>>> +        regulator-max-microvolt = <3300000>;
>>>>>> +        regulator-name = "VCC_SD";
>>>>>> +    };
>>>>>> +};
>>>>>> +
>>>>>> +/* GPIOs */
>>>>>> +&gpio1 {
>>>>>> +    pinctrl-names = "default";
>>>>>> +    pinctrl-0 = <&pinctrl_gpio1>;
>>>>>
>>>>> You are doing more than you describing in your changes log.
>>>>> Here you are forcing a gpio-only functionality for the X16 header.
>>>>> But
>>>>> the pins we route down to the X16 expansion connector can be also
>>>>> used
>>>>> differently.
>>>>
>>>> i think the word "forcing" is little bit hard in this case. It doesn't
>>>> define a gpio-hog.
>>>
>>> You are defaulting it to be a GPIO.
>> Sure, but i still cannot see the problem. Are you concerned about
>> hardware damage, different behavior in comparison to your downstream BSP
>> or overwriting the bootloader defaults?
>>>
>>>>
>>>>> Typically we provide device tree overlays for different use cases on
>>>>> this expansion connectors.
>>>>
>>>> Can you please explain why the device tree overlays cannot
>>>> overwrite the
>>>> pinmuxing?
>>>
>>> It can, and it should. Thats why I mentioned to use different overlays
>>> for different use cases.
>>> I think it is nicer to have a board only defining it's static
>>> components.
>> Yes and i would consider the line names as static and board specific.
>>> At this point we do not know what users will use the expansion
>>> connector for.
>>> Adding this kind of functionality with overlays follows the idea of
>>> defining components where they are actually used/implemented: soc,
>>> som/board level.
>>> You can find a few of the adapters we provide as dtsi files in
>>>   arch/arm/boot/dts/nxp/imx/*peb*
>>> Nowadays we have overlays and can use them instead.
>>>
>>>
>>>>
>>>>>
>>>>> Please drop the muxing.
>>>>>
>>>>> Same applies for the gpio names.
>>>> What's the problem with defining gpio line names for user
>>>> friendliness?
>>>> The Raspberry Pi has also an expansion header, all the pins can be
>>>> muxed
>>>> to different functions but still have gpio line names.
>>>
>>> This may cause confusion if you use overlays defining other
>>> functionalities as the names you define.
>> I agree most of the line names on the Raspberry Pi contains a function,
>> which wasn't the best idea for an expansion header. But this doesn't
>> mean we must do this here, too.
>>
>> I just want to give you feedback from my point of view as a user. I
>> would expect that the gpio line names are defined regardless of the used
>> overlay.
>
> I appreciate the feedback :)
Thanks

> Defining line names should be fine. But I would still prefer to have
> the muxing in an overlay bound to a specific use case.
I'm fine with this. Unfortunately Mathieu dropped the line names in V5
today :-(

AFAIR reviewers should have 2 weeks time maximum. This was just 2 days.
>
> Regards,
> Wadim
>
>>
>> But at the end it's your product.
>>>
>>> Regards,
>>> Wadim
>>>
>>>
>>>>
>>>> Best regards
>>


2024-01-24 14:26:00

by Mathieu Othacehe

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support


Hello Stefan,

>> Defining line names should be fine. But I would still prefer to have
>> the muxing in an overlay bound to a specific use case.
> I'm fine with this. Unfortunately Mathieu dropped the line names in V5
> today :-(
>
> AFAIR reviewers should have 2 weeks time maximum. This was just 2 days.

I am sorry but it is not easy for me to deal with contradictory input. I
chose to remove the gpio-line-names even though it also seemed like a
nice addition to me. The idea was to not interfere with Phytec plans in
the future.

There is no hurry and I can always restore them in a v6.

Let me know what you think,

Thanks,

Mathieu

2024-01-24 17:33:25

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Hello Mathieu,

Am 24.01.24 um 14:48 schrieb Mathieu Othacehe:
> Hello Stefan,
>
>>> Defining line names should be fine. But I would still prefer to have
>>> the muxing in an overlay bound to a specific use case.
>> I'm fine with this. Unfortunately Mathieu dropped the line names in V5
>> today :-(
>>
>> AFAIR reviewers should have 2 weeks time maximum. This was just 2 days.
> I am sorry but it is not easy for me to deal with contradictory input. I
> chose to remove the gpio-line-names even though it also seemed like a
> nice addition to me. The idea was to not interfere with Phytec plans in
> the future.
tbh sending v5 before the discussion between Wadim and me was finished
made it more complicated. Please keep in mind that some reviewers do
this in their spare time, so a response could take some time.

In this particular case Wadim and me agreed on a solution, so no action
from your side was necessary except a little bit patience.

The reason why i suggested the gpio-line-names in the first place is
that users doesn't need to care about different versions of the DT files
(except the downstream one). Changing the line names afterwards leads to
confusion.

So before we discuss on a v6, just a question: are on the X16 connector
just 2 pins muxable as GPIO? This is hard to believe.

Best regards
> There is no hurry and I can always restore them in a v6.
>
> Let me know what you think,
>
> Thanks,
>
> Mathieu
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


2024-01-25 06:05:57

by Wadim Egorov

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support


Am 24.01.24 um 18:28 schrieb Stefan Wahren:
> Hello Mathieu,
>
> Am 24.01.24 um 14:48 schrieb Mathieu Othacehe:
>> Hello Stefan,
>>
>>>> Defining line names should be fine. But I would still prefer to have
>>>> the muxing in an overlay bound to a specific use case.
>>> I'm fine with this. Unfortunately Mathieu dropped the line names in V5
>>> today :-(
>>>
>>> AFAIR reviewers should have 2 weeks time maximum. This was just 2 days.
>> I am sorry but it is not easy for me to deal with contradictory input. I
>> chose to remove the gpio-line-names even though it also seemed like a
>> nice addition to me. The idea was to not interfere with Phytec plans in
>> the future.
> tbh sending v5 before the discussion between Wadim and me was finished
> made it more complicated. Please keep in mind that some reviewers do
> this in their spare time, so a response could take some time.
>
> In this particular case Wadim and me agreed on a solution, so no action
> from your side was necessary except a little bit patience.
>
> The reason why i suggested the gpio-line-names in the first place is
> that users doesn't need to care about different versions of the DT files
> (except the downstream one). Changing the line names afterwards leads to
> confusion.
>
> So before we discuss on a v6, just a question: are on the X16 connector
> just 2 pins muxable as GPIO? This is hard to believe.

In theory you can use more of the Pins as GPIOs. But at this point I
should mention that the Segin board became slightly more complicated
since it started to support more SoMs with different SoCs. We have
routings for various pins to help with the compatibility. So the naming
in the schematics is not really trivial. And IMO the dt should follow
the naming of the schematics.

I would prefer to go with v5 without having any namings for now.

Regards,
Wadim

>
> Best regards
>> There is no hurry and I can always restore them in a v6.
>>
>> Let me know what you think,
>>
>> Thanks,
>>
>> Mathieu
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

2024-01-25 07:42:52

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support

Am 25.01.24 um 07:05 schrieb Wadim Egorov:
>
> Am 24.01.24 um 18:28 schrieb Stefan Wahren:
>> Hello Mathieu,
>>
>> Am 24.01.24 um 14:48 schrieb Mathieu Othacehe:
>>> Hello Stefan,
>>>
>>>>> Defining line names should be fine. But I would still prefer to have
>>>>> the muxing in an overlay bound to a specific use case.
>>>> I'm fine with this. Unfortunately Mathieu dropped the line names in V5
>>>> today :-(
>>>>
>>>> AFAIR reviewers should have 2 weeks time maximum. This was just 2
>>>> days.
>>> I am sorry but it is not easy for me to deal with contradictory
>>> input. I
>>> chose to remove the gpio-line-names even though it also seemed like a
>>> nice addition to me. The idea was to not interfere with Phytec plans in
>>> the future.
>> tbh sending v5 before the discussion between Wadim and me was finished
>> made it more complicated. Please keep in mind that some reviewers do
>> this in their spare time, so a response could take some time.
>>
>> In this particular case Wadim and me agreed on a solution, so no action
>> from your side was necessary except a little bit patience.
>>
>> The reason why i suggested the gpio-line-names in the first place is
>> that users doesn't need to care about different versions of the DT files
>> (except the downstream one). Changing the line names afterwards leads to
>> confusion.
>>
>> So before we discuss on a v6, just a question: are on the X16 connector
>> just 2 pins muxable as GPIO? This is hard to believe.
>
> In theory you can use more of the Pins as GPIOs. But at this point I
> should mention that the Segin board became slightly more complicated
> since it started to support more SoMs with different SoCs. We have
> routings for various pins to help with the compatibility. So the
> naming in the schematics is not really trivial. And IMO the dt should
> follow the naming of the schematics.
Thanks for the explanation
>
> I would prefer to go with v5 without having any namings for now.
Okay
>
> Regards,
> Wadim
>