[PATCH v5 0/2] Xilinx ZynqMP USB fixes

[PATCH v5 0/2] Xilinx ZynqMP USB fixes

[Robert Hancock]

Some fixes related to the 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 v4:
-dropped DWC3 core patches as they are superseded by Sean Anderson's
patchset "usb: dwc3: Calculate REFCLKPER et. al. from reference clock",
ZynqMP-specific patches unchanged

Changes since v3:
-fixed DT schema dt-doc-validate error

Changes since v2:
-additional kerneldoc fixes

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

Robert Hancock (2):
  usb: dwc3: xilinx: Fix PIPE clock selection for USB2.0 mode
  usb: dwc3: xilinx: Fix error handling when getting USB3 PHY

 drivers/usb/dwc3/dwc3-xilinx.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
2.31.1

[PATCH v5 1/2] 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 v5 2/2] 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

Re: [PATCH v5 1/2] usb: dwc3: xilinx: Fix PIPE clock selection for USB2.0 mode

[Michal Simek]

On 1/20/22 18:08, Robert Hancock wrote:
> 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

nit: this is likely comment for net not for the rest.
You should use multiline comment in this format.
/*
  * Set...

M

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

[Michal Simek]

On 1/20/22 18:08, Robert Hancock wrote:
> 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");

nit: Doesn't it fit to one line?

M

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

[Robert Hancock]

On Fri, 2022-01-21 at 08:54 +0100, Michal Simek wrote:
> 
> On 1/20/22 18:08, Robert Hancock wrote:
> > 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");
> 
> nit: Doesn't it fit to one line?
> 
> M

Yup, not sure why I wrapped the lines. Will change.

-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com

Re: [PATCH v5 1/2] usb: dwc3: xilinx: Fix PIPE clock selection for USB2.0 mode

[Robert Hancock]

On Fri, 2022-01-21 at 08:53 +0100, Michal Simek wrote:
> 
> On 1/20/22 18:08, Robert Hancock wrote:
> > 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
> 
> nit: this is likely comment for net not for the rest.
> You should use multiline comment in this format.
> /*
>   * Set...
> 
> M

Yup, will change.

-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com