[PATCH v2 0/2] Add Phytec i.MX93 Segin support

[PATCH v2 0/2] Add Phytec i.MX93 Segin support

[Mathieu Othacehe]

Hello,

This adds support for the Phytec i.MX93 Segin board.

Thanks,

Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
---
Changes in v2: 
- Remove useless line
- Add missing reserved-memory entries

v1: https://lore.kernel.org/linux-devicetree/20240116113939.17339-1-othacehe@gnu.org/

Mathieu Othacehe (2):
  dt-bindings: arm: fsl: Add i.MX93 PHYTEC with Segin
  arm64: dts: imx93-phycore-segin: Add Phytec i.MX93 Segin

 .../devicetree/bindings/arm/fsl.yaml          |  6 ++
 arch/arm64/boot/dts/freescale/Makefile        |  1 +
 .../dts/freescale/imx93-phycore-segin.dts     | 93 +++++++++++++++++++
 .../boot/dts/freescale/imx93-phycore-som.dtsi | 64 +++++++++++++
 4 files changed, 164 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx93-phycore-segin.dts
 create mode 100644 arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi

-- 
2.41.0

[PATCH v2 1/2] dt-bindings: arm: fsl: Add i.MX93 PHYTEC with Segin

[Mathieu Othacehe]

Add support for i.MX93 PHYTEC with Segin board.

Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 228dcc5c7d6f..196935d3abf0 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -1275,6 +1275,12 @@ properties:
           - const: tq,imx93-tqma9352        # TQ-Systems GmbH i.MX93 TQMa93xxCA/LA SOM
           - const: fsl,imx93
 
+      - description: i.MX93 PHYTEC phyBOARD-Segin
+        items:
+          - const: phytec,imx93-phycore-segin
+          - const: phytec,imx93-phycore-som
+          - const: fsl,imx93
+
       - description:
           Freescale Vybrid Platform Device Tree Bindings
 
-- 
2.41.0

[PATCH v2 2/2] arm64: dts: imx93-phycore-segin: Add Phytec i.MX93 Segin

[Mathieu Othacehe]

Add DTSI for Phytec i.MX93 System on Module and DTS for Phytec
i.MX93 on Segin evaluation board.

This version comes with:
 - 1GB LPDDR4 RAM
 - external SD
 - debug UART
 - 1x 100Mbit Ethernet

Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
---
 arch/arm64/boot/dts/freescale/Makefile        |  1 +
 .../dts/freescale/imx93-phycore-segin.dts     | 93 +++++++++++++++++++
 .../boot/dts/freescale/imx93-phycore-som.dtsi | 64 +++++++++++++
 3 files changed, 158 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx93-phycore-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..f078d6ef75f7 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-phycore-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-phycore-segin.dts b/arch/arm64/boot/dts/freescale/imx93-phycore-segin.dts
