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

Linus Walleij
2025-12-26T09:40:19Z

On Thu, Nov 27, 2025 at 11:58 AM Conor Dooley <conor@kernel.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