Re: [PATCH 1/6] gpio: make the gpiochip a real device

[Johan Hovold <johan@kernel.org>]

On Thu, Dec 03, 2015 at 03:04:14PM +0100, Linus Walleij wrote:
> On Mon, Nov 16, 2015 at 3:27 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> > If I don't use device_add()/device_del(), I see this must mean I have to use
> > device_register()/device_unregister() as the latter only decrease the refcount
> > and wait for the instance to die off.
> >
> > So am I reading it correctly if I understand that this is what we should be
> > doing? I.e device_register()/device_unregister() and then wait for the
> > .remove() call to do its deed, so the kobj/struct device is kept around if
> > e.g. sysfs or the chardev has open files.
> 
> Hard to get advice on design here I see. Anyways, the clean thing to
> do (after some weeks of thinking) seems to be this:
> 
> 1. Introduce a void *data into gpio_chip and gpiochip_set_data()
>    gpiochip_get_data() set/get it.
> 
> 2. Replace all occurences of container_of() dereferencing in all
>    GPIO drivers with gpiochip_[set|get]_data() so as to avoid having
>    gpio_chip to be embedded into state containers.
> 
> 3-1. Split gpio_chip in a static descriptor with a vtable for callbacks
>    and other static config called struct gpio_chip, and
>    struct gpio_device that is returned as a pointer from
>    gpiochip_add(). It will need to be free:ed by gpiodevice_remove()
>    after that.
> 
> 3-2. In the same patch set rename all functions to indicate that
>    we are now operating on a gpio_device not a gpio_chip.
> 
> 3-3. After a GPIO driver issues gpio_device_unregister()
>    it may stay around if there are still references to the
>    struct device.
> 
> This is a very tiresome refactoring, but I'm growing confident that
> it is what needs to happen to manage gpio devices in the future.
> 
> After step 3-3 we have a clean gpio_device containing a
> struct device, and from there we apply the initial chardev
> interface.

This sounds unnecessarily complicated.

Why not use a more standard approach of

	gpiochip_create()

that allocates the state container that the driver can then initialise
further (e.g. callback pointers) and finally register with a call to:

	gpiochip_add()

When the parent device is going away, it calls

	gpiochip_remove()

which unregisters the gpiochip device (i.e. calls device_del), updates
its state to "disconnected" and drops the gpiolib's reference.

There may still be other devices (consumers) holding references to the
gpiochip device, which is not deallocated until the final reference is
dropped (i.e. at final gpiod_free).

Johan