[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACRpkdaJNK4+Viv+kdZUSXH6r6jRfGt0KixsTuRTP56qwQccYA@mail.gmail.com>
Date: Fri, 5 Sep 2025 11:24:14 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Sudeep Holla <sudeep.holla@....com>, AKASHI Takahiro <takahiro.akashi@...aro.org>,
Michal Simek <michal.simek@....com>, Cristian Marussi <cristian.marussi@....com>,
arm-scmi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v2 4/7] pinctrl-scmi: add PIN_CONFIG_INPUT_VALUE
On Fri, Sep 5, 2025 at 10:31 AM Linus Walleij <linus.walleij@...aro.org> wrote:
> On Fri, Sep 5, 2025 at 10:27 AM Linus Walleij <linus.walleij@...aro.org> wrote:
> > On Sun, Jul 20, 2025 at 9:39 PM Dan Carpenter <dan.carpenter@...aro.org> wrote:
> >
> > > In SCMI the value of the pin is just another configuration option. Add
> > > this as an option in the pin_config_param enum and creating a mapping to
> > > SCMI_PIN_INPUT_VALUE in pinctrl_scmi_map_pinconf_type()
> > >
> > > Since this is an RFC patch, I'm going to comment that I think the SCMI
> > > pinctrl driver misuses the PIN_CONFIG_OUTPUT enum. It should be for
> > > enabling and disabling output on pins which can serve as both input and
> > > output. Enabling it is supposed to write a 1 and disabling it is
> > > supposed to write a 0 but we use that side effect to write 1s and 0s. I
> > > did't change this because it would break userspace but I'd like to add a
> > > PIN_CONFIG_OUTPUT_VALUE enum as well and use that in the GPIO driver.
> > > But in this patchset I just use PIN_CONFIG_OUTPUT.
> > >
> > > Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> >
> > I tweaked this patch around a bit and applied: removed the second comment
> > in the commit message and wrote the docs to be more generic since
> > in the future other things than SCMI might want to use this
> > config option.
>
> Then I thought about it some more. ...
>
> Isn't it more intuitive that we rewrite the curren PIN_CONFIG_OUTPUT_VALUE
> to just PIN_CONFIG_VALUE that can be used for both reading and
> writing binary low/high instead of having two different things like this?
>
> I will look over current users and maybe propose a patch.
I discovered that several in-tree drivers are already *reading* the
property PIN_CONFIG_OUTPUT_VALUE to get the logic level of
the line.
I sent a patch renaming this property to PIN_CONFIG_LEVEL so
it is clear that this can also be read, and you can drop this patch
and just read/write PIN_CONFIG_LEVEL instead for the GPIO
driver.
Yours,
Linus Walleij
Powered by blists - more mailing lists