linux-kernel - Re: [RFC v2 3/5] pinctrl: add polarfire soc mssio pinctrl driver

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: <CAD++jLkxLJRZocHenBASLzoUAbw=oPpMajNF6a5z-Lzds+5Ecw@mail.gmail.com>
Date: Fri, 26 Dec 2025 10:40:07 +0100
From: Linus Walleij <linusw@...nel.org>
To: Conor Dooley <conor@...nel.org>
Cc: linus.walleij@...aro.org, Conor Dooley <conor.dooley@...rochip.com>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, linux-kernel@...r.kernel.org, 
	linux-gpio@...r.kernel.org, devicetree@...r.kernel.org, 
	Valentina.FernandezAlanis@...rochip.com
Subject: Re: [RFC v2 3/5] pinctrl: add polarfire soc mssio pinctrl driver

On Thu, Nov 27, 2025 at 11:58 AM Conor Dooley <conor@...nel.org> wrote:

>  drivers/pinctrl/Kconfig              |   7 +-
>  drivers/pinctrl/Makefile             |   1 +
>  drivers/pinctrl/pinctrl-mpfs-mssio.c | 750 +++++++++++++++++++++++++++

Time to move the drivers to drivers/pinctrl/microchip
before it becomes an overpopulation problem?

(The previous drivers can be moved in a separate patch.)


> +       select GENERIC_PINCTRL_GROUPS
> +       select GENERIC_PINMUX_FUNCTIONS
> +       select GENERIC_PINCTRL_BELLS_AND_WHISTLES

Just the bottom select will bring it all in, right?

> +static int mpfs_pinctrl_pin_to_iocfg_reg(unsigned int pin)
> +{
> +       u32 reg = MPFS_PINCTRL_IOCFG01_REG;
> +
> +       if (pin >= MPFS_PINCTRL_BANK2_START)
> +               reg += MPFS_PINCTRL_INTER_BANK_GAP;
> +
> +       // 2 pins per 32-bit register
> +       reg += (pin / 2) * 0x4;

It's helpful with these nice comments that ease the reading of the code
quite a bit.

> +static int mpfs_pinctrl_set_mux(struct pinctrl_dev *pctrl_dev, unsigned int fsel,
> +                               unsigned int gsel)
> +{
> +       struct mpfs_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev);
> +       const struct group_desc *group;
> +       const char **functions;
> +
> +       group = pinctrl_generic_get_group(pctrl_dev, gsel);
> +       if (!group)
> +               return -EINVAL;
> +
> +       functions = group->data;
> +
> +       for (int i = 0; i < group->grp.npins; i++) {
> +               u32 function;
> +
> +               //TODO @Linus my new function being actually generic means that
> +               // the mapping of function string to something the hardware
> +               // understands only happens at this point.
> +               // I think this is fine, because dt validation would whinge
> +               // about something invalid, but it's the "catch" with my approach.
> +               // The other option I considered was to provide a mapping
> +               // function pointer that the driver can populate, but I think
> +               // that's overkill.
> +               function = mpfs_pinctrl_function_map(functions[i]);
> +               if (function < 0) {
> +                       dev_err(pctrl->dev, "invalid function %s\n", functions[i]);
> +                       return function;
> +               }

This is fine with me.

Ideally I would like code that does a lot of string stacking and comparing
to be using Rust, but we cannot yet use that in core code so that is for
another day.

Yours,
Linus Walleij