[PATCH v2 0/5] Xilinx ZynqMP USB fixes

[PATCH v2 0/5] Xilinx ZynqMP USB fixes

[Robert Hancock]

Some fixes related to the DWC3 USB driver and Xilinx ZynqMP DWC3
wrapper driver to allow ZynqMP USB to work properly when the hardware
is configured in USB 2.0-only mode.

Changes since v1:
-added DT binding documentation for new attribute
-kerneldoc formatting and reworded comments

Robert Hancock (5):
  usb: dwc3: xilinx: Fix PIPE clock selection for USB2.0 mode
  usb: dwc3: xilinx: Fix error handling when getting USB3 PHY
  dt-bindings: usb: dwc3: add reference clock period fractional
    adjustment
  usb: dwc3: add reference clock FLADJ configuration
  arm64: dts: zynqmp: Add DWC3 USB reference clock period configuration

 .../devicetree/bindings/usb/snps,dwc3.yaml    | 12 +++++++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |  4 +++
 drivers/usb/dwc3/core.c                       | 35 +++++++++++++++++++
 drivers/usb/dwc3/core.h                       |  3 ++
 drivers/usb/dwc3/dwc3-xilinx.c                | 17 +++++----
 5 files changed, 64 insertions(+), 7 deletions(-)

-- 
2.31.1

[PATCH v2 1/5] usb: dwc3: xilinx: Fix PIPE clock selection for USB2.0 mode

[Robert Hancock]

It appears that the PIPE clock should not be selected when only USB 2.0
is being used in the design and no USB 3.0 reference clock is used. Fix
to set the correct value depending on whether a USB3 PHY is present.

Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/usb/dwc3/dwc3-xilinx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index 9cc3ad701a29..3bc035376394 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -167,8 +167,11 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 	/* Set PIPE Power Present signal in FPD Power Present Register*/
 	writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
 
