From: Conor Dooley <conor.dooley@microchip.com>

Hey Linus,

Got the other driver that I was talking about here for you...
It's in RFC state because I'd like to get some feedback on the approach
while I try to figure out a) what ibufmd is and b) how the bank voltage
interacts with the schmitt trigger setting. Although, I am pretty sure
for the latter that it is not forced on for low voltages and that the
commented code should be deleted.

There's some specific @Linus questions in the driver, mostly where I was
unsure about how config bits should be handled and looking around at
other drivers wasn't sufficient because they did different things.

Finally, on the dt side, this was using the pinmux property before the
other drivers were submitted, but I didn't really like it to begin with
(shoving two things into entries of the same property gives me the ick).
I moved to using pins + function which at least looks prettier in the
devicetree. I had been hoping that I could move to some sort of generic
dt_node_to_map function, but I couldn't figure out one that'd work
without creating groups in the driver. If there is, I'd love to get rid
of the custom dt_node_to_map stuff.
I want to avoid doing having set groups, because of the number of
possible configurations in the MSS Configurator FPGA tool is fairly
large, and I believe that MSS Configurator actually undersells the
number of possible combinations for ease of use. I haven't tested that
and the driver technically doesn't support it, but I feel like not trying
to define groups statically and using pins instead would permit those
combos in the future should that use case ever materialise.

Cheers,
Conor.

CC: Linus Walleij <linus.walleij@linaro.org>
CC: Rob Herring <robh@kernel.org>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: linux-kernel@vger.kernel.org
CC: linux-gpio@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: Valentina.FernandezAlanis@microchip.com

Conor Dooley (4):
  dt-bindings: pinctrl: document polarfire soc mssio pin controller
  pinctrl: add polarfire soc mssio pinctrl driver
  MAINTAINERS: add Microchip mpfs mssio driver/bindings to entry
  riscv: dts: microchip: add pinctrl nodes for mpfs/icicle kit

 .../pinctrl/microchip,mpfs-pinctrl-mssio.yaml | 108 +++
 .../microchip,mpfs-mss-top-sysreg.yaml        |   4 +
 MAINTAINERS                                   |   2 +
 .../dts/microchip/mpfs-icicle-kit-common.dtsi |   1 -
 .../dts/microchip/mpfs-icicle-kit-fabric.dtsi |  63 ++
 .../boot/dts/microchip/mpfs-pinctrl.dtsi      | 165 ++++
 arch/riscv/boot/dts/microchip/mpfs.dtsi       |  16 +
 drivers/pinctrl/Kconfig                       |   5 +-
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/pinctrl-mpfs-mssio.c          | 798 ++++++++++++++++++
 10 files changed, 1161 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/microchip,mpfs-pinctrl-mssio.yaml
 create mode 100644 arch/riscv/boot/dts/microchip/mpfs-pinctrl.dtsi
 create mode 100644 drivers/pinctrl/pinctrl-mpfs-mssio.c

-- 
2.51.0

On Wed, Nov 12, 2025 at 3:33 PM Conor Dooley <conor@kernel.org> wrote:

> Got the other driver that I was talking about here for you...
> It's in RFC state because I'd like to get some feedback on the approach
> while I try to figure out a) what ibufmd is

I was going to ask about that :D

> and b) how the bank voltage
> interacts with the schmitt trigger setting.

Please check if "bank voltage" is somewhat analogous to
this generic config:

* @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
 *      supplies, the argument to this parameter (on a custom format) tells
 *      the driver which alternative power source to use.

> There's some specific @Linus questions in the driver, mostly where I was
> unsure about how config bits should be handled and looking around at
> other drivers wasn't sufficient because they did different things.

I tried to answer as best I could.

> Finally, on the dt side, this was using the pinmux property before the
> other drivers were submitted, but I didn't really like it to begin with
> (shoving two things into entries of the same property gives me the ick).
> I moved to using pins + function which at least looks prettier in the
> devicetree.

I think this looks way better than any pinmux properties.

