diff mbox

[RFC,v2,1/3] usb: dwc3: msm: Add device tree binding information

Message ID 1376042027-9798-2-git-send-email-iivanov@mm-sol.com (mailing list archive)
State RFC
Headers show

Commit Message

Ivan T. Ivanov Aug. 9, 2013, 9:53 a.m. UTC
From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
and HS, SS PHY's controll 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>
---
 .../devicetree/bindings/usb/msm-ssusb.txt          |  101 ++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt

Comments

Felipe Balbi Aug. 12, 2013, 6:04 p.m. UTC | #1
On Fri, Aug 09, 2013 at 10:31:58AM -0500, Kumar Gala wrote:
> 
> On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote:
> 
> > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> > 
> > MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
> 
> probably good to spell out Synopsys rather than SNPS

Synopsys (the company) has always used snps in their bindings though.

> > +Required properities :
> > +- compatible : sould be "qcom,dwc3-hsphy";
> > +- reg : offset and length of the register set in the memory map
> > +- clocks : phandles to clock instances of the device tree nodes
> > +- clock-names :
> > +	"xo" : External reference clock 19 MHz
> > +	"sleep_a_clk" : Sleep clock, used when USB3 core goes into low
> > +	power mode (U3).
> > +<supply-name>-supply : phandle to the regulator device tree node
> > +Required "supply-name" are:
> > +	"v1p8" : 1.8v supply for HSPHY
> > +	"v3p3" : 3.3v supply for HSPHY
> > +	"vbus" : vbus supply for host mode
> > +	"vddcx" : vdd supply for HS-PHY digital circuit operation

I believe these regulators belong to the PHY, not DWC3. Please write a
PHY driver.

> > +Required properities :
> > +- compatible : sould be "qcom,dwc3-ssphy";
> > +- reg : offset and length of the register set in the memory map
> > +- clocks : phandles to clock instances of the device tree nodes
> > +- clock-names :
> > +	"xo" : External reference clock 19 MHz
> > +	"ref_clk" : Reference clock - used in host mode.
> > +<supply-name>-supply : phandle to the regulator device tree node
> > +Required "supply-name" are:
> > +	"v1p8" : 1.8v supply for SS-PHY
> > +	"vddcx" : vdd supply for SS-PHY digital circuit operation

likewise, these regulators should be moved to the PHY driver.

> > +Required properties :
> > +- compatible : should be "qcom,dwc3"
> > +- reg : offset and length of the register set in the memory map
> > +	offset and length of the TCSR register for routing USB
> > +	signals to either picoPHY0 or picoPHY1.
> > +- clocks : phandles to clock instances of the device tree nodes
> > +- clock-names :
> > +	"core_clk" : Master/Core clock, have to be >= 125 MHz for SS
> > +	operation and >= 60MHz for HS operation
> > +	"iface_clk" : System bus AXI clock
> > +	"sleep_clk" : Sleep clock, used when USB3 core goes into low
> > +	power mode (U3).
> > +	"utmi_clk" : Generated by HS-PHY. Used to clock the low power
> > +	parts of thr HS Link layer.
> > +
> > +Optional properties :
> > +- gdsc-supply : phandle to the globally distributed switch controller
> > +  regulator node to the USB controller.
> > +
> > +Sub nodes:
> > +- Sub node for "DWC3 USB3 controller".
> > +  This sub node is required property for device node. The properties
> > +  of this subnode are specified in dwc3.txt.
> 
> Is tx-fifo-resize required for qcom,dwc3? if so we should call that
> out in the binding.

AFAICT that's only needed for OMAP5 ES1.0. Unless Qualcomm also screwed
up default TX FIFO sizes :-p
Kumar Gala Aug. 12, 2013, 6:49 p.m. UTC | #2
On Aug 12, 2013, at 1:04 PM, Felipe Balbi wrote:

> On Fri, Aug 09, 2013 at 10:31:58AM -0500, Kumar Gala wrote:
>> 
>> On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote:
>> 
>>> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>>> 
>>> MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
>> 
>> probably good to spell out Synopsys rather than SNPS
> 
> Synopsys (the company) has always used snps in their bindings though.

