diff mbox

[v9,1/3] usb: dwc3: qcom: Add device tree binding

Message ID 1410550088-8754-2-git-send-email-agross@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Gross Sept. 12, 2014, 7:28 p.m. UTC 
From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

QCOM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
(SNPS) and HS, SS PHY's control and configuration registers.

It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 .../devicetree/bindings/phy/qcom-dwc3-usb-phy.txt  |   39 ++++++++++++
 .../devicetree/bindings/usb/qcom,dwc3.txt          |   66 ++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt

Comments

Jack Pham Sept. 16, 2014, 6:15 p.m. UTC | #1 
Hi Andy,

On Fri, Sep 12, 2014 at 02:28:06PM -0500, Andy Gross wrote:
> +Example device nodes:
> +
> +		hs_phy: phy@100f8800 {
> +			compatible = "qcom,dwc3-hs-usb-phy";
> +			reg = <0x100f8800 0x30>;

Just wanted to point out that in our downstream code, the glue
device/driver, i.e. "qcom,dwc3", does claim an entire register region
that encompasses the same registers used by these PHYs. I noticed you're
not adding a reg resource to the glue node below in this patchset. In
order to allow multiple devices to use the overlapping regions, we avoid
calling devm_ioremap_resource() and instead call devm_ioremap_nocache()
directly which bypasses the request_mem_region() call, which I don't
think is an entirely correct thing to do.

On the other hand I'm trying to think of use cases on our other SOCs
(MSM8974, APQ8084) where the glue actually would need access to the same
or adjacent IO registers that couldn't just be handled directly by these
PHY drivers. Should we accommodate for the potential of overlapping
regions or should we just hold the line here and maybe only expose with
finer granularity the registers that will actually be needed by the PHYs
& glue respectively?

> +			clocks = <&gcc USB30_0_UTMI_CLK>;
> +			clock-names = "ref";
> +			#phy-cells = <0>;
> +
> +			status = "ok";
> +		};
> +
> +		ss_phy: phy@100f8830 {
> +			compatible = "qcom,dwc3-ss-usb-phy";
> +			reg = <0x100f8830 0x30>;
> +			clocks = <&gcc USB30_0_MASTER_CLK>;
> +			clock-names = "ref";
> +			#phy-cells = <0>;
> +
> +			status = "ok";
> +		};
> +
> +		usb3_0: usb30@0 {
> +			compatible = "qcom,dwc3";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			clocks = <&gcc USB30_0_MASTER_CLK>;
> +			clock-names = "core";
> +
> +			ranges;
> +
> +			status = "ok";
> +
> +			dwc3@10000000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x10000000 0xcd00>;
> +				interrupts = <0 205 0x4>;
> +				phys = <&hs_phy>, <&ss_phy>;
> +				phy-names = "usb2-phy", "usb3-phy";
> +				tx-fifo-resize;
> +				dr_mode = "host";
> +			};
> +		};
> +

Thanks,
Jack
Felipe Balbi Sept. 16, 2014, 6:29 p.m. UTC | #2 
Hi,

On Tue, Sep 16, 2014 at 11:15:43AM -0700, Jack Pham wrote:
> Hi Andy,
> 
> On Fri, Sep 12, 2014 at 02:28:06PM -0500, Andy Gross wrote:
> > +Example device nodes:
> > +
> > +		hs_phy: phy@100f8800 {
> > +			compatible = "qcom,dwc3-hs-usb-phy";
> > +			reg = <0x100f8800 0x30>;
> 
> Just wanted to point out that in our downstream code, the glue
> device/driver, i.e. "qcom,dwc3", does claim an entire register region
> that encompasses the same registers used by these PHYs. I noticed you're
> not adding a reg resource to the glue node below in this patchset. In
> order to allow multiple devices to use the overlapping regions, we avoid
> calling devm_ioremap_resource() and instead call devm_ioremap_nocache()
> directly which bypasses the request_mem_region() call, which I don't
> think is an entirely correct thing to do.
> 
> On the other hand I'm trying to think of use cases on our other SOCs
> (MSM8974, APQ8084) where the glue actually would need access to the same
> or adjacent IO registers that couldn't just be handled directly by these
> PHY drivers. Should we accommodate for the potential of overlapping
> regions or should we just hold the line here and maybe only expose with
> finer granularity the registers that will actually be needed by the PHYs
> & glue respectively?

I prefer to keep things separated. Glue only needs to access whatever
belongs to glue. If a register related to PHY settings, it should be in
the PHY address space.

cheers
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt b/Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
new file mode 100644
index 0000000..86f2dbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
@@ -0,0 +1,39 @@ 
+Qualcomm DWC3 HS AND SS PHY CONTROLLER
+--------------------------------------
+
+DWC3 PHY nodes are defined to describe on-chip Synopsis Physical layer
+controllers.  Each DWC3 PHY controller should have its own node.
+
+Required properties:
+- compatible: should contain one of the following:
+	- "qcom,dwc3-hs-usb-phy" for High Speed Synopsis PHY controller
+	- "qcom,dwc3-ss-usb-phy" for Super Speed Synopsis PHY controller
+- reg: offset and length of the DWC3 PHY controller register set
+- #phy-cells: must be zero
+- clocks: a list of phandles and clock-specifier pairs, one for each entry in
+  clock-names.
+- clock-names: Should contain "ref" for the PHY reference clock
+
+Optional clocks:
+  "xo"		External reference clock
+
+Example:
+		phy@100f8800 {
+			compatible = "qcom,dwc3-hs-usb-phy";
+			reg = <0x100f8800 0x30>;
+			clocks = <&gcc USB30_0_UTMI_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
+
+		phy@100f8830 {
+			compatible = "qcom,dwc3-ss-usb-phy";
+			reg = <0x100f8830 0x30>;
+			clocks = <&gcc USB30_0_MASTER_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
new file mode 100644
index 0000000..ca164e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -0,0 +1,66 @@ 
+Qualcomm SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:	should contain "qcom,dwc3"
+- clocks:		A list of phandle + clock-specifier pairs for the
+				clocks listed in clock-names
+- clock-names:	Should contain the following:
+  "core"		Master/Core clock, have to be >= 125 MHz for SS
+				operation and >= 60MHz for HS operation
+
+Optional clocks:
+  "iface"		System bus AXI clock.  Not present on all platforms
+  "sleep"		Sleep clock, used when USB3 core goes into low
+				power mode (U3).
+
+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.
+
+Phy documentation is provided in the following places:
+Documentation/devicetree/bindings/phy/qcom,dwc3-usb-phy.txt
+
+Example device nodes:
+
+		hs_phy: phy@100f8800 {
+			compatible = "qcom,dwc3-hs-usb-phy";
+			reg = <0x100f8800 0x30>;
+			clocks = <&gcc USB30_0_UTMI_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
+
+		ss_phy: phy@100f8830 {
+			compatible = "qcom,dwc3-ss-usb-phy";
+			reg = <0x100f8830 0x30>;
+			clocks = <&gcc USB30_0_MASTER_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
+
+		usb3_0: usb30@0 {
+			compatible = "qcom,dwc3";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			clocks = <&gcc USB30_0_MASTER_CLK>;
+			clock-names = "core";
+
+			ranges;
+
+			status = "ok";
+
+			dwc3@10000000 {
+				compatible = "snps,dwc3";
+				reg = <0x10000000 0xcd00>;
+				interrupts = <0 205 0x4>;
+				phys = <&hs_phy>, <&ss_phy>;
+				phy-names = "usb2-phy", "usb3-phy";
+				tx-fifo-resize;
+				dr_mode = "host";
+			};
+		};
+