| Message ID | 20240718102637.3964232-5-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2919d888794ec25ae0af79c7ed7f5d48fe4d698f |
| Headers | show |
| Series | [1/6] usb: phy: mxs: Using regulator phy-3p0 | expand |
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@nxp.com> > --- > 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 >
On Thu, Jul 18, 2024 at 11:12:56AM -0400, Frank Li wrote: > 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. Will change. > > > > > 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. Okay, will change. > > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > > 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. > Yes, you are right, but is_imx6ul_phy() may be used in other place and only 6ul phy has this issue, so I'd prefer to keep this form. Thanks, Xu Yang > > +} > > + > > /* > > * 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 > >
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; +} + /* * 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)
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. 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. echo enabled > /sys/devices/platform/soc/2000000.aips-bus/20c9000.usbphy /power/wakeup Signed-off-by: Xu Yang <xu.yang_2@nxp.com> --- drivers/usb/phy/phy-mxs-usb.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)