mbox series

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

Message ID 20200529213808.2815-1-p.yadav@ti.com
Headers show
Series gpio: Add a managed API | expand

Message

Pratyush Yadav May 29, 2020, 9:38 p.m. UTC
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 at 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

Comments

Simon Glass May 31, 2020, 2:08 p.m. UTC | #1
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 at 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?

Regards,
Simon