On Sun, Feb 03, 2008 at 01:17:43PM -0800, David Brownell wrote:
> On Sunday 03 February 2008, Anton Vorontsov wrote:
> > This patch implements support for the GPIO LIB API. Two calls
> > unimplemented though: irq_to_gpio and gpio_to_irq.
>
> Also gpio_cansleep().
>
>
> > +Example of two SOC GPIO banks defined as gpio-controller nodes:
> > +
> > + qe_pio_a: [EMAIL PROTECTED] {
> > + #gpio-cells = <2>;
> > + compatible = "fsl,qe-pario-bank";
> > + reg = <0x1400 0x18>;
> > + gpio-controller;
> > + };
> > +
> > + qe_pio_e: [EMAIL PROTECTED] {
> > + #gpio-cells = <2>;
> > + compatible = "fsl,qe-pario-bank";
> > + reg = <0x1460 0x18>;
> > + gpio-controller;
> > + };
>
> Let me suggest another example to provide: an I2C GPIO expander
> such as a pcf8574 or pca9354 (both eight bit expanders). The SOC
> case is probably the easiest to cover.
I2C expander will be easy to support via device tree too, something
like:
[EMAIL PROTECTED] {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <0>;
compatible = "fsl-i2c";
reg = <0x3000 0x100>;
interrupts = <14 8>;
interrupt-parent = <&ipic>;
dfsrr;
pcf_pio: [EMAIL PROTECTED] {
#gpio-cells = <2>;
compatible = "ti,pcf8574";
reg = <0x27>;
gpio-controller;
};
};
Of course, to support this, pcf857x driver needs OF bindings.
> > +#define ARCH_OF_GPIOS_PER_CHIP 32
>
> Well, ARCH_OF_* is clearly not the now-removed ARCH_GPIOS_PER_CHIP,
> but I still suggest moving away from that concept.
Yes, I noticed that ARCH_GPIOS_PER_CHIP is removed from the
gpiolib. But I solely use it to assign gpio_bases to the chips:
static int of_get_gpiochip_base(struct device_node *np)
{
struct device_node *gc = NULL;
int gpiochip_base = 0;
while ((gc = of_find_all_nodes(gc))) {
if (!of_get_property(gc, "gpio-controller", NULL))
continue;
if (gc != np) {
gpiochip_base += ARCH_OF_GPIOS_PER_CHIP;
continue;
}
of_node_put(gc);
if (gpiochip_base >= ARCH_OF_GPIOS_END)
return -ENOSPC;
return gpiochip_base;
}
return -ENOENT;
}
This function walks the device tree and assigns gpio_base 0 to
the first encountered gpio-controller, ARCH_OF_GPIOS_PER_CHIP * N
to the Nth controller...
Technically, I can replace
if (gc != np) {
gpiochip_base += ARCH_OF_GPIOS_PER_CHIP;
continue;
}
With
if (gc != np) {
struct gpio_chip *chip = gc->data;
if (chip)
gpiochip_base += chip->ngpio;
continue;
}
Here we're getting next available GPIO base. But then this code can't
handle gpio chip removal. So, we need smart "dynamic GPIO base
allocator" as described asm-generic/gpio.h, gpio_find_base(ngpios)
that will find free gpio base suitable for ngpios to place. Until
that allocator implemented, we use simple allocator with
OF_GPIOS_PER_CHIP...
> > +static inline int gpio_get_value(unsigned int gpio)
> > +{
> > + return __gpio_get_value(gpio);
> > +}
> > +
> > +static inline void gpio_set_value(unsigned int gpio, int value)
> > +{
> > + __gpio_set_value(gpio, value);
> > +}
>
> static inline int gpio_cansleep(unsigned int gpio)
> {
> return __gpio_cansleep(gpio);
> }
Will fix.
> > +static inline int irq_to_gpio(unsigned int irq)
> > +{
> > + return -ENOSYS;
>
> Minor nit: "-EINVAL" would be better here, since the argument
> is constrained to have been returned from gpio_to_irq(), and
> as you wrote that call there can be no such values.
Will change.
Thanks!
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
