Re: [PATCH 3/3] of/gpio: Introduce of_put_gpio(), add ref counting for OF GPIO chips

[Anton Vorontsov <avorontsov@ru.mvista.com>]

On Mon, Feb 15, 2010 at 12:49:56PM -0700, Grant Likely wrote:
[...]
> Okay, I'm convinced now.  The model is wrong.  struct of_gc does need
> to be a member of struct gpio_chip and conditionally compiled against
> CONFIG_OF_GPIO.  This locking requirement is just too plain ugly,

And how would you implement of_get_gpio(np, index) then?
You don't have 'struct gpio_chip' in of_get_gpio(), so you can't
get of_gc out of it.

It's possible to lookup all GPIOs (gpio[0..ARCH_NR_GPIOS]) matching
on chip->dev->archdata.node == np, but you're just moving this stuff
into gpiolib, where you'll have to grab global gpio_lock instead of
devtree lock. And just as with np->data_lock or global devtree lock,
you'll have to grab gpio_lock in of_gpio_put(), of_gpio_chip_register
and of_gpio_chip_unregister().

See? Your suggestion doesn't make things better or simpler, instead
it turns the OF GPIO inside out, exposing arch details to the generic
code.

There is really no excuse for the arch-specific (OF) stuff being in
the generic code. Not in the irq subsystem, not in the gpio
subsystem. My implementation actually proves that.

> and
> dereferencing the np->data pointer in this way is dangerous
> (what if
> something that is not struct of_gc is stored there).

Not possible with the safe np->data accessors.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2