Segher Boessenkool wrote:
>> + l) USB EHCI controllers
>> +
>> + 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.
>> + - 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.
>
> Maybe "usb-ehci" is best -- can anyone think of a better name?
Again, why not type="usb", compatible="ehci"?
>
>> + - reg : Offset and length of the register set for the device
>
> Address and length. You also should declare here how the optional
> EHCI register blocks should be encoded (the USB debug port, etc.)
>
>> + - 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?
>
> Instead, you should just say how many interrupts should be here,
> and which is which in the EHCI standard.
>
>> + - interrupt-parent : the phandle for the interrupt controller that
>> + services interrupts for this device.
>
> Not every EHCI node needs this; just don't mention this at all,
> interrupt mapping is fully defined for all devices elsewhere
> already (in the "interrupt mapping" recommended practice).
>
>> + 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?
>
> I have no opinion which is best; it depends on what configurations
> actually exist, and how popular those are.
>
>> + [EMAIL PROTECTED] {
>> + device_type = "usb";
>> + compatible = "ibm,ehci-440epx", "ehci-be-desc", "ehci";
>> + interrupts = <1a 4>;
>> + interrupt-parent = <&UIC0>;
>> + reg = <0 e0000300 ff>;
>
> Length should be 100 here.
>
>> + big-endian;
>> + };
>
>
> Segher
>
Thanks,
Valentine.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
