[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08ef805a-b041-4db0-aaf7-51d5d06596ff@gmx.net>
Date: Tue, 23 Jan 2024 11:21:11 +0100
From: Stefan Wahren <wahrenst@....net>
To: Wadim Egorov <w.egorov@...tec.de>, Mathieu Othacehe <othacehe@....org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, NXP Linux Team <linux-imx@....com>,
Li Yang <leoyang.li@....com>, Primoz Fiser <primoz.fiser@...ik.com>,
Christoph Stoidner <c.stoidner@...tec.de>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, upstream@...ts.phytec.de
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 <othacehe@....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@...tec.de>, Christoph Stoidner
>>>> <c.stoidner@...tec.de>
>>>> + * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@...il.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
Powered by blists - more mailing lists