[PATCH 1/6] usb: phy: mxs: Using regulator phy-3p0

[PATCH 1/6] usb: phy: mxs: Using regulator phy-3p0

[Xu Yang]

It is one of PHY's power, and we need to enable it to keep signal
quality good, and pass eye diagram test.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 920a32cd094d..42fcc8ad9492 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -18,6 +18,7 @@
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/iopoll.h>
+#include <linux/regulator/consumer.h>
 
 #define DRIVER_NAME "mxs_phy"
 
@@ -204,6 +205,7 @@ struct mxs_phy {
 	int port_id;
 	u32 tx_reg_set;
 	u32 tx_reg_mask;
+	struct regulator *phy_3p0;
 };
 
 static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
@@ -288,6 +290,16 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 	if (ret)
 		goto disable_pll;
 
+	if (mxs_phy->phy_3p0) {
+		ret = regulator_enable(mxs_phy->phy_3p0);
+		if (ret) {
+			dev_err(mxs_phy->phy.dev,
+				"Failed to enable 3p0 regulator, ret=%d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	/* Power up the PHY */
 	writel(0, base + HW_USBPHY_PWD);
 
@@ -448,6 +460,9 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
 	if (is_imx7ulp_phy(mxs_phy))
 		mxs_phy_pll_enable(phy->io_priv, false);
 
+	if (mxs_phy->phy_3p0)
+		regulator_disable(mxs_phy->phy_3p0);
+
 	clk_disable_unprepare(mxs_phy->clk);
 }
 
@@ -789,6 +804,21 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	mxs_phy->clk = clk;
 	mxs_phy->data = of_device_get_match_data(&pdev->dev);
 
+	mxs_phy->phy_3p0 = devm_regulator_get(&pdev->dev, "phy-3p0");
+	if (PTR_ERR(mxs_phy->phy_3p0) == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (PTR_ERR(mxs_phy->phy_3p0) == -ENODEV) {
+		/* not exist */
+		mxs_phy->phy_3p0 = NULL;
+	} else if (IS_ERR(mxs_phy->phy_3p0)) {
+		dev_err(&pdev->dev, "Getting regulator error: %ld\n",
+			PTR_ERR(mxs_phy->phy_3p0));
+		return PTR_ERR(mxs_phy->phy_3p0);
+	}
+
+	if (mxs_phy->phy_3p0)
+		regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
+
 	platform_set_drvdata(pdev, mxs_phy);
 
 	device_set_wakeup_capable(&pdev->dev, true);
-- 
2.34.1

[PATCH 2/6] usb: phy: mxs: keep USBPHY2's clk always on

[Xu Yang]

Per IC engineer request, we need to keep USBPHY2's clk always on,
in this way, the USBPHY2 (PLL7) power can be controlled by
hardware suspend signal totally. It is benefit of USB remote wakeup
case which needs the resume signal be sent out as soon as
possible (without software interfere). Without this, we may see usb
remote wakeup issue since the host does not send resume in time.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 36 ++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 42fcc8ad9492..b6868cc22c1e 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -150,6 +150,16 @@
 #define MXS_PHY_TX_D_CAL_MIN			79
 #define MXS_PHY_TX_D_CAL_MAX			119
 
+/*
+ * At some versions, the PHY2's clock is controlled by hardware directly,
+ * eg, according to PHY's suspend status. In these PHYs, we only need to
+ * open the clock at the initialization and close it at its shutdown routine.
+ * It will be benefit for remote wakeup case which needs to send resume
+ * signal as soon as possible, and in this case, the resume signal can be sent
+ * out without software interfere.
+ */
+#define MXS_PHY_HARDWARE_CONTROL_PHY2_CLK	BIT(4)
+
 struct mxs_phy_data {
 	unsigned int flags;
 };
@@ -161,12 +171,14 @@ static const struct mxs_phy_data imx23_phy_data = {
 static const struct mxs_phy_data imx6q_phy_data = {
 	.flags = MXS_PHY_SENDING_SOF_TOO_FAST |
 		MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
-		MXS_PHY_NEED_IP_FIX,
+		MXS_PHY_NEED_IP_FIX |
+		MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
 };
 
 static const struct mxs_phy_data imx6sl_phy_data = {
 	.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
-		MXS_PHY_NEED_IP_FIX,
+		MXS_PHY_NEED_IP_FIX |
+		MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
 };
 
 static const struct mxs_phy_data vf610_phy_data = {
@@ -175,7 +187,8 @@ static const struct mxs_phy_data vf610_phy_data = {
 };
 
 static const struct mxs_phy_data imx6sx_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 imx6ul_phy_data = {
@@ -206,6 +219,7 @@ struct mxs_phy {
 	u32 tx_reg_set;
 	u32 tx_reg_mask;
 	struct regulator *phy_3p0;
+	bool hardware_control_phy2_clk;
 };
 
 static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
@@ -518,12 +532,17 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
 		}
 		writel(BM_USBPHY_CTRL_CLKGATE,
 		       x->io_priv + HW_USBPHY_CTRL_SET);
-		clk_disable_unprepare(mxs_phy->clk);
+		if (!(mxs_phy->port_id == 1 &&
+				mxs_phy->hardware_control_phy2_clk))
+			clk_disable_unprepare(mxs_phy->clk);
 	} else {
 		mxs_phy_clock_switch_delay();
-		ret = clk_prepare_enable(mxs_phy->clk);
-		if (ret)
-			return ret;
+		if (!(mxs_phy->port_id == 1 &&
+				mxs_phy->hardware_control_phy2_clk)) {
+			ret = clk_prepare_enable(mxs_phy->clk);
+			if (ret)
+				return ret;
+		}
 		writel(BM_USBPHY_CTRL_CLKGATE,
 		       x->io_priv + HW_USBPHY_CTRL_CLR);
 		writel(0, x->io_priv + HW_USBPHY_PWD);
@@ -819,6 +838,9 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	if (mxs_phy->phy_3p0)
 		regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
 
+	if (mxs_phy->data->flags & MXS_PHY_HARDWARE_CONTROL_PHY2_CLK)
+		mxs_phy->hardware_control_phy2_clk = true;
+
 	platform_set_drvdata(pdev, mxs_phy);
 
 	device_set_wakeup_capable(&pdev->dev, true);
-- 
2.34.1

Re: [PATCH 2/6] usb: phy: mxs: keep USBPHY2's clk always on

[Frank Li]

On Thu, Jul 18, 2024 at 06:26:33PM +0800, Xu Yang wrote:
> Per IC engineer request, we need to keep USBPHY2's clk always on,

"IP require keep keep USBPHY2's clk always on."

Not personal request, even it is IC expert. It should base on the "fact"
instead of personal's opinion.

> in this way, the USBPHY2 (PLL7) power can be controlled by
> hardware suspend signal totally. It is benefit of USB remote wakeup
> case which needs the resume signal be sent out as soon as
> possible (without software interfere). Without this, we may see usb
> remote wakeup issue since the host does not send resume in time.

So USBPHY2 (PLL7) power can be controlled by suspend signal. USB remote
wakeup needs resume signal be sent out as soon as possible to match

"spec requirement" or some other requirement.

> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 36 ++++++++++++++++++++++++++++-------
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 42fcc8ad9492..b6868cc22c1e 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -150,6 +150,16 @@
>  #define MXS_PHY_TX_D_CAL_MIN			79
>  #define MXS_PHY_TX_D_CAL_MAX			119
>  
> +/*
> + * At some versions, the PHY2's clock is controlled by hardware directly,

It better declear which version, for example, which chip use if no version
info in IP.

> + * eg, according to PHY's suspend status. In these PHYs, we only need to
> + * open the clock at the initialization and close it at its shutdown routine.
> + * It will be benefit for remote wakeup case which needs to send resume
> + * signal as soon as possible, and in this case, the resume signal can be sent
> + * out without software interfere.

These PHYs can send resume signal without software interfere if not gate
clock.

> + */
> +#define MXS_PHY_HARDWARE_CONTROL_PHY2_CLK	BIT(4)
> +
>  struct mxs_phy_data {
>  	unsigned int flags;
>  };
> @@ -161,12 +171,14 @@ static const struct mxs_phy_data imx23_phy_data = {
>  static const struct mxs_phy_data imx6q_phy_data = {
>  	.flags = MXS_PHY_SENDING_SOF_TOO_FAST |
>  		MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> -		MXS_PHY_NEED_IP_FIX,
> +		MXS_PHY_NEED_IP_FIX |
> +		MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
>  };
>  
>  static const struct mxs_phy_data imx6sl_phy_data = {
>  	.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> -		MXS_PHY_NEED_IP_FIX,
> +		MXS_PHY_NEED_IP_FIX |
> +		MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
>  };
>  
>  static const struct mxs_phy_data vf610_phy_data = {
> @@ -175,7 +187,8 @@ static const struct mxs_phy_data vf610_phy_data = {
>  };
>  
>  static const struct mxs_phy_data imx6sx_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 imx6ul_phy_data = {
> @@ -206,6 +219,7 @@ struct mxs_phy {
>  	u32 tx_reg_set;
>  	u32 tx_reg_mask;
>  	struct regulator *phy_3p0;
> +	bool hardware_control_phy2_clk;

Needn't it. just check MXS_PHY_HARDWARE_CONTROL_PHY2_CLK flag is enough.

>  };
>  
>  static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> @@ -518,12 +532,17 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
>  		}
>  		writel(BM_USBPHY_CTRL_CLKGATE,
>  		       x->io_priv + HW_USBPHY_CTRL_SET);
> -		clk_disable_unprepare(mxs_phy->clk);
> +		if (!(mxs_phy->port_id == 1 &&
> +				mxs_phy->hardware_control_phy2_clk))
> +			clk_disable_unprepare(mxs_phy->clk);
>  	} else {
>  		mxs_phy_clock_switch_delay();
> -		ret = clk_prepare_enable(mxs_phy->clk);
> -		if (ret)
> -			return ret;
> +		if (!(mxs_phy->port_id == 1 &&
> +				mxs_phy->hardware_control_phy2_clk)) {
> +			ret = clk_prepare_enable(mxs_phy->clk);
> +			if (ret)
> +				return ret;
> +		}
>  		writel(BM_USBPHY_CTRL_CLKGATE,
>  		       x->io_priv + HW_USBPHY_CTRL_CLR);
>  		writel(0, x->io_priv + HW_USBPHY_PWD);
> @@ -819,6 +838,9 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  	if (mxs_phy->phy_3p0)
>  		regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
>  
> +	if (mxs_phy->data->flags & MXS_PHY_HARDWARE_CONTROL_PHY2_CLK)
> +		mxs_phy->hardware_control_phy2_clk = true;
> +

Needn't it.

>  	platform_set_drvdata(pdev, mxs_phy);
>  
>  	device_set_wakeup_capable(&pdev->dev, true);
> -- 
> 2.34.1
> 

Re: [PATCH 2/6] usb: phy: mxs: keep USBPHY2's clk always on

[Xu Yang]

On Thu, Jul 18, 2024 at 11:00:06AM -0400, Frank Li wrote:
> On Thu, Jul 18, 2024 at 06:26:33PM +0800, Xu Yang wrote:
> > Per IC engineer request, we need to keep USBPHY2's clk always on,
> 
> "IP require keep keep USBPHY2's clk always on."
> 
> Not personal request, even it is IC expert. It should base on the "fact"
> instead of personal's opinion.

Okay.

> 
> > in this way, the USBPHY2 (PLL7) power can be controlled by
> > hardware suspend signal totally. It is benefit of USB remote wakeup
> > case which needs the resume signal be sent out as soon as
> > possible (without software interfere). Without this, we may see usb
> > remote wakeup issue since the host does not send resume in time.
> 
> So USBPHY2 (PLL7) power can be controlled by suspend signal. USB remote
> wakeup needs resume signal be sent out as soon as possible to match
> 
> "spec requirement" or some other requirement.

Will change.

> 
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/phy/phy-mxs-usb.c | 36 ++++++++++++++++++++++++++++-------
> >  1 file changed, 29 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> > index 42fcc8ad9492..b6868cc22c1e 100644
> > --- a/drivers/usb/phy/phy-mxs-usb.c
> > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > @@ -150,6 +150,16 @@
> >  #define MXS_PHY_TX_D_CAL_MIN			79
> >  #define MXS_PHY_TX_D_CAL_MAX			119
> >  
> > +/*
> > + * At some versions, the PHY2's clock is controlled by hardware directly,
> 
> It better declear which version, for example, which chip use if no version
> info in IP.

Okay, will add.

> 
> > + * eg, according to PHY's suspend status. In these PHYs, we only need to
> > + * open the clock at the initialization and close it at its shutdown routine.
> > + * It will be benefit for remote wakeup case which needs to send resume
> > + * signal as soon as possible, and in this case, the resume signal can be sent
> > + * out without software interfere.
> 
> These PHYs can send resume signal without software interfere if not gate
> clock.

Will change.

> 
> > + */
> > +#define MXS_PHY_HARDWARE_CONTROL_PHY2_CLK	BIT(4)
> > +
> >  struct mxs_phy_data {
> >  	unsigned int flags;
> >  };
> > @@ -161,12 +171,14 @@ static const struct mxs_phy_data imx23_phy_data = {
> >  static const struct mxs_phy_data imx6q_phy_data = {
> >  	.flags = MXS_PHY_SENDING_SOF_TOO_FAST |
> >  		MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> > -		MXS_PHY_NEED_IP_FIX,
> > +		MXS_PHY_NEED_IP_FIX |
> > +		MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
> >  };
> >  
> >  static const struct mxs_phy_data imx6sl_phy_data = {
> >  	.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> > -		MXS_PHY_NEED_IP_FIX,
> > +		MXS_PHY_NEED_IP_FIX |
> > +		MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
> >  };
> >  
> >  static const struct mxs_phy_data vf610_phy_data = {
> > @@ -175,7 +187,8 @@ static const struct mxs_phy_data vf610_phy_data = {
> >  };
> >  
> >  static const struct mxs_phy_data imx6sx_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 imx6ul_phy_data = {
> > @@ -206,6 +219,7 @@ struct mxs_phy {
> >  	u32 tx_reg_set;
> >  	u32 tx_reg_mask;
> >  	struct regulator *phy_3p0;
> > +	bool hardware_control_phy2_clk;
> 
> Needn't it. just check MXS_PHY_HARDWARE_CONTROL_PHY2_CLK flag is enough.

Okay.

> 
> >  };
> >  
> >  static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> > @@ -518,12 +532,17 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
> >  		}
> >  		writel(BM_USBPHY_CTRL_CLKGATE,
> >  		       x->io_priv + HW_USBPHY_CTRL_SET);
> > -		clk_disable_unprepare(mxs_phy->clk);
> > +		if (!(mxs_phy->port_id == 1 &&
> > +				mxs_phy->hardware_control_phy2_clk))
> > +			clk_disable_unprepare(mxs_phy->clk);
> >  	} else {
> >  		mxs_phy_clock_switch_delay();
> > -		ret = clk_prepare_enable(mxs_phy->clk);
> > -		if (ret)
> > -			return ret;
> > +		if (!(mxs_phy->port_id == 1 &&
> > +				mxs_phy->hardware_control_phy2_clk)) {
> > +			ret = clk_prepare_enable(mxs_phy->clk);
> > +			if (ret)
> > +				return ret;
> > +		}
> >  		writel(BM_USBPHY_CTRL_CLKGATE,
> >  		       x->io_priv + HW_USBPHY_CTRL_CLR);
> >  		writel(0, x->io_priv + HW_USBPHY_PWD);
> > @@ -819,6 +838,9 @@ static int mxs_phy_probe(struct platform_device *pdev)
> >  	if (mxs_phy->phy_3p0)
> >  		regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
> >  
> > +	if (mxs_phy->data->flags & MXS_PHY_HARDWARE_CONTROL_PHY2_CLK)
> > +		mxs_phy->hardware_control_phy2_clk = true;
> > +
> 
> Needn't it.

Okay. Will remove this.

> 
> >  	platform_set_drvdata(pdev, mxs_phy);
> >  
> >  	device_set_wakeup_capable(&pdev->dev, true);
> > -- 
> > 2.34.1
> > 

[PATCH 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim property

[Xu Yang]

i.MX7ULP need properly set System Integration Module(SIM) module to make
usb wakeup work well. This will add a "nxp,sim" property.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml    | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
index f4b1ca2fb562..2141f271f8f1 100644
--- a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
@@ -87,6 +87,12 @@ properties:
     maximum: 119
     default: 100
 
+  nxp,sim:
+    description:
+      The system integration module (SIM) provides system control and chip
+      configuration registers.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
 required:
   - compatible
   - reg
@@ -110,6 +116,14 @@ allOf:
       required:
         - fsl,anatop
 
+  - if:
+      properties:
+        compatible:
+          const: fsl,imx7ulp-usbphy
+    then:
+      required:
+        - nxp,sim
+
 additionalProperties: false
 
 examples:
-- 
2.34.1

Re: [PATCH 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim property

[Rob Herring]

On Thu, Jul 18, 2024 at 06:26:34PM +0800, Xu Yang wrote:
> i.MX7ULP need properly set System Integration Module(SIM) module to make
> usb wakeup work well. This will add a "nxp,sim" property.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml    | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> index f4b1ca2fb562..2141f271f8f1 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> @@ -87,6 +87,12 @@ properties:
>      maximum: 119
>      default: 100
>  
> +  nxp,sim:
> +    description:
> +      The system integration module (SIM) provides system control and chip
> +      configuration registers.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
>  required:
>    - compatible
>    - reg
> @@ -110,6 +116,14 @@ allOf:
>        required:
>          - fsl,anatop
>  
> +  - if:
> +      properties:
> +        compatible:
> +          const: fsl,imx7ulp-usbphy
> +    then:
> +      required:
> +        - nxp,sim

       else:
         properties:
           nxp,sim: false


> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.34.1
> 

Re: [PATCH 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim property

[Xu Yang]

On Mon, Jul 22, 2024 at 08:51:10PM -0600, Rob Herring wrote:
> On Thu, Jul 18, 2024 at 06:26:34PM +0800, Xu Yang wrote:
> > i.MX7ULP need properly set System Integration Module(SIM) module to make
> > usb wakeup work well. This will add a "nxp,sim" property.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml    | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > index f4b1ca2fb562..2141f271f8f1 100644
> > --- a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > @@ -87,6 +87,12 @@ properties:
> >      maximum: 119
> >      default: 100
> >  
> > +  nxp,sim:
> > +    description:
> > +      The system integration module (SIM) provides system control and chip
> > +      configuration registers.
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -110,6 +116,14 @@ allOf:
> >        required:
> >          - fsl,anatop
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: fsl,imx7ulp-usbphy
> > +    then:
> > +      required:
> > +        - nxp,sim
> 
>        else:
>          properties:
>            nxp,sim: false

Okay, I will add it.

Thanks,
Xu Yang

> 
> 
> > +
> >  additionalProperties: false
> >  
> >  examples:
> > -- 
> > 2.34.1
> > 

[PATCH 4/6] usb: phy: mxs: add wakeup enable for imx7ulp

[Xu Yang]

This wakeup setting can enable USB wakeup function even the
controller's power is lost, and both A7 and M4 are in VLLS mode.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 41 +++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index b6868cc22c1e..627733a982d1 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -118,6 +118,11 @@
 #define BM_ANADIG_USB2_MISC_RX_VPIN_FS		BIT(29)
 #define BM_ANADIG_USB2_MISC_RX_VMIN_FS		BIT(28)
 
+/* System Integration Module (SIM) Registers */
+#define SIM_GPR1				0x30
+
+#define USB_PHY_VLLS_WAKEUP_EN			BIT(0)
+
 #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
 
 /* Do disconnection between PHY and controller without vbus */
@@ -215,6 +220,7 @@ struct mxs_phy {
 	struct clk *clk;
 	const struct mxs_phy_data *data;
 	struct regmap *regmap_anatop;
+	struct regmap *regmap_sim;
 	int port_id;
 	u32 tx_reg_set;
 	u32 tx_reg_mask;
@@ -772,6 +778,17 @@ static int mxs_phy_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Currently, only imx7ulp has SIM module */
+	if (of_get_property(np, "nxp,sim", NULL)) {
+		mxs_phy->regmap_sim = syscon_regmap_lookup_by_phandle
+			(np, "nxp,sim");
+		if (IS_ERR(mxs_phy->regmap_sim)) {
+			dev_dbg(&pdev->dev,
+				"failed to find regmap for sim\n");
+			return PTR_ERR(mxs_phy->regmap_sim);
+		}
+	}
+
 	/* Precompute which bits of the TX register are to be updated, if any */
 	if (!of_property_read_u32(np, "fsl,tx-cal-45-dn-ohms", &val) &&
 	    val >= MXS_PHY_TX_CAL45_MIN && val <= MXS_PHY_TX_CAL45_MAX) {
@@ -856,6 +873,22 @@ static void mxs_phy_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM_SLEEP
+static void mxs_phy_wakeup_enable(struct mxs_phy *mxs_phy, bool on)
+{
+	u32 mask = USB_PHY_VLLS_WAKEUP_EN;
+
+	/* If the SoCs don't have SIM, quit */
+	if (!mxs_phy->regmap_sim)
+		return;
+
+	if (on) {
+		regmap_update_bits(mxs_phy->regmap_sim, SIM_GPR1, mask, mask);
+		udelay(500);
+	} else {
+		regmap_update_bits(mxs_phy->regmap_sim, SIM_GPR1, mask, 0);
+	}
+}
+
 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;
@@ -876,8 +909,10 @@ static int mxs_phy_system_suspend(struct device *dev)
 {
 	struct mxs_phy *mxs_phy = dev_get_drvdata(dev);
 
-	if (device_may_wakeup(dev))
+	if (device_may_wakeup(dev)) {
 		mxs_phy_enable_ldo_in_suspend(mxs_phy, true);
+		mxs_phy_wakeup_enable(mxs_phy, true);
+	}
 
 	return 0;
 }
@@ -886,8 +921,10 @@ static int mxs_phy_system_resume(struct device *dev)
 {
 	struct mxs_phy *mxs_phy = dev_get_drvdata(dev);
 
-	if (device_may_wakeup(dev))
+	if (device_may_wakeup(dev)) {
 		mxs_phy_enable_ldo_in_suspend(mxs_phy, false);
+		mxs_phy_wakeup_enable(mxs_phy, false);
+	}
 
 	return 0;
 }
-- 
2.34.1

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

[Xu Yang]

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(-)

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)
-- 
2.34.1

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

[Frank Li]

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
> 

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

[Xu Yang]

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
> > 

[PATCH 6/6] ARM: dts: imx7ulp: add "nxp,sim" property for usbphy1

[Xu Yang]

i.MX7ULP need properly set System Integration Module(SIM) module to make
usb wakeup work well. This will add a "nxp,sim" property for usbphy1.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi b/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi
index ac338320ac1d..b093f2a447ae 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi
@@ -214,6 +214,7 @@ usbphy1: usb-phy@40350000 {
 			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&pcc2 IMX7ULP_CLK_USB_PHY>;
 			#phy-cells = <0>;
+			nxp,sim = <&sim>;
 		};
 
 		usdhc0: mmc@40370000 {
-- 
2.34.1

Re: [PATCH 1/6] usb: phy: mxs: Using regulator phy-3p0

[Frank Li]

On Thu, Jul 18, 2024 at 06:26:32PM +0800, Xu Yang wrote:
> It is one of PHY's power, and we need to enable it to keep signal
> quality good, and pass eye diagram test.

"Enable regulator 'phy-3p0' to pass eye diagram test since it improve signal
qualilty."

My question is why it just improve signal quality instead of make it work.
It should not work if no power supply. 

> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 920a32cd094d..42fcc8ad9492 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -18,6 +18,7 @@
>  #include <linux/regmap.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/iopoll.h>
> +#include <linux/regulator/consumer.h>
>  
>  #define DRIVER_NAME "mxs_phy"
>  
> @@ -204,6 +205,7 @@ struct mxs_phy {
>  	int port_id;
>  	u32 tx_reg_set;
>  	u32 tx_reg_mask;
> +	struct regulator *phy_3p0;
>  };
>  
>  static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> @@ -288,6 +290,16 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  	if (ret)
>  		goto disable_pll;
>  
> +	if (mxs_phy->phy_3p0) {
> +		ret = regulator_enable(mxs_phy->phy_3p0);
> +		if (ret) {
> +			dev_err(mxs_phy->phy.dev,
> +				"Failed to enable 3p0 regulator, ret=%d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
>  	/* Power up the PHY */
>  	writel(0, base + HW_USBPHY_PWD);
>  
> @@ -448,6 +460,9 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
>  	if (is_imx7ulp_phy(mxs_phy))
>  		mxs_phy_pll_enable(phy->io_priv, false);
>  
> +	if (mxs_phy->phy_3p0)
> +		regulator_disable(mxs_phy->phy_3p0);
> +
>  	clk_disable_unprepare(mxs_phy->clk);
>  }
>  
> @@ -789,6 +804,21 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  	mxs_phy->clk = clk;
>  	mxs_phy->data = of_device_get_match_data(&pdev->dev);
>  
> +	mxs_phy->phy_3p0 = devm_regulator_get(&pdev->dev, "phy-3p0");

Does binding doc update?

> +	if (PTR_ERR(mxs_phy->phy_3p0) == -EPROBE_DEFER) {
> +		return -EPROBE_DEFER;
> +	} else if (PTR_ERR(mxs_phy->phy_3p0) == -ENODEV) {
> +		/* not exist */
> +		mxs_phy->phy_3p0 = NULL;
> +	} else if (IS_ERR(mxs_phy->phy_3p0)) {
> +		dev_err(&pdev->dev, "Getting regulator error: %ld\n",
> +			PTR_ERR(mxs_phy->phy_3p0));
> +		return PTR_ERR(mxs_phy->phy_3p0);
> +	}

just need call dev_err_probe()

> +
> +	if (mxs_phy->phy_3p0)
> +		regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
> +
>  	platform_set_drvdata(pdev, mxs_phy);
>  
>  	device_set_wakeup_capable(&pdev->dev, true);
> -- 
> 2.34.1
> 

Re: [PATCH 1/6] usb: phy: mxs: Using regulator phy-3p0

[Xu Yang]

On Thu, Jul 18, 2024 at 10:42:33AM -0400, Frank Li wrote:
> On Thu, Jul 18, 2024 at 06:26:32PM +0800, Xu Yang wrote:
> > It is one of PHY's power, and we need to enable it to keep signal
> > quality good, and pass eye diagram test.
> 
> "Enable regulator 'phy-3p0' to pass eye diagram test since it improve signal
> qualilty."
> 
> My question is why it just improve signal quality instead of make it work.
> It should not work if no power supply. 

I'm cleaning up these old local patch and I'm not very clear about the history.

But I think this 3p0 is not the main power of phy, it's just one of phy's power.
I can only get below info about this reg-3p0:

37.6.2 Regulator 3P0 Register (PMU_REG_3P0)
  "This register defines the control and status bits for the 3.0V regulator
powered by the host USB VBUS pin."

I've just tested it works well without this phy-3p0. I'll rewrite the commit
message to avoid confusion.

> 
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/phy/phy-mxs-usb.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> > index 920a32cd094d..42fcc8ad9492 100644
> > --- a/drivers/usb/phy/phy-mxs-usb.c
> > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/regmap.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/iopoll.h>
> > +#include <linux/regulator/consumer.h>
> >  
> >  #define DRIVER_NAME "mxs_phy"
> >  
> > @@ -204,6 +205,7 @@ struct mxs_phy {
> >  	int port_id;
> >  	u32 tx_reg_set;
> >  	u32 tx_reg_mask;
> > +	struct regulator *phy_3p0;
> >  };
> >  
> >  static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> > @@ -288,6 +290,16 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> >  	if (ret)
> >  		goto disable_pll;
> >  
> > +	if (mxs_phy->phy_3p0) {
> > +		ret = regulator_enable(mxs_phy->phy_3p0);
> > +		if (ret) {
> > +			dev_err(mxs_phy->phy.dev,
> > +				"Failed to enable 3p0 regulator, ret=%d\n",
> > +				ret);
> > +			return ret;
> > +		}
> > +	}
> > +
> >  	/* Power up the PHY */
> >  	writel(0, base + HW_USBPHY_PWD);
> >  
> > @@ -448,6 +460,9 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
> >  	if (is_imx7ulp_phy(mxs_phy))
> >  		mxs_phy_pll_enable(phy->io_priv, false);
> >  
> > +	if (mxs_phy->phy_3p0)
> > +		regulator_disable(mxs_phy->phy_3p0);
> > +
> >  	clk_disable_unprepare(mxs_phy->clk);
> >  }
> >  
> > @@ -789,6 +804,21 @@ static int mxs_phy_probe(struct platform_device *pdev)
> >  	mxs_phy->clk = clk;
> >  	mxs_phy->data = of_device_get_match_data(&pdev->dev);
> >  
> > +	mxs_phy->phy_3p0 = devm_regulator_get(&pdev->dev, "phy-3p0");
> 
> Does binding doc update?

Yes.

> 
> > +	if (PTR_ERR(mxs_phy->phy_3p0) == -EPROBE_DEFER) {
> > +		return -EPROBE_DEFER;
> > +	} else if (PTR_ERR(mxs_phy->phy_3p0) == -ENODEV) {
> > +		/* not exist */
> > +		mxs_phy->phy_3p0 = NULL;
> > +	} else if (IS_ERR(mxs_phy->phy_3p0)) {
> > +		dev_err(&pdev->dev, "Getting regulator error: %ld\n",
> > +			PTR_ERR(mxs_phy->phy_3p0));
> > +		return PTR_ERR(mxs_phy->phy_3p0);
> > +	}
> 
> just need call dev_err_probe()

Ok. Will change it.

> 
> > +
> > +	if (mxs_phy->phy_3p0)
> > +		regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
> > +
> >  	platform_set_drvdata(pdev, mxs_phy);
> >  
> >  	device_set_wakeup_capable(&pdev->dev, true);
> > -- 
> > 2.34.1
> >