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 <redacted>
Cc: gregkh@linuxfoundation.org; Michal Simek <redacted>;
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 [off-list ref]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: dwc3-xilinx: Add
documentation for Versal DWC3 Controller

Rob Herring [off-list ref] writes:

quoted

On Thu, Sep 10, 2020 at 12:33:04AM +0530, Manish Narani wrote:

quoted

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 <redacted>
---
 .../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

quoted

quoted

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"

quoted

quoted

+- 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

quoted

quoted

+		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