I just meant in the commit message as SNPS wasn't obvious to me and I think when someone comes back and looks the commit message doing "Synopsys (SNPS)" is a little more readable.

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ivan T. Ivanov Aug. 13, 2013, 7:21 a.m. UTC | #3
Hi, 

On Fri, 2013-08-09 at 10:31 -0500, Kumar Gala wrote: 
> On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote:
> 
> > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> > 
> > MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
> 
> probably good to spell out Synopsys rather than SNPS

I could make it look like this? Synopsys (SNPS)

> 
> > and HS, SS PHY's controll 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>
> > ---
> > .../devicetree/bindings/usb/msm-ssusb.txt          |  101 ++++++++++++++++++++
> > 1 file changed, 101 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> > new file mode 100644
> > index 0000000..7a97163
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> > @@ -0,0 +1,101 @@
> > +MSM SuperSpeed DWC3 USB SoC controller
> > +
> 
> We should have a heading here like:
> 
> DWC3 Highspeed USB PHY

Ok, I will add these headings.

> 
> > +Required properities :
> > +- compatible : sould be "qcom,dwc3-hsphy";
> > +- reg : offset and length of the register set in the memory map
> > +- clocks : phandles to clock instances of the device tree nodes
> > +- clock-names :
> > +	"xo" : External reference clock 19 MHz
> > +	"sleep_a_clk" : Sleep clock, used when USB3 core goes into low
> > +	power mode (U3).
> > +<supply-name>-supply : phandle to the regulator device tree node
> > +Required "supply-name" are:
> > +	"v1p8" : 1.8v supply for HSPHY
> > +	"v3p3" : 3.3v supply for HSPHY
> > +	"vbus" : vbus supply for host mode
> > +	"vddcx" : vdd supply for HS-PHY digital circuit operation
> > +
> 
> DWC3 Superspeed USB PHY
> 
> > +Required properities :
> > +- compatible : sould be "qcom,dwc3-ssphy";
> > +- reg : offset and length of the register set in the memory map
> > +- clocks : phandles to clock instances of the device tree nodes
> > +- clock-names :
> > +	"xo" : External reference clock 19 MHz
> > +	"ref_clk" : Reference clock - used in host mode.
> > +<supply-name>-supply : phandle to the regulator device tree node
> > +Required "supply-name" are:
> > +	"v1p8" : 1.8v supply for SS-PHY
> > +	"vddcx" : vdd supply for SS-PHY digital circuit operation
> > +
> 
> DWC3 controller
> 
> > +Required properties :
> > +- compatible : should be "qcom,dwc3"
> > +- reg : offset and length of the register set in the memory map
> > +	offset and length of the TCSR register for routing USB
> > +	signals to either picoPHY0 or picoPHY1.
> > +- clocks : phandles to clock instances of the device tree nodes
> > +- clock-names :
> > +	"core_clk" : Master/Core clock, have to be >= 125 MHz for SS
> > +	operation and >= 60MHz for HS operation
> > +	"iface_clk" : System bus AXI clock
> > +	"sleep_clk" : Sleep clock, used when USB3 core goes into low
> > +	power mode (U3).
> > +	"utmi_clk" : Generated by HS-PHY. Used to clock the low power
> > +	parts of thr HS Link layer.
> > +
> > +Optional properties :
> > +- gdsc-supply : phandle to the globally distributed switch controller
> > +  regulator node to the USB controller.
> > +
> > +Sub nodes:
> > +- Sub node for "DWC3 USB3 controller".
> > +  This sub node is required property for device node. The properties
> > +  of this subnode are specified in dwc3.txt.
> 
> Is tx-fifo-resize required for qcom,dwc3? if so we should call that out in the binding.

It looks like this is used in apq8084.dtsi (codeaurora repo)

