[PATCH 1/2] usb: doc: dwc3-xilinx: Add devicetree bindings

[PATCH 1/2] usb: doc: dwc3-xilinx: Add devicetree bindings

[Subbaraya Sundeep Bhatta]

This patch adds binding doc for Xilinx DWC3 glue driver.

Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
---
 .../devicetree/bindings/usb/dwc3-xilinx.txt        | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/dwc3-xilinx.txt

diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
new file mode 100644
index 0000000..30361b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
@@ -0,0 +1,33 @@
+Xilinx SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:	Should contain "xlnx,zynqmp-dwc3"
+- clocks:	A list of phandles for the clocks listed in clock-names
+- clock-names:	Should contain the following:
+  "bus_clk"	 Master/Core clock, have to be >= 125 MHz for SS
+		 operation and >= 60MHz for HS operation
+
+  "ref_clk"	 Clock source to core during PHY power down
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Example device node:
+
+		usb@0 {
+			#address-cells = <0x2>;
+			#size-cells = <0x1>;
+			status = "okay";
+			compatible = "xlnx,zynqmp-dwc3";
+			clock-names = "bus_clk" "ref_clk";
+			clocks = <&clk125>, <&clk125>;
+			ranges;
+
+			dwc3@fe200000 {
+				compatible = "snps,dwc3";
+				reg = <0x0 0xfe200000 0x40000>;
+				interrupts = <0x0 0x41 0x4>;
+				dr_mode = "host";
+			};
+		};
-- 
2.1.2

Re: [PATCH 1/2] usb: doc: dwc3-xilinx: Add devicetree bindings

[Rob Herring]

On Wed, Nov 18, 2015 at 06:20:31PM +0530, Subbaraya Sundeep Bhatta wrote:
> This patch adds binding doc for Xilinx DWC3 glue driver.
> 
> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>

I really dislike how the DWC3 binding has been done. The sub-node here 
is pointless. The only thing the outer node does is add clocks which 
should simply be part of the DWC3 node. Presumably the IP block needs 
some clocks...

Anyway, it's self-contained within the DWC3, so I won't make you clean 
up the mess.

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  .../devicetree/bindings/usb/dwc3-xilinx.txt        | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
> new file mode 100644
> index 0000000..30361b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
> @@ -0,0 +1,33 @@
> +Xilinx SuperSpeed DWC3 USB SoC controller
> +
> +Required properties:
> +- compatible:	Should contain "xlnx,zynqmp-dwc3"
> +- clocks:	A list of phandles for the clocks listed in clock-names
> +- clock-names:	Should contain the following:
> +  "bus_clk"	 Master/Core clock, have to be >= 125 MHz for SS
> +		 operation and >= 60MHz for HS operation
> +
> +  "ref_clk"	 Clock source to core during PHY power down
> +
> +Required child node:
> +A child node must exist to represent the core DWC3 IP block. The name of
> +the node is not important. The content of the node is defined in dwc3.txt.
> +
> +Example device node:
> +
> +		usb@0 {
> +			#address-cells = <0x2>;
> +			#size-cells = <0x1>;
> +			status = "okay";
> +			compatible = "xlnx,zynqmp-dwc3";
> +			clock-names = "bus_clk" "ref_clk";
> +			clocks = <&clk125>, <&clk125>;
> +			ranges;
> +
> +			dwc3@fe200000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x0 0xfe200000 0x40000>;
> +				interrupts = <0x0 0x41 0x4>;
> +				dr_mode = "host";
> +			};
> +		};
> -- 
> 2.1.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: doc: dwc3-xilinx: Add devicetree bindings

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]


Hi,

Rob Herring <robh@kernel.org> writes:
> On Wed, Nov 18, 2015 at 06:20:31PM +0530, Subbaraya Sundeep Bhatta wrote:
>> This patch adds binding doc for Xilinx DWC3 glue driver.
>> 
>> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
>
> I really dislike how the DWC3 binding has been done. The sub-node here 
> is pointless. The only thing the outer node does is add clocks which 
> should simply be part of the DWC3 node. Presumably the IP block needs 
> some clocks...
>
> Anyway, it's self-contained within the DWC3, so I won't make you clean 
> up the mess.

heh, it's definitely not pointless. We get a lot of free goodies by
doing it that way. I'm just not going to repeat it once again. But in
summary:

a) we force people to write glue layers for *only* their HW specific
details

b) there's really no way to abuse dwc3 core because there's no symbol
exported from dwc3 core to glue

c) PM (both system sleep and runtime) becomes a lot simpler with core
only caring about what PM details delivered by SNPS (e.g. Hibernation
mode from DWC3) and glue only has to consider the SoC integration parts
of PM (for OMAP that would be PRCM details, hwmod, etc).

I'm definitely not going to change dwc3 because it has made my life a
lot saner. Definitely a lot saner than MUSB. Besides, DTS is supposed to
describe the HW and that's, really, how the HW is.

There's a piece of HW which is somewhat platform agnostic and delivered
by SNPS as a black box (SNPS customers don't touch SNPS RTL) and there's
another piece of HW which bridges this dwc3 to the platform specific
details and integration context of the platform (for OMAP that would the
SYSCONFIG/SYSSTATUS registers, Clock autogating, PRCM, etc, etc etc).

So you _do_ in fact, have two separate pieces of HW which are SW visible
and controllable independently. They each have their own IRQs (though in
some SoCs they share the same line), they're own address space, yada
yada yada.

cheers

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 1/2] usb: doc: dwc3-xilinx: Add devicetree bindings

[Rob Herring]

