[U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration

[Patrick DELAUNAY]

Hi Simon,

> From: Simon Glass <sjg at chromium.org>
> Sent: mercredi 30 octobre 2019 02:49
> 
> Hi Patrick,
> 
> On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay at st.com>
> wrote:
> >
> > This commit manages the flags that can be used in GPIO specifiers to
> > indicate if a pull-up resistor or pull-down resistor should be enabled
> > for output GPIO and the Open Drain/Open Source configuration for input
> > GPIO.
> >
> > It is managed in driver with a new ops in gpio uclass set_config.
> >
> > These flags are already support in Linux kernel in gpiolib.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> > ---
> >
> >  drivers/gpio/gpio-uclass.c | 62
> > +++++++++++++++++++++++++++++++++++++-
> >  include/asm-generic/gpio.h | 56 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 117 insertions(+), 1 deletion(-)
> >
> 
> To me this seems like a pretty annoying interface. The uclass has to call the driver
> multiple times with each enum and the driver may end up reprogramming the pins
> multiple times to get it right.
> 
> Normally we want to program things correctly once, before enabling the function.
> 
> On the other handle I think what you have is better than adding new methods like
> set_open_drain().
> 
> But overall I think it would be better to define a new struct like gpio_config that
> holds some flags and perhaps a few other things. Then the uclass can set up that
> struct and call the driver once with it, to set up the pin. It could include input/output
> too, so that if
> set_config() is defined, the uclass uses that instead of direction_output(), etc.
> 
> What do you think?

I understand the issue.... 
You think something like the serial ops setconfig/getconfig.

So the API can evaluate without add new ops for each new parameter... 
It is clearly better.

I will think about and try to propose something without break nothing.

> Also we should update the sandbox driver to include tests for new methods. It
> looks like you have done pinctrl but not this?

I think test are done in 
[PATCH 12/13] test: dm: update test for pins configuration in gpio
- dm_test_gpio_pin_config

Do think about other tests ?

> Regards,
> Simon

Regards

Patrick