MIME-Version: 1.0
References: <20230209143702.44408-1-hal.feng@starfivetech.com>
 <CACRpkdbao9M5dMHFO_QE8z4_E6DZLZsMUW1OnUNjKSfQc59=ew@mail.gmail.com> <4211d93d-702c-6759-6f25-f86160c0be5d@starfivetech.com>
In-Reply-To: <4211d93d-702c-6759-6f25-f86160c0be5d@starfivetech.com>
From:   Emil Renner Berthing <emil.renner.berthing@canonical.com>
Date:   Mon, 20 Feb 2023 11:15:44 +0100
Message-ID: <CAJM55Z-+Cxdebcn4MLXfQdOVhx4c2SQ+zMH8cjn-Yq35xO8g0A@mail.gmail.com>
Subject: Re: [PATCH v5 0/4] Basic pinctrl support for StarFive JH7110 RISC-V SoC
To:     Hal Feng <hal.feng@starfivetech.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        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
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, 11 Feb 2023 at 09:25, Hal Feng <hal.feng@starfivetech.com> wrote:
> On Fri, 10 Feb 2023 23:45:05 +0100, Linus Walleij wrote:
> > On Thu, Feb 9, 2023 at 3:37 PM Hal Feng <hal.feng@starfivetech.com> wrote:
> >
> > > This patch series adds basic pinctrl support for StarFive JH7110 SoC.
> >
> > This v4 version applied, the driver is in good shape and all bindings ACKed,
> > nice work on this driver!
>
> v4? Is this a typo? This series is v5.
> Anyway, thank you so much!

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.

/Emil

> Best regards,
> Hal