diff mbox series

[v3,1/6] dt-bindings: add binding for USBSS-DRD controller.

Message ID 1548935553-452-2-git-send-email-pawell@cadence.com (mailing list archive)
State Superseded
Headers show
Series Introduced new Cadence USBSS DRD Driver. | expand

Commit Message

Pawel Laszczak Jan. 31, 2019, 11:52 a.m. UTC 
This patch aim at documenting USB related dt-bindings for the
Cadence USBSS-DRD controller.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 .../devicetree/bindings/usb/cdns-usb3.txt     | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3.txt

Comments

Rob Herring (Arm) Feb. 13, 2019, 10:48 p.m. UTC | #1 
On Thu, Jan 31, 2019 at 11:52:28AM +0000, Pawel Laszczak wrote:
> This patch aim at documenting USB related dt-bindings for the
> Cadence USBSS-DRD controller.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  .../devicetree/bindings/usb/cdns-usb3.txt     | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3.txt b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> new file mode 100644
> index 000000000000..6dc38be77f5c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> @@ -0,0 +1,33 @@
> +Binding for the Cadence USBSS-DRD controller
> +
> +Required properties:
> +  - reg: Physical base address and size of the controller's register areas.
> +	 Controller has 3 different regions:
> +	 region 1 - HOST registers area
> +	 region 2 - DEVICE registers area
> +	 region 3 - OTG/DRD registers area
> +  - reg-names - register memory area names:
> +	"xhci" - for HOST registers space
> +	"dev" - for DEVICE registers space
> +	"otg" - for OTG/DRD registers space
> +  - compatible: Should contain: "cdns,usb3-1.0.0" or "cdns,usb3-1.0.1"
> +  - interrupts: Interrupt specifier. Refer to interrupt bindings.
> +		Driver supports only single interrupt line.

Driver supports or h/w only has 1 interrupt?

> +                This single interrupt is shared between Device,
> +		host and OTG/DRD part of driver.
> +
> +Optional properties:
> + - maximum-speed : valid arguments are "super-speed", "high-speed" and
> +                   "full-speed"; refer to usb/generic.txt
> + - dr_mode: Should be one of "host", "peripheral" or "otg".
> + - phys: reference to the USB PHY
> +
> +Example:
> +	usb@f3000000 {
> +		compatible = "cdns,usb3";

Doesn't match above.

> +		interrupts = <USB_IRQ  7 IRQ_TYPE_LEVEL_HIGH>;
> +		reg = <0xf3000000 0x10000	/* memory area for HOST registers */
> +			0xf3010000 0x10000	/* memory area for DEVICE registers */
> +			0xf3020000 0x10000>;	/* memory area for OTG/DRD registers */

Are these really 64KB long? That wastes virtual address space on 32-bit 
systems if not.

> +		reg-names = "xhci", "dev", "otg";
> +	};
> -- 
> 2.17.1
>
Pawel Laszczak Feb. 14, 2019, 6:08 a.m. UTC | #2 
Hi Rob,
>On Thu, Jan 31, 2019 at 11:52:28AM +0000, Pawel Laszczak wrote:
>> This patch aim at documenting USB related dt-bindings for the
>> Cadence USBSS-DRD controller.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>>  .../devicetree/bindings/usb/cdns-usb3.txt     | 33 +++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3.txt b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
>> new file mode 100644
>> index 000000000000..6dc38be77f5c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
>> @@ -0,0 +1,33 @@
>> +Binding for the Cadence USBSS-DRD controller
>> +
>> +Required properties:
>> +  - reg: Physical base address and size of the controller's register areas.
>> +	 Controller has 3 different regions:
>> +	 region 1 - HOST registers area
>> +	 region 2 - DEVICE registers area
>> +	 region 3 - OTG/DRD registers area
>> +  - reg-names - register memory area names:
>> +	"xhci" - for HOST registers space
>> +	"dev" - for DEVICE registers space
>> +	"otg" - for OTG/DRD registers space
>> +  - compatible: Should contain: "cdns,usb3-1.0.0" or "cdns,usb3-1.0.1"
>> +  - interrupts: Interrupt specifier. Refer to interrupt bindings.
>> +		Driver supports only single interrupt line.
>
>Driver supports or h/w only has 1 interrupt?

