[PATCH 1/2] dt-bindings: phy: imx8mq-usb: improve some tuning properties

[PATCH 1/2] dt-bindings: phy: imx8mq-usb: improve some tuning properties

[Xu Yang]

Commit b2e75563dc39 ("dt-bindings: phy: imx8mq-usb: add phy tuning
properties") add many tuning properties, but some parameter value doesn't
match the register description. It made some changes based on the original
value: add offset to a negative number so turn it to a non-negative number.
However, this conversion bring many pain when some properties need to be
improved.

Because device-tree supports negative parameter number, this will improve
some propertie's parameter.

Mainly include below properties:
 - fsl,phy-tx-vref-tune-percent
 - fsl,phy-tx-rise-tune-percent
 - fsl,phy-comp-dis-tune-percent

The parameter value of above 3 properties are USB PHY specific. i.MX8MP
and i.MX95 USB PHY has different meanings. So this add restrictions for
them.

 - fsl,phy-tx-vboost-level-microvolt

For this property, the parameter value is wrong in register description.
This will correct it according to true value.

For detailed info, please refer to i.MX8MP and i.MX95 latest reference
manual.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 43 +++++++++++++++----
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
index daee0c0fc915..b09e3dab4c59 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
@@ -43,15 +43,11 @@ properties:
   fsl,phy-tx-vref-tune-percent:
     description:
       Tunes the HS DC level relative to the nominal level
-    minimum: 94
-    maximum: 124
 
   fsl,phy-tx-rise-tune-percent:
     description:
       Adjusts the rise/fall time duration of the HS waveform relative to
       its nominal value
-    minimum: 97
-    maximum: 103
 
   fsl,phy-tx-preemp-amp-tune-microamp:
     description:
@@ -63,15 +59,12 @@ properties:
   fsl,phy-tx-vboost-level-microvolt:
     description:
       Adjust the boosted transmit launch pk-pk differential amplitude
-    minimum: 880
-    maximum: 1120
+    enum: [844, 1008, 1156]
 
   fsl,phy-comp-dis-tune-percent:
     description:
       Adjust the voltage level used to detect a disconnect event at the host
       relative to the nominal value
-    minimum: 91
-    maximum: 115
 
   fsl,phy-pcs-tx-deemph-3p5db-attenuation-db:
     description:
@@ -112,6 +105,40 @@ allOf:
         reg:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx8mq-usb-phy
+            - fsl,imx8mp-usb-phy
+    then:
+      properties:
+        fsl,phy-tx-vref-tune-percent:
+          minimum: -6
+          maximum: 24
+        fsl,phy-tx-rise-tune-percent:
+          enum: [-3, -1, 0, 3]
+        fsl,phy-comp-dis-tune-percent:
+          enum: [-9, -6, -3, 0, 4, 7, 11, 15]
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx95-usb-phy
+    then:
+      properties:
+        fsl,phy-tx-vref-tune-percent:
+          description: 100x the original
+          minimum: -1000
+          maximum: 875
+        fsl,phy-tx-rise-tune-percent:
+          enum: [-10, 0, 15, 20]
+        fsl,phy-comp-dis-tune-percent:
+          description: 10x the original
+          enum: [-60, -45, -30, -15, 0, 15, 30, 45]
+
   - if:
       required:
         - orientation-switch
-- 
2.34.1

[PATCH 2/2] phy: fsl-imx8mq-usb: improve some tuning parameters and support i.MX95 tuning

[Xu Yang]

Since device tree support negative number, this will improve some
parameter parsing functions to handle negative number. Because i.MX8MP
and i.MX95 USB3 PHY have different tuning parameter for same tuning
field, this will add i.MX95 tuning functions too.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 94 ++++++++++++++++------
 1 file changed, 68 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index a974ef94de9a..59b9b4704b03 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -113,11 +113,11 @@ struct imx8mq_usb_phy {
 	struct tca_blk *tca;
 	u32 pcs_tx_swing_full;
 	u32 pcs_tx_deemph_3p5db;
-	u32 tx_vref_tune;
-	u32 tx_rise_tune;
+	s32 tx_vref_tune;
+	s32 tx_rise_tune;
 	u32 tx_preemp_amp_tune;
 	u32 tx_vboost_level;
-	u32 comp_dis_tune;
+	s32 comp_dis_tune;
 };
 
 
@@ -286,24 +286,45 @@ static void imx95_usb_phy_put_tca(struct imx8mq_usb_phy *imx_phy)
 	tca_blk_put_typec_switch(tca->sw);
 }
 