new file mode 100644
index 000000000000..748b779a9dc1
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx93-phycore-segin.dts
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 PHYTEC Messtechnik GmbH
+ * Christoph Stoidner <c.stoidner@phytec.de>
+ * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com>
+ *
+ */
+/dts-v1/;
+
+#include "imx93-phycore-som.dtsi"
+
+/{
+	model = "PHYTEC phyBOARD-Segin-i.MX93";
+	compatible = "phytec,imx93-phycore-segin", "phytec,imx93-phycore-som",
+		     "fsl,imx93";
+
+	chosen {
+		stdout-path = &lpuart1;
+	};
+
+	reg_usdhc2_vmmc: regulator-usdhc2 {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
+		regulator-name = "VSD_3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+};
+
+/* Console */
+&lpuart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart1>;
+	status = "okay";
+};
+
+/* SD-Card */
+&usdhc2 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
+	pinctrl-1 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
+	pinctrl-2 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
+	cd-gpios = <&gpio3 00 GPIO_ACTIVE_LOW>;
+	vmmc-supply = <&reg_usdhc2_vmmc>;
+	bus-width = <4>;
+	status = "okay";
+	no-sdio;
+	no-mmc;
+};
+
+/* Watchdog */
+&wdog3 {
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl-names = "default";
+	status = "okay";
+
+	pinctrl_uart1: uart1grp {
+		fsl,pins = <
+			MX93_PAD_UART1_RXD__LPUART1_RX			0x31e
+			MX93_PAD_UART1_TXD__LPUART1_TX			0x31e
+		>;
+	};
+
+	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
+		fsl,pins = <
+			MX93_PAD_SD2_RESET_B__GPIO3_IO07		0x31e
+		>;
+	};
+
+	pinctrl_usdhc2_gpio: usdhc2gpiogrp {
+		fsl,pins = <
+			MX93_PAD_SD2_CD_B__GPIO3_IO00			0x31e
+		>;
+	};
+
+	pinctrl_usdhc2: usdhc2grp {
+		fsl,pins = <
+			MX93_PAD_SD2_CLK__USDHC2_CLK			0x178e
+			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
+		>;
+	};
+};
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..4edff4a50b2b
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 PHYTEC Messtechnik GmbH
+ * Christoph Stoidner <c.stoidner@phytec.de>
+ * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com>
+ *
+ */
+/dts-v1/;
+
+#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;
+		};
+
+		ele_reserved: ele-reserved@a4120000 {
+			compatible = "shared-dma-pool";
+			reg = <0 0xa4120000 0 0x100000>;
+			no-map;
+		};
+	};
+};
+
+/* eMMC */
+&usdhc1 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc1>;
+	pinctrl-1 = <&pinctrl_usdhc1>;
+	pinctrl-2 = <&pinctrl_usdhc1>;
+	bus-width = <8>;
+	non-removable;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_usdhc1: usdhc1grp {
+		fsl,pins = <
+			MX93_PAD_SD1_CLK__USDHC1_CLK		0x15fe
+			MX93_PAD_SD1_CMD__USDHC1_CMD		0x13fe
+			MX93_PAD_SD1_DATA0__USDHC1_DATA0	0x13fe
+			MX93_PAD_SD1_DATA1__USDHC1_DATA1	0x13fe
+			MX93_PAD_SD1_DATA2__USDHC1_DATA2	0x13fe
+			MX93_PAD_SD1_DATA3__USDHC1_DATA3	0x13fe
+			MX93_PAD_SD1_DATA4__USDHC1_DATA4	0x13fe
+			MX93_PAD_SD1_DATA5__USDHC1_DATA5	0x13fe
+			MX93_PAD_SD1_DATA6__USDHC1_DATA6	0x13fe
+			MX93_PAD_SD1_DATA7__USDHC1_DATA7	0x13fe
+			MX93_PAD_SD1_STROBE__USDHC1_STROBE	0x15fe
+		>;
+	};
+};
-- 
2.41.0

Re: [PATCH v2 1/2] dt-bindings: arm: fsl: Add i.MX93 PHYTEC with Segin

[Krzysztof Kozlowski]

On 17/01/2024 08:49, Mathieu Othacehe wrote:
> Add support for i.MX93 PHYTEC with Segin board.
> 
> Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: arm: fsl: Add i.MX93 PHYTEC with Segin

[Primoz Fiser]

Hi Mathieu,

On 17. 01. 24 08:49, Mathieu Othacehe wrote:
> Add support for i.MX93 PHYTEC with Segin board.
> 
> Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 228dcc5c7d6f..196935d3abf0 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -1275,6 +1275,12 @@ properties:
>            - const: tq,imx93-tqma9352        # TQ-Systems GmbH i.MX93 TQMa93xxCA/LA SOM
>            - const: fsl,imx93
>  
> +      - description: i.MX93 PHYTEC phyBOARD-Segin
> +        items:
> +          - const: phytec,imx93-phycore-segin
> +          - const: phytec,imx93-phycore-som
> +          - const: fsl,imx93
> +

PHYTEC has the following in their downstream kernel:

>       - description: PHYTEC phyCORE-i.MX93 SoM based boards
>         items:
>           - const: phytec,imx93-phyboard-segin # phyBOARD-Segin with i.MX93
>           - const: phytec,imx93-phycore-som    # phyCORE-i.MX93 SoM
>           - const: fsl,imx93

so there will be additional boards based on phyCORE-i.MX93 SoMs just
like with phyCORE-i.MX8MM.

Please consider fixing this in v3.


>        - description:
>            Freescale Vybrid Platform Device Tree Bindings
>  

Re: [PATCH v2 2/2] arm64: dts: imx93-phycore-segin: Add Phytec i.MX93 Segin

[Primoz Fiser]

Hi Mathieu,

first of all, nice to see an attempt to upstream the board. A bit fast
though as upstreaming was planned after the official PHYTEC release.

However, there is quite a lot of differences in respect to the
downstream PHYTEC kernel tree.

I am sure PHYTEC wants to sync here to avoid general confusion.

CC-ing: upstream@list.phytec.de

So lets start:

- the board should be named "imx93-phyboard-segin" instead of
"imx93-phycore-segin".

PHYTEC naming convention:
phyCORE -> the SoM
phyBOARD -> the base board (with the SoM)


On 17. 01. 24 08:49, Mathieu Othacehe wrote:
> Add DTSI for Phytec i.MX93 System on Module and DTS for Phytec
> i.MX93 on Segin evaluation board.
> 
> This version comes with:
>  - 1GB LPDDR4 RAM
>  - external SD
>  - debug UART
>  - 1x 100Mbit Ethernet

