| Message ID | 20240122095306.14084-1-othacehe@gnu.org |
|---|---|
| Headers | show |
| Series | Add phyBOARD-Segin-i.MX93 support | expand |
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 <othacehe@gnu.org> >>> --- >>> 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 >>> <c.stoidner@phytec.de> >>> + * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com> >>> + * >>> + * 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
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 <othacehe@gnu.org> >>>> --- >>>> 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 >>>> <c.stoidner@phytec.de> >>>> + * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com> >>>> + * >>>> + * 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
Hello, This adds support for the phyBOARD-Segin-i.MX93 board. Note that the v4 enables the FEC ethernet port to ease device-tree debugging, as well as the heartbeat LED and the gpio-line-names for the GPIOs on the phyBOARD X16 header. It also adds a new documentation patch to fix a dt check warning regarding gpio-line-names property. Thanks, Signed-off-by: Mathieu Othacehe <othacehe@gnu.org> --- Changes in v4: - Add gpio-line-names to gpio-vf610 dt-bindings documentation - Add the heartbeat SoM LED - Add support for the FEC ethernet port - Restore the original authors in the copyright - Add gpio-line-names for the GPIOs on the X16 header Changes in v3: - Update documentation to match PHYTEC naming - Remove useless properties - Update pinmux from PHYTEC downstream kernel Changes in v2: - Remove useless line - Add missing reserved-memory entries v3: https://lore.kernel.org/linux-devicetree/20240119092835.21462-1-othacehe@gnu.org/ Mathieu Othacehe (3): dt-bindings: arm: fsl: Add phyBOARD-Segin-i.MX93 dt-bindings: gpio: gpio-vf610: add gpio-line-names arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support .../devicetree/bindings/arm/fsl.yaml | 6 + .../devicetree/bindings/gpio/gpio-vf610.yaml | 1 + arch/arm64/boot/dts/freescale/Makefile | 1 + .../dts/freescale/imx93-phyboard-segin.dts | 141 ++++++++++++++++++ .../boot/dts/freescale/imx93-phycore-som.dtsi | 127 ++++++++++++++++ 5 files changed, 276 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