From: Conor Dooley <conor@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Conor Dooley <conor.dooley@microchip.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org
Subject: Re: [RFC 3/5] pinctrl: add polarfire soc iomux0 pinmux driver
Date: Mon, 13 Oct 2025 12:42:39 +0100 [thread overview]
Message-ID: <20251013-spoiling-halogen-4e56c4bc83dd@spud> (raw)
In-Reply-To: <CACRpkdaEsa5gSpGxWG8xudMePt12nZaZRCRrW5Bf5JQ0f1K9Zw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
On Mon, Oct 13, 2025 at 01:02:35PM +0200, Linus Walleij wrote:
> On Wed, Oct 1, 2025 at 5:45 PM Conor Dooley <conor@kernel.org> wrote:
>
> > They're not actually package pins at all that are being configured here,
> > it's internal routing inside the FPGA. It does operate on a function
> > level, but I don't think there's a neat mapping to the pinctrl subsystem
> > which (AFAIU) considers functions to contain groups, which in turn
> > contain pins. I suppose it could be thought of that, for example, spi0
> > is actually a function containing 4 (or 5, don't ask - or do if you want
> > to read a rant about pointlessly confusing design) "pins" in 1 group.
> >
> > If I could just work in terms of functions only, and avoid groups or
> > pins at all, feels (to me ofc) like it'd maybe match the purpose of this
> > aspect of the hardware better.
>
> What I would ask myself is whether the abstraction fits the bill,
> like if there is a natural set of functions in the silicon, then the code
> should reflect that.
>
> When it comes to those:
>
> +static const struct pinctrl_pin_desc mpfs_iomux0_pinctrl_pins[] = {
> + PINCTRL_PIN(0, "spi0"),
> + PINCTRL_PIN(1, "spi1"),
>
> At least be careful with the nouns used: are they really "pins"?
> Should they be described as "pins"?
I think that my choices if talking to someone outside of the context of
the structure of the pinctrl subsystem would be functions (for what's in
the driver as pins) and routings (instead of groups). pinctrl's function
doesn't really do very much in this context, although the devicetree
function and groups I think actually work fairly well: "function ABC is
assigned to pin 1".
Regarding nouns, I think it depends on how far you go with that...
> Maybe it is best to come up with some custom struct if not?
...because taking that to an extreme means forsaking a lot (all) of the
common pinctrl infrastructure, no? If it's just choosing more apt names
for variables/functions in the driver or redefining PINCTRL_PIN to be
something more suitably named, then sure. But if I have to avoid using
pinctrl_pin_desc et al, am I going to lose out on a bunch of useful
common code?
I'll send my v2 on tomorrow probably, and we can talk about what exact
changes should be made there?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2025-10-13 11:42 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-26 14:33 [RFC 0/5] microchip mpfs/pic64gx pinctrl questions Conor Dooley
2025-09-26 14:33 ` [RFC 1/5] dt-bindings: pinctrl: add polarfire soc iomux0 pinmux Conor Dooley
2025-09-26 14:33 ` [RFC 2/5] dt-bindings: pinctrl: add pic64gx "gpio2" pinmux Conor Dooley
2025-10-01 11:32 ` Linus Walleij
2025-10-01 15:47 ` Conor Dooley
2025-10-01 15:48 ` Conor Dooley
2025-10-13 10:56 ` Linus Walleij
2025-10-13 11:22 ` Conor Dooley
2025-09-26 14:33 ` [RFC 3/5] pinctrl: add polarfire soc iomux0 pinmux driver Conor Dooley
2025-10-01 11:34 ` Linus Walleij
2025-10-01 11:36 ` Linus Walleij
2025-10-01 15:45 ` Conor Dooley
2025-10-13 11:02 ` Linus Walleij
2025-10-13 11:42 ` Conor Dooley [this message]
2025-10-14 10:27 ` Linus Walleij
2025-09-26 14:33 ` [RFC 4/5] pinctrl: add pic64gx "gpio2" " Conor Dooley
2025-09-26 14:33 ` [RFC 5/5] riscv: dts: microchip: add pinctrl nodes for iomux0 Conor Dooley
2025-10-01 11:29 ` [RFC 0/5] microchip mpfs/pic64gx pinctrl questions Linus Walleij
2025-10-01 16:00 ` Conor Dooley
2025-10-01 16:15 ` Conor Dooley
2025-10-09 15:55 ` Conor Dooley
2025-10-13 13:27 ` Linus Walleij
2025-10-13 13:55 ` Conor Dooley
2025-10-14 10:33 ` Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251013-spoiling-halogen-4e56c4bc83dd@spud \
--to=conor@kernel.org \
--cc=conor.dooley@microchip.com \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).