[PATCH v2 1/3] usb: chipidea: imx: improve logic if samsung,picophy-* parameter is 0

[PATCH v2 1/3] usb: chipidea: imx: improve logic if samsung,picophy-* parameter is 0

[Xu Yang]

In current driver, the value of tuning parameter will not take effect
if samsung,picophy-* is assigned as 0. Because 0 is also a valid value
acccording to the description of USB_PHY_CFG1 register, this will improve
the logic to let it work.

Fixes: 58a3cefb3840 ("usb: chipidea: imx: add two samsung picophy parameters tuning implementation")
cc: <stable@vger.kernel.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - no change
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 10 ++++++----
 drivers/usb/chipidea/usbmisc_imx.c |  6 ++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 336ef6dd8e7d..873539f9a2c0 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -175,10 +175,12 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 	if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
 		data->ulpi = 1;
 
-	of_property_read_u32(np, "samsung,picophy-pre-emp-curr-control",
-			&data->emp_curr_control);
-	of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
-			&data->dc_vol_level_adjust);
+	if (of_property_read_u32(np, "samsung,picophy-pre-emp-curr-control",
+			&data->emp_curr_control))
+		data->emp_curr_control = -1;
+	if (of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
+			&data->dc_vol_level_adjust))
+		data->dc_vol_level_adjust = -1;
 
 	return data;
 }
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 9ee9621e2ccc..1c7932f22218 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -659,13 +659,15 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 			usbmisc->base + MX7D_USBNC_USB_CTRL2);
 		/* PHY tuning for signal quality */
 		reg = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