-	/* Set the PIPE Clock Select bit in FPD PIPE Clock register */
-	writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
+	/* Set the PIPE Clock Select bit in FPD PIPE Clock register if a USB3
+	 * PHY is in use, deselect otherwise
+	 */
+	writel(usb3_phy ? PIPE_CLK_SELECT : PIPE_CLK_DESELECT,
+	       priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
 
 	ret = reset_control_deassert(crst);
 	if (ret < 0) {
-- 
2.31.1

[PATCH v2 2/5] usb: dwc3: xilinx: Fix error handling when getting USB3 PHY

[Robert Hancock]

The code that looked up the USB3 PHY was ignoring all errors other than
EPROBE_DEFER in an attempt to handle the PHY not being present. Fix and
simplify the code by using devm_phy_optional_get and dev_err_probe so
that a missing PHY is not treated as an error and unexpected errors
are handled properly.

Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/usb/dwc3/dwc3-xilinx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index 3bc035376394..3b16e7610009 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -102,12 +102,12 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
 	int			ret;
 	u32			reg;
 
-	usb3_phy = devm_phy_get(dev, "usb3-phy");
-	if (PTR_ERR(usb3_phy) == -EPROBE_DEFER) {
-		ret = -EPROBE_DEFER;
+	usb3_phy = devm_phy_optional_get(dev, "usb3-phy");
+	if (IS_ERR(usb3_phy)) {
+		ret = PTR_ERR(usb3_phy);
+		dev_err_probe(dev, ret,
+			      "failed to get USB3 PHY\n");
 		goto err;
-	} else if (IS_ERR(usb3_phy)) {
-		usb3_phy = NULL;
 	}
 
 	crst = devm_reset_control_get_exclusive(dev, "usb_crst");
-- 
2.31.1

[PATCH v2 3/5] dt-bindings: usb: dwc3: add reference clock period fractional adjustment

[Robert Hancock]

Document the new snps,ref-clock-fladj property which can be used to set
the fractional portion of the reference clock period.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 25ac2c93dc6c..88d8d831e04a 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -260,6 +260,18 @@ properties:
     minimum: 1
     maximum: 0x3ff
 
+  snps,ref-clock-fladj:
+    description:
+      Value for GFLADJ_REFCLK_FLADJ field of GFLADJ register for the
+      fractional portion of the reference clock period in nanoseconds,
+      when the hardware set default does not match the actual
+      clock. Calculated via
+      ((125000/ref_clk_period_integer)-(125000/ref_clk_period)) * ref_clk_period
+      where ref_clk_period_integer is the period specified in GUCTL_REFCLKPER and
+      ref_clk_period is the period including fractional value.
+    minimum: 0
+    maximum: 124999
+
   snps,rx-thr-num-pkt-prd:
     description:
       Periodic ESS RX packet threshold count (host mode only). Set this and
-- 
2.31.1

[PATCH v2 4/5] usb: dwc3: add reference clock FLADJ configuration

[Robert Hancock]

Previously a device tree property was added to allow overriding the
reference clock period parameter if the default value used was incorrect.
However, there is another register field, GFLADJ_REFCLK_FLADJ, which
reflects the fractional nanosecond portion of the reference clock
period. Add a snps,ref-clock-fladj property to allow configuring this
as well.

On the Xilinx ZynqMP platform, the reference clock appears to always
be 20 MHz, giving a clock period of 50 ns. However, the default value
of GFLADJ_REFCLK_FLADJ was 1008 rather than 0 as it should have been,
which prevented many USB devices from functioning properly. The
psu_init code run by the Xilinx first-stage boot loader sets this
value to 0, however when the controller is reset by the dwc3-xilinx
layer, the incorrect default value is restored. This configuration
property allows ensuring that the correct value is always used.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/usb/dwc3/core.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 38 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f4c09951b517..ea11fd1e3b42 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -359,6 +359,37 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
 }
 
 
+/**
+ * dwc3_ref_clk_fladj - Reference clock period adjustment configuration
+ * GFLADJ_REFCLK_FLADJ should be set based on the fractional portion of the
+ * reference clock period, where the integer portion is set in GUCTL_REFCLKPER.
+ * Calculated as: ((125000/ref_clk_period_integer)-(125000/ref_clk_period)) * ref_clk_period
+ * where ref_clk_period_integer is the period specified in GUCTL_REFCLKPER and
+ * ref_clk_period is the period including fractional value.
+ * This value can be specified in the device tree if the default value is incorrect.
+ * Note that 0 is a valid value.
+ *
+ * @dwc3: Pointer to our controller context structure
+ */
+static void dwc3_ref_clk_fladj(struct dwc3 *dwc)
+{
+	u32 reg;
+	u32 reg_new;
+
+	if (DWC3_VER_IS_PRIOR(DWC3, 250A))
+		return;
+
+	if (!dwc->ref_clk_fladj_set)
+		return;
+
+	reg = dwc3_readl(dwc->regs, DWC3_GFLADJ);
+	reg_new = reg & ~DWC3_GFLADJ_REFCLK_FLADJ_MASK;
+	reg_new |= FIELD_PREP(DWC3_GFLADJ_REFCLK_FLADJ_MASK, dwc->ref_clk_fladj);
+	if (reg_new != reg)
+		dwc3_writel(dwc->regs, DWC3_GFLADJ, reg_new);
+}
+
+
 /**
  * dwc3_free_one_event_buffer - Frees one event buffer
  * @dwc: Pointer to our controller context structure
@@ -1033,6 +1064,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	/* Adjust Reference Clock Period */
 	dwc3_ref_clk_period(dwc);
+	dwc3_ref_clk_fladj(dwc);
 
 	dwc3_set_incr_burst_type(dwc);
 
@@ -1418,6 +1450,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				 &dwc->fladj);
 	device_property_read_u32(dev, "snps,ref-clock-period-ns",
 				 &dwc->ref_clk_per);
+	if (!device_property_read_u32(dev, "snps,ref-clock-fladj",
+				      &dwc->ref_clk_fladj))
+		dwc->ref_clk_fladj_set = true;
 
 	dwc->dis_metastability_quirk = device_property_read_bool(dev,
 				"snps,dis_metastability_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e1cc3f7398fb..650d4c2e7a67 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -388,6 +388,7 @@
 /* Global Frame Length Adjustment Register */
 #define DWC3_GFLADJ_30MHZ_SDBND_SEL		BIT(7)
 #define DWC3_GFLADJ_30MHZ_MASK			0x3f
+#define DWC3_GFLADJ_REFCLK_FLADJ_MASK		0x3fff00
 
 /* Global User Control Register*/
 #define DWC3_GUCTL_REFCLKPER_MASK		0xffc00000
@@ -1166,6 +1167,8 @@ struct dwc3 {
 
 	u32			fladj;
 	u32			ref_clk_per;
+	bool			ref_clk_fladj_set;
+	u32			ref_clk_fladj;
 	u32			irq_gadget;
 	u32			otg_irq;
 	u32			current_otg_role;
-- 
2.31.1

Re: [PATCH v2 4/5] usb: dwc3: add reference clock FLADJ configuration

[Thinh Nguyen]

Robert Hancock wrote:
> Previously a device tree property was added to allow overriding the
> reference clock period parameter if the default value used was incorrect.
> However, there is another register field, GFLADJ_REFCLK_FLADJ, which
> reflects the fractional nanosecond portion of the reference clock
> period. Add a snps,ref-clock-fladj property to allow configuring this
> as well.
> 
> On the Xilinx ZynqMP platform, the reference clock appears to always
> be 20 MHz, giving a clock period of 50 ns. However, the default value
> of GFLADJ_REFCLK_FLADJ was 1008 rather than 0 as it should have been,
> which prevented many USB devices from functioning properly. The
> psu_init code run by the Xilinx first-stage boot loader sets this
> value to 0, however when the controller is reset by the dwc3-xilinx
> layer, the incorrect default value is restored. This configuration
> property allows ensuring that the correct value is always used.
> 
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>  drivers/usb/dwc3/core.c | 35 +++++++++++++++++++++++++++++++++++
>  drivers/usb/dwc3/core.h |  3 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index f4c09951b517..ea11fd1e3b42 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -359,6 +359,37 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
>  }
>  
>  
> +/**
> + * dwc3_ref_clk_fladj - Reference clock period adjustment configuration
> + * GFLADJ_REFCLK_FLADJ should be set based on the fractional portion of the
> + * reference clock period, where the integer portion is set in GUCTL_REFCLKPER.
> + * Calculated as: ((125000/ref_clk_period_integer)-(125000/ref_clk_period)) * ref_clk_period
> + * where ref_clk_period_integer is the period specified in GUCTL_REFCLKPER and
> + * ref_clk_period is the period including fractional value.
> + * This value can be specified in the device tree if the default value is incorrect.
> + * Note that 0 is a valid value.

Minor nit. Can you follow the kernel-doc format for brief/long description

https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html

> + *
> + * @dwc3: Pointer to our controller context structure
> + */
> +static void dwc3_ref_clk_fladj(struct dwc3 *dwc)
> +{
> +	u32 reg;
> +	u32 reg_new;
> +
> +	if (DWC3_VER_IS_PRIOR(DWC3, 250A))
> +		return;
> +
> +	if (!dwc->ref_clk_fladj_set)
> +		return;
> +
> +	reg = dwc3_readl(dwc->regs, DWC3_GFLADJ);
> +	reg_new = reg & ~DWC3_GFLADJ_REFCLK_FLADJ_MASK;
> +	reg_new |= FIELD_PREP(DWC3_GFLADJ_REFCLK_FLADJ_MASK, dwc->ref_clk_fladj);
> +	if (reg_new != reg)
> +		dwc3_writel(dwc->regs, DWC3_GFLADJ, reg_new);
> +}
> +
> +
>  /**
>   * dwc3_free_one_event_buffer - Frees one event buffer
>   * @dwc: Pointer to our controller context structure
> @@ -1033,6 +1064,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	/* Adjust Reference Clock Period */
>  	dwc3_ref_clk_period(dwc);
> +	dwc3_ref_clk_fladj(dwc);
>  
>  	dwc3_set_incr_burst_type(dwc);
>  
> @@ -1418,6 +1450,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>  				 &dwc->fladj);
>  	device_property_read_u32(dev, "snps,ref-clock-period-ns",
>  				 &dwc->ref_clk_per);
> +	if (!device_property_read_u32(dev, "snps,ref-clock-fladj",
> +				      &dwc->ref_clk_fladj))
> +		dwc->ref_clk_fladj_set = true;
>  
>  	dwc->dis_metastability_quirk = device_property_read_bool(dev,
>  				"snps,dis_metastability_quirk");
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index e1cc3f7398fb..650d4c2e7a67 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -388,6 +388,7 @@
>  /* Global Frame Length Adjustment Register */
>  #define DWC3_GFLADJ_30MHZ_SDBND_SEL		BIT(7)
>  #define DWC3_GFLADJ_30MHZ_MASK			0x3f
> +#define DWC3_GFLADJ_REFCLK_FLADJ_MASK		0x3fff00
>  
>  /* Global User Control Register*/
>  #define DWC3_GUCTL_REFCLKPER_MASK		0xffc00000
> @@ -1166,6 +1167,8 @@ struct dwc3 {
>  
>  	u32			fladj;
>  	u32			ref_clk_per;
> +	bool			ref_clk_fladj_set;
> +	u32			ref_clk_fladj;

Can you also document these new struct members.

>  	u32			irq_gadget;
>  	u32			otg_irq;
>  	u32			current_otg_role;


For this patch:
Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks!
Thinh

[PATCH v2 5/5] arm64: dts: zynqmp: Add DWC3 USB reference clock period configuration

[Robert Hancock]

Set the reference clock period and FLADJ fields in the DWC3 USB driver
to 50ns with no fractional portion to match the ZynqMP configuration.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 74e66443e4ce..2f531707d5d4 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -828,6 +828,8 @@ dwc3_0: usb@fe200000 {
 				#stream-id-cells = <1>;
 				iommus = <&smmu 0x860>;
 				snps,quirk-frame-length-adjustment = <0x20>;
+				snps,ref-clock-period-ns = <50>;
+				snps,ref-clock-fladj = <0>;
 				/* dma-coherent; */
 			};
 		};
@@ -855,6 +857,8 @@ dwc3_1: usb@fe300000 {
 				#stream-id-cells = <1>;
 				iommus = <&smmu 0x861>;
 				snps,quirk-frame-length-adjustment = <0x20>;
+				snps,ref-clock-period-ns = <50>;
+				snps,ref-clock-fladj = <0>;
 				/* dma-coherent; */
 			};
 		};
-- 
2.31.1