On Thu, Jul 18, 2024 at 06:26:36PM +0800, Xu Yang wrote: > For imx6ul PHY, when the system enters suspend, its 1p1 is off by default, > that may cause the PHY get inaccurate USB DP/DM value. If the USB wakeup > is enabled at this time, the unexpected wakeup may occur when the system > enters suspend. 1p1 is off when the system enters suspend at iMX6UL. It cause the PHY get wrong USB DP/DM value, then unexpected wakeup may occur if USB wakeup enabled. > > In this patch, when the vbus is there, we enable weak 1p1 during the PHY > suspend API, in that case, the USB DP/DM will be accurate for USB PHY, > then unexpected usb wakeup will not be occurred, especially for the USB > charger is connected scenario. The user needs to enable PHY wakeup for > USB wakeup function using below setting. Avoid use word "this patch", "this commit." Enable weak 1p1 during PHY suspend if vbus exist. So USB DP/DM is correct when system suspend. Reproduce step: > > echo enabled > /sys/devices/platform/soc/2000000.aips-bus/20c9000.usbphy > /power/wakeup echo mem > /sys/power/state, then some error happen. Or just remove it. > > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx> > --- > drivers/usb/phy/phy-mxs-usb.c | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > index 627733a982d1..dcd032678814 100644 > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -71,6 +71,9 @@ > #define BM_USBPHY_PLL_EN_USB_CLKS BIT(6) > > /* Anatop Registers */ > +#define ANADIG_REG_1P1_SET 0x114 > +#define ANADIG_REG_1P1_CLR 0x118 > + > #define ANADIG_ANA_MISC0 0x150 > #define ANADIG_ANA_MISC0_SET 0x154 > #define ANADIG_ANA_MISC0_CLR 0x158 > @@ -123,6 +126,9 @@ > > #define USB_PHY_VLLS_WAKEUP_EN BIT(0) > > +#define BM_ANADIG_REG_1P1_ENABLE_WEAK_LINREG BIT(18) > +#define BM_ANADIG_REG_1P1_TRACK_VDD_SOC_CAP BIT(19) > + > #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy) > > /* Do disconnection between PHY and controller without vbus */ > @@ -197,7 +203,8 @@ static const struct mxs_phy_data imx6sx_phy_data = { > }; > > static const struct mxs_phy_data imx6ul_phy_data = { > - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS, > + .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS | > + MXS_PHY_HARDWARE_CONTROL_PHY2_CLK, > }; > > static const struct mxs_phy_data imx7ulp_phy_data = { > @@ -243,6 +250,11 @@ static inline bool is_imx7ulp_phy(struct mxs_phy *mxs_phy) > return mxs_phy->data == &imx7ulp_phy_data; > } > > +static inline bool is_imx6ul_phy(struct mxs_phy *mxs_phy) > +{ > + return mxs_phy->data == &imx6ul_phy_data; You'd better define, MXS_PHY_POWER_OFF_AT_SUSPEND. is_phy_power_off_at_suspend(). Actually, you just need know if phy power off instead if it is 6ul phy. > +} > + > /* > * PHY needs some 32K cycles to switch from 32K clock to > * bus (such as AHB/AXI, etc) clock. > @@ -891,18 +903,30 @@ static void mxs_phy_wakeup_enable(struct mxs_phy *mxs_phy, bool on) > > static void mxs_phy_enable_ldo_in_suspend(struct mxs_phy *mxs_phy, bool on) > { > - unsigned int reg = on ? ANADIG_ANA_MISC0_SET : ANADIG_ANA_MISC0_CLR; > + unsigned int reg; > + u32 value; > > /* If the SoCs don't have anatop, quit */ > if (!mxs_phy->regmap_anatop) > return; > > - if (is_imx6q_phy(mxs_phy)) > + if (is_imx6q_phy(mxs_phy)) { > + reg = on ? ANADIG_ANA_MISC0_SET : ANADIG_ANA_MISC0_CLR; > regmap_write(mxs_phy->regmap_anatop, reg, > BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG); > - else if (is_imx6sl_phy(mxs_phy)) > + } else if (is_imx6sl_phy(mxs_phy)) { > + reg = on ? ANADIG_ANA_MISC0_SET : ANADIG_ANA_MISC0_CLR; > regmap_write(mxs_phy->regmap_anatop, > reg, BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG_SL); > + } else if (is_imx6ul_phy(mxs_phy)) { > + reg = on ? ANADIG_REG_1P1_SET : ANADIG_REG_1P1_CLR; > + value = BM_ANADIG_REG_1P1_ENABLE_WEAK_LINREG | > + BM_ANADIG_REG_1P1_TRACK_VDD_SOC_CAP; > + if (mxs_phy_get_vbus_status(mxs_phy) && on) > + regmap_write(mxs_phy->regmap_anatop, reg, value); > + else if (!on) > + regmap_write(mxs_phy->regmap_anatop, reg, value); > + } > } > > static int mxs_phy_system_suspend(struct device *dev) > -- > 2.34.1 >