linux-hardening - Re: [PATCH v3 12/15] pinctrl: allow to mark pin functions as requestable GPIOs

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] [day] [month] [year] [list]

Message-ID: <CAHp75Vc4vsJh_-GbP+YO50veoGoGtfAPL4tjcF+73uophfmnGw@mail.gmail.com>
Date: Thu, 24 Jul 2025 14:21:53 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, Alexey Klimov <alexey.klimov@...aro.org>, 
	Lorenzo Bianconi <lorenzo@...nel.org>, Sean Wang <sean.wang@...nel.org>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	Paul Cercueil <paul@...pouillou.net>, Kees Cook <kees@...nel.org>, 
	Andy Shevchenko <andy@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>, 
	David Hildenbrand <david@...hat.com>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	Mike Rapoport <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>, 
	Dong Aisheng <aisheng.dong@....com>, Fabio Estevam <festevam@...il.com>, 
	Shawn Guo <shawnguo@...nel.org>, Jacky Bai <ping.bai@....com>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, NXP S32 Linux Team <s32@....com>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Tony Lindgren <tony@...mide.com>, 
	Haojian Zhuang <haojian.zhuang@...aro.org>, Geert Uytterhoeven <geert+renesas@...der.be>, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, linux-mediatek@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org, linux-mips@...r.kernel.org, 
	linux-hardening@...r.kernel.org, linux-mm@...ck.org, imx@...ts.linux.dev, 
	linux-omap@...r.kernel.org, linux-renesas-soc@...r.kernel.org, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v3 12/15] pinctrl: allow to mark pin functions as
 requestable GPIOs

On Thu, Jul 24, 2025 at 11:25 AM Bartosz Golaszewski <brgl@...ev.pl> wrote:
>
> The name of the pin function has no real meaning to pinctrl core and is
> there only for human readability of device properties. Some pins are
> muxed as GPIOs but for "strict" pinmuxers it's impossible to request
> them as GPIOs if they're bound to a devide - even if their function name
> explicitly says "gpio". Add a new field to struct pinfunction that
> allows to pass additional flags to pinctrl core. While we could go with

passing
to the pinctrl

> a boolean "is_gpio" field, a flags field is more future-proof.
>
> If the PINFUNCTION_FLAG_GPIO is set for a given function, the pin muxed
> to it can be requested as GPIO even on strict pin controllers. Add a new

"...the pin, which is muxed to it, ..."

> callback to struct pinmux_ops - function_is_gpio() - that allows pinmux
> core to inspect a function and see if it's a GPIO one. Provide a generic
> implementation of this callback.

...

> -       if (ops->strict && desc->mux_usecount)
> +       if (ops->function_is_gpio && mux_setting)

Seems mux_setting presence is prior to the GPIO checks, I would swap
the parameters of &&.

> +               func_is_gpio = ops->function_is_gpio(pctldev,
> +                                                    mux_setting->func);

One line is okay.

> +       if (ops->strict && desc->mux_usecount && !func_is_gpio)
>                 return false;
>
>         return !(ops->strict && !!desc->gpio_owner);

I think this whole if/return chain can be made slightly more readable,
but I haven't had something to provide right now. Lemme think about
it,

...

> +               if (ops->function_is_gpio && mux_setting)
> +                       func_is_gpio = ops->function_is_gpio(pctldev,
> +                                                            mux_setting->func);
> +               if ((!gpio_range || ops->strict) && !func_is_gpio &&
>                     desc->mux_usecount && strcmp(desc->mux_owner, owner)) {

This is very similar to the above check, I think at bare minimum here
can be a helper for both cases.

...

> +/**
> + * pinmux_generic_function_is_gpio() - returns true if given function is a GPIO
> + * @pctldev: pin controller device
> + * @selector: function number

Missing Return section. Please run kernel-doc validator against new kernel-docs.

> + */
> +bool pinmux_generic_function_is_gpio(struct pinctrl_dev *pctldev,
> +                                    unsigned int selector)
> +{
> +       struct function_desc *function;
> +
> +       function = radix_tree_lookup(&pctldev->pin_function_tree,
> +                                    selector);

One line is okay.

> +       if (!function)
> +               return false;
> +
> +       return function->func->flags & PINFUNCTION_FLAG_GPIO;
> +}

...

>  struct pinfunction {
>         const char *name;
>         const char * const *groups;
>         size_t ngroups;
> +       unsigned long flags;

Not sure we need this. If the function is GPIO, pin control already
knows about this. The pin muxing has gpio request / release callbacks
that change the state. Why do we need an additional flag(s)?

>  };

-- 
With Best Regards,
Andy Shevchenko