linux-kernel - Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ldx5htm3.fsf@prevas.dk>
Date: Wed, 27 Nov 2024 11:13:40 +0100
From: Rasmus Villemoes <ravi@...vas.dk>
To: "Arnd Bergmann" <arnd@...db.de>
Cc: "Fabio Estevam" <festevam@...il.com>,  "Guenter Roeck"
 <linux@...ck-us.net>,  "Linus Walleij" <linus.walleij@...aro.org>,  "Esben
 Haabendal" <esben@...nix.com>,  "Russell King" <linux@...linux.org.uk>,
  "Shawn Guo" <shawnguo@...nel.org>,  "Sascha Hauer"
 <s.hauer@...gutronix.de>,  "Pengutronix Kernel Team"
 <kernel@...gutronix.de>,  "Dong Aisheng" <aisheng.dong@....com>,  "Jacky
 Bai" <ping.bai@....com>,  linux-arm-kernel@...ts.infradead.org,
  imx@...ts.linux.dev,  linux-kernel@...r.kernel.org,  "open list:GPIO
 SUBSYSTEM" <linux-gpio@...r.kernel.org>,  "Fabio Estevam"
 <festevam@...x.de>
Subject: Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl

On Wed, Nov 27 2024, "Arnd Bergmann" <arnd@...db.de> wrote:

> On Wed, Nov 27, 2024, at 10:13, Rasmus Villemoes wrote:
>> On Tue, Nov 26 2024, Fabio Estevam <festevam@...il.com> wrote:
>>>> Fabio submitted a patch enabling PINCTRL for imx_v4_v5_defconfig and
>>>> imx_v6_v7_defconfig explicitly [1]. I don't know if that fixes the
>>>> problem for good - I see CONFIG_ARCH_MXC in other configurations as
>>>> well.
>>>
>>> Good point. I can send a v2 adding CONFIG_PINCTRL=y to the other defconfigs.
>>>
>>
>> Instead of doing that, isn't this exactly what the 'imply' keyword is
>> for?
>>
>> - weak reverse dependencies: "imply" <symbol> ["if" <expr>]
>>
>>   This is similar to "select" as it enforces a lower limit on another
>>   symbol except that the "implied" symbol's value may still be set to n
>>   from a direct dependency or with a visible prompt.
>>
>>
>> So how about adding 'imply PINCTRL' in lieu of the previous 'select
>> PINCTRL'? And that would also better match the intention of the patch in
>> question (namely that the user needs to take explicit action to disable
>> PINCTRL).
>
> Please never use imply. Even if you think it's the right
> thing in a particular case, it will come back to bite you
> later.

Could you elaborate?

> See also https://en.wikipedia.org/wiki/COMEFROM ;-)

Yes yes, we've probably all seen that at some point and chuckled, but I
fail to see why imply would be worse than select.

> I would prefer we completely kill off that keyword from the Kconfig
> language and replace it with the reverse 'default'. In this
> particular case, having 'default ARCH_IMX' in 'PINCTRL'
> would of course not be a great idea,

Just to be clear, it would be 'default y if ARCH_MXC', not 'default
ARCH_IMX', right? Yes, given that the PINCTRL entry doesn't currently
have a list of "default y if ...", it would seem inappropriate.

> but for the exact same reason, the 'imply' is wrong here.

I don't follow, sorry.

Rasmus