Hi Robert,
Thanks for the patch! Please see my comments below inline!
> -----Original Message-----
> From: Robert Hancock <robert.hancock@xxxxxxxxxx>
> Sent: Friday, January 21, 2022 11:49 PM
> To: linux-usb@xxxxxxxxxxxxxxx
> Cc: balbi@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; Michal Simek
> <michals@xxxxxxxxxx>; Manish Narani <MNARANI@xxxxxxxxxx>;
> sean.anderson@xxxxxxxx; Robert Hancock <robert.hancock@xxxxxxxxxx>
> Subject: [PATCH v6 1/2] usb: dwc3: xilinx: Fix PIPE clock selection for USB2.0
> mode
>
> 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@xxxxxxxxxx>
> ---
> drivers/usb/dwc3/dwc3-xilinx.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index 9cc3ad701a29..dd6218d05159 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -167,8 +167,12 @@ 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);
When USB3.0 is enabled in the design, FSBL will set this bit to PIPE_CLK_SELECT
And it's state will be persistent till Linux stage. When this driver finds the usb3-phy property
In the device tree, it will again set this bit.
But in case if the usb3-phy is not present in the device tree and design has
USB3.0 enabled, then this will clear this bit and ultimately it will fail.
It will be better to skip touching that bit in case the device tree does not have the usb3-phy property.
This will skip the whole sequence of PHY initialization (reset assert/deassert are done in order to help initialize PHY).
Something like below should work.
---
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;
}
+ if (!usb3_phy)
+ goto skip_usb3_phy;
+
crst = devm_reset_control_get_exclusive(dev, "usb_crst");
if (IS_ERR(crst)) {
ret = PTR_ERR(crst);
@@ -188,6 +190,7 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
goto err;
}
+skip_usb3_phy:
/*
* This routes the USB DMA traffic to go through FPD path instead
* of reaching DDR directly. This traffic routing is needed to
---
Thanks,
Manish