> 
> > +
> > +Example device nodes:
> > +
> > +	dwc3_hsphy: phy@f92f8800 {
> > +		compatible = "qcom,dwc3-hsphy";
> > +		reg = <0xf92f8800 0x30>;
> > +
> > +		clocks = <&cxo>, <&usb2a_phy_sleep_cxc>;
> > +		clock-names = "xo", "sleep_a_clk";
> > +
> > +		vbus-supply = <&supply>;
> > +		vddcx-supply = <&supply>;
> > +		v1p8-supply = <&supply>;
> > +		v3p3-supply = <&supply>;
> > +	};
> > +
> > +	dwc3_ssphy: phy@f92f8830 {
> > +		compatible = "qcom,dwc3-ssphy";
> > +		reg = <0xf92f8830 0x30>;
> > +
> > +		clocks = <&cxo>, <&usb30_mock_utmi_cxc>;
> > +		clock-names = "xo", "ref_clk";
> > +
> > +		vddcx-supply = <&supply>;
> > +		v1p8-supply = <&supply>;
> > +	};
> > +
> > +	usb@fd4ab000 {
> > +		compatible = "qcom,dwc3";
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		reg = <0xfd4ab000 0x4>;
> > +
> > +		clocks = <&usb30_master_cxc>, <&sys_noc_usb3_axi_cxc>,
> > +				<&usb30_sleep_cxc>, <&usb30_mock_utmi_cxc>;
> > +		clock-names = "core_clk", "iface_clk", "sleep_clk", "utmi_clk";
> > +
> > +		gdsc-supply = <&supply>;
> > +		ranges;
> > +
> > +		dwc3@f9200000 {
> > +			compatible = "snps,dwc3";
> > +			reg = <0xf9200000 0xcd00>;
> > +			interrupts = <0 131 0>;
> > +			interrupt-names = "irq";
> 
> I'd drop interrupt-names since its not terribly meaningful w/a single IRQ

Ok.

Thanks, 
Ivan

> 
> > +			usb-phy = <&dwc3_hsphy>, <&dwc3_ssphy>;
> > +			tx-fifo-resize;
> > +		};
> > +	};
> > -- 
> > 1.7.9.5
> > 

> 


--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ivan T. Ivanov Aug. 13, 2013, 7:32 a.m. UTC | #4
Hi, 

On Mon, 2013-08-12 at 13:04 -0500, Felipe Balbi wrote: 
> On Fri, Aug 09, 2013 at 10:31:58AM -0500, Kumar Gala wrote:
> > 
> > On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote:
> > 
> > > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> > > 
> > > MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
> > 
> > probably good to spell out Synopsys rather than SNPS
> 
> Synopsys (the company) has always used snps in their bindings though.
> 
> > > +Required properities :
> > > +- compatible : sould be "qcom,dwc3-hsphy";
> > > +- reg : offset and length of the register set in the memory map
> > > +- clocks : phandles to clock instances of the device tree nodes
> > > +- clock-names :
> > > +	"xo" : External reference clock 19 MHz
> > > +	"sleep_a_clk" : Sleep clock, used when USB3 core goes into low
> > > +	power mode (U3).
> > > +<supply-name>-supply : phandle to the regulator device tree node
> > > +Required "supply-name" are:
> > > +	"v1p8" : 1.8v supply for HSPHY
> > > +	"v3p3" : 3.3v supply for HSPHY
> > > +	"vbus" : vbus supply for host mode
> > > +	"vddcx" : vdd supply for HS-PHY digital circuit operation
> 
> I believe these regulators belong to the PHY, not DWC3. Please write a
> PHY driver.
> 

There is already usb phy drivers for these?? 

