diff mbox

[v2,1/2] usb: samsung: Update Exynos EHCI/OHCI bindings documentation

Message ID 1377776006-22972-1-git-send-email-sachin.kamat@linaro.org
State Deferred
Headers show

Commit Message

Sachin Kamat Aug. 29, 2013, 11:33 a.m. UTC
Updated the document as per the latest implementation.
While at it also fixed some trivial typos.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
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(-)

Comments

Mark Rutland Sept. 6, 2013, 2:30 p.m. UTC | #1
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@linaro.org>
> ---
> 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.
diff mbox

Patch

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.
+ - clocks: from common clock binding: handle to USB clock.
+ - clock-names: shall be "usbhost".
 
 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>;
 
 		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".
 
 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
  - 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".
 
 Sub-nodes:
 The dwc3 core should be added as subnode to Exynos dwc3 glue.