Re: [PATCH 0/4] add support for bias pull-disable

From: "Nuno Sá" <noname.nuno@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"ACPI Devel Maling List" <linux-acpi@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Bartosz Golaszewski" <brgl@bgdev.pl>,
	"Frank Rowand" <frowand.list@gmail.com>,
	"Mika Westerberg" <mika.westerberg@linux.intel.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Linus Walleij" <linus.walleij@linaro.org>
Subject: Re: [PATCH 0/4] add support for bias pull-disable
Date: Thu, 14 Jul 2022 11:49:34 +0200	[thread overview]
Message-ID: <fb3112dab2e614cba3feaf0b6a962186f07cb61b.camel@gmail.com> (raw)
In-Reply-To: <CAHp75VcEBE3kMDi5Q+89GmS9V=aF+pOcyROY9MdfyB_5OaruPg@mail.gmail.com>

On Thu, 2022-07-14 at 11:12 +0200, Andy Shevchenko wrote:
> On Thu, Jul 14, 2022 at 9:10 AM Nuno Sá <noname.nuno@gmail.com>
> wrote:
> > On Wed, 2022-07-13 at 20:39 +0300, Andy Shevchenko wrote:
> > > On Wed, Jul 13, 2022 at 03:14:17PM +0200, Nuno Sá wrote:
> > > > The gpio core looks at 'FLAG_BIAS_DISABLE' in preparation of
> > > > calling the
> > > > gpiochip 'set_config()' hook. However, AFAICT, there's no way
> > > > that
> > > > this
> > > > flag is set because there's no support for it in firwmare code.
> > > > Moreover,
> > > > in 'gpiod_configure_flags()', only pull-ups and pull-downs are
> > > > being
> > > > handled.
> > > 
> > > Isn't it enough?
> > 
> > I might be missing something but don't think so. Look at this
> > driver
> > which seems a lot like the reference i put in the cover:
> > 
> > https://elixir.bootlin.com/linux/v5.19-rc6/source/drivers/gpio/gpio-pca953x.c#L573
> > 
> > I just don't see an in-kernel path (I'm aware now that we can get
> > here
> > through gpio cdev) to get to the point where we want to disable the
> > pin
> > BIAS.
> 
> Ah, that driver should be converted to pin control. It's definitely a
> problem with the driver.
> 

I'm not too familiar with pinctrl or even gpiochips to argue much in
here so just looking to better understand it...

The driver has it's own way to control the pin BIAS and does not rely
on any other pinctrl chip on the system. Are you pointing that this
driver should be converted in a pinctrl one which registers the
gpiochip and drops the 'set_config()' callback so pin consumers could
use pinctrl to let's say, disable BIAS? If so, then why do we have
PIN_CONFIG_BIAS_PULL_UP|DOWN in 'set_config()'? Legacy? 

And for my test device which is an input keymap, having it converted
into a pinctrl driver does not make much sense  (and it also supports
pull-up pull-disable) and I guess that might make sense for other
devices that have some optional gpiochip support...

- Nuno Sá