> I had been hoping that I could move to some sort of generic
> dt_node_to_map function, but I couldn't figure out one that'd work
> without creating groups in the driver. If there is, I'd love to get rid
> of the custom dt_node_to_map stuff.

It seems like something that could be added to the core
(drivers/pinctrl/devicetree.c), if you feel like and have time for going
the extra mile. Maybe it would be simple to move some drivers
over to using it if done right.

> I want to avoid doing having set groups, because of the number of
> possible configurations in the MSS Configurator FPGA tool is fairly
> large, and I believe that MSS Configurator actually undersells the
> number of possible combinations for ease of use.

FPGA:s often have this "phone exchange" property.

> I haven't tested that
> and the driver technically doesn't support it, but I feel like not trying
> to define groups statically and using pins instead would permit those
> combos in the future should that use case ever materialise.

It makes sense for a driver for this type of very flexible hardware.

Yours,
Linus Walleij

On Wed, Nov 19, 2025 at 01:16:16PM +0100, Linus Walleij wrote:
> On Wed, Nov 12, 2025 at 3:33 PM Conor Dooley <conor@kernel.org> wrote:
> 
> > Got the other driver that I was talking about here for you...
> > It's in RFC state because I'd like to get some feedback on the approach
> > while I try to figure out a) what ibufmd is
> 
> I was going to ask about that :D
> 
> > and b) how the bank voltage
> > interacts with the schmitt trigger setting.
> 
> Please check if "bank voltage" is somewhat analogous to
> this generic config:
> 
> * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
>  *      supplies, the argument to this parameter (on a custom format) tells
>  *      the driver which alternative power source to use.

It's not pin based, the whole bank it is connected to has to be changed.
I'm not entirely sure if I want the driver to be able to control this at
all (at least not right now) because I don't know exactly what the
ramifications of changing it away from what the configuration tool
decided that it was are. Probably it could be done, but I don't know how
safe it is to change, as I think most of these kinds of settings were
never really seen as something "application" software would change
(linux is an application from the perspective of the design folks),
since it was all modifiable from the MSS configuration tool.

> > There's some specific @Linus questions in the driver, mostly where I was
> > unsure about how config bits should be handled and looking around at
> > other drivers wasn't sufficient because they did different things.
> 
> I tried to answer as best I could.

Cool, thanks.

> > Finally, on the dt side, this was using the pinmux property before the
> > other drivers were submitted, but I didn't really like it to begin with
> > (shoving two things into entries of the same property gives me the ick).
> > I moved to using pins + function which at least looks prettier in the
> > devicetree.
> 
> I think this looks way better than any pinmux properties.
> 
> > I had been hoping that I could move to some sort of generic
> > dt_node_to_map function, but I couldn't figure out one that'd work
> > without creating groups in the driver. If there is, I'd love to get rid
> > of the custom dt_node_to_map stuff.
> 
> It seems like something that could be added to the core
> (drivers/pinctrl/devicetree.c), if you feel like and have time for going
> the extra mile. Maybe it would be simple to move some drivers
> over to using it if done right.

Yeah I might, especially if it pushes people away from these pinmux
properties! Might also just submit this as-is and do it on top, depends
on what stuff I have going on.

> 
> > I want to avoid doing having set groups, because of the number of
> > possible configurations in the MSS Configurator FPGA tool is fairly
> > large, and I believe that MSS Configurator actually undersells the
> > number of possible combinations for ease of use.
> 
> FPGA:s often have this "phone exchange" property.
> 
> > I haven't tested that
> > and the driver technically doesn't support it, but I feel like not trying
> > to define groups statically and using pins instead would permit those
> > combos in the future should that use case ever materialise.
> 
> It makes sense for a driver for this type of very flexible hardware.


Thanks for taking a look.

On Wed, Nov 19, 2025 at 7:06 PM Conor Dooley <conor@kernel.org> wrote:
> On Wed, Nov 19, 2025 at 01:16:16PM +0100, Linus Walleij wrote:
> > On Wed, Nov 12, 2025 at 3:33 PM Conor Dooley <conor@kernel.org> wrote:

