| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <008317aa-4dd1-4889-8c64-5e4396d83931@phytec.de>
Date: Wed, 24 Jan 2024 06:13:39 +0100
From: Wadim Egorov <w.egorov@...tec.de>
To: Stefan Wahren <wahrenst@....net>, 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,
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@....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.
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
>
Powered by blists - more mailing lists