linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/4] Add DT-based gear and rate limiting support
@ 2025-08-26 15:08 Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties Ram Kumar Dwivedi
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-26 15:08 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, robh, krzk+dt, conor+dt,
	mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

From: Ram Kumar Dwivedi <rdwivedi@qti.qualcomm.com>

This patch series adds support for limiting the maximum high-speed
gear and rate used by the UFS controller via device tree properties.

Some platforms may have signal integrity, clock configuration, or
layout issues that prevent reliable operation at higher gears or rates.
This is especially critical in automotive and other platforms where
stability is prioritized over peak performance.

The series follows this logical progression:
1. Document the new DT properties in the common UFS binding
2. Clean up existing redundant code in the qcom driver
3. Add platform-level parsing support for the new properties
4. Integrate the platform support in the qcom driver

This approach makes the functionality available to other UFS host
drivers and provides a cleaner, more maintainable implementation.

Changes from V1:
- Restructured patch series for better logical flow and maintainability.
- Moved DT bindings to ufs-common.yaml making it available for all UFS
  controllers.
- Added platform-level support in ufshcd-pltfrm.c for code reusability.
- Separated the cleanup patch to remove redundant hs_rate assignment in
  qcom driver.
- Removed SA8155 DTS changes to keep the series focused on core
  functionality.
- Improved commit messages with better technical rationale.

Changes from V2:
- Documented default values of limit-rate and limit-hs-gear in DT bindings
  as per Krzysztof's suggestion.

Ram Kumar Dwivedi (4):
  ufs: dt-bindings: Document gear and rate limit properties
  ufs: ufs-qcom: Remove redundant re-assignment to hs_rate
  ufs: pltfrm: Allow limiting HS gear and rate via DT
  ufs: ufs-qcom: Add support for limiting HS gear and rate

 .../devicetree/bindings/ufs/ufs-common.yaml   | 18 ++++++++++++
 drivers/ufs/host/ufs-qcom.c                   | 21 ++++++++++----
 drivers/ufs/host/ufshcd-pltfrm.c              | 29 +++++++++++++++++++
 drivers/ufs/host/ufshcd-pltfrm.h              |  1 +
 4 files changed, 63 insertions(+), 6 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-26 15:08 [PATCH V3 0/4] Add DT-based gear and rate limiting support Ram Kumar Dwivedi
@ 2025-08-26 15:08 ` Ram Kumar Dwivedi
  2025-08-26 15:35   ` Bart Van Assche
  2025-08-26 15:08 ` [PATCH V3 2/4] ufs: ufs-qcom: Remove redundant re-assignment to hs_rate Ram Kumar Dwivedi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-26 15:08 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, robh, krzk+dt, conor+dt,
	mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

Add optional "limit-hs-gear" and "limit-rate" properties to the
UFS controller common binding. These properties allow limiting
the maximum HS gear and rate.

This is useful in cases where the customer board may have signal
integrity, clock configuration or layout issues that prevent reliable
operation at higher gears. Such limitations are especially critical in
those platforms, where stability is prioritized over peak performance.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 .../devicetree/bindings/ufs/ufs-common.yaml    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
index 31fe7f30ff5b..1af658207901 100644
--- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml
+++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
@@ -89,6 +89,24 @@ properties:
 
   msi-parent: true
 
+  limit-hs-gear:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 5
+    default: 5
+    description:
+      Restricts the maximum HS gear used in both TX and RX directions,
+      typically for hardware or power constraints in automotive use cases.
+
+  limit-rate:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    default: 2
+    description:
+      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
+      TX and RX directions, often required in automotive environments due
+      to hardware limitations.
+
 dependencies:
   freq-table-hz: [ clocks ]
   operating-points-v2: [ clocks, clock-names ]
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V3 2/4] ufs: ufs-qcom: Remove redundant re-assignment to hs_rate
  2025-08-26 15:08 [PATCH V3 0/4] Add DT-based gear and rate limiting support Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties Ram Kumar Dwivedi
@ 2025-08-26 15:08 ` Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 3/4] ufs: pltfrm: Allow limiting HS gear and rate via DT Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate Ram Kumar Dwivedi
  3 siblings, 0 replies; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-26 15:08 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, robh, krzk+dt, conor+dt,
	mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

