Messages in this thread | | | Date | Wed, 24 Jan 2024 06:13:39 +0100 | Subject | Re: [PATCH v4 3/3] arm64: dts: imx93: Add phyBOARD-Segin-i.MX93 support | From | 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 <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.
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 >
| |