Hi Pratyush, On Mon, 1 Jun 2020 at 05:22, Pratyush Yadav <p.ya...@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.ya...@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-jjhib...@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