Remove the redundant else block that assigns PA_HS_MODE_B to hs_rate,
as it is already assigned in ufshcd_init_host_params(). This avoids
unnecessary reassignment and prevents overwriting hs_rate when it is
explicitly set to a different value.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 9574fdc2bb0f..1a93351fb70e 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -494,12 +494,8 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 	 * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A,
 	 * so that the subsequent power mode change shall stick to Rate-A.
 	 */
-	if (host->hw_ver.major == 0x5) {
-		if (host->phy_gear == UFS_HS_G5)
-			host_params->hs_rate = PA_HS_MODE_A;
-		else
-			host_params->hs_rate = PA_HS_MODE_B;
-	}
+	if (host->hw_ver.major == 0x5 && host->phy_gear == UFS_HS_G5)
+		host_params->hs_rate = PA_HS_MODE_A;
 
 	mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A;
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V3 3/4] ufs: pltfrm: Allow limiting HS gear and rate via DT
  2025-08-26 15:08 [PATCH V3 0/4] Add DT-based gear and rate limiting support Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 2/4] ufs: ufs-qcom: Remove redundant re-assignment to hs_rate Ram Kumar Dwivedi
@ 2025-08-26 15:08 ` Ram Kumar Dwivedi
  2025-08-26 15:08 ` [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate Ram Kumar Dwivedi
  3 siblings, 0 replies; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-26 15:08 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, robh, krzk+dt, conor+dt,
	mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

Add support for parsing 'limit-hs-gear' and 'limit-rate' device tree
properties to restrict high-speed gear and rate during initialization.

This is useful in cases where the customer board may have signal
integrity, clock configuration or layout issues that prevent reliable
operation at higher gears. Such limitations are especially critical in
those platforms, where stability is prioritized over peak performance.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 drivers/ufs/host/ufshcd-pltfrm.c | 29 +++++++++++++++++++++++++++++
 drivers/ufs/host/ufshcd-pltfrm.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index ffe5d1d2b215..d9be6c86f044 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -430,6 +430,35 @@ int ufshcd_negotiate_pwr_params(const struct ufs_host_params *host_params,
 }
 EXPORT_SYMBOL_GPL(ufshcd_negotiate_pwr_params);
 
+/**
+ * ufshcd_parse_limits - Parse DT-based gear and rate limits for UFS
+ * @hba: Pointer to UFS host bus adapter instance
+ * @host_params: Pointer to UFS host parameters structure to be updated
+ *
+ * This function reads optional device tree properties to apply
+ * platform-specific constraints.
+ *
+ * "limit-hs-gear": Specifies the max HS gear.
+ * "limit-rate": Specifies the max High-Speed rate.
+ */
+void ufshcd_parse_limits(struct ufs_hba *hba, struct ufs_host_params *host_params)
+{
+	struct device_node *np = hba->dev->of_node;
+	u32 hs_gear, hs_rate;
+
+	if (!np)
+		return;
+
+	if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {
+		host_params->hs_tx_gear = hs_gear;
+		host_params->hs_rx_gear = hs_gear;
+	}
+
+	if (!of_property_read_u32(np, "limit-rate", &hs_rate))
+		host_params->hs_rate = hs_rate;
+}
+EXPORT_SYMBOL_GPL(ufshcd_parse_limits);
+
 void ufshcd_init_host_params(struct ufs_host_params *host_params)
 {
 	*host_params = (struct ufs_host_params){
diff --git a/drivers/ufs/host/ufshcd-pltfrm.h b/drivers/ufs/host/ufshcd-pltfrm.h
index 3017f8e8f93c..1617f2541273 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.h
+++ b/drivers/ufs/host/ufshcd-pltfrm.h
@@ -29,6 +29,7 @@ int ufshcd_negotiate_pwr_params(const struct ufs_host_params *host_params,
 				const struct ufs_pa_layer_attr *dev_max,
 				struct ufs_pa_layer_attr *agreed_pwr);
 void ufshcd_init_host_params(struct ufs_host_params *host_params);
+void ufshcd_parse_limits(struct ufs_hba *hba, struct ufs_host_params *host_params);
 int ufshcd_pltfrm_init(struct platform_device *pdev,
 		       const struct ufs_hba_variant_ops *vops);
 void ufshcd_pltfrm_remove(struct platform_device *pdev);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate
  2025-08-26 15:08 [PATCH V3 0/4] Add DT-based gear and rate limiting support Ram Kumar Dwivedi
                   ` (2 preceding siblings ...)
  2025-08-26 15:08 ` [PATCH V3 3/4] ufs: pltfrm: Allow limiting HS gear and rate via DT Ram Kumar Dwivedi
@ 2025-08-26 15:08 ` Ram Kumar Dwivedi
  2025-08-26 15:37   ` Bart Van Assche
  3 siblings, 1 reply; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-26 15:08 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, robh, krzk+dt, conor+dt,
	mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

Add support to limit Tx/Rx gear and rate during UFS initialization
based on DT property.

Also update the phy_gear to ensure PHY calibrations align with
the required gear and rate.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 1a93351fb70e..53c64d5fb95d 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1092,6 +1092,18 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
 	}
 }
 
+static void ufs_qcom_parse_limits(struct ufs_hba *hba)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	struct ufs_host_params *host_params = &host->host_params;
+	u32 hs_gear_old = host_params->hs_tx_gear;
+
+	ufshcd_parse_limits(hba, host_params);
+	if (host_params->hs_tx_gear != hs_gear_old) {
+		host->phy_gear = host_params->hs_tx_gear;
+	}
+}
+
 static void ufs_qcom_set_host_params(struct ufs_hba *hba)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
@@ -1333,6 +1345,7 @@ static int ufs_qcom_init(struct ufs_hba *hba)
 	ufs_qcom_advertise_quirks(hba);
 	ufs_qcom_set_host_params(hba);
 	ufs_qcom_set_phy_gear(host);
+	ufs_qcom_parse_limits(hba);
 
 	err = ufs_qcom_ice_init(host);
 	if (err)
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-26 15:08 ` [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties Ram Kumar Dwivedi
@ 2025-08-26 15:35   ` Bart Van Assche
  2025-08-28 16:25     ` Ram Kumar Dwivedi
                       ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-08-26 15:35 UTC (permalink / raw)
  To: Ram Kumar Dwivedi, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
> +  limit-hs-gear:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 5
> +    default: 5
> +    description:
> +      Restricts the maximum HS gear used in both TX and RX directions,
> +      typically for hardware or power constraints in automotive use cases.

The UFSHCI 5.0 spec will add gear 6 soon. So why to restrict the maximum
gear to 5?

> +  limit-rate:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    default: 2
> +    description:
> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
> +      TX and RX directions, often required in automotive environments due
> +      to hardware limitations.

As far as I know no numeric values are associated with these rates in
the UFSHCI 4.1 standard nor in any of the previous versions of this
standard. Does the .yaml syntax support something like "enum: [A, B]"?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate
  2025-08-26 15:08 ` [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate Ram Kumar Dwivedi
@ 2025-08-26 15:37   ` Bart Van Assche
  2025-08-28 16:28     ` Ram Kumar Dwivedi
  0 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-08-26 15:37 UTC (permalink / raw)
  To: Ram Kumar Dwivedi, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
> +	if (host_params->hs_tx_gear != hs_gear_old) {
> +		host->phy_gear = host_params->hs_tx_gear;
> +	}

The recommended style in Linux kernel code is not to surround single
statements with braces.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-26 15:35   ` Bart Van Assche
@ 2025-08-28 16:25     ` Ram Kumar Dwivedi
  2025-08-28 17:41       ` Krzysztof Kozlowski
  2025-08-28 16:45     ` Ram Kumar Dwivedi
  2025-08-28 17:40     ` Krzysztof Kozlowski
  2 siblings, 1 reply; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-28 16:25 UTC (permalink / raw)
  To: Bart Van Assche, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm



On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>> +  limit-hs-gear:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 5
>> +    default: 5
>> +    description:
>> +      Restricts the maximum HS gear used in both TX and RX directions,
>> +      typically for hardware or power constraints in automotive use cases.
> 
> The UFSHCI 5.0 spec will add gear 6 soon. So why to restrict the maximum
> gear to 5?

Hi Bart,

Thanks for the suggestion. I limited it to Gear 5 based on current upstream support. 
But I am fine with updating it to Gear 6 if you prefer, to align with the upcoming
spec and avoid future revisions.

Thanks,
Ram.

> 
>> +  limit-rate:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [1, 2]
>> +    default: 2
>> +    description:
>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>> +      TX and RX directions, often required in automotive environments due
>> +      to hardware limitations.
> 
> As far as I know no numeric values are associated with these rates in
> the UFSHCI 4.1 standard nor in any of the previous versions of this
> standard. Does the .yaml syntax support something like "enum: [A, B]"?
> 
> Thanks,
> 
> Bart.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate
  2025-08-26 15:37   ` Bart Van Assche
@ 2025-08-28 16:28     ` Ram Kumar Dwivedi
  0 siblings, 0 replies; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-28 16:28 UTC (permalink / raw)
  To: Bart Van Assche, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm



On 26-Aug-25 9:07 PM, Bart Van Assche wrote:
> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>> +    if (host_params->hs_tx_gear != hs_gear_old) {
>> +        host->phy_gear = host_params->hs_tx_gear;
>> +    }
> 
> The recommended style in Linux kernel code is not to surround single
> statements with braces.
> 
Hi Bart,

Sure, I will update it in the next patchset.

Thanks,
Ram> Thanks,
> 
> Bart.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-26 15:35   ` Bart Van Assche
  2025-08-28 16:25     ` Ram Kumar Dwivedi
@ 2025-08-28 16:45     ` Ram Kumar Dwivedi
  2025-08-28 17:22       ` Bart Van Assche
  2025-08-28 17:41       ` Krzysztof Kozlowski
  2025-08-28 17:40     ` Krzysztof Kozlowski
  2 siblings, 2 replies; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-08-28 16:45 UTC (permalink / raw)
  To: Bart Van Assche, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm



On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>> +  limit-hs-gear:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 5
>> +    default: 5
>> +    description:
>> +      Restricts the maximum HS gear used in both TX and RX directions,
>> +      typically for hardware or power constraints in automotive use cases.
> 
> The UFSHCI 5.0 spec will add gear 6 soon. So why to restrict the maximum
> gear to 5?
> 
>> +  limit-rate:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [1, 2]
>> +    default: 2
>> +    description:
>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>> +      TX and RX directions, often required in automotive environments due
>> +      to hardware limitations.
> 
> As far as I know no numeric values are associated with these rates in
> the UFSHCI 4.1 standard nor in any of the previous versions of this
> standard. Does the .yaml syntax support something like "enum: [A, B]"?
Hi Bart,

As per the MIPI UniPro spec:

In Section 5.7.12.3.2, the hs_series is defined as:
hs_series = Flags[3] + 1;

In Section 5.7.7.1, Flags[3] is described as:
Set to ‘0’ for Series A and ‘1’ for Series B (PA_HSSeries).

While issuing the DME command from the UFS driver to set the rate,
the values 1 and 2 are passed as arguments for Rate A and Rate B
respectively. Additionally, the hs_rate variable is of type u32.

Thanks,
Ram

> 
> Thanks,
> 
> Bart.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-28 16:45     ` Ram Kumar Dwivedi
@ 2025-08-28 17:22       ` Bart Van Assche
  2025-09-01  4:07         ` Manivannan Sadhasivam
  2025-08-28 17:41       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-08-28 17:22 UTC (permalink / raw)
  To: Ram Kumar Dwivedi, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 8/28/25 9:45 AM, Ram Kumar Dwivedi wrote:
> On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
>> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>>> +  limit-rate:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [1, 2]
>>> +    default: 2
>>> +    description:
>>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>>> +      TX and RX directions, often required in automotive environments due
>>> +      to hardware limitations.
>>
>> As far as I know no numeric values are associated with these rates in
>> the UFSHCI 4.1 standard nor in any of the previous versions of this
>> standard. Does the .yaml syntax support something like "enum: [A, B]"?
> Hi Bart,
> 
> As per the MIPI UniPro spec:
> 
> In Section 5.7.12.3.2, the hs_series is defined as:
> hs_series = Flags[3] + 1;
> 
> In Section 5.7.7.1, Flags[3] is described as:
> Set to ‘0’ for Series A and ‘1’ for Series B (PA_HSSeries).
> 
> While issuing the DME command from the UFS driver to set the rate,
> the values 1 and 2 are passed as arguments for Rate A and Rate B
> respectively. Additionally, the hs_rate variable is of type u32.

Hi Ram,

Thanks for having looked this up.

Since it is much more common to refer to these rates as "Rate A" and
"Rate B" rather than using numbers for these rates, please change the
enumeration labels into something like "Rate_A" and "Rate_B".

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-26 15:35   ` Bart Van Assche
  2025-08-28 16:25     ` Ram Kumar Dwivedi
  2025-08-28 16:45     ` Ram Kumar Dwivedi
@ 2025-08-28 17:40     ` Krzysztof Kozlowski
  2025-08-28 18:27       ` Bart Van Assche
  2 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-28 17:40 UTC (permalink / raw)
  To: Bart Van Assche, Ram Kumar Dwivedi, alim.akhtar, avri.altman,
	robh, krzk+dt, conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 26/08/2025 17:35, Bart Van Assche wrote:
> 
>> +  limit-rate:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [1, 2]
>> +    default: 2
>> +    description:
>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>> +      TX and RX directions, often required in automotive environments due
>> +      to hardware limitations.
> 
> As far as I know no numeric values are associated with these rates in
> the UFSHCI 4.1 standard nor in any of the previous versions of this
> standard. Does the .yaml syntax support something like "enum: [A, B]"?

That's what I also requested and answer was "1" and "2" are coming from
the spec. So now I am confused.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-28 16:25     ` Ram Kumar Dwivedi
@ 2025-08-28 17:41       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-28 17:41 UTC (permalink / raw)
  To: Ram Kumar Dwivedi, Bart Van Assche, alim.akhtar, avri.altman,
	robh, krzk+dt, conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 28/08/2025 18:25, Ram Kumar Dwivedi wrote:
> 
> 
> On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
>> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>>> +  limit-hs-gear:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 1
>>> +    maximum: 5
>>> +    default: 5
>>> +    description:
>>> +      Restricts the maximum HS gear used in both TX and RX directions,
>>> +      typically for hardware or power constraints in automotive use cases.
>>
>> The UFSHCI 5.0 spec will add gear 6 soon. So why to restrict the maximum
>> gear to 5?
> 
> Hi Bart,
> 
> Thanks for the suggestion. I limited it to Gear 5 based on current upstream support.

Upstream of what? Bindings describe hardware.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-28 16:45     ` Ram Kumar Dwivedi
  2025-08-28 17:22       ` Bart Van Assche
@ 2025-08-28 17:41       ` Krzysztof Kozlowski
  1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-28 17:41 UTC (permalink / raw)
  To: Ram Kumar Dwivedi, Bart Van Assche, alim.akhtar, avri.altman,
	robh, krzk+dt, conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 28/08/2025 18:45, Ram Kumar Dwivedi wrote:
> 
> 
> On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
>> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>>> +  limit-hs-gear:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 1
>>> +    maximum: 5
>>> +    default: 5
>>> +    description:
>>> +      Restricts the maximum HS gear used in both TX and RX directions,
>>> +      typically for hardware or power constraints in automotive use cases.
>>
>> The UFSHCI 5.0 spec will add gear 6 soon. So why to restrict the maximum
>> gear to 5?
>>
>>> +  limit-rate:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [1, 2]
>>> +    default: 2
>>> +    description:
>>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>>> +      TX and RX directions, often required in automotive environments due
>>> +      to hardware limitations.
>>
>> As far as I know no numeric values are associated with these rates in
>> the UFSHCI 4.1 standard nor in any of the previous versions of this
>> standard. Does the .yaml syntax support something like "enum: [A, B]"?
> Hi Bart,
> 
> As per the MIPI UniPro spec:
> 
> In Section 5.7.12.3.2, the hs_series is defined as:
> hs_series = Flags[3] + 1;
> 
> In Section 5.7.7.1, Flags[3] is described as:
> Set to ‘0’ for Series A and ‘1’ for Series B (PA_HSSeries).

That's register value. Why are you using values as argument of actual
meaning of rates?


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-28 17:40     ` Krzysztof Kozlowski
@ 2025-08-28 18:27       ` Bart Van Assche
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-08-28 18:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Ram Kumar Dwivedi, alim.akhtar, avri.altman,
	robh, krzk+dt, conor+dt, mani, James.Bottomley, martin.petersen
  Cc: linux-scsi, devicetree, linux-kernel, linux-arm-msm

On 8/28/25 10:40 AM, Krzysztof Kozlowski wrote:
> On 26/08/2025 17:35, Bart Van Assche wrote:
>>
>>> +  limit-rate:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [1, 2]
>>> +    default: 2
>>> +    description:
>>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>>> +      TX and RX directions, often required in automotive environments due
>>> +      to hardware limitations.
>>
>> As far as I know no numeric values are associated with these rates in
>> the UFSHCI 4.1 standard nor in any of the previous versions of this
>> standard. Does the .yaml syntax support something like "enum: [A, B]"?
> 
> That's what I also requested and answer was "1" and "2" are coming from
> the spec. So now I am confused.

Ram quoted from the MIPI spec. That's another standard than what I
referred to (UFSHCI).

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-08-28 17:22       ` Bart Van Assche
@ 2025-09-01  4:07         ` Manivannan Sadhasivam
  2025-09-01 16:01           ` Ram Kumar Dwivedi
  0 siblings, 1 reply; 17+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-01  4:07 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Ram Kumar Dwivedi, alim.akhtar, avri.altman, robh, krzk+dt,
	conor+dt, James.Bottomley, martin.petersen, linux-scsi,
	devicetree, linux-kernel, linux-arm-msm

On Thu, Aug 28, 2025 at 10:22:28AM GMT, Bart Van Assche wrote:
> On 8/28/25 9:45 AM, Ram Kumar Dwivedi wrote:
> > On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
> > > On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
> > > > +  limit-rate:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [1, 2]
> > > > +    default: 2
> > > > +    description:
> > > > +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
> > > > +      TX and RX directions, often required in automotive environments due
> > > > +      to hardware limitations.
> > > 
> > > As far as I know no numeric values are associated with these rates in
> > > the UFSHCI 4.1 standard nor in any of the previous versions of this
> > > standard. Does the .yaml syntax support something like "enum: [A, B]"?
> > Hi Bart,
> > 
> > As per the MIPI UniPro spec:
> > 
> > In Section 5.7.12.3.2, the hs_series is defined as:
> > hs_series = Flags[3] + 1;
> > 
> > In Section 5.7.7.1, Flags[3] is described as:
> > Set to ‘0’ for Series A and ‘1’ for Series B (PA_HSSeries).
> > 
> > While issuing the DME command from the UFS driver to set the rate,
> > the values 1 and 2 are passed as arguments for Rate A and Rate B
> > respectively. Additionally, the hs_rate variable is of type u32.
> 
> Hi Ram,
> 
> Thanks for having looked this up.
> 
> Since it is much more common to refer to these rates as "Rate A" and
> "Rate B" rather than using numbers for these rates, please change the
> enumeration labels into something like "Rate_A" and "Rate_B".
> 

+1. Since this binding describes the HCI, let's stick to the terminologies in
UFSHCI spec.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties
  2025-09-01  4:07         ` Manivannan Sadhasivam
@ 2025-09-01 16:01           ` Ram Kumar Dwivedi
  0 siblings, 0 replies; 17+ messages in thread
From: Ram Kumar Dwivedi @ 2025-09-01 16:01 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Bart Van Assche
  Cc: alim.akhtar, avri.altman, robh, krzk+dt, conor+dt,
	James.Bottomley, martin.petersen, linux-scsi, devicetree,
	linux-kernel, linux-arm-msm



On 01-Sep-25 9:37 AM, Manivannan Sadhasivam wrote:
> On Thu, Aug 28, 2025 at 10:22:28AM GMT, Bart Van Assche wrote:
>> On 8/28/25 9:45 AM, Ram Kumar Dwivedi wrote:
>>> On 26-Aug-25 9:05 PM, Bart Van Assche wrote:
>>>> On 8/26/25 8:08 AM, Ram Kumar Dwivedi wrote:
>>>>> +  limit-rate:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [1, 2]
>>>>> +    default: 2
>>>>> +    description:
>>>>> +      Restricts the UFS controller to Rate A (1) or Rate B (2) for both
>>>>> +      TX and RX directions, often required in automotive environments due
>>>>> +      to hardware limitations.
>>>>
>>>> As far as I know no numeric values are associated with these rates in
>>>> the UFSHCI 4.1 standard nor in any of the previous versions of this
>>>> standard. Does the .yaml syntax support something like "enum: [A, B]"?
>>> Hi Bart,
>>>
>>> As per the MIPI UniPro spec:
>>>
>>> In Section 5.7.12.3.2, the hs_series is defined as:
>>> hs_series = Flags[3] + 1;
>>>
>>> In Section 5.7.7.1, Flags[3] is described as:
>>> Set to ‘0’ for Series A and ‘1’ for Series B (PA_HSSeries).
>>>
>>> While issuing the DME command from the UFS driver to set the rate,
>>> the values 1 and 2 are passed as arguments for Rate A and Rate B
>>> respectively. Additionally, the hs_rate variable is of type u32.
>>
>> Hi Ram,
>>
>> Thanks for having looked this up.
>>
>> Since it is much more common to refer to these rates as "Rate A" and
>> "Rate B" rather than using numbers for these rates, please change the
>> enumeration labels into something like "Rate_A" and "Rate_B".
>>
> 
> +1. Since this binding describes the HCI, let's stick to the terminologies in
> UFSHCI spec.

I have taken care of this in the next patchset.

Thanks,
Ram


> 
> - Mani
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-09-01 16:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 15:08 [PATCH V3 0/4] Add DT-based gear and rate limiting support Ram Kumar Dwivedi
2025-08-26 15:08 ` [PATCH V3 1/4] ufs: dt-bindings: Document gear and rate limit properties Ram Kumar Dwivedi
2025-08-26 15:35   ` Bart Van Assche
2025-08-28 16:25     ` Ram Kumar Dwivedi
2025-08-28 17:41       ` Krzysztof Kozlowski
2025-08-28 16:45     ` Ram Kumar Dwivedi
2025-08-28 17:22       ` Bart Van Assche
2025-09-01  4:07         ` Manivannan Sadhasivam
2025-09-01 16:01           ` Ram Kumar Dwivedi
2025-08-28 17:41       ` Krzysztof Kozlowski
2025-08-28 17:40     ` Krzysztof Kozlowski
2025-08-28 18:27       ` Bart Van Assche
2025-08-26 15:08 ` [PATCH V3 2/4] ufs: ufs-qcom: Remove redundant re-assignment to hs_rate Ram Kumar Dwivedi
2025-08-26 15:08 ` [PATCH V3 3/4] ufs: pltfrm: Allow limiting HS gear and rate via DT Ram Kumar Dwivedi
2025-08-26 15:08 ` [PATCH V3 4/4] ufs: ufs-qcom: Add support for limiting HS gear and rate Ram Kumar Dwivedi
2025-08-26 15:37   ` Bart Van Assche
2025-08-28 16:28     ` Ram Kumar Dwivedi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).