Maybe you sync this commit msg + title with downstream commit like so:

>     arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support
>     
>     Add basic support for phyBOARD-Segin-i.MX93.
>     Main features are:
>     * Ethernet
>     * SD-Card
>     * UART
>     

plus add eMMC to the list if you decided to enable it?


> 
> Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
> ---
>  arch/arm64/boot/dts/freescale/Makefile        |  1 +
>  .../dts/freescale/imx93-phycore-segin.dts     | 93 +++++++++++++++++++
>  .../boot/dts/freescale/imx93-phycore-som.dtsi | 64 +++++++++++++
>  3 files changed, 158 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx93-phycore-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..f078d6ef75f7 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-phycore-segin.dtb

Like I said, this has to be renamed to imx93-phyboard-segin.dts as the
official kit name is "phyBOARD-Segin-i.MX93".

>  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-phycore-segin.dts b/arch/arm64/boot/dts/freescale/imx93-phycore-segin.dts
> new file mode 100644
> index 000000000000..748b779a9dc1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-phycore-segin.dts> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
> + * Christoph Stoidner <c.stoidner@phytec.de>
> + * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com>
> + *
> + */
> +/dts-v1/;
> +
> +#include "imx93-phycore-som.dtsi"
> +
> +/{
> +	model = "PHYTEC phyBOARD-Segin-i.MX93";
> +	compatible = "phytec,imx93-phycore-segin", "phytec,imx93-phycore-som",
> +		     "fsl,imx93";
> +
> +	chosen {
> +		stdout-path = &lpuart1;
> +	};
> +
> +	reg_usdhc2_vmmc: regulator-usdhc2 {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;

Order properties here alphabetically like in the downstream kernel.

Comment applies for the entire patch.

> +	};
> +};
> +
> +/* Console */
> +&lpuart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +/* SD-Card */
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> +	pinctrl-1 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> +	pinctrl-2 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> +	cd-gpios = <&gpio3 00 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&reg_usdhc2_vmmc>;
> +	bus-width = <4>;
> +	status = "okay";
> +	no-sdio;
> +	no-mmc;
> +};
> +

Please consider adding pinctrl_100mhz and pinctrl_200mhz from the
downstream kernel which were determined by HW measurements at those
operating frequencies.


> +/* Watchdog */
> +&wdog3 {
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";
> +	status = "okay";

Remove this pinctrl + status left-over?

> +
> +	pinctrl_uart1: uart1grp {
> +		fsl,pins = <
> +			MX93_PAD_UART1_RXD__LPUART1_RX			0x31e
> +			MX93_PAD_UART1_TXD__LPUART1_TX			0x31e

Please consider pinctrl settings from the down-stream kernel. They differ.

> +		>;
> +	};
> +
> +	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
> +		fsl,pins = <
> +			MX93_PAD_SD2_RESET_B__GPIO3_IO07		0x31e
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_gpio: usdhc2gpiogrp {
> +		fsl,pins = <
> +			MX93_PAD_SD2_CD_B__GPIO3_IO00			0x31e
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX93_PAD_SD2_CLK__USDHC2_CLK			0x178e
> +			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
> +		>;
> +	};
> +};
> 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..4edff4a50b2b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2023 PHYTEC Messtechnik GmbH
> + * Christoph Stoidner <c.stoidner@phytec.de>
> + * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com>
> + *
> + */
> +/dts-v1/;
> +
> +#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;
> +		};
> +
> +		ele_reserved: ele-reserved@a4120000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0 0xa4120000 0 0x100000>;
> +			no-map;
> +		};

Remove ele_reserved if you are not using it anywhere. This only applies
to vendor kernel at the moment.

> +	};
> +};
> +
> +/* eMMC */
> +&usdhc1 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc1>;
> +	pinctrl-1 = <&pinctrl_usdhc1>;
> +	pinctrl-2 = <&pinctrl_usdhc1>;
> +	bus-width = <8>;
> +	non-removable;
> +	status = "okay";

Currently only default DDR50 is supported.

So no need for 100mhz + 200mhz pinctrls.

> +};
> +
> +&iomuxc {
> +	pinctrl_usdhc1: usdhc1grp {
> +		fsl,pins = <
> +			MX93_PAD_SD1_CLK__USDHC1_CLK		0x15fe
> +			MX93_PAD_SD1_CMD__USDHC1_CMD		0x13fe
> +			MX93_PAD_SD1_DATA0__USDHC1_DATA0	0x13fe
> +			MX93_PAD_SD1_DATA1__USDHC1_DATA1	0x13fe
> +			MX93_PAD_SD1_DATA2__USDHC1_DATA2	0x13fe
> +			MX93_PAD_SD1_DATA3__USDHC1_DATA3	0x13fe
> +			MX93_PAD_SD1_DATA4__USDHC1_DATA4	0x13fe
> +			MX93_PAD_SD1_DATA5__USDHC1_DATA5	0x13fe
> +			MX93_PAD_SD1_DATA6__USDHC1_DATA6	0x13fe
> +			MX93_PAD_SD1_DATA7__USDHC1_DATA7	0x13fe
> +			MX93_PAD_SD1_STROBE__USDHC1_STROBE	0x15fe
> +		>;
> +	};
> +};

BR,
Primoz

Re: [PATCH v2 2/2] arm64: dts: imx93-phycore-segin: Add Phytec i.MX93 Segin

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

On Thu, Jan 18, 2024 at 10:58:36AM +0100, Primoz Fiser wrote:
> > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > +		compatible = "regulator-fixed";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
> > +		regulator-name = "VSD_3V3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> 
> Order properties here alphabetically like in the downstream kernel.
> 
> Comment applies for the entire patch.

Please do not order properties alphabetically. Instead, please read the
new documentation on property ordering that makes explicit what has just
been convention until now:
https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst?h=for-next&id=83a368a3fc8ae8538bccb713dc0cae9eacc04790#n112

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] arm64: dts: imx93-phycore-segin: Add Phytec i.MX93 Segin

[Mathieu Othacehe]

Hey,

> Please do not order properties alphabetically. Instead, please read
> the new documentation on property ordering that makes explicit what
> has just been convention until now:
> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst?h=for-next&id=83a368a3fc8ae8538bccb713dc0cae9eacc04790#n112

Thanks for the link.

I have a question though. Regarding that section:

--8<---------------cut here---------------start------------->8---
/* 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 00 GPIO_ACTIVE_LOW>;
	no-sdio;
	no-mmc;
	vmmc-supply = <&reg_usdhc2_vmmc>;
	status = "okay";
};
--8<---------------cut here---------------end--------------->8---

The documentation states:

--8<---------------cut here---------------start------------->8---
Order of Properties in Device Node
----------------------------------

The following order of properties in device nodes is preferred:

1. "compatible"
2. "reg"
3. "ranges"
4. Standard/common properties (defined by common bindings, e.g. without
   vendor-prefixes)
5. Vendor-specific properties
6. "status" (if applicable)
7. Child nodes, where each node is preceded with a blank line
--8<---------------cut here---------------end--------------->8---

All of the properties in my example are falling into the "4" category I
guess, except for "status" that should come last. Now, how am I supposed
to order those properties? I had a look to other IMX device trees and it
is hard to establish a pattern. Pinctrl first, then alphabetical order?
Anything else?

Thanks for the guidance,

Mathieu

Re: [PATCH v2 2/2] arm64: dts: imx93-phycore-segin: Add Phytec i.MX93 Segin

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

On Thu, Jan 18, 2024 at 02:43:07PM +0100, Mathieu Othacehe wrote:
> 
> Hey,
> 
> > Please do not order properties alphabetically. Instead, please read
> > the new documentation on property ordering that makes explicit what
> > has just been convention until now:
> > https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst?h=for-next&id=83a368a3fc8ae8538bccb713dc0cae9eacc04790#n112
> 
> Thanks for the link.
> 
> I have a question though. Regarding that section:
> 
> --8<---------------cut here---------------start------------->8---
> /* 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 00 GPIO_ACTIVE_LOW>;
> 	no-sdio;
> 	no-mmc;
> 	vmmc-supply = <&reg_usdhc2_vmmc>;
> 	status = "okay";
> };
> --8<---------------cut here---------------end--------------->8---
> 
> The documentation states:
> 
> --8<---------------cut here---------------start------------->8---
> Order of Properties in Device Node
> ----------------------------------
> 
> The following order of properties in device nodes is preferred:
> 
> 1. "compatible"
> 2. "reg"
> 3. "ranges"
> 4. Standard/common properties (defined by common bindings, e.g. without
>    vendor-prefixes)
> 5. Vendor-specific properties
> 6. "status" (if applicable)
> 7. Child nodes, where each node is preceded with a blank line
> --8<---------------cut here---------------end--------------->8---
> 
> All of the properties in my example are falling into the "4" category I
> guess, except for "status" that should come last. Now, how am I supposed
> to order those properties? I had a look to other IMX device trees and it
> is hard to establish a pattern. Pinctrl first, then alphabetical order?
> Anything else?

If that is the established order for imx devicetrees, then yeah, I would
follow that ordering. From my own quick check of recently added boards,
that is the way things seem to be.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]