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:

quoted

Am 23.01.24 um 08:42 schrieb Stefan Wahren:

quoted

Hi Wadim,

Am 23.01.24 um 07:11 schrieb Wadim Egorov:

quoted

Hey Mathieu,

Am 22.01.24 um 10:53 schrieb Mathieu Othacehe:

quoted

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

Signed-off-by: Mathieu Othacehe <redacted>
---
  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 <w.egorov@phytec.de>, Christoph Stoidner

[off-list ref]
+ * Copyright (C) 2024 Mathieu Othacehe [off-list ref]
+ *
+ * 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?

quoted

quoted

quoted

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.

quoted

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.

quoted

quoted

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.

quoted

Regards,
Wadim

quoted

Best regards

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel