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

[Segher Boessenkool <segher@kernel.crashing.org>]

>>> +  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