-		if (data->emp_curr_control && data->emp_curr_control <=
+		if (data->emp_curr_control >= 0 &&
+			data->emp_curr_control <=
 			(TXPREEMPAMPTUNE0_MASK >> TXPREEMPAMPTUNE0_BIT)) {
 			reg &= ~TXPREEMPAMPTUNE0_MASK;
 			reg |= (data->emp_curr_control << TXPREEMPAMPTUNE0_BIT);
 		}
 
-		if (data->dc_vol_level_adjust && data->dc_vol_level_adjust <=
+		if (data->dc_vol_level_adjust >= 0 &&
+			data->dc_vol_level_adjust <=
 			(TXVREFTUNE0_MASK >> TXVREFTUNE0_BIT)) {
 			reg &= ~TXVREFTUNE0_MASK;
 			reg |= (data->dc_vol_level_adjust << TXVREFTUNE0_BIT);
-- 
2.34.1

[PATCH v2 2/3] dt-bindings: usb: ci-hdrc-usb2: add fsl,picophy-rise-fall-time-adjust property

[Xu Yang]

The fsl,picophy-rise-fall-time-adjust property can help to adjust the
rise/fall times of the high-speed transmitter waveform. The value can be
0~3. It has no unit. According to the description of USBNC_n_PHY_CFG1
register, the rise/fall time will be increased or decreased by a certain
percentage relative to design default time if a value is given to this
property.

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

---
Changes in v2:
 - rename property to fsl,picophy-rise-fall-time-adjust
 - add default: 1
 - add some description in yaml and commit msg
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml        | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 782402800d4a..532d6464c8b3 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -292,6 +292,18 @@ properties:
     minimum: 0x0
     maximum: 0xf
 
+  fsl,picophy-rise-fall-time-adjust:
+    description:
+      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times
+      of the high-speed transmitter waveform. It has no unit. The rise/fall
+      time will be increased or decreased by a certain percentage relative
+      to design default time. (0:-10%; 1:design default; 2:+15%; 3:+20%)
+      Details can refer to TXRISETUNE0 bit of USBNC_n_PHY_CFG1.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 3
+    default: 1
+
   usb-phy:
     description: phandle for the PHY device. Use "phys" instead.
     $ref: /schemas/types.yaml#/definitions/phandle
-- 
2.34.1

Re: [PATCH v2 2/3] dt-bindings: usb: ci-hdrc-usb2: add fsl,picophy-rise-fall-time-adjust property

[Krzysztof Kozlowski]

On 27/06/2023 13:21, Xu Yang wrote:
> The fsl,picophy-rise-fall-time-adjust property can help to adjust the
> rise/fall times of the high-speed transmitter waveform. The value can be
> 0~3. It has no unit. According to the description of USBNC_n_PHY_CFG1
> register, the rise/fall time will be increased or decreased by a certain
> percentage relative to design default time if a value is given to this
> 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 2/3] dt-bindings: usb: ci-hdrc-usb2: add fsl,picophy-rise-fall-time-adjust property

[Peter Chen]

On 23-06-27 19:21:25, Xu Yang wrote:
> The fsl,picophy-rise-fall-time-adjust property can help to adjust the
> rise/fall times of the high-speed transmitter waveform. The value can be
> 0~3. It has no unit. According to the description of USBNC_n_PHY_CFG1
> register, the rise/fall time will be increased or decreased by a certain
> percentage relative to design default time if a value is given to this
> property.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

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

Peter
> 
> ---
> Changes in v2:
>  - rename property to fsl,picophy-rise-fall-time-adjust
>  - add default: 1
>  - add some description in yaml and commit msg
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml        | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index 782402800d4a..532d6464c8b3 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -292,6 +292,18 @@ properties:
>      minimum: 0x0
>      maximum: 0xf
>  
> +  fsl,picophy-rise-fall-time-adjust:
> +    description:
> +      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times
> +      of the high-speed transmitter waveform. It has no unit. The rise/fall
> +      time will be increased or decreased by a certain percentage relative
> +      to design default time. (0:-10%; 1:design default; 2:+15%; 3:+20%)
> +      Details can refer to TXRISETUNE0 bit of USBNC_n_PHY_CFG1.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    default: 1
> +
>    usb-phy:
>      description: phandle for the PHY device. Use "phys" instead.
>      $ref: /schemas/types.yaml#/definitions/phandle
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

[PATCH v2 3/3] usb: chipidea: imx: add one fsl picophy parameter tuning implementation

[Xu Yang]

In some cases, the user may need to tune the rise/fall time of the
high-speed transmitter waveform for USB Certification. This will add
a parameter for this purpose. The value will be fetched from dtb and
finally written to the register.

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

---
Changes in v2:
 - rename parameter name
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++
 drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
 drivers/usb/chipidea/usbmisc_imx.c | 9 +++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 873539f9a2c0..159509c5fe58 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -181,6 +181,9 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 	if (of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
 			&data->dc_vol_level_adjust))
 		data->dc_vol_level_adjust = -1;
+	if (of_property_read_u32(np, "fsl,picophy-rise-fall-time-adjust",
+			&data->rise_fall_time_adjust))
+		data->rise_fall_time_adjust = -1;
 
 	return data;
 }
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 7135b9a5d913..88b8da79d518 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -28,6 +28,7 @@ struct imx_usbmisc_data {
 	enum usb_dr_mode available_role; /* runtime usb dr mode */
 	int emp_curr_control;
 	int dc_vol_level_adjust;
+	int rise_fall_time_adjust;
 };
 
 int imx_usbmisc_init(struct imx_usbmisc_data *data);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 1c7932f22218..56570100ffb8 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -130,6 +130,8 @@
 #define MX7D_USB_OTG_PHY_CFG1		0x30
 #define TXPREEMPAMPTUNE0_BIT		28
 #define TXPREEMPAMPTUNE0_MASK		(3 << 28)
+#define TXRISETUNE0_BIT			24
+#define TXRISETUNE0_MASK		(3 << 24)
 #define TXVREFTUNE0_BIT			20
 #define TXVREFTUNE0_MASK		(0xf << 20)
 
@@ -673,6 +675,13 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 			reg |= (data->dc_vol_level_adjust << TXVREFTUNE0_BIT);
 		}
 
+		if (data->rise_fall_time_adjust >= 0 &&
+			data->rise_fall_time_adjust <=
+			(TXRISETUNE0_MASK >> TXRISETUNE0_BIT)) {
+			reg &= ~TXRISETUNE0_MASK;
+			reg |= (data->rise_fall_time_adjust << TXRISETUNE0_BIT);
+		}
+
 		writel(reg, usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
 	}
 
-- 
2.34.1

Re: [PATCH v2 3/3] usb: chipidea: imx: add one fsl picophy parameter tuning implementation

[Peter Chen]

On 23-06-27 19:21:26, Xu Yang wrote:
> In some cases, the user may need to tune the rise/fall time of the
> high-speed transmitter waveform for USB Certification. This will add
> a parameter for this purpose. The value will be fetched from dtb and
> finally written to the register.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

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

