diff mbox series

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

Message ID 1544445555-17325-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 Dec. 10, 2018, 12:39 p.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/cdns3-usb.txt     | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt

Comments

Roger Quadros Dec. 11, 2018, 10:16 a.m. UTC | #1
Pawel,

On 10/12/18 14:39, 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/cdns3-usb.txt     | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
> new file mode 100644
> index 000000000000..ae4a255f0b10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
> @@ -0,0 +1,31 @@
> +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
> +  - compatible: Should contain: "cdns,usb3"
> +  - 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 propertiesi:

s/propertiesi/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
> + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
> +
> +
> +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

Use "/* <comment> */" instead.

> +	};
> +
> 

cheers,
-roger
Peter Chen Dec. 13, 2018, 9:20 a.m. UTC | #2
On Tue, Dec 11, 2018 at 6:19 PM Roger Quadros <rogerq@ti.com> wrote:
>
> Pawel,
>
> On 10/12/18 14:39, 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/cdns3-usb.txt     | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
> > new file mode 100644
> > index 000000000000..ae4a255f0b10
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
> > @@ -0,0 +1,31 @@
> > +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
> > +  - compatible: Should contain: "cdns,usb3"
> > +  - 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 propertiesi:
>
> s/propertiesi/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
> > + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
> > +
> > +
> > +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
>
> Use "/* <comment> */" instead.
>
> > +     };
> > +
> >

When running git am to apply patch, it has below warning, please fix them.

Applying: dt-bindings: add binding for USBSS-DRD controller.
.git/rebase-apply/patch:42: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Pawel Laszczak Dec. 13, 2018, 9:25 a.m. UTC | #3
Hi, 

>On Tue, Dec 11, 2018 at 6:19 PM Roger Quadros <rogerq@ti.com> wrote:
>>
>> Pawel,
>>
>> On 10/12/18 14:39, 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/cdns3-usb.txt     | 31 +++++++++++++++++++
>> >  1 file changed, 31 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
>> > new file mode 100644
>> > index 000000000000..ae4a255f0b10
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
>> > @@ -0,0 +1,31 @@
>> > +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
>> > +  - compatible: Should contain: "cdns,usb3"
>> > +  - 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 propertiesi:
>>
>> s/propertiesi/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
>> > + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
>> > +
>> > +
>> > +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
>>
>> Use "/* <comment> */" instead.
>>
>> > +     };
>> > +
>> >
>
>When running git am to apply patch, it has below warning, please fix them.
>
>Applying: dt-bindings: add binding for USBSS-DRD controller.
>.git/rebase-apply/patch:42: new blank line at EOF.
>+
>warning: 1 line adds whitespace errors.

Ok, thanks
Cheers 
Pawel
Rob Herring (Arm) Dec. 20, 2018, 8:01 p.m. UTC | #4
On Mon, Dec 10, 2018 at 12:39:14PM +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/cdns3-usb.txt     | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
> new file mode 100644
> index 000000000000..ae4a255f0b10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt

cdns-usb3.txt?

> @@ -0,0 +1,31 @@
> +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
> +  - compatible: Should contain: "cdns,usb3"

Only 1 version of the IP block?

> +  - 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 propertiesi:

typo

> + - 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
> + - phy-names: name of the USB PHY, should be "cdns3,usbphy"

Don't need -names when there is only one.

> +
> +
> +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
> +	};
> +
> -- 
> 2.17.1
>
Pawel Laszczak Dec. 22, 2018, 10:24 p.m. UTC | #5
Hi Rob,

>On Mon, Dec 10, 2018 at 12:39:14PM +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/cdns3-usb.txt     | 31 +++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
>> new file mode 100644
>> index 000000000000..ae4a255f0b10
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
>
>cdns-usb3.txt?

In all functions we use prefix cdns3, so it was a reason for cdns3-usb.txt.
I agree  with you and I think that cdns-usb3.txt in this place will be better.
3 in cdns3  could be associated with  company name, but that's not true. 

>
>> @@ -0,0 +1,31 @@
>> +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
>> +  - compatible: Should contain: "cdns,usb3"
>
>Only 1 version of the IP block?

I will add one more. Now I know that we should have 
at least one additional version. 
Controller version will be recognized at runtime, but in my opinion 
we should also  have information  about this in dt-binding documentation. 
>
>> +  - 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 propertiesi:
>
>typo
Thank, I know that already. Someone has already reported me this typo. 
"i" - insert mode in vim :)
>
>> + - 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
>> + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
>
>Don't need -names when there is only one.

But in the future probably we will need to add next phy version.
So maybe it's better to leave this name ?

>
>> +
>> +
>> +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
>> +	};
>> +
>> --
>> 2.17.1
>>

Thanks for comments 
Cheers,
Pawel
Rob Herring (Arm) Dec. 27, 2018, 9:01 p.m. UTC | #6
On Sat, Dec 22, 2018 at 4:24 PM Pawel Laszczak <pawell@cadence.com> wrote:
>
> Hi Rob,
>
> >On Mon, Dec 10, 2018 at 12:39:14PM +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>

[...]

> >> + - phys: reference to the USB PHY
> >> + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
> >
> >Don't need -names when there is only one.
>
> But in the future probably we will need to add next phy version.
> So maybe it's better to leave this name ?

'-names' are for when there is more than one phy connected, not
different phy versions. For example, if you had separate phys for HS
and SS.

Rob
Pawel Laszczak Dec. 31, 2018, 5:35 a.m. UTC | #7
Hi Bob,
>>
>> >On Mon, Dec 10, 2018 at 12:39:14PM +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>
>
>[...]
>
>> >> + - phys: reference to the USB PHY
>> >> + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
>> >
>> >Don't need -names when there is only one.
>>
>> But in the future probably we will need to add next phy version.
>> So maybe it's better to leave this name ?
>
>'-names' are for when there is more than one phy connected, not
>different phy versions. For example, if you had separate phys for HS
>and SS.
>

Thanks for the clarification.
I will remove  "-names" 

Cheers,
Pawel
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
new file mode 100644
index 000000000000..ae4a255f0b10
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
@@ -0,0 +1,31 @@ 
+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
+  - compatible: Should contain: "cdns,usb3"
+  - 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 propertiesi:
+ - 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
+ - phy-names: name of the USB PHY, should be "cdns3,usbphy"
+
+
+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
+	};
+