-static u32 phy_tx_vref_tune_from_property(u32 percent)
+static u32 phy_tx_vref_tune_from_property(s32 percent)
 {
-	percent = clamp(percent, 94U, 124U);
+	percent = clamp(percent, -6, 24);
 
-	return DIV_ROUND_CLOSEST(percent - 94U, 2);
+	return DIV_ROUND_CLOSEST(percent + 6, 2);
 }
 
-static u32 phy_tx_rise_tune_from_property(u32 percent)
+static u32 imx95_phy_tx_vref_tune_from_property(s32 percent)
+{
+	percent = clamp(percent, -1000, 875);
+
+	return DIV_ROUND_CLOSEST(percent + 1000, 125);
+}
+
+static u32 phy_tx_rise_tune_from_property(s32 percent)
 {
 	switch (percent) {
-	case 0 ... 98:
+	case -3:
 		return 3;
-	case 99:
+	case -1:
 		return 2;
-	case 100 ... 101:
-		return 1;
+	case 3:
+		return 0;
 	default:
+		return 1;
+	}
+}
+
+static u32 imx95_phy_tx_rise_tune_from_property(s32 percent)
+{
+	switch (percent) {
+	case -10:
+		return 3;
+	case 15:
+		return 1;
+	case 20:
 		return 0;
+	default:
+		return 2;
 	}
 }
 
@@ -317,12 +338,12 @@ static u32 phy_tx_preemp_amp_tune_from_property(u32 microamp)
 static u32 phy_tx_vboost_level_from_property(u32 microvolt)
 {
 	switch (microvolt) {
-	case 0 ... 960:
-		return 0;
-	case 961 ... 1160:
-		return 2;
-	default:
+	case 1156:
+		return 5;
+	case 844:
 		return 3;
+	default:
+		return 4;
 	}
 }
 
@@ -331,27 +352,35 @@ static u32 phy_pcs_tx_deemph_3p5db_from_property(u32 decibel)
 	return min(decibel, 36U);
 }
 
-static u32 phy_comp_dis_tune_from_property(u32 percent)
+static u32 phy_comp_dis_tune_from_property(s32 percent)
 {
 	switch (percent) {
-	case 0 ... 92:
+	case -9:
 		return 0;
-	case 93 ... 95:
+	case -6:
 		return 1;
-	case 96 ... 97:
+	case -3:
 		return 2;
-	case 98 ... 102:
-		return 3;
-	case 103 ... 105:
+	case 4:
 		return 4;
-	case 106 ... 109:
+	case 7:
 		return 5;
-	case 110 ... 113:
+	case 11:
 		return 6;
-	default:
+	case 15:
 		return 7;
+	default:
+		return 3;
 	}
 }
