[PATCH 02/10] support 88pm8606 in 860x driver

[Jean Delvare]

On Mon, 7 Dec 2009 07:47:44 -0500, Haojian Zhuang wrote:
> >
> > Some more background would be welcome, yes. In particular on how the
> > 8806 and 8807 relate to each other, and what kind of chips they are.
> > And what problem you are trying to solve in the first place ;)
> 
> These two chips can be used together for power management. 8807 is
> focus on regulator, RTC, touch, vibrator. 8806 is focus on backlight,
> led, charger, and so on. When charger is in use, it also need to
> access registers in 8807. In original implementation, one static mixed
> structure is defined to link these two chips together. One i2c driver
> was serviced for both 8806 and 8807 by i2c id table. Now Samuel is
> help me to remove the static mixed structure.

I agree that a static structure is not a good idea and it's better to
avoid it if possible.

> User can use them together or use only one.

By "user" you mean hardware designer? Or end-user?

> > In any case, you never get to duplicate the i2c-pxa driver code
> > anywhere. If you have several I2C bus segments controlled by PXA-like
> > hardware, then you instantiate that many i2c_adapters, all driven by
> > the same driver (i2c-pxa.)
> >
> > Calling i2c_new_device() from an i2c client's probe() routine is rather
> > unusual. I can't really think of a good reason to do this. If the first
> > client was declared as part of platform init data, I'd expect the
> > second to be declared there as well.
> >
> > There's one which is somewhat similar though: chips which reply to more
> > than one I2C address. The extra addresses are reserved using
> > i2c_new_dummy(). This gives you an i2c_client handle for easy register
> > access. This might be suitable for the problem at hand, but I can't
> > tell for sure without knowing the details.
> >
> 
> Does it mean that i2c_new_dummy()/i2c_new_device() is doing the same
> thing like declaring of i2c chip in platform init data?

i2c_new_device() is indeed very similar to declaring the i2c chip in
platform init data. The key difference is that you need an i2c_adapter
handle for i2c_new_device(), while the adapter is referenced by its
number in the platform init data.

i2c_new_dummy() is a little different, it doesn't create a real device,
no probe/remove method will be called. It only reserves a given I2C
address (on the specified adapter) for your own use.

Now that I understand better what problem you are trying to solve, here
are a few hints.

As suggested by Samuel, you can have a single device declared in the
platform init data. Attach custom data to this device, where you
declare if the other chip is present and at which address it lives.
Then in the probe() function of the driver, check the data and call
i2c_new_dummy() on the address in question if present.

There are constraints and requirements to this approach: both chips
must be connected to the same I2C adapter, the same driver must handle
both devices, and your driver must be ready to deal with the main device
being either a 8807 or a 8806, and the secondary chip being present or
not. Shouldn't be overly difficult though.

Calling i2c_new_device() instead of i2c_new_dummy() may lead to cleaner
driver code, however I am far from certain than i2c-core will let you
do it. It might deadlock. I just don't know.

An alternative approach is to declare both devices as part of the
platform init data, and let them be registered separately. Then have a
dedicated registration/removal calls at the end of the probe() method /
beginning of the remove() method. The registration function would track
which devices are up and operational, and when it spots two devices
which would work together, it adds the respective information to each
device so that it can reach its sibling. The removal function would
clean up the links when either sibling is removed.

This also has limitations, as you need a way to decide which devices go
by pair. But assuming that both devices are always on the same I2C
adapter, it shouldn't be too difficult. It is probably cleaner than the
other options, as it is symmetrical, but it might be overkill depending
on the cases you really must handle in practice.

-- 
Jean Delvare