[PATCH v2 1/6] usb: phy: mxs: enable regulator phy-3p0 to improve signal qualilty

[PATCH v2 1/6] usb: phy: mxs: enable regulator phy-3p0 to improve signal qualilty

[Xu Yang]

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

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - rewrite commit message
 - use dev_err_probe() as suggested by Frank Li
---
 drivers/usb/phy/phy-mxs-usb.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 920a32cd094d..d9d29f0b37de 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,17 @@ 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) == -ENODEV)
+		/* not exist */
+		mxs_phy->phy_3p0 = NULL;
+	else if (IS_ERR(mxs_phy->phy_3p0))
+		return dev_err_probe(&pdev->dev, PTR_ERR(mxs_phy->phy_3p0),
+				"Getting regulator error\n");
+
+	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 v2 2/6] usb: phy: mxs: keep USBPHY2's clk always on

[Xu Yang]

IP require keep USBPHY2's clk always on, 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 USB requirements.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - modify commit message
 - remove hardware_control_phy2_clk
---
 drivers/usb/phy/phy-mxs-usb.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index d9d29f0b37de..e172af75f602 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -150,6 +150,15 @@
 #define MXS_PHY_TX_D_CAL_MIN			79
 #define MXS_PHY_TX_D_CAL_MAX			119
 
