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

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

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

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

Maybe "usb-ehci" is best -- can anyone think of a better name?

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

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.

I have no opinion which is best; it depends on what configurations
actually exist, and how popular those are.

> +	ehci@e0000300 {
> +		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