linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc: Hal Feng <hal.feng@starfivetech.com>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-riscv@lists.infradead.org, Andreas Schwab <schwab@suse.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Jianlong Huang <jianlong.huang@starfivetech.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 0/4] Basic pinctrl support for StarFive JH7110 RISC-V SoC
Date: Mon, 6 Mar 2023 14:43:06 +0100	[thread overview]
Message-ID: <CACRpkdYvzbK-ZJVzh6mGY3Sf5fT3u9LQYkW52BBtiFd2REgwNA@mail.gmail.com> (raw)
In-Reply-To: <CAJM55Z-+Cxdebcn4MLXfQdOVhx4c2SQ+zMH8cjn-Yq35xO8g0A@mail.gmail.com>

On Mon, Feb 20, 2023 at 11:16 AM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:

> Hi Hal and Linus
>
> I'm curious if there is a plan to address Icenowy's concerns here:
> https://lore.kernel.org/linux-gpio/52dcbe48dbf5f2561713a9642943353216fef15a.camel@icenowy.me/
>
> The problem is that input from "GPIO" pins is configured a little
> differently on the StarFive SoCs. Instead of having a register pr.
> pin(grroup) there is a register pr. control line to the peripherals,
> and into these you write the pin number + 2 of the pin you want the
> peripheral to react to. Why +2? That's because 0 is a special "always
> low" signal and similarly 1 is a special "always high" signal.
>
> With the current bindings one hacky way to solve this is to treat
> those two special values as kind of "virtual pins" that will always be
> high or low. So that would be something like
>
> pinmux = <GPIOMUX(GPIO_ALWAYS_LOW, GPOUT_IGNORED, GPOEN_DISABLE,
> GPI_SYS_USB_OVERCURRENT)>;
>
> ..but this means we might need to mux these two virtual pins to
> multiple peripherals. I'm not sure the pinmux framework is prepared
> for that.

I am not a fan of the magic numbers approach to pin muxing at all and I
often point out that I prefer that you use strings and connect groups with
functions using strings.

The above mentioned problems is one of the reasons.

As maintainer I have been repeatedly hammered down by maintainers
who think magic numbers is good for them, usually because it saves
space in the DTS file and is easier to code a driver for. So this alternative
approach is acceptable, as a compromise.

My apologies if I didn't point this out as sternly as I often do for
StarFive. Really sorry. I guess I have just started to give up on insisting,
as so many people like their magic numbers.

If you switch to stop using the "pinmux" property and instead use
string arrays function and groups as defined in
Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml
this should fix it, the old pinmux device trees can be supported
as fallback, if pinmux is present but function and groups are
missing.

So that is my suggestion.

Usually people don't like this suggestion, so I suspect you might
not like it either and come up with something else to work around
it.

Yours,
Linus Walleij

      reply	other threads:[~2023-03-06 13:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-09 14:36 [PATCH v5 0/4] Basic pinctrl support for StarFive JH7110 RISC-V SoC Hal Feng
2023-02-09 14:36 ` [PATCH v5 1/4] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl Hal Feng
2023-02-09 14:37 ` [PATCH v5 2/4] dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl Hal Feng
2023-02-09 14:37 ` [PATCH v5 3/4] pinctrl: starfive: Add StarFive JH7110 sys controller driver Hal Feng
2023-02-09 14:37 ` [PATCH v5 4/4] pinctrl: starfive: Add StarFive JH7110 aon " Hal Feng
2023-02-10 22:45 ` [PATCH v5 0/4] Basic pinctrl support for StarFive JH7110 RISC-V SoC Linus Walleij
2023-02-11  8:24   ` Hal Feng
2023-02-20 10:15     ` Emil Renner Berthing
2023-03-06 13:43       ` Linus Walleij [this message]

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=CACRpkdYvzbK-ZJVzh6mGY3Sf5fT3u9LQYkW52BBtiFd2REgwNA@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=emil.renner.berthing@canonical.com \
    --cc=hal.feng@starfivetech.com \
    --cc=jianlong.huang@starfivetech.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=robh+dt@kernel.org \
    --cc=schwab@suse.de \
    /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).