[PATCH v2 0/2] gpio: Add a managed API

[Simon Glass]

Hi Pratyush,

On Mon, 1 Jun 2020 at 05:22, Pratyush Yadav <p.yadav at ti.com> wrote:
>
> On 31/05/20 08:08AM, Simon Glass wrote:
> > Hi Pratyush,
> >
> > On Fri, 29 May 2020 at 15:39, Pratyush Yadav <p.yadav at ti.com> wrote:
> > >
> > > Hi,
> > >
> > > This is a re-submission of Jean-Jacques' earlier work in October last
> > > year. It can be found at [0]. The goal is to facilitate porting drivers
> > > from the linux kernel. Most of the series will be about adding managed
> > > API to existing infrastructure (GPIO, reset, regmap (already
> > > submitted)).
> > >
> > > This particular series is about GPIOs. It adds a managed API using the
> > > API as Linux. To make it 100% compatible with linux, there is a small
> > > deviation from u-boot's way of naming the gpio lists: the managed
> > > equivalent of gpio_request_by_name(..,"blabla-gpios", ...) is
> > > devm_gpiod_get_index(..., "blabla", ...)
> > >
> > > Changes in v2:
> > > - The original series had a patch that checked for NULL pointers in the
> > >   core GPIO functions. The checks were needed because of the addition of
> > >   devm_gpiod_get_index_optional() which would return NULL when when no
> > >   GPIO was assigned to the requested function. This is convenient for
> > >   drivers that need to handle optional GPIOs.
> > >
> > >   Simon argued that those should be behind a Kconfig option because of
> > >   code size concerns. He also argued against implicit return in the
> > >   macro that checked for the optional GPIOs.
> > >
> > >   This submission removes the controversial patch so that base
> > >   functionality can get unblocked.
> > >
> > >   We still need to take a stance on who is responsible for the NULL
> > >   check: the driver or the GPIO core? Do we want to trust drivers to
> > >   take care of the NULL checks, or do we want to distrust them and make
> > >   sure they don't send us anything bogus in the GPIO core. For now the
> > >   responsibility lies on the drivers by default. I will send a separate
> > >   RFC of the NULL check patch and we can probably discuss the issue
> > >   there.
> > >
> > > [0] https://patchwork.ozlabs.org/project/uboot/cover/20191001115130.18886-1-jjhiblot@ti.com/
> > >
> > > Jean-Jacques Hiblot (2):
> > >   drivers: gpio: Add a managed API to get a GPIO from the device-tree
> > >   test: gpio: Add tests for the managed API
> > >
> > >  arch/sandbox/dts/test.dts  |  10 ++++
> > >  drivers/gpio/gpio-uclass.c |  70 +++++++++++++++++++++++++
> > >  include/asm-generic/gpio.h |  47 +++++++++++++++++
> > >  test/dm/gpio.c             | 102 +++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 229 insertions(+)
> > >
> > > --
> > > 2.26.2
> > >
> >
> > The first question I have is why do you want to allocate the gpio_desc
> > and return it? Doesn't the caller have a place for that in its private
> > struct?
>
> Ask the Linux folks that ;-)
>
> The main aim of this series is to make it easier to port and maintain
> drivers from Linux. The less changes we have to make when porting a
> driver, the easier it is to port future fixes and features.
>
> Linux drivers (like the TI J721E WIZ [0] for which this effort is mainly
> being made) use these APIs. FWIW, the docs in Linux say the optional
> wrappers to the functions are added as a convenience for drivers that
> need to handle optional GPIOs.

U-Boot already supports optional GPIOs.

Can we put this behind a CONFIG_LINUX_COMPAT_GPIO flag perhaps, so
people know they are trading off code / memory size for compatibility?

>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/ti/phy-j721e-wiz.c
>
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India

Regards,
Simon