Re: [patch 2.6.20-rc1 1/6] GPIO core

Pavel Machek
2006-12-28T08:38:19Z

Hi!

> +Identifying GPIOs
> +-----------------
> +GPIOs are identified by unsigned integers in the range 0..MAX_INT. That
> +reserves "negative" numbers for other purposes like marking signals as
> +"not available on this board", or indicating faults.
> +
> +Platforms define how they use those integers, and usually #define symbols
> +for the GPIO lines so that board-specific setup code directly corresponds
> +to the relevant schematics. In contrast, drivers should only use GPIO

Perhaps these should not be integers, then?

typedef struct { int mydata } pin_t; prevents people from looking
inside, allows you to typecheck, and allows expansion in (unlikely) case where
more than int is needed? ...hotpluggable gpio pins?

> +Spinlock-Safe GPIO access
> +-------------------------
> +Most GPIO controllers can be accessed with memory read/write instructions.
> +That doesn't need to sleep, and can safely be done from inside IRQ handlers.
> +
> +Use these calls to access such GPIOs:
> +
> + /* GPIO INPUT: return zero or nonzero */
> + int gpio_get_value(unsigned gpio);
> +
> + /* GPIO OUTPUT */
> + void gpio_set_value(unsigned gpio, int value);
> +
> +The values are boolean, zero for low, nonzero for high. When reading the
> +value of an output pin, the value returned should be what's seen on the
> +pin ... that won't always match the specified output value, because of
> +issues including wire-OR and output latencies.
> +
> +The get/set calls have no error returns because "invalid GPIO" should have
> +been reported earlier in gpio_set_direction(). However, note that not all
> +platforms can read the value of output pins; those that can't should always
> +return zero.
> Also, these calls will be ignored for GPIOs that can't safely
> +be accessed wihtout sleeping (see below).

'Silently ignored' is ugly. BUG() would be okay there.

> +Platforms that support this type of GPIO distinguish them from other GPIOs
> +by returning nonzero from this call:
> +
> + int gpio_cansleep(unsigned gpio);

This is ugly :-(. But I don't see easy way around...

> +GPIOs mapped to IRQs
> +--------------------
> +GPIO numbers are unsigned integers; so are IRQ numbers. These make up
> +two logically distinct namespaces (GPIO 0 need not use IRQ 0). You can
> +map between them using calls like:
> +
> + /* map GPIO numbers to IRQ numbers */
> + int gpio_to_irq(unsigned gpio);
> +
> + /* map IRQ numbers to GPIO numbers */
> + int irq_to_gpio(unsigned irq);

. Don't we have irq_t already?

> +Those return either the corresponding number in the other namespace, or
> +else a negative errno code if the mapping can't be done. (For example,
> +some GPIOs can't used as IRQs.) It is an unchecked error to use a GPIO
> +number that hasn't been marked as an input using gpio_set_direction(), or

It should be valid to do irqs on outputs, if those outputs are really
tristates (wire-or or how you call it?)?

Pavel

--
Thanks for all the (sleeping) penguins.