Hi Alexandre, Thank you for the patch. On Friday 25 July 2014 00:04:58 Alexandre Courbot wrote: > The huge majority of GPIOs have their direction and initial value set > right after being obtained by one of the gpiod_get() functions. The > integer GPIO API had gpio_request_one() that took a convenience flags > parameter allowing to specify an direction and value applied to the > returned GPIO. This feature greatly simplifies client code and ensures > errors are always handled properly. > > A similar feature has been requested for the gpiod API. Since GPIOs need > a direction to be used anyway, we prefer to extend the existing > functions instead of introducing new functions that would raise the > number of gpiod getters to 16 (!). > > The drawback of this approach is that all gpiod clients need to be > updated, but there aren't that many and the moment and this results in > smaller (and hopefully safer) code. > > Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx> > --- > This change will be difficult to apply without breaking things, but > let's try to do it right. Hopefully the benefit will outweight the > disturbance. > > This is a patch against -next to list and update all current gpiod > consumers. Updates are trivial at first sight, but it would be nice to > get as many acks as possible from the respective subsystem maintainers. > > I'm not sure how this could be applied harmlessly though - maybe through > a dedicated branch for -next? Problem is that a lot of new code is not > yet merged into mainline, and conflicts are very likely to occur. Linus, > do you have any suggestion as to how this can be done without blood being > spilled? > > Documentation/gpio/consumer.txt | 26 ++++++++--- > drivers/gpio/devres.c | 24 ++++++---- > drivers/gpio/gpiolib.c | 53 +++++++++++++------ > drivers/gpu/drm/panel/panel-ld9040.c | 7 +-- > drivers/gpu/drm/panel/panel-s6e8aa0.c | 7 +-- > drivers/gpu/drm/panel/panel-simple.c | 16 ++----- > drivers/hsi/clients/nokia-modem.c | 7 +-- > drivers/i2c/muxes/i2c-mux-pca954x.c | 4 +- > drivers/iio/accel/kxcjk-1013.c | 6 +-- > drivers/input/keyboard/clps711x-keypad.c | 6 +-- > drivers/input/misc/gpio-beeper.c | 6 +-- > drivers/input/misc/soc_button_array.c | 2 +- > drivers/media/i2c/adv7604.c | 6 +-- > drivers/mfd/intel_soc_pmic_core.c | 2 +- > drivers/mmc/core/slot-gpio.c | 6 +-- > drivers/net/phy/at803x.c | 4 +- > drivers/power/reset/gpio-poweroff.c | 21 ++------- > drivers/tty/serial/serial_mctrl_gpio.c | 2 +- > drivers/video/backlight/pwm_bl.c | 6 +-- > drivers/video/fbdev/omap2/displays-new/panel-dpi.c | 12 ++--- > .../omap2/displays-new/panel-lgphilips-lb035q02.c | 6 +-- > .../omap2/displays-new/panel-sharp-ls037v7dw01.c | 7 +-- > include/linux/gpio/consumer.h | 38 ++++++++++++---- > net/rfkill/rfkill-gpio.c | 16 ++----- > sound/soc/codecs/adau1977.c | 11 ++--- > sound/soc/codecs/cs4265.c | 5 +- > sound/soc/codecs/sta350.c | 9 ++-- > sound/soc/codecs/tas2552.c | 4 +- > sound/soc/jz4740/qi_lb60.c | 10 +--- > sound/soc/omap/rx51.c | 29 +++--------- > sound/soc/soc-jack.c | 9 ++-- > 31 files changed, 160 insertions(+), 207 deletions(-) > > diff --git a/Documentation/gpio/consumer.txt > b/Documentation/gpio/consumer.txt index 7ff30d2..a3fb1d7 100644 > --- a/Documentation/gpio/consumer.txt > +++ b/Documentation/gpio/consumer.txt > @@ -29,13 +29,24 @@ gpiod_get() functions. Like many other kernel > subsystems, gpiod_get() takes the device that will use the GPIO and the > function the requested GPIO is supposed to fulfill: > > - struct gpio_desc *gpiod_get(struct device *dev, const char *con_id) > + struct gpio_desc *gpiod_get(struct device *dev, const char *con_id, > + enum gpio_flags flags) I assume you mean enum gpiod_flags here and in the rest of the documentation. > If a function is implemented by using several GPIOs together (e.g. a simple > LED device that displays digits), an additional index argument can be > specified: > > struct gpio_desc *gpiod_get_index(struct device *dev, > - const char *con_id, unsigned int idx) > + const char *con_id, unsigned int idx, > + enum gpio_flags flags) > + > +The flags parameter is used to optionally specify a direction and initial > value +for the GPIO. Values can be: > + > +* AS_IS or 0 to not initialize the GPIO at all. The direction must be set > later > + with one of the dedicated functions. > +* INPUT to initialize the GPIO as input. > +* OUTPUT_LOW to initialize the GPIO as output with a value of 0. > +* OUTPUT_HIGH to initialize the GPIO as output with a value of 1. Pretty please, could you at least prefix that with GPIOD_ ? Those names are too generic. How about renaming them to GPIOD_AS_IS, GPIOD_IN, GPIOD_OUT_INIT_LOW and GPIOD_OUT_INIT_HIGH in order to match the GPIOF_ flags ? > Both functions return either a valid GPIO descriptor, or an error code > checkable with IS_ERR() (they will never return a NULL pointer). -ENOENT > will be returned @@ -49,11 +60,13 @@ GPIO has been assigned to the > requested function: > > Device-managed variants of these functions are also defined: > > - struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id) > + struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id, > + enum gpio_flags flags) > > struct gpio_desc *devm_gpiod_get_index(struct device *dev, > const char *con_id, > - unsigned int idx) > + unsigned int idx, > + enum gpio_flags flags) > > devm_gpiod_get_optional() and devm_gpiod_get_index_optional() exist as > well. > > @@ -72,8 +85,9 @@ Using GPIOs > > Setting Direction > ----------------- > -The first thing a driver must do with a GPIO is setting its direction. This > is -done by invoking one of the gpiod_direction_*() functions: > +The first thing a driver must do with a GPIO is setting its direction. If > no +direction-setting flags as been given to one of the gpiod_get*() > functions, this +is done by invoking one of the gpiod_direction_*() > functions: > > int gpiod_direction_input(struct gpio_desc *desc) > int gpiod_direction_output(struct gpio_desc *desc, int value) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