+/*
+ * At imx6q/6sl/6sx, 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.
+ * 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 +170,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 +186,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 = {
@@ -518,12 +530,19 @@ 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->data->flags &
+				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->data->flags &
+				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);
-- 
2.34.1

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

[Peter Chen]

On 24-07-26 19:32:03, Xu Yang wrote:
> IP require keep USBPHY2's clk always on, 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 USB requirements.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Reviewed-by: Peter Chen <peter.chen@kernel.org>
> 
> ---
> Changes in v2:
>  - modify commit message
>  - remove hardware_control_phy2_clk
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index d9d29f0b37de..e172af75f602 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -150,6 +150,15 @@
>  #define MXS_PHY_TX_D_CAL_MIN			79
>  #define MXS_PHY_TX_D_CAL_MAX			119
>  
> +/*
> + * At imx6q/6sl/6sx, 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.
> + * 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 +170,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 +186,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 = {
> @@ -518,12 +530,19 @@ 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->data->flags &
> +				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->data->flags &
> +				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);
> -- 
> 2.34.1
> 

[PATCH v2 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>

---
Changes in v2:
 - add else branch suggested by Rob
---
 .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
index f4b1ca2fb562..ce665a2779b7 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,17 @@ 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 v2 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim property

[Peng Fan]

> Subject: [PATCH v2 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim
> property
> 
> 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>
> 
> ---
> Changes in v2:
>  - add else branch suggested by Rob
> ---
>  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17
> +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-
> usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-
> usbphy.yaml
> index f4b1ca2fb562..ce665a2779b7 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:

"nxp,imx7ulp-sim" should be better. SIM is specific to i.MX7ULP as of now.

Regards,
Peng.

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

[Krzysztof Kozlowski]

On 29/07/2024 03:03, Peng Fan wrote:
>> Subject: [PATCH v2 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim
>> property
>>
>> 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>
>>
>> ---
>> Changes in v2:
>>  - add else branch suggested by Rob
>> ---
>>  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17
>> +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-
>> usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-
>> usbphy.yaml
>> index f4b1ca2fb562..ce665a2779b7 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:
> 
> "nxp,imx7ulp-sim" should be better. SIM is specific to i.MX7ULP as of now.

Can you guarantee that? IOW, if you ever, ever come with another "sim"
property for another SoC NXP agrees that it will be rejected?

Properties should be defined once and naming such per-soc kind of
contradicts it.

Best regards,
Krzysztof

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

[Peng Fan]

> Subject: Re: [PATCH v2 3/6] dt-bindings: phy: mxs-usb-phy: add
> nxp,sim property
> 
> On 29/07/2024 03:03, Peng Fan wrote:
> >> Subject: [PATCH v2 3/6] dt-bindings: phy: mxs-usb-phy: add nxp,sim
> >> property
> >>
> >> 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>
> >>
> >> ---
> >> Changes in v2:
> >>  - add else branch suggested by Rob
> >> ---
> >>  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17
> >> +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-
> >> usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-
> >> usbphy.yaml
> >> index f4b1ca2fb562..ce665a2779b7 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:
> >
> > "nxp,imx7ulp-sim" should be better. SIM is specific to i.MX7ULP as of
> now.
> 
> Can you guarantee that? IOW, if you ever, ever come with another
> "sim"
> property for another SoC NXP agrees that it will be rejected?

I could not judge here. I was thinking, vendor,soc-ip is the preferred
property.
> 
> Properties should be defined once and naming such per-soc kind of
> contradicts it.

It is fine to keep nxp,sim as what the patch did. 

Regards,
Peng.

> 
> Best regards,
> Krzysztof

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

[Xu Yang]

On Fri, Jul 26, 2024 at 07:32:04PM +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>
> 
> ---
> Changes in v2:
>  - add else branch suggested by Rob
> ---
>  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> index f4b1ca2fb562..ce665a2779b7 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,17 @@ allOf:
>        required:
>          - fsl,anatop
>  
> +  - if:
> +      properties:
> +        compatible:
> +          const: fsl,imx7ulp-usbphy
> +    then:
> +      required:
> +        - nxp,sim
> +    else:
> +      properties:
> +        nxp,sim: false
> +
>  additionalProperties: false
>  
>  examples:

A gentle ping.

If it's fine, please add tag for this binding patch. The rest of the
patches (expect dts) are already merged to mainline.

Thanks,
Xu Yang


> -- 
> 2.34.1
> 

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

[Krzysztof Kozlowski]

On 26/07/2024 13:32, 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>
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Xu Yang]

Hi Greg,

On Fri, Jul 26, 2024 at 07:32:04PM +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>
> 
> ---
> Changes in v2:
>  - add else branch suggested by Rob
> ---
>  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> index f4b1ca2fb562..ce665a2779b7 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,17 @@ allOf:
>        required:
>          - fsl,anatop
>  
> +  - if:
> +      properties:
> +        compatible:
> +          const: fsl,imx7ulp-usbphy
> +    then:
> +      required:
> +        - nxp,sim
> +    else:
> +      properties:
> +        nxp,sim: false
> +
>  additionalProperties: false
>  
>  examples:


Could you please pick up patch #3? Rest of the patches are mainlined.

Thanks,
Xu Yang

> -- 
> 2.34.1
> 

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

[Greg KH]

On Thu, Aug 29, 2024 at 05:09:35PM +0800, Xu Yang wrote:
> Hi Greg,
> 
> On Fri, Jul 26, 2024 at 07:32:04PM +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>
> > 
> > ---
> > Changes in v2:
> >  - add else branch suggested by Rob
> > ---
> >  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > index f4b1ca2fb562..ce665a2779b7 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,17 @@ allOf:
> >        required:
> >          - fsl,anatop
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: fsl,imx7ulp-usbphy
> > +    then:
> > +      required:
> > +        - nxp,sim
> > +    else:
> > +      properties:
> > +        nxp,sim: false
> > +
> >  additionalProperties: false
> >  
> >  examples:
> 
> 
> Could you please pick up patch #3? Rest of the patches are mainlined.

Can you please resend it as an individual patch with the reviewed-by
added to it?

thanks,

greg k-h

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

[Xu Yang]

On Tue, Sep 03, 2024 at 09:08:52AM +0200, Greg KH wrote:
> On Thu, Aug 29, 2024 at 05:09:35PM +0800, Xu Yang wrote:
> > Hi Greg,
> > 
> > On Fri, Jul 26, 2024 at 07:32:04PM +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>
> > > 
> > > ---
> > > Changes in v2:
> > >  - add else branch suggested by Rob
> > > ---
> > >  .../devicetree/bindings/phy/fsl,mxs-usbphy.yaml | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > > index f4b1ca2fb562..ce665a2779b7 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,17 @@ allOf:
> > >        required:
> > >          - fsl,anatop
> > >  
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          const: fsl,imx7ulp-usbphy
> > > +    then:
> > > +      required:
> > > +        - nxp,sim
> > > +    else:
> > > +      properties:
> > > +        nxp,sim: false
> > > +
> > >  additionalProperties: false
> > >  
> > >  examples:
> > 
> > 
> > Could you please pick up patch #3? Rest of the patches are mainlined.
> 
> Can you please resend it as an individual patch with the reviewed-by
> added to it?

Sure. Will do it later.

Thanks,
Xu Yang

[PATCH v2 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>

---
Changes in v2:
 - no changes
---
 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 e172af75f602..acaae22df3ba 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 */