[PATCH v2 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core

> > > +Required properities :
> > > +- compatible : sould be "qcom,dwc3-ssphy";
> > > +- reg : offset and length of the register set in the memory map
> > > +- clocks : phandles to clock instances of the device tree nodes
> > > +- clock-names :
> > > +	"xo" : External reference clock 19 MHz
> > > +	"ref_clk" : Reference clock - used in host mode.
> > > +<supply-name>-supply : phandle to the regulator device tree node
> > > +Required "supply-name" are:
> > > +	"v1p8" : 1.8v supply for SS-PHY
> > > +	"vddcx" : vdd supply for SS-PHY digital circuit operation
> 
> likewise, these regulators should be moved to the PHY driver.
> 
> > > +Required properties :
> > > +- compatible : should be "qcom,dwc3"
> > > +- reg : offset and length of the register set in the memory map
> > > +	offset and length of the TCSR register for routing USB
> > > +	signals to either picoPHY0 or picoPHY1.
> > > +- clocks : phandles to clock instances of the device tree nodes
> > > +- clock-names :
> > > +	"core_clk" : Master/Core clock, have to be >= 125 MHz for SS
> > > +	operation and >= 60MHz for HS operation
> > > +	"iface_clk" : System bus AXI clock
> > > +	"sleep_clk" : Sleep clock, used when USB3 core goes into low
> > > +	power mode (U3).
> > > +	"utmi_clk" : Generated by HS-PHY. Used to clock the low power
> > > +	parts of thr HS Link layer.
> > > +
> > > +Optional properties :
> > > +- gdsc-supply : phandle to the globally distributed switch controller
> > > +  regulator node to the USB controller.
> > > +
> > > +Sub nodes:
> > > +- Sub node for "DWC3 USB3 controller".
> > > +  This sub node is required property for device node. The properties
> > > +  of this subnode are specified in dwc3.txt.
> > 
> > Is tx-fifo-resize required for qcom,dwc3? if so we should call that
> > out in the binding.
> 
> AFAICT that's only needed for OMAP5 ES1.0. Unless Qualcomm also screwed
> up default TX FIFO sizes :-p

Or it is intentional? :-) Look at [1] dwc3@f9200000

Ivan

