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

[Haojian Zhuang]

On Mon, Dec 7, 2009 at 8:59 AM, Samuel Ortiz <sameo at linux.intel.com> wrote:
> Hi Jean,
>
> On Mon, Dec 07, 2009 at 02:38:32PM +0100, Jean Delvare wrote:
>> 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?
> I think he means hardware designer.
>
>
>> > > 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.
> Yes, the driver fits all those requirements, so it shouldnt be too difficult.
>
>> 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.
> I thought about that solution as well, but couldnt see how we could cleanly
> tell how 2 devices relate to each other. Now, as you're saying below, we could
> assume that both devices are on the same i2c adapter, that sounds like a
> fair requirement.
> Personally, I'd prefer to use the i2c_new_dummy() method, but it's Haojian's
> call to decide which one he prefers.
>
>
>> 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, thanks a lot spending your time on this long and detailed explanation, I
> appreciate.
>

Hi Jean & Samuel,

Thanks for your great help. Now I update these patches with
i2c_new_device(). I'll paste it in a new thread.

Best Regards
Haojian