gpiodev - API definitions
RFC PATCH 1/3 gpiodev - API definitions
Hello linux-arm-kernel,
GPIODEV API: Core API definitions. Provided are:
1. struct gpiodev_ops which must be included into platform_data structure of a device which will provide GPIODEV API; driver for a device must initialize this structure.
2. Structural definition of generalized GPIO identifier (struct gpio).
2. Set of API calls for clients. This fully follow Generic GPIO API naming and semantics, except that they have "gpiodev" prefix and accept struct gpio instead of integer gpio identifiers.
Index: include/linux/gpiodev.h
===================================================================
RCS file: include/linux/gpiodev.h
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 renamed from dev/null 1 Jan 1970 00:00:00 -0000 +43 -0
@@ -0,0 +1,43 @@ +#ifndef __GPIODEV_H +#define __GPIODEV_H + +#include <linux/device.h> +#include <linux/platform_device.h> +#include <asm/gpio.h> + +/* Interface */ + +/* This structure must be first member of device platform_data structure + of a device which provides gpiodev interface */ +struct gpiodev_ops { + int (*get)(struct device *this, int gpio_no); + void (*set)(struct device *this, int gpio_no, int val); + int (*to_irq)(struct device *this, int gpio_no); +}; + +/* Generalized GPIO structure */ + +struct gpio { + struct platform_device *gpio_dev; + int gpio_no; +}; + +/* API functions */ + +static inline int gpiodev_get_value(struct gpio *gpio) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data; + return ops->get(&gpio->gpio_dev->dev, gpio->gpio_no); +} +static inline void gpiodev_set_value(struct gpio *gpio, int val) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data; + ops->set(&gpio->gpio_dev->dev, gpio->gpio_no, val); +} +static inline int gpiodev_to_irq(struct gpio *gpio) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data; + return ops->to_irq(&gpio->gpio_dev->dev, gpio->gpio_no); +} + +#endif /* __GPIODEV_H */
Signature · 2 lines
Best regards, Paul mailto:[email protected]
it looks ok, but I have several questions:
1. why should we bind this to platform_device, what if the gpio device is not actually a "platform_device", say, a I2C device, a SPI device or even a USB device?
2. I still doubt the benefit of using of a structure for a gpio, isn't a gpio number not enough?
3. If one is going to use a GPIO, he has to initialize a "struct gpio" before that, how is he suppose to know the value for "gpio->gpio_dev"?
4. how can we optimize to a direct register access instruction (e.g. to GPDR in PXA) for bit-banging operation (pardon me, I don't exactly remember the name for such operation, maybe bit-banging)
Hello Eric,
Quoted 5 lines · you
it looks ok, but I have several questions:
1. why should we bind this to
platform_device, what if the gpio device is not actually a "platform_device", say, a I2C device, a SPI device or even a USB device?
Good point. That was handhelds.org-specific "optimization", as we so
far mostly dealing with GPIO devices on platform bus, and could save us writing "&pdev.dev". This must be changed to use struct device, sure.
Quoted 2 lines
2. I still doubt the benefit of using of a structure for a gpio, isn't a gpio number not enough?
Well, I replied to David's earlier post to LAKML after posting this,
and discussed both approach in more detail (and even more color). To sum up, yes, we can use either structures for GPIOs, or scalar ints. But both these approaches leads to some tradeoffs. Our argument is that "scalar" (aka flat) approach leads to more tradeoffs - you need to "flatten" your GPIO space first, and this is least evil, as indeed this could be done using compile-time defines. But then you need to "unflatten" it, and this happens at runtime, wasting cycles.
Using structure GPIO ids skips this "flattening"/"unflattening"
(maybe multiplexing/demultiplexing are better terms) phases. Trade off is necessity to write:
.gpio = {&gpio_device, GPIO_DEVICE_GPIO_1}
instead of:
#define BASE_FOR_GPIO_DEVICE_ON_MY_BOARD 1000
.gpio = BASE_FOR_GPIO_DEVICE_ON_MY_BOARD + GPIO_DEVICE_GPIO_1
IMHO, pretty acceptable syntactical tradeoff ;-)
Quoted 2 lines
3. If one is going to use a GPIO, he has to initialize a "struct gpio" before that, how is he suppose to know the value for "
gpio->gpio_dev"?
Yes, so the target usage of GPIODEV API is to write
device-independent driver. Such driver simply shouldn't care what device is used for GPIO - it should be able to accept any, - and how exactly GPIO ops are performed.
But something should care what actual GPIO device is being used. And
we know what's that - something which people call "platform code", but I better call old good "machine definition file", to remove any ambiguities. Such a definition is written for a specific machine, so obviously you know what GPIO devices it has, and for what purpose pins of them I used for. So, you have something like:
struct platform_device my_gpio_device = { .dev = {.name = "asic_foo"}};
struct some_driver_platform_data some_driver_pdata = {
...
.pullup = {&my_gpio_device.dev, ASIC_FOO_GPIO_PULLUP},
...
}
Important thing to note here is that structured GPIO id's are still
constants! Link-time constant, not compile-time, as scalar id's would be, but still there's zero overhead at runtime.
Quoted 3 lines
4. how can we optimize to a direct register access instruction (e.g. to GPDR in PXA) for bit-banging operation (pardon me, I don't exactly remember the name for such operation, maybe bit-banging)
Well, you should first decide what exactly you want. If you want to
do bit-banging on PXA, then well, - you don't need GPIODEV and its polymorphic support at all! If you know GPIO# beforehand, just use David's GPIO API - it will constant-optimize it to GPCR/GPSR access rigth away.
If you need to support arbitrary GPIO#, you can get better
frequency/throughput by skipping GPIO API still, like in the following pseudocode:
MASK = 1 << GPIO#;
for (;count < SIZE; count--) {
GPSR = MASK;
nop;
GPCR = MASK;
} If you'd use GPIO API, it would recompute MASK on each call, wasting
your cycles.
But now it's completely different story if you have a need to
write a driver which will bang bits on any GPIO of any device, including such devices which you can't imagine at the time of driver's writing at all - that's the GPIODEV estate. Obviously, for such general usage it doesn't make too much sense to speak about optimization for a one tiny specific device out of infinity of its domain - it's purpose is exactly generality, not optimality, and it's well known that these notions are reciprocals.
If you still want super-optimal handling for PXA case, while being
able to still support anything else, solution is also known - have two drivers, one fast and PXA-adhoc, another generic but slower, and use extraneous logic to select which one to use.
Back to GPIODEV's traits, it's still offers best performance which
can be achieved for infinitely extendable solution (within bounds of the interface defined, of course), because it uses best known solution for such problem (and such solution is simple and fast) - indirect function call, and all data needed for such call are available right away due to the magic of structural GPIO id's.
In this regard, your implementation draft, which uses scalars and
looping to find out owning GPIO device/handler, pretty bad suited for bit-banging at all: just consider that GPIO operation overhead in this case is dependent on number of GPIO devices present. Add new one, and your (supposedly) carefully tuned latencies and delay "float".
David's code, which is essentially a simple hashing scheme, at least
has O(1) complexity in regard to number of GPIO devices present, but still wastes precious cycles for useless things, like making a lookup in hash table and subtracting GPIO base.
Signature · 2 lines
Best regards, Paul mailto:[email protected]
Am Dienstag, 10. April 2007 23:30 schrieb Paul Sokolovsky:
Quoted 38 lines
Hello linux-arm-kernel,
GPIODEV API: Core API definitions. Provided are:
1. structgpiodev_opswhich must be included intoplatform_datastructure of a device which will provide GPIODEV API; driver for a device must initialize this structure.
2. Structural definition of generalized GPIO identifier (struct gpio).
2. Set of API calls for clients. This fully follow Generic GPIO API naming and semantics, except that they have "gpiodev" prefix and accept struct gpio instead of integer gpio identifiers.Index:
include/linux/gpiodev.h
===================================================================
RCS file:include/linux/gpiodev.h
diff -Ninclude/linux/gpiodev.hlinux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 renamed from dev/null 1 Jan 1970 00:00:00 -0000 +19
-0@@ -0,0 +1,43 @@ +#ifndef __GPIODEV_H +#define __GPIODEV_H + +#include <linux/device.h> +#include <linux/platform_device.h> +#include <asm/gpio.h> + +/* Interface */ + +/* This structure must be first member of device platform_data structure + of a device which provides gpiodev interface */ +struct gpiodev_ops { + int (*get)(struct device *this, int gpio_no); + void (*set)(struct device *this, int gpio_no, int val); + int (*to_irq)(struct device *this, int gpio_no); +}; + +/* Generalized GPIO structure */ +
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 line 29
Quoted 10 lines
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 renamed from dev/null 1 Jan 1970 00:00:00 -0000 +10
-0+struct gpio { + struct platform_device *gpio_dev; + int gpio_no; +}; + +/* API functions */ + +static inline int gpiodev_get_value(struct gpio *gpio) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data;
wouldn't it be more sure to verify if xxx function is NOT null before using it ?? Perhaps something like that
BUG_ON(!ops->get);
Quoted 14 lines
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 renamed from dev/null 1 Jan 1970 00:00:00 -0000 +14
-0+ return ops->get(&gpio->gpio_dev->dev, gpio->gpio_no); +} +static inline void gpiodev_set_value(struct gpio *gpio, int val) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data; + ops->set(&gpio->gpio_dev->dev, gpio->gpio_no, val); +} +static inline int gpiodev_to_irq(struct gpio *gpio) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data; + return ops->to_irq(&gpio->gpio_dev->dev, gpio->gpio_no); +} + +#endif /* __GPIODEV_H */
Signature · 27 lines
-------------------------------------------------------------- Jürgen Schindele Software-Entwicklung NENTEC Netzwerktechnologie GmbH Greschbachstr. 12 76229 Karlsruhe Deutschland Telefon: +49/721/94249-51 Telefax: +49/721/94249-10 E-Mail: [email protected] WEB: www.nentec.de Geschäftsführung: Klaus Becker, Roland Knapp Sitz der Gesellschaft: Karlsruhe Handelsregister: Amtsgericht Mannheim HRB 107658 Diese E-Mail enthält vertrauliche oder rechtlich geschützte Informationen. Wenn Sie nicht der vorgesehene Empfänger sind, informieren Sie bitte sofort den Absender und löschen Sie diese E-Mail. Das unbefugte Kopieren dieser E-Mail oder die unbefugte Weitergabe der enthaltenen Informationen ist nicht gestattet. The information contained in this message is confidential or protected by law. If you are not the intended recipient, please contact the sender and delete this message. Any unauthorised copying of this message or unauthorised distribution of the information contained herein is prohibited. --------------------------------------------------------------
Quoted 10 lines · Juergen Schindele
Am Dienstag, 10. April 2007 23:30 schrieb Paul Sokolovsky:
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 line 29
Quoted 3 lines
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 renamed from dev/null 1 Jan 1970 00:00:00 -0000 +3
-0+static inline int gpiodev_get_value(struct gpio *gpio) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data;wouldn't it be more sure to verify if xxx function is NOT null before using it ?? Perhaps something like that
BUG_ON(!ops->get);Quoted 1 line
+ return
ops->get(&gpio->gpio_dev->dev, gpio->gpio_no);
What does the BUG_ON buy us that oopsing due to a NULL pointer deref doesn't? Both cases you end up with a register dump and backtrace. In fact, on ARM the NULL pointer deref provides a more accurate register dump and backtrace than BUG_ON() can ever do.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Hello Juergen,
Quoted 13 lines · you
Am Dienstag, 10. April 2007 23:30 schrieb Paul Sokolovsky:
Quoted 12 lines
Hello linux-arm-kernel,
GPIODEV API: Core API definitions. Provided are:
1. structgpiodev_opswhich must be included intoplatform_datastructure of a device which will provide GPIODEV API; driver for a device must initialize this structure.
2. Structural definition of generalized GPIO identifier (struct gpio).
2. Set of API calls for clients. This fully follow Generic GPIO API naming and semantics, except that they have "gpiodev" prefix and accept struct gpio instead of integer gpio identifiers.
[]
Quoted 9 lines
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 line 29
Quoted 5 lines
linux/gpiodev.h 10 Apr 2007 19:17:12 -0000 1.5 renamed from dev/null 1 Jan 1970 00:00:00 -0000 +5
-0+/* API functions */ + +static inline int gpiodev_get_value(struct gpio *gpio) +{ + struct gpiodev_ops *ops = gpio->gpio_dev->dev.platform_data;wouldn't it be more sure to verify if xxx function is NOT null before using it ?? Perhaps something like that
BUG_ON(!ops->get);
GPIODEV is considered to be low-level one and critical for speed, so
all method pointers assumed to be set properly. In particular, if some operation is not available for a device (say, GPI/GPO case), a method must be set to a stab function.
BUG_ON would be acceptable, as it can be compiled out based on
CONFIG setting, but as was pointed out, doesn't add much into picture anyway.
But I'd be happy to add comment to struct gpiodev_ops declaration
about the described method constraints, thanks for comment.
[]
Signature · 2 lines
Best regards, Paul mailto:[email protected]