> > > and b) how the bank voltage
> > > interacts with the schmitt trigger setting.
> >
> > Please check if "bank voltage" is somewhat analogous to
> > this generic config:
> >
> > * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
> >  *      supplies, the argument to this parameter (on a custom format) tells
> >  *      the driver which alternative power source to use.
>
> It's not pin based, the whole bank it is connected to has to be changed.

So there *is* such a thing as a group pin config setting for a
whole group of pins. Groups are not just for functions...

And I don't know what is meant by a bank here, but it seems
to be exactly a group of pins.

From arch/arm/boot/dts/gemini/gemini-sq201.dts:

 /* Set up drive strength on GMAC0 and GMAC1 to 16 mA */
conf9 {
    groups = "gmii_gmac0_grp", "gmii_gmac1_grp";
    drive-strength = <16>;
};

If you look in driver/pinctrl/pinctrl-gemini.c you find:
gemini_pinconf_group_set()

static const struct pinconf_ops gemini_pinconf_ops = {
        .pin_config_get = gemini_pinconf_get,
        .pin_config_set = gemini_pinconf_set,
        .pin_config_group_set = gemini_pinconf_group_set,
        .is_generic = true,
};

OTOMH it's actually *fine* to *just* use groups for pin config like this
and *not* use it for muxing, i.e. have this group correspond to
a bank and not use that group for anything else than to set this
or any other per-bank property. But have a look!

Yours,
Linus Walleij

On Wed, Nov 19, 2025 at 10:31:47PM +0100, Linus Walleij wrote:
> On Wed, Nov 19, 2025 at 7:06 PM Conor Dooley <conor@kernel.org> wrote:
> > On Wed, Nov 19, 2025 at 01:16:16PM +0100, Linus Walleij wrote:
> > > On Wed, Nov 12, 2025 at 3:33 PM Conor Dooley <conor@kernel.org> wrote:
> 
> > > > and b) how the bank voltage
> > > > interacts with the schmitt trigger setting.
> > >
> > > Please check if "bank voltage" is somewhat analogous to
> > > this generic config:
> > >
> > > * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
> > >  *      supplies, the argument to this parameter (on a custom format) tells
> > >  *      the driver which alternative power source to use.
> >
> > It's not pin based, the whole bank it is connected to has to be changed.
> 
> So there *is* such a thing as a group pin config setting for a
> whole group of pins. Groups are not just for functions...
> 
> And I don't know what is meant by a bank here, but it seems
> to be exactly a group of pins.

Yeah, it's a whole group of pins. There's two banks that the IOs this
driver controls are split between, they don't really neatly correspond
to a particular function (although the configurator doesn't permit a
function belong to two banks, but it is technically possible for them
to).
> 
> From arch/arm/boot/dts/gemini/gemini-sq201.dts:
> 
>  /* Set up drive strength on GMAC0 and GMAC1 to 16 mA */
> conf9 {
>     groups = "gmii_gmac0_grp", "gmii_gmac1_grp";
>     drive-strength = <16>;
> };
> 
> If you look in driver/pinctrl/pinctrl-gemini.c you find:
> gemini_pinconf_group_set()
> 
> static const struct pinconf_ops gemini_pinconf_ops = {
>         .pin_config_get = gemini_pinconf_get,
>         .pin_config_set = gemini_pinconf_set,
>         .pin_config_group_set = gemini_pinconf_group_set,
>         .is_generic = true,
> };
> 
> OTOMH it's actually *fine* to *just* use groups for pin config like this
> and *not* use it for muxing, i.e. have this group correspond to
> a bank and not use that group for anything else than to set this
> or any other per-bank property. But have a look!

I could just put it in as a per-pin thing, even if the control isn't
actually that granular. The bank lockdown has a per-pin bit, but is
actually a bank wide toggle in the configurator, it wouldn't be too
different.