Re: [PATCH 2/3] usb: ehci-ppc-of dts bindings.

[Valentine Barshak <vbarshak@ru.mvista.com>]

Segher Boessenkool wrote:
>>>> +  Required properties:
>>>> +  - device_type : should be "usb".
>>> No device_type please.  The published USB binding doesn't define
>>> one on purpose.
>>
>> Could you please, explain why?
>> Sorry, I don't think I get the concept of device description here.
> 
> "device_type" is meant to be used only by OF for determining the OF
> programming model for a device.  No such thing has been defined for
> USB busses, so the USB binding does not define a "device_type" either.
> 
> Nothing in a flat device tree should ever define a device_type, except
> perhaps for compatibility with legacy kernel code.
> 
>>>> +  - compatible : should be "ehci".
>>> Just "ehci" isn't enough -- compare to OHCI, which is the name for
>>> a kind of USB host controller as well as for a kind of Firewire
>>> host controller.
>>
>> Actually, I though device type="usb" + compatible="ehci" would be enough.
> 
> "compatible" values are their own namespace, you should in principle
> be able to find a driver for a device with them without having to look
> at other properties.
> 
>>> Maybe "usb-ehci" is best -- can anyone think of a better name?
>>
>> Again, why not type="usb", compatible="ehci"?
> 
> Because an OF client (i.e., the Linux kernel) is not supposed to use
> "device_type" at all for its own driver matching.
> 
>>>> +  - interrupts : <a b> where a is the interrupt number and b is a
>>>> +    field that represents an encoding of the sense and level
>>>> +    information for the interrupt.
>>> This is incorrect; not all interrupt domains use two cells, and
>>> not all that do have this meaning for those cells.
>>
>> Well, this was just copied from other descriptions in 
>> Documentation/powerpc/booting-without-of.txt
>> Do we need to fix them all?
> 
> Sounds like it, yes.
> 
>>>> +  If device registers are implemented in big endian mode, the device
>>>> +  node should have "big-endian" property.
>>>> +  If controller implementation operates with big endian descriptors,
>>>> +  compatible should also have "ehci-be-desc"
>>> Ah, I understand what this is about, finally.
>>> Don't put this in "compatible"; instead, do a "big-endian-descriptors"
>>> property similar to the "big-endian" property.  That last one should
>>> maybe be "big-endian-registers" here then, to avoid confusion.
>>> Or make "big-endian" mean both big-endian registers *and* big-endian
>>> descriptors.
>>
>> But doesn't "big-endian" property actually mean "big-endian-registers"?
>> Do we have to overload property meaning in this case?
> 
> It doesn't *have* a standard meaning, it is defined per device binding
> that uses it.  Just make it mean whatever is most logical for this
> device.
> 
>>> I have no opinion which is best; it depends on what configurations
>>> actually exist, and how popular those are.
> 
> 
> Segher
> 

OK, thanks a lot for the explanations.
I'll modify the code.
I think may be usb-ohci compatible values should be made properties as well,
so that both drivers follow the same rules.
Thanks,
Valentine.