On Thu, Sep 05, 2013 at 12:44:34PM +0900, Alexandre Courbot wrote:

On Thu, Sep 5, 2013 at 4:56 AM, Stephen Warren [off-list ref] wrote:

quoted

On 09/04/2013 05:29 AM, Alexandre Courbot wrote:

quoted

Add gpiod_get() and gpiod_put() functions that provide safer handling of
GPIOs.

These functions put the GPIO framework in line with the conventions of
other frameworks in the kernel, and help ensure every GPIO is declared
properly and valid while it is used.

quoted

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h

quoted

+struct gpio_desc *__must_check gpiod_get(struct device *dev,
+                                      const char *con_id);
+void gpiod_put(struct gpio_desc *desc);

It might be nice to add an "int index" parameter to this function. For
example, a bit-banged parallel bus protocol driver might have 1
chip-select GPIO, 1 clock GPIO, and 8 data GPIOs. gpiod_get(dev, "bus",
0)..gpiod_get(dev, "bus", 7) might be nicer than gpiod_get(dev,
"bus0")..gpiod_get(dev, "bus7")? Possibly for client-simplicity,
implement both gpiod_get(dev, con_id) (as an inline wrapper for ...) and
gpiod_get_index(dev, con_id, index)?

In DT terms, this would map to:

cs-gpios = <&gpio 3 0>;
clock-gpios = <&gpio 5 0>;
bus-gpios = <&gpio 10 0 ... &gpio 17 0>;

... and with the mapping table registration mechanism, we could
presumably add "int index" to struct gpiod_lookup.

Having the index argument of of_get_named_gpiod_flags() propagated
into gpiod_get() is something I also thought about (see the cover
letter), but I really can't make up my mind about it.

On the one hand, having several GPIO per DT property is already
allowed, and presumably used in bindings already. It might also make
sense to have several lines under the same name, e.g. for bitbanging.

On the other hand, I'm afraid people will abuse this, and group all
the GPIOs for a device under a single property instead of using proper
names.

I wouldn't worry about this too much. Preventing abuse is a large part
of why we have DT bindings reviews and I think that regardless of what
the implementation allows us to, that shouldn't influence the way how
bindings are defined.

After all the bindings would ideally be usable on any other OS as well,
so it should be sound irrespective of the specific implementation.

I think having a gpiod_get_index(dev, con_id, index) with an inline
wrapper gpiod_get(dev, con_id) will give us the same functionality that
we have using the current set of helpers from include/linux/of_gpio.h
and allows non-DT to specify them in an analogous way.

In my opinion that's enough for now. We can always refactor common
patterns (such as busses with 0..n GPIOs) when they start to emerge.

Thierry