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: <20260115-wired-botanical-042f7cda4449@spud>
Date: Thu, 15 Jan 2026 17:55:57 +0000
From: Conor Dooley <conor@...nel.org>
To: Linus Walleij <linusw@...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 Fri, Dec 26, 2025 at 10:40:07AM +0100, Linus Walleij wrote:
> 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?

Sure, no 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?

I'll make it do that if it's not already. Just didn't know if you were a
"select everything you use" kinda guy or didn't mind selects selecting.

> > +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.

Eh, I feel like sometimes a comment like this is just better than trying
to insert silly defines to unmagic the numbers.

> 
> > +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.

Yeah, would be nice. My problem with it was more about the point in time
where this happens rather than doing it in the first place though.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)