Re: [PATCH v3] USB: PHY: Palmas USB Transceiver Driver

[Graeme Gregory <gg@slimlogic.co.uk>]

On 26/03/13 20:23, Stephen Warren wrote:
> On 03/26/2013 10:57 AM, Graeme Gregory wrote:
>> On 26/03/13 16:22, Stephen Warren wrote:
>>> On 03/26/2013 03:27 AM, Graeme Gregory wrote:
>>> ...
>>>> If we are tightly coupling as above then using platform_irq is an extra
>>>> inefficiency. You both have to populate this then parse it afterwards.
>>>> Why not just use the regmap helper? Ill admit this code is like this as
>>>> there was a period where platform irqs in DT just was not working right!
>>>>
>>>> We should really agree now if we are going for loose or tight coupling
>>>> now rather than keep switching?
>>> Yes, this is something that I think needs to be fully resolved before
>>> any more Palmas patches are discussed.
>>>
>>> So you can have the MFD components fully coupled or completely
>>> standalone. We should fully pick one or the other, not some mish-mash of
>>> the two.
>>>
>>> In practical terms, this means that:
>>>
>>> a) Tightly coupled
>>>
>>> The top-level MFD device knows exactly which child devices are present.
>>> It has an internal table to defined the set of child devices, and
>>> instantiate them. It provides them with IRQs, I2C addresses and register
>>> base addresses (or regmaps), etc. etc., using purely Palmas-internal
>>> APIs. If using device tree, the DT won't include any representation of
>>> which child devices are present, nor their I2C addresses, nor their
>>> register addresses, nor their IRQs, etc. That's all inside the driver.
>>>
>>> b) Completely decoupled.
>>>
>>> The top-level MFD device knows nothing about its children. It simply
>>> provides a bus into which they can be instantiated, and a generic IRQ
>>> controller into which they can hook.
>>>
>>> Platform data or device tree is solely used to define which children
>>> exist, which of the top-level MFD's I2C addresses is used for each
>>> child, the base register address for each child device, the IRQs used by
>>> each child device, etc.
>>>
>>>
>>> Which of those two models are different people expecting?
>>>
>>> (b) appears to be the most flexible, and since you have defined the DT
>>> bindings to contain a separate node for each MFD child device, each with
>>> its own compatible value, seems to be what you're aiming for. In this
>>> scenario, there should be no private APIs between the top-level MFD
>>> device and the children though; everything should be using standard bus
>>> APIs.
>> I was aiming towards (b) which would allow drivers for IP blocks that I
>> know are shared between multiple devices such as RTC which is shared by
>> twl6030, twl6032, tps80032, tps65910, tps65911, tps65912, tps80035,
>> tps80036 and probably many more.
>>
>> Finishing (b) implementation is currently beyond the time I have
>> available I think.
>>
>> If we choose to ignore path (b) and ignore the code duplication of half
>> a dozen RTC drivers for the same IP than the path to (a) is much quicker
>> and easier. Can just rip out a lot of the DT stuff, use mfd_add_devices.
>> Makes the binding much simpler. Means we don't have to use platform
>> resources for IRQs. Makes palmas and palmas-charger just 2 black boxes
>> which is in line with other MFDs.
>>
>> So I think I have come around to just do it the easy way and select (a)
>>
>> I shall work on the main palmas series to implement (a).
>>
>> This will obviously invalidate some comments on this patch and the main
>> series.
> Well, my question was more directed at which way we want to model the HW
> in the device tree, rather than how we want to implement the driver. The
> driver implementation style might end up being derived from the DT
> structure, but it shouldn't be the other way around.
>
> I think if people think (b) is the right way to go, we should just do
> it, and ignore any time issues; if it takes a while to get it right
> upstream, what is the issue with that?
I'm going to take a timeout now, I have to travel on an emergency
tonight and not sure when I will be back.

Graeme