| Message ID | 1599678185-119412-2-git-send-email-manish.narani@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a separate DWC3 OF driver for Xilinx platforms | expand |
On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote: > Add documentation for Versal DWC3 controller. Add required property > 'reg' for the same. Also add optional properties for snps,dwc3. > > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > --- > .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > index 4aae5b2cef56..219b5780dbee 100644 > --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > @@ -1,7 +1,8 @@ > Xilinx SuperSpeed DWC3 USB SoC controller > > Required properties: > -- compatible: Should contain "xlnx,zynqmp-dwc3" > +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal-dwc3" > +- reg: Base address and length of the register control block > - 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 > @@ -13,12 +14,24 @@ 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. > > +Optional properties for snps,dwc3: > +- dma-coherent: Enable this flag if CCI is enabled in design. Adding this > + flag configures Global SoC bus Configuration Register and > + Xilinx USB 3.0 IP - USB coherency register to enable CCI. > +- snps,enable-hibernation: Add this flag to enable hibernation support for > + peripheral mode. This belongs in the DWC3 binding. It also implies that hibernation is not supported by any other DWC3 based platform. Can't this be implied by the compatible string (in the parent)? > +- interrupt-names: Should contain the following: > + "dwc_usb3" USB gadget mode interrupts > + "otg" USB OTG mode interrupts > + "hiber" USB hibernation interrupts > + > Example device node: > > usb@0 { > #address-cells = <0x2>; > #size-cells = <0x1>; > compatible = "xlnx,zynqmp-dwc3"; > + reg = <0x0 0xff9d0000 0x0 0x100>; > clock-names = "bus_clk" "ref_clk"; > clocks = <&clk125>, <&clk125>; > ranges; > @@ -26,7 +39,10 @@ Example device node: > dwc3@fe200000 { > compatible = "snps,dwc3"; > reg = <0x0 0xfe200000 0x40000>; > - interrupts = <0x0 0x41 0x4>; > + interrupt-names = "dwc_usb3", "otg", "hiber"; > + interrupts = <0 65 4>, <0 69 4>, <0 75 4>; > dr_mode = "host"; > + dma-coherent; > + snps,enable-hibernation; > }; > }; > -- > 2.17.1 >
Rob Herring <robh@kernel.org> writes: > On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote: >> Add documentation for Versal DWC3 controller. Add required property >> 'reg' for the same. Also add optional properties for snps,dwc3. >> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> >> --- >> .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +++++++++++++++++-- >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> index 4aae5b2cef56..219b5780dbee 100644 >> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> @@ -1,7 +1,8 @@ >> Xilinx SuperSpeed DWC3 USB SoC controller >> >> Required properties: >> -- compatible: Should contain "xlnx,zynqmp-dwc3" >> +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal-dwc3" >> +- reg: Base address and length of the register control block >> - 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 >> @@ -13,12 +14,24 @@ 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. >> >> +Optional properties for snps,dwc3: >> +- dma-coherent: Enable this flag if CCI is enabled in design. Adding this >> + flag configures Global SoC bus Configuration Register and >> + Xilinx USB 3.0 IP - USB coherency register to enable CCI. >> +- snps,enable-hibernation: Add this flag to enable hibernation support for >> + peripheral mode. > > This belongs in the DWC3 binding. It also implies that hibernation is > not supported by any other DWC3 based platform. Can't this be implied by > the compatible string (in the parent)? hibernation support is detectable in runtime, and we've been using that.
Hi Rob/Felipe, Thanks for the review. > -----Original Message----- > From: Felipe Balbi <balbi@kernel.org> > Sent: Thursday, September 24, 2020 12:47 PM > To: Rob Herring <robh@kernel.org>; Manish Narani <MNARANI@xilinx.com> > Cc: gregkh@linuxfoundation.org; Michal Simek <michals@xilinx.com>; > p.zabel@pengutronix.de; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; git <git@xilinx.com> > Subject: Re: [PATCH v2 1/2] dt-bindings: usb: dwc3-xilinx: Add > documentation for Versal DWC3 Controller > > Rob Herring <robh@kernel.org> writes: > > > On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote: > >> Add documentation for Versal DWC3 controller. Add required property > >> 'reg' for the same. Also add optional properties for snps,dwc3. > >> > >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> > >> --- > >> .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +++++++++++++++++-- > >> 1 file changed, 18 insertions(+), 2 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> index 4aae5b2cef56..219b5780dbee 100644 > >> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> @@ -1,7 +1,8 @@ > >> Xilinx SuperSpeed DWC3 USB SoC controller > >> > >> Required properties: > >> -- compatible: Should contain "xlnx,zynqmp-dwc3" > >> +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal- > dwc3" > >> +- reg: Base address and length of the register control block > >> - 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 > >> @@ -13,12 +14,24 @@ 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. > >> > >> +Optional properties for snps,dwc3: > >> +- dma-coherent: Enable this flag if CCI is enabled in design. Adding this > >> + flag configures Global SoC bus Configuration Register and > >> + Xilinx USB 3.0 IP - USB coherency register to enable CCI. > >> +- snps,enable-hibernation: Add this flag to enable hibernation support > for > >> + peripheral mode. > > > > This belongs in the DWC3 binding. It also implies that hibernation is > > not supported by any other DWC3 based platform. Can't this be implied by > > the compatible string (in the parent)? Rob, We can move this to dwc3 bindings. If Felipe is okay with below response. > > hibernation support is detectable in runtime, and we've been using that. Felipe, Yes, this flag is to control the enable/disable hibernation. I did not see has_hibernation flag being set anywhere in the driver. Can we control the hibernation enable/disable through DT entry? See below: ----- diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 2eb34c8b4065..1baf44d8d566 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -769,8 +769,15 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc) reg &= ~DWC3_GCTL_DSBLCLKGTNG; break; case DWC3_GHWPARAMS1_EN_PWROPT_HIB: - /* enable hibernation here */ - dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); + if (!device_property_read_bool(dwc->dev, + "snps,enable-hibernation")) { + dev_dbg(dwc->dev, "Hibernation not enabled\n"); + } else { + /* enable hibernation here */ + dwc->nr_scratch = + DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); + dwc->has_hibernation = 1; + } /* * REVISIT Enabling this bit so that host-mode hibernation ----- Please provide your inputs. Thanks, Manish
Hi, Manish Narani <MNARANI@xilinx.com> writes: > Hi Rob/Felipe, > > Thanks for the review. > >> -----Original Message----- >> From: Felipe Balbi <balbi@kernel.org> >> Sent: Thursday, September 24, 2020 12:47 PM >> To: Rob Herring <robh@kernel.org>; Manish Narani <MNARANI@xilinx.com> >> Cc: gregkh@linuxfoundation.org; Michal Simek <michals@xilinx.com>; >> p.zabel@pengutronix.de; linux-usb@vger.kernel.org; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >> kernel@vger.kernel.org; git <git@xilinx.com> >> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: dwc3-xilinx: Add >> documentation for Versal DWC3 Controller >> >> Rob Herring <robh@kernel.org> writes: >> >> > On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote: >> >> Add documentation for Versal DWC3 controller. Add required property >> >> 'reg' for the same. Also add optional properties for snps,dwc3. >> >> >> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> >> >> --- >> >> .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +++++++++++++++++-- >> >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> >> index 4aae5b2cef56..219b5780dbee 100644 >> >> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> >> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt >> >> @@ -1,7 +1,8 @@ >> >> Xilinx SuperSpeed DWC3 USB SoC controller >> >> >> >> Required properties: >> >> -- compatible: Should contain "xlnx,zynqmp-dwc3" >> >> +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal- >> dwc3" >> >> +- reg: Base address and length of the register control block >> >> - 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 >> >> @@ -13,12 +14,24 @@ 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. >> >> >> >> +Optional properties for snps,dwc3: >> >> +- dma-coherent: Enable this flag if CCI is enabled in design. Adding this >> >> + flag configures Global SoC bus Configuration Register and >> >> + Xilinx USB 3.0 IP - USB coherency register to enable CCI. >> >> +- snps,enable-hibernation: Add this flag to enable hibernation support >> for >> >> + peripheral mode. >> > >> > This belongs in the DWC3 binding. It also implies that hibernation is >> > not supported by any other DWC3 based platform. Can't this be implied by >> > the compatible string (in the parent)? > > Rob, We can move this to dwc3 bindings. If Felipe is okay with below response. > >> >> hibernation support is detectable in runtime, and we've been using that. > > Felipe, Yes, this flag is to control the enable/disable hibernation. > I did not see has_hibernation flag being set anywhere in the driver. > Can we control the hibernation enable/disable through DT entry? See below: > ----- > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 2eb34c8b4065..1baf44d8d566 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -769,8 +769,15 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc) > reg &= ~DWC3_GCTL_DSBLCLKGTNG; > break; > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > - /* enable hibernation here */ > - dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > + if (!device_property_read_bool(dwc->dev, > + "snps,enable-hibernation")) { > + dev_dbg(dwc->dev, "Hibernation not enabled\n"); > + } else { > + /* enable hibernation here */ > + dwc->nr_scratch = > + DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > + dwc->has_hibernation = 1; > + } I left it off because I didn't have HW to validate. Don't add a new binding for this. Set has_hibernation to true and make sure it works. Then send me a patch that sets has_hibernation to true whenever DWC3_GHWPARAMS1_EN_PWROPT_HIB is valid.
Hi Felipe, > -----Original Message----- > From: Felipe Balbi <balbi@kernel.org> > Sent: Friday, September 25, 2020 12:42 PM > To: Manish Narani <MNARANI@xilinx.com>; Rob Herring <robh@kernel.org> > Cc: gregkh@linuxfoundation.org; Michal Simek <michals@xilinx.com>; > p.zabel@pengutronix.de; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; git <git@xilinx.com> > Subject: RE: [PATCH v2 1/2] dt-bindings: usb: dwc3-xilinx: Add > documentation for Versal DWC3 Controller > > > Hi, > > Manish Narani <MNARANI@xilinx.com> writes: > > Hi Rob/Felipe, > > > > Thanks for the review. > > > >> -----Original Message----- > >> From: Felipe Balbi <balbi@kernel.org> > >> Sent: Thursday, September 24, 2020 12:47 PM > >> To: Rob Herring <robh@kernel.org>; Manish Narani > <MNARANI@xilinx.com> > >> Cc: gregkh@linuxfoundation.org; Michal Simek <michals@xilinx.com>; > >> p.zabel@pengutronix.de; linux-usb@vger.kernel.org; > >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > >> kernel@vger.kernel.org; git <git@xilinx.com> > >> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: dwc3-xilinx: Add > >> documentation for Versal DWC3 Controller > >> > >> Rob Herring <robh@kernel.org> writes: > >> > >> > On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote: > >> >> Add documentation for Versal DWC3 controller. Add required property > >> >> 'reg' for the same. Also add optional properties for snps,dwc3. > >> >> > >> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> > >> >> --- > >> >> .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +++++++++++++++++- > - > >> >> 1 file changed, 18 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> >> index 4aae5b2cef56..219b5780dbee 100644 > >> >> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> >> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt > >> >> @@ -1,7 +1,8 @@ > >> >> Xilinx SuperSpeed DWC3 USB SoC controller > >> >> > >> >> Required properties: > >> >> -- compatible: Should contain "xlnx,zynqmp-dwc3" > >> >> +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal- > >> dwc3" > >> >> +- reg: Base address and length of the register control block > >> >> - 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 > >> >> @@ -13,12 +14,24 @@ 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. > >> >> > >> >> +Optional properties for snps,dwc3: > >> >> +- dma-coherent: Enable this flag if CCI is enabled in design. Adding this > >> >> + flag configures Global SoC bus Configuration Register and > >> >> + Xilinx USB 3.0 IP - USB coherency register to enable CCI. > >> >> +- snps,enable-hibernation: Add this flag to enable hibernation support > >> for > >> >> + peripheral mode. > >> > > >> > This belongs in the DWC3 binding. It also implies that hibernation is > >> > not supported by any other DWC3 based platform. Can't this be implied > by > >> > the compatible string (in the parent)? > > > > Rob, We can move this to dwc3 bindings. If Felipe is okay with below > response. > > > >> > >> hibernation support is detectable in runtime, and we've been using that. > > > > Felipe, Yes, this flag is to control the enable/disable hibernation. > > I did not see has_hibernation flag being set anywhere in the driver. > > Can we control the hibernation enable/disable through DT entry? See > below: > > ----- > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 2eb34c8b4065..1baf44d8d566 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -769,8 +769,15 @@ static void dwc3_core_setup_global_control(struct > dwc3 *dwc) > > reg &= ~DWC3_GCTL_DSBLCLKGTNG; > > break; > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > - /* enable hibernation here */ > > - dwc->nr_scratch = > DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > + if (!device_property_read_bool(dwc->dev, > > + "snps,enable-hibernation")) { > > + dev_dbg(dwc->dev, "Hibernation not enabled\n"); > > + } else { > > + /* enable hibernation here */ > > + dwc->nr_scratch = > > + > DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > + dwc->has_hibernation = 1; > > + } > > I left it off because I didn't have HW to validate. Don't add a new > binding for this. Set has_hibernation to true and make sure it > works. Then send me a patch that sets has_hibernation to true whenever > DWC3_GHWPARAMS1_EN_PWROPT_HIB is valid. OK Felipe. I will remove this property from binding. We have validated Device-mode hibernation on Xilinx ZynqMP and Versal platform. I am planning to send a separate patch series for hibernation after this. Thanks, Manish
diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt index 4aae5b2cef56..219b5780dbee 100644 --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt @@ -1,7 +1,8 @@ Xilinx SuperSpeed DWC3 USB SoC controller Required properties: -- compatible: Should contain "xlnx,zynqmp-dwc3" +- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal-dwc3" +- reg: Base address and length of the register control block - 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 @@ -13,12 +14,24 @@ 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. +Optional properties for snps,dwc3: +- dma-coherent: Enable this flag if CCI is enabled in design. Adding this + flag configures Global SoC bus Configuration Register and + Xilinx USB 3.0 IP - USB coherency register to enable CCI. +- snps,enable-hibernation: Add this flag to enable hibernation support for + peripheral mode. +- interrupt-names: Should contain the following: + "dwc_usb3" USB gadget mode interrupts + "otg" USB OTG mode interrupts + "hiber" USB hibernation interrupts + Example device node: usb@0 { #address-cells = <0x2>; #size-cells = <0x1>; compatible = "xlnx,zynqmp-dwc3"; + reg = <0x0 0xff9d0000 0x0 0x100>; clock-names = "bus_clk" "ref_clk"; clocks = <&clk125>, <&clk125>; ranges; @@ -26,7 +39,10 @@ Example device node: dwc3@fe200000 { compatible = "snps,dwc3"; reg = <0x0 0xfe200000 0x40000>; - interrupts = <0x0 0x41 0x4>; + interrupt-names = "dwc_usb3", "otg", "hiber"; + interrupts = <0 65 4>, <0 69 4>, <0 75 4>; dr_mode = "host"; + dma-coherent; + snps,enable-hibernation; }; };
Add documentation for Versal DWC3 controller. Add required property 'reg' for the same. Also add optional properties for snps,dwc3. Signed-off-by: Manish Narani <manish.narani@xilinx.com> --- .../devicetree/bindings/usb/dwc3-xilinx.txt | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)