mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Icenowy Zheng <uwu@icenowy.me>
Cc: Emil Renner Berthing <kernel@esmil.dk>,
	Jianlong Huang <jianlong.huang@starfivetech.com>,
	 Hal Feng <hal.feng@starfivetech.com>,
	Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-gpio@vger.kernel.org,  devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,  linux-riscv@lists.infradead.org
Subject: Re: [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs
Date: Thu, 24 Apr 2025 10:48:44 +0200	[thread overview]
Message-ID: <CACRpkdbmF5BucsRjGZYgT709QA_Qnd3VrDoXg9dJdtAb_QLPwA@mail.gmail.com> (raw)
In-Reply-To: <c5efd677fd2199cbf8f9d6006905acdf19da02bc.camel@icenowy.me>

On Wed, Apr 23, 2025 at 4:22 PM Icenowy Zheng <uwu@icenowy.me> wrote:

> > So clearly the entities that you affect are in the same numberspace,
> > and that is all we care about. They are not enumerated in any way
> > orthogonal to any other pins AFAICT.
>
> They just share the field width, they're not in the same numberspace.

OK I trust you on this, I just had to put a bit of pressure so we try
to stay with standard bindings.

> The design of the JH7110 pin mux control is quite simple and stupid:
>
> - First per-GPIO map configuration to map the GPIO's DOEn pin to
> internal tri-stating signals.
> - Then per-GPIO map configuration to map the GPIO's DOUT pin to
> internal output signals.
> - Then per-input-signal configuration (note that it's no longer per-
> GPIO) map configuration to map the signal to a GPIO's DIN (or fixed
> low/high).

I get it, I think.

So if I understand correctly this set-up is necessary to use any one
pin as a GPIO pin?

In that case, consider that this must probably be deeply integrated
with the GPIO subsystem rather than the pin control subsystem.

For example GPIO usually has this:

gpio-ranges = <&pinctrl1 0 20 10>, <&pinctrl2 10 50 20>;

Indicating which pins are actually routed as GPIO and implicitly
contains the information you need as to which pins are
affected.

There is also

gpio-reserved-ranges = <0 4>, <12 2>;

that can be used to say certain number ranges in the GPIO
controller can *not* be used for GPIO.

This type of inferred information should ideally be used to
infer the configuration rather than hardcoded properties.

And if you have pin control as a back-end to GPIO, the
callbacks in struct pinmux_ops:

        int (*gpio_request_enable) (struct pinctrl_dev *pctldev,
                                    struct pinctrl_gpio_range *range,
                                    unsigned int offset);
        void (*gpio_disable_free) (struct pinctrl_dev *pctldev,
                                   struct pinctrl_gpio_range *range,
                                   unsigned int offset);
        int (*gpio_set_direction) (struct pinctrl_dev *pctldev,
                                   struct pinctrl_gpio_range *range,
                                   unsigned int offset,
                                   bool input);

which you could implement in jh7110_pinmux_ops to get
a tighter connection between you pinmux and GPIO controller
portions.

I have a strong feeling that the missing piece is using these
callbacks along with the gpio-ranges to connect the GPIO and
pin mux systems together so that you can set this stuff up
in the above callbacks instead.

That would save you the weird DT properties that will be a real
pain to keep in sync with the actual use.

Yours,
Linus Walleij

  reply	other threads:[~2025-04-24  8:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-22 16:22 [RFC PATCH 0/3] pinctrl: starfive: jh7110: support " Icenowy Zheng
2025-04-22 16:22 ` [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add " Icenowy Zheng
2025-04-23  9:09   ` Linus Walleij
2025-04-23  9:40     ` Icenowy Zheng
2025-04-23 10:55       ` Linus Walleij
2025-04-23 14:21         ` Icenowy Zheng
2025-04-24  8:48           ` Linus Walleij [this message]
2025-04-22 16:22 ` [RFC PATCH 2/3] pinctrl: starfive: jh7110: support forcing inputs to low/high Icenowy Zheng
2025-04-22 16:22 ` [RFC PATCH 3/3] riscv: dts: starfive: jh7110-pine64-star64: force no USB overcurrent Icenowy Zheng

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=CACRpkdbmF5BucsRjGZYgT709QA_Qnd3VrDoXg9dJdtAb_QLPwA@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hal.feng@starfivetech.com \
    --cc=jianlong.huang@starfivetech.com \
    --cc=kernel@esmil.dk \
    --cc=krzk+dt@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=robh@kernel.org \
    --cc=uwu@icenowy.me \
    /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

all inboxes | Powered by JetHome®