| Message ID | 20240205141221.56076-6-rogerq@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3-am62: module removal and errata fixes | expand |
On 2/5/24 8:12 AM, Roger Quadros wrote: > All AM62 devices have Errata i2409 [1] due to which > USB2 PHY may lock up due to short suspend. > > Workaround involves setting bit 5 and 4 PLL_REG12 > in PHY2 register space after USB controller is brought > out of LPSC reset but before controller initialization. > > Handle this workaround. > > [1] - https://www.ti.com/lit/er/sprz487d/sprz487d.pdf > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: Conor Dooley <conor+dt@kernel.org> > Signed-off-by: Roger Quadros <rogerq@kernel.org> > --- > > Notes: > Changelog: > > v2: > - don't add phy read/write helpers or add phy to private data > > v1: https://lore.kernel.org/all/20240201121220.5523-5-rogerq@kernel.org/ > > drivers/usb/dwc3/dwc3-am62.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c > index af1ce934e7fb..5ae5c3087b0f 100644 > --- a/drivers/usb/dwc3/dwc3-am62.c > +++ b/drivers/usb/dwc3/dwc3-am62.c > @@ -101,6 +101,11 @@ > #define PHY_CORE_VOLTAGE_MASK BIT(31) > #define PHY_PLL_REFCLK_MASK GENMASK(3, 0) > > +/* USB PHY2 register offsets */ > +#define USB_PHY_PLL_REG12 0x130 > +#define USB_PHY_PLL_LDO_REF_EN BIT(5) > +#define USB_PHY_PLL_LDO_REF_EN_EN BIT(4) > + > #define DWC3_AM62_AUTOSUSPEND_DELAY 100 > > struct dwc3_am62 { > @@ -184,8 +189,9 @@ static int dwc3_ti_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *node = pdev->dev.of_node; > struct dwc3_am62 *am62; > - int i, ret; > unsigned long rate; > + void __iomem *phy; > + int i, ret; > u32 reg; > > am62 = devm_kzalloc(dev, sizeof(*am62), GFP_KERNEL); > @@ -201,6 +207,12 @@ static int dwc3_ti_probe(struct platform_device *pdev) > return PTR_ERR(am62->usbss); > } > > + phy = devm_platform_ioremap_resource(pdev, 1); > + if (IS_ERR(phy)) { > + dev_err(dev, "can't map PHY IOMEM resource. Won't apply i2409 fix.\n"); > + phy = NULL; > + } Why not move this down to where you use it below, then just have it be an if/else with the work around in the if (!IS_ERR(phy)) and the warning in the else. Andrew > + > am62->usb2_refclk = devm_clk_get(dev, "ref"); > if (IS_ERR(am62->usb2_refclk)) { > dev_err(dev, "can't get usb2_refclk\n"); > @@ -227,6 +239,13 @@ static int dwc3_ti_probe(struct platform_device *pdev) > if (ret) > return ret; > > + /* Workaround Errata i2409 */ > + if (phy) { > + reg = readl(phy + USB_PHY_PLL_REG12); > + reg |= USB_PHY_PLL_LDO_REF_EN | USB_PHY_PLL_LDO_REF_EN_EN; > + writel(reg, phy + USB_PHY_PLL_REG12); > + } > + > /* VBUS divider select */ > am62->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); > reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG);
On 05/02/2024 20:07, Andrew Davis wrote: > On 2/5/24 8:12 AM, Roger Quadros wrote: >> All AM62 devices have Errata i2409 [1] due to which >> USB2 PHY may lock up due to short suspend. >> >> Workaround involves setting bit 5 and 4 PLL_REG12 >> in PHY2 register space after USB controller is brought >> out of LPSC reset but before controller initialization. >> >> Handle this workaround. >> >> [1] - https://www.ti.com/lit/er/sprz487d/sprz487d.pdf >> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> >> Cc: Conor Dooley <conor+dt@kernel.org> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> >> Notes: >> Changelog: >> v2: >> - don't add phy read/write helpers or add phy to private data >> v1: https://lore.kernel.org/all/20240201121220.5523-5-rogerq@kernel.org/ >> >> drivers/usb/dwc3/dwc3-am62.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c >> index af1ce934e7fb..5ae5c3087b0f 100644 >> --- a/drivers/usb/dwc3/dwc3-am62.c >> +++ b/drivers/usb/dwc3/dwc3-am62.c >> @@ -101,6 +101,11 @@ >> #define PHY_CORE_VOLTAGE_MASK BIT(31) >> #define PHY_PLL_REFCLK_MASK GENMASK(3, 0) >> +/* USB PHY2 register offsets */ >> +#define USB_PHY_PLL_REG12 0x130 >> +#define USB_PHY_PLL_LDO_REF_EN BIT(5) >> +#define USB_PHY_PLL_LDO_REF_EN_EN BIT(4) >> + >> #define DWC3_AM62_AUTOSUSPEND_DELAY 100 >> struct dwc3_am62 { >> @@ -184,8 +189,9 @@ static int dwc3_ti_probe(struct platform_device *pdev) >> struct device *dev = &pdev->dev; >> struct device_node *node = pdev->dev.of_node; >> struct dwc3_am62 *am62; >> - int i, ret; >> unsigned long rate; >> + void __iomem *phy; >> + int i, ret; >> u32 reg; >> am62 = devm_kzalloc(dev, sizeof(*am62), GFP_KERNEL); >> @@ -201,6 +207,12 @@ static int dwc3_ti_probe(struct platform_device *pdev) >> return PTR_ERR(am62->usbss); >> } >> + phy = devm_platform_ioremap_resource(pdev, 1); >> + if (IS_ERR(phy)) { >> + dev_err(dev, "can't map PHY IOMEM resource. Won't apply i2409 fix.\n"); >> + phy = NULL; >> + } > > Why not move this down to where you use it below, then just have > it be an if/else with the work around in the if (!IS_ERR(phy)) > and the warning in the else. Seems reasonable. Will fix. > > Andrew > >> + >> am62->usb2_refclk = devm_clk_get(dev, "ref"); >> if (IS_ERR(am62->usb2_refclk)) { >> dev_err(dev, "can't get usb2_refclk\n"); >> @@ -227,6 +239,13 @@ static int dwc3_ti_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> + /* Workaround Errata i2409 */ >> + if (phy) { >> + reg = readl(phy + USB_PHY_PLL_REG12); >> + reg |= USB_PHY_PLL_LDO_REF_EN | USB_PHY_PLL_LDO_REF_EN_EN; >> + writel(reg, phy + USB_PHY_PLL_REG12); >> + } >> + >> /* VBUS divider select */ >> am62->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); >> reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG);
On Mon, Feb 05, 2024 at 04:12:21PM +0200, Roger Quadros wrote: > All AM62 devices have Errata i2409 [1] due to which > USB2 PHY may lock up due to short suspend. Is there any visible log trace when we have this phy lock up situation? Eventually it would be nice to have this in the commit message. Francesco
On 11/02/2024 18:18, Francesco Dolcini wrote: > On Mon, Feb 05, 2024 at 04:12:21PM +0200, Roger Quadros wrote: >> All AM62 devices have Errata i2409 [1] due to which >> USB2 PHY may lock up due to short suspend. > > Is there any visible log trace when we have this phy lock up situation? > Eventually it would be nice to have this in the commit message. > I have not been able to reproduce this issue here so no log trace.
diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c index af1ce934e7fb..5ae5c3087b0f 100644 --- a/drivers/usb/dwc3/dwc3-am62.c +++ b/drivers/usb/dwc3/dwc3-am62.c @@ -101,6 +101,11 @@ #define PHY_CORE_VOLTAGE_MASK BIT(31) #define PHY_PLL_REFCLK_MASK GENMASK(3, 0) +/* USB PHY2 register offsets */ +#define USB_PHY_PLL_REG12 0x130 +#define USB_PHY_PLL_LDO_REF_EN BIT(5) +#define USB_PHY_PLL_LDO_REF_EN_EN BIT(4) + #define DWC3_AM62_AUTOSUSPEND_DELAY 100 struct dwc3_am62 { @@ -184,8 +189,9 @@ static int dwc3_ti_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *node = pdev->dev.of_node; struct dwc3_am62 *am62; - int i, ret; unsigned long rate; + void __iomem *phy; + int i, ret; u32 reg; am62 = devm_kzalloc(dev, sizeof(*am62), GFP_KERNEL); @@ -201,6 +207,12 @@ static int dwc3_ti_probe(struct platform_device *pdev) return PTR_ERR(am62->usbss); } + phy = devm_platform_ioremap_resource(pdev, 1); + if (IS_ERR(phy)) { + dev_err(dev, "can't map PHY IOMEM resource. Won't apply i2409 fix.\n"); + phy = NULL; + } + am62->usb2_refclk = devm_clk_get(dev, "ref"); if (IS_ERR(am62->usb2_refclk)) { dev_err(dev, "can't get usb2_refclk\n"); @@ -227,6 +239,13 @@ static int dwc3_ti_probe(struct platform_device *pdev) if (ret) return ret; + /* Workaround Errata i2409 */ + if (phy) { + reg = readl(phy + USB_PHY_PLL_REG12); + reg |= USB_PHY_PLL_LDO_REF_EN | USB_PHY_PLL_LDO_REF_EN_EN; + writel(reg, phy + USB_PHY_PLL_REG12); + } + /* VBUS divider select */ am62->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG);
All AM62 devices have Errata i2409 [1] due to which USB2 PHY may lock up due to short suspend. Workaround involves setting bit 5 and 4 PLL_REG12 in PHY2 register space after USB controller is brought out of LPSC reset but before controller initialization. Handle this workaround. [1] - https://www.ti.com/lit/er/sprz487d/sprz487d.pdf Cc: Rob Herring <robh+dt@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> Cc: Conor Dooley <conor+dt@kernel.org> Signed-off-by: Roger Quadros <rogerq@kernel.org> --- Notes: Changelog: v2: - don't add phy read/write helpers or add phy to private data v1: https://lore.kernel.org/all/20240201121220.5523-5-rogerq@kernel.org/ drivers/usb/dwc3/dwc3-am62.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)