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@nvidia.com
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)