On Mon, Dec 09, 2024, Roger Quadros wrote: > If controller looses power during PM suspend then re-initialize > it. We use the DEBUG_CFG register to track if controller lost power > or was reset in PM suspend. > > Move all initialization code into dwc3_ti_init() so it can be re-used. > > Signed-off-by: Roger Quadros <rogerq@xxxxxxxxxx> > --- > Changes in v2: > - fixed macro USBSS_DEBUG_CFG_OFF to 0 > - Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/r/20241122-am62-dwc3-io-ddr-v1-1-cc4956449420@kernel.org__;!!A4F2R9G_pg!fQlfE8tlmLW59YBhswZnfOSf_zypGRcqWV312B5A0NF0rLaOFPvTkWaPzCoKpz9E-2iihXpR87fFTrfubb-v$ > --- > drivers/usb/dwc3/dwc3-am62.c | 82 +++++++++++++++++++++++++++++--------------- > 1 file changed, 55 insertions(+), 27 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c > index 5e3d1741701f..7eef945b1f89 100644 > --- a/drivers/usb/dwc3/dwc3-am62.c > +++ b/drivers/usb/dwc3/dwc3-am62.c > @@ -108,6 +108,9 @@ > > #define DWC3_AM62_AUTOSUSPEND_DELAY 100 > > +#define USBSS_DEBUG_CFG_OFF 0x0 > +#define USBSS_DEBUG_CFG_DISABLED 0x7 > + > struct dwc3_am62 { > struct device *dev; > void __iomem *usbss; > @@ -117,6 +120,7 @@ struct dwc3_am62 { > unsigned int offset; > unsigned int vbus_divider; > u32 wakeup_stat; > + void __iomem *phy; This is an odd way to name the iomem. I would associate "phy" to a device more than an iomem. How about phy_regs? In any case, it's just minor nit. Regardless whether you want to rename it or not: Acked-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> BR, Thinh > }; > > static const int dwc3_ti_rate_table[] = { /* in KHZ */ > @@ -184,15 +188,47 @@ static int phy_syscon_pll_refclk(struct dwc3_am62 *am62) > return 0; > } > > +static int dwc3_ti_init(struct dwc3_am62 *am62) > +{ > + int ret; > + u32 reg; > + > + /* Read the syscon property and set the rate code */ > + ret = phy_syscon_pll_refclk(am62); > + if (ret) > + return ret; > + > + /* Workaround Errata i2409 */ > + if (am62->phy) { > + reg = readl(am62->phy + USB_PHY_PLL_REG12); > + reg |= USB_PHY_PLL_LDO_REF_EN | USB_PHY_PLL_LDO_REF_EN_EN; > + writel(reg, am62->phy + USB_PHY_PLL_REG12); > + } > + > + /* VBUS divider select */ > + reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG); > + if (am62->vbus_divider) > + reg |= 1 << USBSS_PHY_VBUS_SEL_SHIFT; > + > + dwc3_ti_writel(am62, USBSS_PHY_CONFIG, reg); > + > + clk_prepare_enable(am62->usb2_refclk); > + > + /* Set mode valid bit to indicate role is valid */ > + reg = dwc3_ti_readl(am62, USBSS_MODE_CONTROL); > + reg |= USBSS_MODE_VALID; > + dwc3_ti_writel(am62, USBSS_MODE_CONTROL, reg); > + > + return 0; > +} > + > 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; > unsigned long rate; > - void __iomem *phy; > int i, ret; > - u32 reg; > > am62 = devm_kzalloc(dev, sizeof(*am62), GFP_KERNEL); > if (!am62) > @@ -228,29 +264,17 @@ static int dwc3_ti_probe(struct platform_device *pdev) > > am62->rate_code = i; > > - /* Read the syscon property and set the rate code */ > - ret = phy_syscon_pll_refclk(am62); > - if (ret) > - return ret; > - > - /* Workaround Errata i2409 */ > - phy = devm_platform_ioremap_resource(pdev, 1); > - if (IS_ERR(phy)) { > + am62->phy = devm_platform_ioremap_resource(pdev, 1); > + if (IS_ERR(am62->phy)) { > dev_err(dev, "can't map PHY IOMEM resource. Won't apply i2409 fix.\n"); > - phy = NULL; > - } else { > - 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); > + am62->phy = NULL; > } > > - /* VBUS divider select */ > am62->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); > - reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG); > - if (am62->vbus_divider) > - reg |= 1 << USBSS_PHY_VBUS_SEL_SHIFT; > > - dwc3_ti_writel(am62, USBSS_PHY_CONFIG, reg); > + ret = dwc3_ti_init(am62); > + if (ret) > + return ret; > > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > @@ -258,7 +282,6 @@ static int dwc3_ti_probe(struct platform_device *pdev) > * Don't ignore its dependencies with its children > */ > pm_suspend_ignore_children(dev, false); > - clk_prepare_enable(am62->usb2_refclk); > pm_runtime_get_noresume(dev); > > ret = of_platform_populate(node, NULL, NULL, dev); > @@ -267,11 +290,6 @@ static int dwc3_ti_probe(struct platform_device *pdev) > goto err_pm_disable; > } > > - /* Set mode valid bit to indicate role is valid */ > - reg = dwc3_ti_readl(am62, USBSS_MODE_CONTROL); > - reg |= USBSS_MODE_VALID; > - dwc3_ti_writel(am62, USBSS_MODE_CONTROL, reg); > - > /* Device has capability to wakeup system from sleep */ > device_set_wakeup_capable(dev, true); > ret = device_wakeup_enable(dev); > @@ -338,6 +356,9 @@ static int dwc3_ti_suspend_common(struct device *dev) > dwc3_ti_writel(am62, USBSS_WAKEUP_STAT, USBSS_WAKEUP_STAT_CLR); > } > > + /* just to track if module resets on suspend */ > + dwc3_ti_writel(am62, USBSS_DEBUG_CFG, USBSS_DEBUG_CFG_DISABLED); > + > clk_disable_unprepare(am62->usb2_refclk); > > return 0; > @@ -348,7 +369,14 @@ static int dwc3_ti_resume_common(struct device *dev) > struct dwc3_am62 *am62 = dev_get_drvdata(dev); > u32 reg; > > - clk_prepare_enable(am62->usb2_refclk); > + reg = dwc3_ti_readl(am62, USBSS_DEBUG_CFG); > + if (reg != USBSS_DEBUG_CFG_DISABLED) { > + /* lost power/context */ > + dwc3_ti_init(am62); > + } else { > + dwc3_ti_writel(am62, USBSS_DEBUG_CFG, USBSS_DEBUG_CFG_OFF); > + clk_prepare_enable(am62->usb2_refclk); > + } > > if (device_may_wakeup(dev)) { > /* Clear wakeup config enable bits */ > > --- > base-commit: cdd30ebb1b9f36159d66f088b61aee264e649d7a > change-id: 20241122-am62-dwc3-io-ddr-3bcb683a91a0 > > Best regards, > -- > Roger Quadros <rogerq@xxxxxxxxxx> >