@@ -214,6 +219,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) {
@@ -849,6 +866,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;
@@ -869,8 +902,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;
 }
@@ -879,8 +914,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

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

[Peter Chen]

On 24-07-26 19:32:05, Xu Yang wrote:
> 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>

Reviewed-by: Peter Chen <peter.chen@kernel.org>
> 
> ---
> Changes in v2:
>  - no changes
> ---
>  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 e172af75f602..acaae22df3ba 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 */
> @@ -214,6 +219,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) {
> @@ -849,6 +866,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;
> @@ -869,8 +902,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;
>  }
> @@ -879,8 +914,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 v2 5/6] usb: phy: mxs: enable weak 1p1 regulator for imx6ul during suspend

[Xu Yang]

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);
+	}
 }
 
 static int mxs_phy_system_suspend(struct device *dev)
-- 
2.34.1

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

[Peter Chen]

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
> 

[PATCH v2 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>

---
Changes in v2:
 - no changes
---
 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 v2 6/6] ARM: dts: imx7ulp: add "nxp,sim" property for usbphy1

[Xu Yang]

Hi Shawn,

On Fri, Jul 26, 2024 at 07:32:07PM +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 for usbphy1.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - no changes
> ---
>  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 {

Could you please pick up patch 3 and 6?

Thanks in advance!

Best Regards,
Xu Yang

> -- 
> 2.34.1
> 

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

[Shawn Guo]

On Mon, Aug 26, 2024 at 02:56:12PM +0800, Xu Yang wrote:
> Hi Shawn,
> 
> On Fri, Jul 26, 2024 at 07:32:07PM +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 for usbphy1.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > 
> > ---
> > Changes in v2:
> >  - no changes
> > ---
> >  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 {
> 
> Could you please pick up patch 3 and 6?

Patch #3 should go via usb tree, and ping me on #6 after #3 is applied.

Shawn

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

[Xu Yang]

On Wed, Aug 28, 2024 at 09:44:00AM +0800, Shawn Guo wrote:
> On Mon, Aug 26, 2024 at 02:56:12PM +0800, Xu Yang wrote:
> > Hi Shawn,
> > 
> > On Fri, Jul 26, 2024 at 07:32:07PM +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 for usbphy1.
> > > 
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > 
> > > ---
> > > Changes in v2:
> > >  - no changes
> > > ---
> > >  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 {
> > 
> > Could you please pick up patch 3 and 6?
> 
> Patch #3 should go via usb tree, and ping me on #6 after #3 is applied.

Okay.

Thanks,
Xu Yang

> 
> Shawn
> 

Re: [PATCH v2 1/6] usb: phy: mxs: enable regulator phy-3p0 to improve signal qualilty

[Peter Chen]

On 24-07-26 19:32:02, Xu Yang wrote:
> Enable regulator 'phy-3p0' to pass eye diagram test since it improve signal
> qualilty.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Reviewed-by: Peter Chen <peter.chen@kernel.org>

> 
> ---
> Changes in v2:
>  - rewrite commit message
>  - use dev_err_probe() as suggested by Frank Li
> ---
>  drivers/usb/phy/phy-mxs-usb.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 920a32cd094d..d9d29f0b37de 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,17 @@ 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) == -ENODEV)
> +		/* not exist */
> +		mxs_phy->phy_3p0 = NULL;
> +	else if (IS_ERR(mxs_phy->phy_3p0))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(mxs_phy->phy_3p0),
> +				"Getting regulator error\n");
> +
> +	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
>