Hi,
On Thu, Aug 29, 2013 at 12:33:25PM +0100, Sachin Kamat wrote:
> Updated the document as per the latest implementation.
> While at it also fixed some trivial typos.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx>
> ---
> Changes since v1:
> * Updated review comments from Stephen Warren regarding the wording
> and some styling.
> ---
> .../devicetree/bindings/usb/exynos-usb.txt | 29 ++++++++++----------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
> index d967ba1..56468f7 100644
> --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
> @@ -5,13 +5,14 @@ The device node has following properties.
>
> EHCI
> Required properties:
> - - compatible: should be "samsung,exynos4210-ehci" for USB 2.0
> - EHCI controller in host mode.
> + - compatible: should be one of the following for USB 2.0 EHCI controller:
> + (a) "samsung,exynos5440-ehci" for Exynos5440 SoC
> + (b) "samsung,exynos4210-ehci" for all other Exynos4 and 5 SoCs
> - reg: physical base address of the controller and length of memory mapped
> region.
> - - interrupts: interrupt number to the cpu.
> - - clocks: from common clock binding: handle to usb clock.
> - - clock-names: from common clock binding: Shall be "usbhost".
> + - interrupts: interrupt number to the CPU.
If this is going to be reorganised, can we fix the terminology?
Interrupts are defined by interrupt-specifiers:
- interrupts: a single interrupt-specifier for the sole interrupt
generated by the device.
> + - clocks: from common clock binding: handle to USB clock.
> + - clock-names: shall be "usbhost".
Similarly:
- clocks: phandle amd clock-specifier pair for the clocks listed in
clock-names.
- clock-names: shall contain "usbhost" for the USB clock.
>
> Optional properties:
> - samsung,vbus-gpio: if present, specifies the GPIO that
> @@ -23,7 +24,7 @@ Example:
> compatible = "samsung,exynos4210-ehci";
> reg = <0x12110000 0x100>;
> interrupts = <0 71 0>;
> - samsung,vbus-gpio = <&gpx2 6 1 3 3>;
> + samsung,vbus-gpio = <&gpx2 6 0>;
Why does the gpio in the example need to be changed in this way?
>
> clocks = <&clock 285>;
> clock-names = "usbhost";
> @@ -31,13 +32,14 @@ Example:
>
> OHCI
> Required properties:
> - - compatible: should be "samsung,exynos4210-ohci" for USB 2.0
> - OHCI companion controller in host mode.
> + - compatible: should be one of the following for USB 2.0 OHCI controller:
> + (a) "samsung,exynos5440-ohci" for Exynos5440 SoC
> + (b) "samsung,exynos4210-ohci" for all other Exynos4 and 5 SoCs
> - reg: physical base address of the controller and length of memory mapped
> region.
> - - interrupts: interrupt number to the cpu.
> - - clocks: from common clock binding: handle to usb clock.
> - - clock-names: from common clock binding: Shall be "usbhost".
> + - interrupts: interrupt number to the CPU.
> + - clocks: from common clock binding: handle to USB clock.
> + - clock-names: shall be "usbhost".
Similarly it would be nice to fix up the terminology here.
>
> Example:
> usb@12120000 {
> @@ -53,12 +55,11 @@ DWC3
> Required properties:
> - compatible: should be "samsung,exynos5250-dwusb3" for USB 3.0 DWC3
> controller.
> - - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> - with 'reg' property.
> + - #address-cells, #size-cells: should be '1'.
> - ranges: allows valid 1:1 translation between child's address space and
> parent's address space
Huh? What if I'm on an LPAE system. I can't necessarily have both an
empty ranges property (for 1:1 translation) and #address-cells = <1>,
#size-cells = <1>. That's just broken.
Is the driver relying on this, or does it just require some valid
ranges, #address-cells, and #size-cells that the Linux infrastructure
can already handle?
As far as I an see, this should be something like:
- #address-cells: as required to describe any child nodes.
- #size-cells: as required to describe any child nodes.
- ranges: as required to map any child nodes to the parent address space.
> - clocks: Clock IDs array as required by the controller.
> - - clock-names: names of clocks correseponding to IDs in the clock property
> + - clock-names: shall be "usbdrd30".
It would be nice to fix up the terminology here too.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