+
+static u32 imx95_phy_comp_dis_tune_from_property(s32 percent)
+{
+	percent = clamp(percent, -60, 45);
+
+	return DIV_ROUND_CLOSEST(percent + 60, 15);
+}
+
 static u32 phy_pcs_tx_swing_full_from_property(u32 percent)
 {
 	percent = min(percent, 100U);
@@ -362,10 +391,17 @@ static u32 phy_pcs_tx_swing_full_from_property(u32 percent)
 static void imx8m_get_phy_tuning_data(struct imx8mq_usb_phy *imx_phy)
 {
 	struct device *dev = imx_phy->phy->dev.parent;
+	bool is_imx95 = false;
+
+	if (device_is_compatible(dev, "fsl,imx95-usb-phy"))
+		is_imx95 = true;
 
 	if (device_property_read_u32(dev, "fsl,phy-tx-vref-tune-percent",
 				     &imx_phy->tx_vref_tune))
 		imx_phy->tx_vref_tune = PHY_TUNE_DEFAULT;
+	else if (is_imx95)
+		imx_phy->tx_vref_tune =
+			imx95_phy_tx_vref_tune_from_property(imx_phy->tx_vref_tune);
 	else
 		imx_phy->tx_vref_tune =
 			phy_tx_vref_tune_from_property(imx_phy->tx_vref_tune);
@@ -373,6 +409,9 @@ static void imx8m_get_phy_tuning_data(struct imx8mq_usb_phy *imx_phy)
 	if (device_property_read_u32(dev, "fsl,phy-tx-rise-tune-percent",
 				     &imx_phy->tx_rise_tune))
 		imx_phy->tx_rise_tune = PHY_TUNE_DEFAULT;
+	else if (is_imx95)
+		imx_phy->tx_rise_tune =
+			imx95_phy_tx_rise_tune_from_property(imx_phy->tx_rise_tune);
 	else
 		imx_phy->tx_rise_tune =
 			phy_tx_rise_tune_from_property(imx_phy->tx_rise_tune);
@@ -394,6 +433,9 @@ static void imx8m_get_phy_tuning_data(struct imx8mq_usb_phy *imx_phy)
 	if (device_property_read_u32(dev, "fsl,phy-comp-dis-tune-percent",
 				     &imx_phy->comp_dis_tune))
 		imx_phy->comp_dis_tune = PHY_TUNE_DEFAULT;
+	else if (is_imx95)
+		imx_phy->comp_dis_tune =
+			imx95_phy_comp_dis_tune_from_property(imx_phy->comp_dis_tune);
 	else
 		imx_phy->comp_dis_tune =
 			phy_comp_dis_tune_from_property(imx_phy->comp_dis_tune);
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: improve some tuning properties

[Krzysztof Kozlowski]

On Wed, Apr 23, 2025 at 05:46:06PM GMT, Xu Yang wrote:
> Commit b2e75563dc39 ("dt-bindings: phy: imx8mq-usb: add phy tuning
> properties") add many tuning properties, but some parameter value doesn't
> match the register description. It made some changes based on the original
> value: add offset to a negative number so turn it to a non-negative number.
> However, this conversion bring many pain when some properties need to be
> improved.
> 
> Because device-tree supports negative parameter number, this will improve
> some propertie's parameter.
> 
> Mainly include below properties:
>  - fsl,phy-tx-vref-tune-percent
>  - fsl,phy-tx-rise-tune-percent
>  - fsl,phy-comp-dis-tune-percent
> 
> The parameter value of above 3 properties are USB PHY specific. i.MX8MP
> and i.MX95 USB PHY has different meanings. So this add restrictions for
> them.
> 
>  - fsl,phy-tx-vboost-level-microvolt
> 
> For this property, the parameter value is wrong in register description.
> This will correct it according to true value.
> 
> For detailed info, please refer to i.MX8MP and i.MX95 latest reference
> manual.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 43 +++++++++++++++----
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> index daee0c0fc915..b09e3dab4c59 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> @@ -43,15 +43,11 @@ properties:
>    fsl,phy-tx-vref-tune-percent:
>      description:
>        Tunes the HS DC level relative to the nominal level
> -    minimum: 94
> -    maximum: 124

Widest constraints should always stay here.

>  
>    fsl,phy-tx-rise-tune-percent:
>      description:
>        Adjusts the rise/fall time duration of the HS waveform relative to
>        its nominal value
> -    minimum: 97
> -    maximum: 103
>  
>    fsl,phy-tx-preemp-amp-tune-microamp:
>      description:
> @@ -63,15 +59,12 @@ properties:
>    fsl,phy-tx-vboost-level-microvolt:
>      description:
>        Adjust the boosted transmit launch pk-pk differential amplitude
> -    minimum: 880
> -    maximum: 1120
> +    enum: [844, 1008, 1156]
>  
>    fsl,phy-comp-dis-tune-percent:
>      description:
>        Adjust the voltage level used to detect a disconnect event at the host
>        relative to the nominal value
> -    minimum: 91
> -    maximum: 115
>  
>    fsl,phy-pcs-tx-deemph-3p5db-attenuation-db:
>      description:
> @@ -112,6 +105,40 @@ allOf:
>          reg:
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx8mq-usb-phy
> +            - fsl,imx8mp-usb-phy
> +    then:
> +      properties:
> +        fsl,phy-tx-vref-tune-percent:
> +          minimum: -6
> +          maximum: 24

That's ABI break with vague reason "some pain".

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: improve some tuning properties

[Xu Yang]

On Mon, Apr 28, 2025 at 09:59:42AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Apr 23, 2025 at 05:46:06PM GMT, Xu Yang wrote:
> > Commit b2e75563dc39 ("dt-bindings: phy: imx8mq-usb: add phy tuning
> > properties") add many tuning properties, but some parameter value doesn't
> > match the register description. It made some changes based on the original
> > value: add offset to a negative number so turn it to a non-negative number.
> > However, this conversion bring many pain when some properties need to be
> > improved.
> > 
> > Because device-tree supports negative parameter number, this will improve
> > some propertie's parameter.
> > 
> > Mainly include below properties:
> >  - fsl,phy-tx-vref-tune-percent
> >  - fsl,phy-tx-rise-tune-percent
> >  - fsl,phy-comp-dis-tune-percent
> > 
> > The parameter value of above 3 properties are USB PHY specific. i.MX8MP
> > and i.MX95 USB PHY has different meanings. So this add restrictions for
> > them.
> > 
> >  - fsl,phy-tx-vboost-level-microvolt
> > 
> > For this property, the parameter value is wrong in register description.
> > This will correct it according to true value.
> > 
> > For detailed info, please refer to i.MX8MP and i.MX95 latest reference
> > manual.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 43 +++++++++++++++----
> >  1 file changed, 35 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > index daee0c0fc915..b09e3dab4c59 100644
> > --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > @@ -43,15 +43,11 @@ properties:
> >    fsl,phy-tx-vref-tune-percent:
> >      description:
> >        Tunes the HS DC level relative to the nominal level
> > -    minimum: 94
> > -    maximum: 124
> 
> Widest constraints should always stay here.

If so, I will restore and modify them in v2.

> 
> >  
> >    fsl,phy-tx-rise-tune-percent:
> >      description:
> >        Adjusts the rise/fall time duration of the HS waveform relative to
> >        its nominal value
> > -    minimum: 97
> > -    maximum: 103
> >  
> >    fsl,phy-tx-preemp-amp-tune-microamp:
> >      description:
> > @@ -63,15 +59,12 @@ properties:
> >    fsl,phy-tx-vboost-level-microvolt:
> >      description:
> >        Adjust the boosted transmit launch pk-pk differential amplitude
> > -    minimum: 880
> > -    maximum: 1120
> > +    enum: [844, 1008, 1156]
> >  
> >    fsl,phy-comp-dis-tune-percent:
> >      description:
> >        Adjust the voltage level used to detect a disconnect event at the host
> >        relative to the nominal value
> > -    minimum: 91
> > -    maximum: 115
> >  
> >    fsl,phy-pcs-tx-deemph-3p5db-attenuation-db:
> >      description:
> > @@ -112,6 +105,40 @@ allOf:
> >          reg:
> >            maxItems: 1
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,imx8mq-usb-phy
> > +            - fsl,imx8mp-usb-phy
> > +    then:
> > +      properties:
> > +        fsl,phy-tx-vref-tune-percent:
> > +          minimum: -6
> > +          maximum: 24
> 
> That's ABI break with vague reason "some pain".

For these four properties, no DTS files use them now. So that's why I'm
going to improve them. I think no ABI break here.

 - fsl,phy-tx-vref-tune-percent
 - fsl,phy-tx-rise-tune-percent
 - fsl,phy-comp-dis-tune-percent
 - fsl,phy-tx-vboost-level-microvolt

Thanks,
Xu Yang

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: improve some tuning properties

[Krzysztof Kozlowski]

On 28/04/2025 10:20, Xu Yang wrote:
>>>  
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          enum:
>>> +            - fsl,imx8mq-usb-phy
>>> +            - fsl,imx8mp-usb-phy
>>> +    then:
>>> +      properties:
>>> +        fsl,phy-tx-vref-tune-percent:
>>> +          minimum: -6
>>> +          maximum: 24
>>
>> That's ABI break with vague reason "some pain".
> 
> For these four properties, no DTS files use them now. So that's why I'm
> going to improve them. I think no ABI break here.

And which out of tree users did you investigate? Read what is ABI before
you keep insisting that it is not an ABI break.

You change already approved and released interface, right? Or not?

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: improve some tuning properties

[Xu Yang]

On Tue, Apr 29, 2025 at 09:33:02AM +0200, Krzysztof Kozlowski wrote:
> On 28/04/2025 10:20, Xu Yang wrote:
> >>>  
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          enum:
> >>> +            - fsl,imx8mq-usb-phy
> >>> +            - fsl,imx8mp-usb-phy
> >>> +    then:
> >>> +      properties:
> >>> +        fsl,phy-tx-vref-tune-percent:
> >>> +          minimum: -6
> >>> +          maximum: 24
> >>
> >> That's ABI break with vague reason "some pain".
> > 
> > For these four properties, no DTS files use them now. So that's why I'm
> > going to improve them. I think no ABI break here.
> 
> And which out of tree users did you investigate? Read what is ABI before
> you keep insisting that it is not an ABI break.

Given out of tree DTS need also to be considered, I will keep dtschema as
it is.

Thanks,
Xu Yang