h/w internally has 3 separate interrupt line, but in my testing board 
they are ored. So physically driver sees only one h/w interrupt line and 
assumes that h/w has single common interrupt line. 

>
>> +                This single interrupt is shared between Device,
>> +		host and OTG/DRD part of driver.
>> +
>> +Optional properties:
>> + - maximum-speed : valid arguments are "super-speed", "high-speed" and
>> +                   "full-speed"; refer to usb/generic.txt
>> + - dr_mode: Should be one of "host", "peripheral" or "otg".
>> + - phys: reference to the USB PHY
>> +
>> +Example:
>> +	usb@f3000000 {
>> +		compatible = "cdns,usb3";
>
>Doesn't match above.
Right, should be:
		Compatible = " cdns,usb3-1.0.0", "cdns,usb3-1.0.1";
>
>> +		interrupts = <USB_IRQ  7 IRQ_TYPE_LEVEL_HIGH>;
>> +		reg = <0xf3000000 0x10000	/* memory area for HOST registers */
>> +			0xf3010000 0x10000	/* memory area for DEVICE registers */
>> +			0xf3020000 0x10000>;	/* memory area for OTG/DRD registers */
>
>Are these really 64KB long? That wastes virtual address space on 32-bit
>systems if not.

Yes, on my testing platform it takes 64KB. It's only example, and it could be limited. 
>
>> +		reg-names = "xhci", "dev", "otg";
>> +	};
>> --
>> 2.17.1
>>
Thanks 
Pawel
Rob Herring (Arm) Feb. 14, 2019, 2 p.m. UTC | #3 
On Thu, Feb 14, 2019 at 12:08 AM Pawel Laszczak <pawell@cadence.com> wrote:
>
> Hi Rob,
> >On Thu, Jan 31, 2019 at 11:52:28AM +0000, Pawel Laszczak wrote:
> >> This patch aim at documenting USB related dt-bindings for the
> >> Cadence USBSS-DRD controller.
> >>
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> ---
> >>  .../devicetree/bindings/usb/cdns-usb3.txt     | 33 +++++++++++++++++++
> >>  1 file changed, 33 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3.txt b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> >> new file mode 100644
> >> index 000000000000..6dc38be77f5c
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> >> @@ -0,0 +1,33 @@
> >> +Binding for the Cadence USBSS-DRD controller
> >> +
> >> +Required properties:
> >> +  - reg: Physical base address and size of the controller's register areas.
> >> +     Controller has 3 different regions:
> >> +     region 1 - HOST registers area
> >> +     region 2 - DEVICE registers area
> >> +     region 3 - OTG/DRD registers area
> >> +  - reg-names - register memory area names:
> >> +    "xhci" - for HOST registers space
> >> +    "dev" - for DEVICE registers space
> >> +    "otg" - for OTG/DRD registers space
> >> +  - compatible: Should contain: "cdns,usb3-1.0.0" or "cdns,usb3-1.0.1"
> >> +  - interrupts: Interrupt specifier. Refer to interrupt bindings.
> >> +            Driver supports only single interrupt line.
> >
> >Driver supports or h/w only has 1 interrupt?
>
> h/w internally has 3 separate interrupt line, but in my testing board
> they are ored. So physically driver sees only one h/w interrupt line and
> assumes that h/w has single common interrupt line.

If it is possible for others to have 3 separate lines then you should
have 3. Just repeat the connection 3 times. Or the binding can support
1 or 3 interrupts based on the h/w. Either way it's not about what the
driver supports, but what the h/w looks like.