Peter
> 
> ---
> Changes in v2:
>  - rename parameter name
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++
>  drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
>  drivers/usb/chipidea/usbmisc_imx.c | 9 +++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 873539f9a2c0..159509c5fe58 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -181,6 +181,9 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
>  	if (of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
>  			&data->dc_vol_level_adjust))
>  		data->dc_vol_level_adjust = -1;
> +	if (of_property_read_u32(np, "fsl,picophy-rise-fall-time-adjust",
> +			&data->rise_fall_time_adjust))
> +		data->rise_fall_time_adjust = -1;
>  
>  	return data;
>  }
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
> index 7135b9a5d913..88b8da79d518 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -28,6 +28,7 @@ struct imx_usbmisc_data {
>  	enum usb_dr_mode available_role; /* runtime usb dr mode */
>  	int emp_curr_control;
>  	int dc_vol_level_adjust;
> +	int rise_fall_time_adjust;
>  };
>  
>  int imx_usbmisc_init(struct imx_usbmisc_data *data);
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 1c7932f22218..56570100ffb8 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -130,6 +130,8 @@
>  #define MX7D_USB_OTG_PHY_CFG1		0x30
>  #define TXPREEMPAMPTUNE0_BIT		28
>  #define TXPREEMPAMPTUNE0_MASK		(3 << 28)
> +#define TXRISETUNE0_BIT			24
> +#define TXRISETUNE0_MASK		(3 << 24)
>  #define TXVREFTUNE0_BIT			20
>  #define TXVREFTUNE0_MASK		(0xf << 20)
>  
> @@ -673,6 +675,13 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
>  			reg |= (data->dc_vol_level_adjust << TXVREFTUNE0_BIT);
>  		}
>  
> +		if (data->rise_fall_time_adjust >= 0 &&
> +			data->rise_fall_time_adjust <=
> +			(TXRISETUNE0_MASK >> TXRISETUNE0_BIT)) {
> +			reg &= ~TXRISETUNE0_MASK;
> +			reg |= (data->rise_fall_time_adjust << TXRISETUNE0_BIT);
> +		}
> +
>  		writel(reg, usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
>  	}
>  
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v2 1/3] usb: chipidea: imx: improve logic if samsung,picophy-* parameter is 0

[Peter Chen]

On 23-06-27 19:21:24, Xu Yang wrote:
> In current driver, the value of tuning parameter will not take effect
> if samsung,picophy-* is assigned as 0. Because 0 is also a valid value
> acccording to the description of USB_PHY_CFG1 register, this will improve
> the logic to let it work.
> 
> Fixes: 58a3cefb3840 ("usb: chipidea: imx: add two samsung picophy parameters tuning implementation")
> cc: <stable@vger.kernel.org>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

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

Peter

> 
> ---
> Changes in v2:
>  - no change
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 10 ++++++----
>  drivers/usb/chipidea/usbmisc_imx.c |  6 ++++--
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 336ef6dd8e7d..873539f9a2c0 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -175,10 +175,12 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
>  	if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
>  		data->ulpi = 1;
>  
> -	of_property_read_u32(np, "samsung,picophy-pre-emp-curr-control",
> -			&data->emp_curr_control);
> -	of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
> -			&data->dc_vol_level_adjust);
> +	if (of_property_read_u32(np, "samsung,picophy-pre-emp-curr-control",
> +			&data->emp_curr_control))
> +		data->emp_curr_control = -1;
> +	if (of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
> +			&data->dc_vol_level_adjust))
> +		data->dc_vol_level_adjust = -1;
>  
>  	return data;
>  }
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 9ee9621e2ccc..1c7932f22218 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -659,13 +659,15 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
>  			usbmisc->base + MX7D_USBNC_USB_CTRL2);
>  		/* PHY tuning for signal quality */
>  		reg = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
> -		if (data->emp_curr_control && data->emp_curr_control <=
> +		if (data->emp_curr_control >= 0 &&
> +			data->emp_curr_control <=
>  			(TXPREEMPAMPTUNE0_MASK >> TXPREEMPAMPTUNE0_BIT)) {
>  			reg &= ~TXPREEMPAMPTUNE0_MASK;
>  			reg |= (data->emp_curr_control << TXPREEMPAMPTUNE0_BIT);
>  		}
>  
> -		if (data->dc_vol_level_adjust && data->dc_vol_level_adjust <=
> +		if (data->dc_vol_level_adjust >= 0 &&
> +			data->dc_vol_level_adjust <=
>  			(TXVREFTUNE0_MASK >> TXVREFTUNE0_BIT)) {
>  			reg &= ~TXVREFTUNE0_MASK;
>  			reg |= (data->dc_vol_level_adjust << TXVREFTUNE0_BIT);
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen