Re: [PATCH v2 5/6] usb: phy: mxs: enable weak 1p1 regulator for imx6ul during suspend

Peter Chen
2024-08-09T01:34:23Z

On 24-07-26 19:32:06, Xu Yang wrote:
> 1p1 is off when the system enters suspend at i.MX6UL. It cause the PHY
> get wrong USB DP/DM value, then unexpected wakeup may occur if USB wakeup
> enabled. This will enable weak 1p1 during PHY suspend if vbus exist. So
> USB DP/DM is correct when system suspend.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v2:
> - modify commit message
> ---
> 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 acaae22df3ba..cc4156c1b148 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 */
> @@ -196,7 +202,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 = {
> @@ -241,6 +248,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.
> @@ -884,18 +896,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);

Please check if vbus is not there but wakeup is enabled, and see the
behaviour is expected or not.

Peter
> + }
> }
>
> static int mxs_phy_system_suspend(struct device *dev)
> --
> 2.34.1
>