Re: [RFC v1 2/4] pinctrl: add polarfire soc mssio pinctrl driver

From: Linus Walleij
Date: Tue Nov 25 2025 - 14:28:34 EST


On Tue, Nov 25, 2025 at 6:47 PM Conor Dooley <conor@xxxxxxxxxx> wrote:

> > I think it's unfortunate naming, people sometimes use the word
> > "pinmux" as a DT property, sometimes to describe the subsystem,
> > sometimes a part of the subsystem, sometimes anything related
> > to pins.
>
> I think I actually understand the naming now. It's called pinmux because
> the existing function pinconf_generic_dt_node_to_map() doesn't support
> pinmux, so this is the version you need for platforms that are using
> pinmux. But then nothing about it limits it actually to pinmuxes, other
> than arbitrary property checks, it could actually be used for my pins +
> functions use-case, if I added similar code to amlogics in my probe
> function that creates the functions and groups.
>
> I still think the naming is poor though, and that it is not as generic as
> it purports to be, since it depends on having the exact dt configuration
> that amlogic has, and wouldn't work for spacemit that use the first
> multi-group example that I gave above. I'd be inclined to say that it
> should be shunted back to the amlogic driver, to avoid baiting people
> into trying to use it because of the label position problem,

You're probably right. I see the problem here.

> > I know I should perhaps have shepherded this better :/
>
> idk, I think this is the usual "creating something generic but with only
> one user" problem. Hard to know if it actually is generic at all.

The problem is mostly too few people working on genericizing
the pinctrl code I think, it makes me be happy about any such
attempts. But I should pay more attention, clearly I just looked
at it superficially.

> > > It seems to depend on aml_pctl_parse_functions() being called
> > > during probe which creates the groups and functions.
> > > There's a weird warning about expecting a function parent node that seems
> > > very amlogic specific too.
> > >
> > > In my eyes, there should be some generic dt_node_to_map helpers that
> > > do it all for you on the "configuration entirely described in dt"
> > > platforms because that's what stuff like spacemit k1 driver that do
> > > this in their dt_node_to_map implementations.
> >
> > I think you're right!
>
> My dilemma now is what to call them and where to put them.
> pinconf_generic_dt_node_to_map<something>() feels weird for something
> that is also creating functions and groups, which I noticed because I
> was having to include pinmux.h in pinconf.c so that I could call
> pinmux_generic_add_function().

pinctrl_generic_dt_node_parse_config() or so? Is it vague enough?

Yours,
Linus Walleij