> >> +                This single interrupt is shared between Device,
> >> +            host and OTG/DRD part of driver.
> >> +
> >> +Optional properties:
> >> + - maximum-speed : valid arguments are "super-speed", "high-speed" and
> >> +                   "full-speed"; refer to usb/generic.txt
> >> + - dr_mode: Should be one of "host", "peripheral" or "otg".
> >> + - phys: reference to the USB PHY
> >> +
> >> +Example:
> >> +    usb@f3000000 {
> >> +            compatible = "cdns,usb3";
> >
> >Doesn't match above.
> Right, should be:
>                 Compatible = " cdns,usb3-1.0.0", "cdns,usb3-1.0.1";

No, that doesn't match either. Above you said "or".

Rob
Peter Chen Feb. 15, 2019, 1:56 a.m. UTC | #4 
> >
> > Hi Rob,
> > >On Thu, Jan 31, 2019 at 11:52:28AM +0000, Pawel Laszczak wrote:
> > >> This patch aim at documenting USB related dt-bindings for the
> > >> Cadence USBSS-DRD controller.
> > >>
> > >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> > >> ---
> > >>  .../devicetree/bindings/usb/cdns-usb3.txt     | 33 +++++++++++++++++++
> > >>  1 file changed, 33 insertions(+)
> > >>  create mode 100644
> > >> Documentation/devicetree/bindings/usb/cdns-usb3.txt
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> > >> b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> > >> new file mode 100644
> > >> index 000000000000..6dc38be77f5c
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
> > >> @@ -0,0 +1,33 @@
> > >> +Binding for the Cadence USBSS-DRD controller
> > >> +
> > >> +Required properties:
> > >> +  - reg: Physical base address and size of the controller's register areas.
> > >> +     Controller has 3 different regions:
> > >> +     region 1 - HOST registers area
> > >> +     region 2 - DEVICE registers area
> > >> +     region 3 - OTG/DRD registers area
> > >> +  - reg-names - register memory area names:
> > >> +    "xhci" - for HOST registers space
> > >> +    "dev" - for DEVICE registers space
> > >> +    "otg" - for OTG/DRD registers space
> > >> +  - compatible: Should contain: "cdns,usb3-1.0.0" or "cdns,usb3-1.0.1"
> > >> +  - interrupts: Interrupt specifier. Refer to interrupt bindings.
> > >> +            Driver supports only single interrupt line.
> > >
> > >Driver supports or h/w only has 1 interrupt?
> >
> > h/w internally has 3 separate interrupt line, but in my testing board
> > they are ored. So physically driver sees only one h/w interrupt line
> > and assumes that h/w has single common interrupt line.
> 
> If it is possible for others to have 3 separate lines then you should have 3. Just
> repeat the connection 3 times. Or the binding can support
> 1 or 3 interrupts based on the h/w. Either way it's not about what the driver
> supports, but what the h/w looks like.
> 
 
From IP level, it has three interrupt sources for "otg", "host" and "device". But from
SoC level, it usually only has one USB interrupt line, the device driver internal controls
and knows which sources from USB controller.

Peter
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3.txt b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
new file mode 100644
index 000000000000..6dc38be77f5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
@@ -0,0 +1,33 @@ 
+Binding for the Cadence USBSS-DRD controller
+
+Required properties:
+  - reg: Physical base address and size of the controller's register areas.
+	 Controller has 3 different regions:
+	 region 1 - HOST registers area
+	 region 2 - DEVICE registers area
+	 region 3 - OTG/DRD registers area
+  - reg-names - register memory area names:
+	"xhci" - for HOST registers space
+	"dev" - for DEVICE registers space
+	"otg" - for OTG/DRD registers space
+  - compatible: Should contain: "cdns,usb3-1.0.0" or "cdns,usb3-1.0.1"
+  - interrupts: Interrupt specifier. Refer to interrupt bindings.
+		Driver supports only single interrupt line.
+                This single interrupt is shared between Device,
+		host and OTG/DRD part of driver.
+
+Optional properties:
+ - maximum-speed : valid arguments are "super-speed", "high-speed" and
+                   "full-speed"; refer to usb/generic.txt
+ - dr_mode: Should be one of "host", "peripheral" or "otg".
+ - phys: reference to the USB PHY
+
+Example:
+	usb@f3000000 {
+		compatible = "cdns,usb3";
+		interrupts = <USB_IRQ  7 IRQ_TYPE_LEVEL_HIGH>;
+		reg = <0xf3000000 0x10000	/* memory area for HOST registers */
+			0xf3010000 0x10000	/* memory area for DEVICE registers */
+			0xf3020000 0x10000>;	/* memory area for OTG/DRD registers */
+		reg-names = "xhci", "dev", "otg";
+	};