[1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/arch/arm/boot/dts/apq8084.dtsi?h=msm-3.4

> 


--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Aug. 13, 2013, 7:57 p.m. UTC | #5
On 08/09/2013 03:53 AM, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> 
> MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
> and HS, SS PHY's controll and configuration registers.

s/controll/control/

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

> diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt b/Documentation/devicetree/bindings/usb/msm-ssusb.txt

> +MSM SuperSpeed DWC3 USB SoC controller
> +
> +Required properities :
> +- compatible : sould be "qcom,dwc3-hsphy";
...
> +Required properities :
> +- compatible : sould be "qcom,dwc3-ssphy";

I would expect different compatible values to be documented in different
files.

> +Optional properties :
> +- gdsc-supply : phandle to the globally distributed switch controller
> +  regulator node to the USB controller.

Which of the 3 compatible values that were described above is that
property optional for?

> +Sub nodes:
> +- Sub node for "DWC3 USB3 controller".
> +  This sub node is required property for device node. The properties
> +  of this subnode are specified in dwc3.txt.

Why not represent the PHY and USB controller as separate top-level
nodes? They can point at each-other with phandles if they need to know
each-others' identity.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ivan T. Ivanov Aug. 14, 2013, 7:18 a.m. UTC | #6
Hi,

On Tue, 2013-08-13 at 13:57 -0600, Stephen Warren wrote: 
> On 08/09/2013 03:53 AM, Ivan T. Ivanov wrote:
> > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> > 
> > MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
> > and HS, SS PHY's controll and configuration registers.
> 
> s/controll/control/
> 

Thanks.

> > It could operate in device mode (SS, HS, FS) and host
> > mode (SS, HS, FS, LS).
> 
> > diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> 
> > +MSM SuperSpeed DWC3 USB SoC controller
> > +
> > +Required properities :
> > +- compatible : sould be "qcom,dwc3-hsphy";
> ...
> > +Required properities :
> > +- compatible : sould be "qcom,dwc3-ssphy";
> 
> I would expect different compatible values to be documented in different
> files.

It is easy to see connection between them when they are in
one file. Drivers are useless without each other. 

> 
> > +Optional properties :
> > +- gdsc-supply : phandle to the globally distributed switch controller
> > +  regulator node to the USB controller.
> 
> Which of the 3 compatible values that were described above is that
> property optional for?

"qcom,dwc3". I will make this more explicit. 

> 
> > +Sub nodes:
> > +- Sub node for "DWC3 USB3 controller".
> > +  This sub node is required property for device node. The properties
> > +  of this subnode are specified in dwc3.txt.
> 
> Why not represent the PHY and USB controller as separate top-level
> nodes? They can point at each-other with phandles if they need to know
> each-others' identity.

"qcom,dwc3" (glue layer) driver have to be loaded before "snps,dwc3",
actual USB3 IP. "qcom,dwc3" provide required clocks and power supplies
to the USB3 IP core.

Regards,
Ivan


--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
new file mode 100644
index 0000000..7a97163
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
@@ -0,0 +1,101 @@ 
+MSM SuperSpeed DWC3 USB SoC controller
+
+Required properities :
+- compatible : sould be "qcom,dwc3-hsphy";
+- reg : offset and length of the register set in the memory map
+- clocks : phandles to clock instances of the device tree nodes
+- clock-names :
+	"xo" : External reference clock 19 MHz
+	"sleep_a_clk" : Sleep clock, used when USB3 core goes into low
+	power mode (U3).
+<supply-name>-supply : phandle to the regulator device tree node
+Required "supply-name" are:
+	"v1p8" : 1.8v supply for HSPHY
+	"v3p3" : 3.3v supply for HSPHY
+	"vbus" : vbus supply for host mode
+	"vddcx" : vdd supply for HS-PHY digital circuit operation
+
+Required properities :
+- compatible : sould be "qcom,dwc3-ssphy";
+- reg : offset and length of the register set in the memory map
+- clocks : phandles to clock instances of the device tree nodes
+- clock-names :
+	"xo" : External reference clock 19 MHz
+	"ref_clk" : Reference clock - used in host mode.
+<supply-name>-supply : phandle to the regulator device tree node
+Required "supply-name" are:
+	"v1p8" : 1.8v supply for SS-PHY
+	"vddcx" : vdd supply for SS-PHY digital circuit operation
+
+Required properties :
+- compatible : should be "qcom,dwc3"
+- reg : offset and length of the register set in the memory map
+	offset and length of the TCSR register for routing USB
+	signals to either picoPHY0 or picoPHY1.
+- clocks : phandles to clock instances of the device tree nodes
+- clock-names :
+	"core_clk" : Master/Core clock, have to be >= 125 MHz for SS
+	operation and >= 60MHz for HS operation
+	"iface_clk" : System bus AXI clock
+	"sleep_clk" : Sleep clock, used when USB3 core goes into low
+	power mode (U3).
+	"utmi_clk" : Generated by HS-PHY. Used to clock the low power
+	parts of thr HS Link layer.
+
+Optional properties :
+- gdsc-supply : phandle to the globally distributed switch controller
+  regulator node to the USB controller.
+
+Sub nodes:
+- Sub node for "DWC3 USB3 controller".
+  This sub node is required property for device node. The properties
+  of this subnode are specified in dwc3.txt.
+
+Example device nodes:
+
+	dwc3_hsphy: phy@f92f8800 {
+		compatible = "qcom,dwc3-hsphy";
+		reg = <0xf92f8800 0x30>;
+
+		clocks = <&cxo>, <&usb2a_phy_sleep_cxc>;
+		clock-names = "xo", "sleep_a_clk";
+
+		vbus-supply = <&supply>;
+		vddcx-supply = <&supply>;
+		v1p8-supply = <&supply>;
+		v3p3-supply = <&supply>;
+	};
+
+	dwc3_ssphy: phy@f92f8830 {
+		compatible = "qcom,dwc3-ssphy";
+		reg = <0xf92f8830 0x30>;
+
+		clocks = <&cxo>, <&usb30_mock_utmi_cxc>;
+		clock-names = "xo", "ref_clk";
+
+		vddcx-supply = <&supply>;
+		v1p8-supply = <&supply>;
+	};
+
+	usb@fd4ab000 {
+		compatible = "qcom,dwc3";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0xfd4ab000 0x4>;
+
+		clocks = <&usb30_master_cxc>, <&sys_noc_usb3_axi_cxc>,
+				<&usb30_sleep_cxc>, <&usb30_mock_utmi_cxc>;
+		clock-names = "core_clk", "iface_clk", "sleep_clk", "utmi_clk";
+
+		gdsc-supply = <&supply>;
+		ranges;
+
+		dwc3@f9200000 {
+			compatible = "snps,dwc3";
+			reg = <0xf9200000 0xcd00>;
+			interrupts = <0 131 0>;
+			interrupt-names = "irq";
+			usb-phy = <&dwc3_hsphy>, <&dwc3_ssphy>;
+			tx-fifo-resize;
+		};
+	};