[RFC,v2,4/5] dt-bindings: usb: dwc3: of-simple: add compatible for HiSi

Message ID 20191007175553.66940-5-john.stultz@linaro.org
State New
Headers show
Series
  • dwc3: Changes for HiKey960 support
Related show

Commit Message

John Stultz Oct. 7, 2019, 5:55 p.m.
Add necessary compatible flag for HiSi's DWC3 so
dwc3-of-simple will probe.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
v2: Tweaked clock names as clk_usb3phy_ref didn't seem right.
---
 .../devicetree/bindings/usb/hisi,dwc3.txt     | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/hisi,dwc3.txt

-- 
2.17.1

Comments

John Stultz Oct. 7, 2019, 11 p.m. | #1
On Mon, Oct 7, 2019 at 2:11 PM Rob Herring <robh+dt@kernel.org> wrote:
>

> On Mon, Oct 7, 2019 at 2:07 PM John Stultz <john.stultz@linaro.org> wrote:

> >

> > On Mon, Oct 7, 2019 at 11:38 AM Rob Herring <robh+dt@kernel.org> wrote:

> > >

> > > On Mon, Oct 7, 2019 at 12:56 PM John Stultz <john.stultz@linaro.org> wrote:

> > > >

> > > > Add necessary compatible flag for HiSi's DWC3 so

> > > > dwc3-of-simple will probe.

> > > >

> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> > > > Cc: Felipe Balbi <balbi@kernel.org>

> > > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>

> > > > Cc: Rob Herring <robh+dt@kernel.org>

> > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > Cc: Yu Chen <chenyu56@huawei.com>

> > > > Cc: Matthias Brugger <matthias.bgg@gmail.com>

> > > > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>

> > > > Cc: linux-usb@vger.kernel.org

> > > > Cc: devicetree@vger.kernel.org

> > > > Signed-off-by: John Stultz <john.stultz@linaro.org>

> > > > ---

> > > > v2: Tweaked clock names as clk_usb3phy_ref didn't seem right.

> > > > ---

> > > >  .../devicetree/bindings/usb/hisi,dwc3.txt     | 52 +++++++++++++++++++

> > > >  1 file changed, 52 insertions(+)

> > > >  create mode 100644 Documentation/devicetree/bindings/usb/hisi,dwc3.txt

> > >

> > > Can you make this a schema.

> >

> > Sorry, I'm not sure exactly what you're asking. I'm guessing from

> > grepping around you want the bindings in yaml instead (I see a few

> > examples)?

>

> Yes.

>

> > Is there some pointer to documentation? The

> > Documentation/devicetree/bindings/writing-bindings.txt file doesn't

> > seem to say much on it.

>

> You mean Documentation/devicetree/writing-schemas.rst? There's that

> and Documentation/devicetree/bindings/example-schema.yaml which has a

> bunch of annotations on what each part means.


Ah! Sorry for missing that. Thanks for the pointer, though I may get
away with dropping this one.

> > > If it's only clocks and resets for the wrapper node, just make this

> > > all one node.

> >

> > Just to make sure I'm following, you're suggesting I put all the

> > clocks/resets in the dwc3 node (renamed to usb for the node name) and

> > not add the wrapper?

>

> Yes.

>

> > I'll have to see if that's possible. The generic dwc3 binding wants 3

> > clocks, but I only have two in the code I've worked with (similarly it

> > seems to only want two resets, not 4) so I'll have to see if I can

> > figure out how to adapt that.

>

> Possible since commit fe8abf332b8f ("usb: dwc3: support clocks and

> resets for DWC3 core").


Ok. It *seems* like I can get it working with the existing binding
then. There's a little funkiness with the core expecting three clocks
while I only have two (currently I'm duplicating the "bus_early" clk
for "suspend". Is there a preferred way to do this sort of hack?), and
I'm a little worried that only the first reset is being used (instead
of the 4 specified), but it seems to work so far.

I still suspect the dwc3-of-simple.c method might be better since it
can handle arbitrary sets of clks and resets instead of the fixed 3 &
1 in the dwc3.txt binding.
But if I can get away without having to add an extra binding here, I'd
be happier.

thanks
-john
Rob Herring Oct. 11, 2019, 3:51 p.m. | #2
On Mon, Oct 07, 2019 at 04:00:24PM -0700, John Stultz wrote:
> On Mon, Oct 7, 2019 at 2:11 PM Rob Herring <robh+dt@kernel.org> wrote:

> >

> > On Mon, Oct 7, 2019 at 2:07 PM John Stultz <john.stultz@linaro.org> wrote:

> > >

> > > On Mon, Oct 7, 2019 at 11:38 AM Rob Herring <robh+dt@kernel.org> wrote:

> > > >

> > > > On Mon, Oct 7, 2019 at 12:56 PM John Stultz <john.stultz@linaro.org> wrote:

> > > > >

> > > > > Add necessary compatible flag for HiSi's DWC3 so

> > > > > dwc3-of-simple will probe.

> > > > >

> > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> > > > > Cc: Felipe Balbi <balbi@kernel.org>

> > > > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>

> > > > > Cc: Rob Herring <robh+dt@kernel.org>

> > > > > Cc: Mark Rutland <mark.rutland@arm.com>

> > > > > Cc: Yu Chen <chenyu56@huawei.com>

> > > > > Cc: Matthias Brugger <matthias.bgg@gmail.com>

> > > > > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>

> > > > > Cc: linux-usb@vger.kernel.org

> > > > > Cc: devicetree@vger.kernel.org

> > > > > Signed-off-by: John Stultz <john.stultz@linaro.org>

> > > > > ---

> > > > > v2: Tweaked clock names as clk_usb3phy_ref didn't seem right.

> > > > > ---

> > > > >  .../devicetree/bindings/usb/hisi,dwc3.txt     | 52 +++++++++++++++++++

> > > > >  1 file changed, 52 insertions(+)

> > > > >  create mode 100644 Documentation/devicetree/bindings/usb/hisi,dwc3.txt

> > > >

> > > > Can you make this a schema.

> > >

> > > Sorry, I'm not sure exactly what you're asking. I'm guessing from

> > > grepping around you want the bindings in yaml instead (I see a few

> > > examples)?

> >

> > Yes.

> >

> > > Is there some pointer to documentation? The

> > > Documentation/devicetree/bindings/writing-bindings.txt file doesn't

> > > seem to say much on it.

> >

> > You mean Documentation/devicetree/writing-schemas.rst? There's that

> > and Documentation/devicetree/bindings/example-schema.yaml which has a

> > bunch of annotations on what each part means.

> 

> Ah! Sorry for missing that. Thanks for the pointer, though I may get

> away with dropping this one.

> 

> > > > If it's only clocks and resets for the wrapper node, just make this

> > > > all one node.

> > >

> > > Just to make sure I'm following, you're suggesting I put all the

> > > clocks/resets in the dwc3 node (renamed to usb for the node name) and

> > > not add the wrapper?

> >

> > Yes.

> >

> > > I'll have to see if that's possible. The generic dwc3 binding wants 3

> > > clocks, but I only have two in the code I've worked with (similarly it

> > > seems to only want two resets, not 4) so I'll have to see if I can

> > > figure out how to adapt that.

> >

> > Possible since commit fe8abf332b8f ("usb: dwc3: support clocks and

> > resets for DWC3 core").

> 

> Ok. It *seems* like I can get it working with the existing binding

> then. There's a little funkiness with the core expecting three clocks

> while I only have two (currently I'm duplicating the "bus_early" clk

> for "suspend". Is there a preferred way to do this sort of hack?), and

> I'm a little worried that only the first reset is being used (instead

> of the 4 specified), but it seems to work so far.


I would assume that you simply don't know how the 'suspend' clock is 
connected rather than you don't have one. But that's maybe not a 
problem you can fix.

I would make dwc3 use devm_clk_bulk_get_all and allow for less than 3 
clocks. And do a similar change for resets.


> I still suspect the dwc3-of-simple.c method might be better since it

> can handle arbitrary sets of clks and resets instead of the fixed 3 &

> 1 in the dwc3.txt binding.

> But if I can get away without having to add an extra binding here, I'd

> be happier.


Me too.

Rob

Patch

diff --git a/Documentation/devicetree/bindings/usb/hisi,dwc3.txt b/Documentation/devicetree/bindings/usb/hisi,dwc3.txt
new file mode 100644
index 000000000000..3a3e5c320f2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/hisi,dwc3.txt
@@ -0,0 +1,52 @@ 
+HiSi SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:		should contain "hisilicon,hi3660-dwc3" for HiSi SoC
+- clocks:		A list of phandle + clock-specifier pairs for the
+			clocks listed in clock-names
+- clock-names:		Should contain the following:
+  "clk_abb_usb"		USB reference clk
+  "aclk_usb3otg"	USB3 OTG aclk
+
+- assigned-clocks:	Should be:
+				HI3660_ACLK_GATE_USB3OTG
+- assigned-clock-rates: Should be:
+				229Mhz (229000000) for HI3660_ACLK_GATE_USB3OTG
+
+Optional properties:
+- resets:		Phandle to reset control that resets core and wrapper.
+
+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 nodes:
+
+	usb3: hisi_dwc3 {
+		compatible = "hisilicon,hi3660-dwc3";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
+			 <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+		clock-names = "clk_abb_usb", "aclk_usb3otg";
+
+		assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+		assigned-clock-rates = <229 000 000>;
+		resets = <&crg_rst 0x90 8>,
+			 <&crg_rst 0x90 7>,
+			 <&crg_rst 0x90 6>,
+			 <&crg_rst 0x90 5>;
+
+		dwc3: dwc3@ff100000 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xff100000 0x0 0x100000>;
+			interrupts = <0 159 4>, <0 161 4>;
+			phys = <&usb_phy>;
+			phy-names = "usb3-phy";
+			dr_mode = "otg";
+
+			...
+		};
+	};