On Wed, Nov 18, 2015 at 5:02 PM, Felipe Balbi <balbi@ti.com> wrote:
>
> Hi,
>
> Rob Herring <robh@kernel.org> writes:
>> On Wed, Nov 18, 2015 at 06:20:31PM +0530, Subbaraya Sundeep Bhatta wrote:
>>> This patch adds binding doc for Xilinx DWC3 glue driver.
>>>
>>> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
>>
>> I really dislike how the DWC3 binding has been done. The sub-node here
>> is pointless. The only thing the outer node does is add clocks which
>> should simply be part of the DWC3 node. Presumably the IP block needs
>> some clocks...
>>
>> Anyway, it's self-contained within the DWC3, so I won't make you clean
>> up the mess.
>
> heh, it's definitely not pointless. We get a lot of free goodies by
> doing it that way. I'm just not going to repeat it once again. But in
> summary:
>
> a) we force people to write glue layers for *only* their HW specific
> details
>
> b) there's really no way to abuse dwc3 core because there's no symbol
> exported from dwc3 core to glue

Both are doable independent of DT.

> c) PM (both system sleep and runtime) becomes a lot simpler with core
> only caring about what PM details delivered by SNPS (e.g. Hibernation
> mode from DWC3) and glue only has to consider the SoC integration parts
> of PM (for OMAP that would be PRCM details, hwmod, etc).

No doubt OMAP is special.

> I'm definitely not going to change dwc3 because it has made my life a
> lot saner. Definitely a lot saner than MUSB. Besides, DTS is supposed to
> describe the HW and that's, really, how the HW is.

So reading the description, the DWC3 has no clocks?

> There's a piece of HW which is somewhat platform agnostic and delivered
> by SNPS as a black box (SNPS customers don't touch SNPS RTL) and there's
> another piece of HW which bridges this dwc3 to the platform specific
> details and integration context of the platform (for OMAP that would the
> SYSCONFIG/SYSSTATUS registers, Clock autogating, PRCM, etc, etc etc).

It is a black box, but with dozens of configuration options typically.

> So you _do_ in fact, have two separate pieces of HW which are SW visible
> and controllable independently. They each have their own IRQs (though in
> some SoCs they share the same line), they're own address space, yada
> yada yada.

When that is the case, I have no problem with this split, but I don't
see any of these examples in this particular case. So how should the
binding look when there is no vendor specific glue? Are we supposed to
keep the binding structure to appease the needs of the driver?

Rob

Re: [PATCH 1/2] usb: doc: dwc3-xilinx: Add devicetree bindings

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]


Hi,

Rob Herring <robh@kernel.org> writes:
> On Wed, Nov 18, 2015 at 5:02 PM, Felipe Balbi <balbi@ti.com> wrote:
>>
>> Hi,
>>
>> Rob Herring <robh@kernel.org> writes:
>>> On Wed, Nov 18, 2015 at 06:20:31PM +0530, Subbaraya Sundeep Bhatta wrote:
>>>> This patch adds binding doc for Xilinx DWC3 glue driver.
>>>>
>>>> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
>>>
>>> I really dislike how the DWC3 binding has been done. The sub-node here
>>> is pointless. The only thing the outer node does is add clocks which
>>> should simply be part of the DWC3 node. Presumably the IP block needs
>>> some clocks...
>>>
>>> Anyway, it's self-contained within the DWC3, so I won't make you clean
>>> up the mess.
>>
>> heh, it's definitely not pointless. We get a lot of free goodies by
>> doing it that way. I'm just not going to repeat it once again. But in
>> summary:
>>
>> a) we force people to write glue layers for *only* their HW specific
>> details
>>
>> b) there's really no way to abuse dwc3 core because there's no symbol
>> exported from dwc3 core to glue
>
> Both are doable independent of DT.
>
>> c) PM (both system sleep and runtime) becomes a lot simpler with core
>> only caring about what PM details delivered by SNPS (e.g. Hibernation
>> mode from DWC3) and glue only has to consider the SoC integration parts
>> of PM (for OMAP that would be PRCM details, hwmod, etc).
>
> No doubt OMAP is special.

not only OMAP. Every single SoC will integrate a particular IP in its
own way.

>> I'm definitely not going to change dwc3 because it has made my life a
>> lot saner. Definitely a lot saner than MUSB. Besides, DTS is supposed to
>> describe the HW and that's, really, how the HW is.
>
> So reading the description, the DWC3 has no clocks?

of course it has, and you have registers in DWC3 to change some of the
parents of the clocks it uses internally.

>> There's a piece of HW which is somewhat platform agnostic and delivered
>> by SNPS as a black box (SNPS customers don't touch SNPS RTL) and there's
>> another piece of HW which bridges this dwc3 to the platform specific
>> details and integration context of the platform (for OMAP that would the
>> SYSCONFIG/SYSSTATUS registers, Clock autogating, PRCM, etc, etc etc).
>
> It is a black box, but with dozens of configuration options typically.

all of which are detected within the driver. For those which can't be,
we have bindings.

>> So you _do_ in fact, have two separate pieces of HW which are SW visible
>> and controllable independently. They each have their own IRQs (though in
>> some SoCs they share the same line), they're own address space, yada
>> yada yada.
>
> When that is the case, I have no problem with this split, but I don't
> see any of these examples in this particular case. So how should the
> binding look when there is no vendor specific glue? Are we supposed to
> keep the binding structure to appease the needs of the driver?

If there really is *no* vendor specific glue, nothing prevents them from
patching dwc3 to understand *OPTIONAL* clocks and use dwc3 directly. As
long as it doesn't break any of the platforms currently supported and
doesn't look ugly, fine with me.

In fact, there's one company which has been using dwc3 without a glue
layer. I forgot who told me they didn't need a glue layer, but it's in
the archives.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]