[PATCH V1 0/3] Add DT-based gear and rate limiting support

[PATCH V1 0/3] Add DT-based gear and rate limiting support

[Ram Kumar Dwivedi]

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

Some automotive platforms, such as SA8155, require restricting the
maximum gear or rate due to hardware limitations. To support this,
the driver is extended to parse two new optional DT properties and
apply them during initialization. The default behavior remains
unchanged if these properties are not specified.

Ram Kumar Dwivedi (3):
  scsi: ufs: qcom: Add support for DT-based gear and rate limiting
  arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS
  dt-bindings: ufs: qcom: Document HS gear and rate limit properties

 .../devicetree/bindings/ufs/qcom,ufs.yaml     | 10 +++++++
 arch/arm64/boot/dts/qcom/sm8150.dtsi          |  3 ++
 drivers/ufs/host/ufs-qcom.c                   | 29 +++++++++++++++----
 3 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.50.1

[PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Ram Kumar Dwivedi]

Add optional device tree properties to limit Tx/Rx gear and rate during UFS
initialization. Parse these properties in ufs_qcom_init() and apply them to
host->host_params to enforce platform-specific constraints.

Use this mechanism to cap the maximum gear or rate on platforms with
hardware limitations, such as those required by some automotive customers
using SA8155. Preserve the default behavior if the properties are not
specified in the device tree.

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

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 4bbe4de1679b..5e7fd3257aca 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;
 
@@ -1096,6 +1092,25 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
 	}
 }
 
+static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
+{
+	struct ufs_host_params *host_params = &host->host_params;
+	struct device_node *np = host->hba->dev->of_node;
+	u32 hs_gear, hs_rate = 0;
+
+	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;
+		host->phy_gear = hs_gear;
+	}
+
+	if (!of_property_read_u32(np, "limit-rate", &hs_rate))
+		host_params->hs_rate = hs_rate;
+}
+
 static void ufs_qcom_set_host_params(struct ufs_hba *hba)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
@@ -1337,6 +1352,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(host);
 
 	err = ufs_qcom_ice_init(host);
 	if (err)
-- 
2.50.1

Re: [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Dmitry Baryshkov]

On Tue, Jul 22, 2025 at 09:41:01PM +0530, Ram Kumar Dwivedi wrote:
> Add optional device tree properties to limit Tx/Rx gear and rate during UFS
> initialization. Parse these properties in ufs_qcom_init() and apply them to
> host->host_params to enforce platform-specific constraints.
> 
> Use this mechanism to cap the maximum gear or rate on platforms with
> hardware limitations, such as those required by some automotive customers
> using SA8155. Preserve the default behavior if the properties are not
> specified in the device tree.
> 
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 4bbe4de1679b..5e7fd3257aca 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;

Why? This doesn't seem related.

>  
>  	mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A;
>  
> @@ -1096,6 +1092,25 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
>  	}
>  }
>  
> +static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
> +{
> +	struct ufs_host_params *host_params = &host->host_params;
> +	struct device_node *np = host->hba->dev->of_node;
> +	u32 hs_gear, hs_rate = 0;
> +
> +	if (!np)
> +		return;
> +
> +	if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {

These are generic properties, so they need to be handled in a generic
code path.

Also, the patch with bindings should preceed driver and DT changes.

> +		host_params->hs_tx_gear = hs_gear;
> +		host_params->hs_rx_gear = hs_gear;
> +		host->phy_gear = hs_gear;
> +	}
> +
> +	if (!of_property_read_u32(np, "limit-rate", &hs_rate))
> +		host_params->hs_rate = hs_rate;
> +}
> +
>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
>  {
>  	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> @@ -1337,6 +1352,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(host);
>  
>  	err = ufs_qcom_ice_init(host);
>  	if (err)
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Ram Kumar Dwivedi]

On 23-Jul-25 12:24 AM, Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2025 at 09:41:01PM +0530, Ram Kumar Dwivedi wrote:
>> Add optional device tree properties to limit Tx/Rx gear and rate during UFS
>> initialization. Parse these properties in ufs_qcom_init() and apply them to
>> host->host_params to enforce platform-specific constraints.
>>
>> Use this mechanism to cap the maximum gear or rate on platforms with
>> hardware limitations, such as those required by some automotive customers
>> using SA8155. Preserve the default behavior if the properties are not
>> specified in the device tree.
>>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  drivers/ufs/host/ufs-qcom.c | 28 ++++++++++++++++++++++------
>>  1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 4bbe4de1679b..5e7fd3257aca 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;
> 
> Why? This doesn't seem related.

Hi Dmitry,

I have refactored the patch to put this part in a separate patch in latest patchset.

Thanks,
Ram.

> 
>>  
>>  	mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A;
>>  
>> @@ -1096,6 +1092,25 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
>>  	}
>>  }
>>  
>> +static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
>> +{
>> +	struct ufs_host_params *host_params = &host->host_params;
>> +	struct device_node *np = host->hba->dev->of_node;
>> +	u32 hs_gear, hs_rate = 0;
>> +
>> +	if (!np)
>> +		return;
>> +
>> +	if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {
> 
> These are generic properties, so they need to be handled in a generic
> code path.

Hi Dmitry,


Below is the probe path for the UFS-QCOM platform driver:

ufs_qcom_probe
  └─ ufshcd_platform_init
       └─ ufshcd_init
            └─ ufs_qcom_init
                 └─ ufs_qcom_set_host_params
                      └─ ufshcd_init_host_params (initialized with default values)
                           └─ ufs_qcom_get_hs_gear (overrides gear based on controller capability)
                                └─ ufs_qcom_set_phy_gear (further overrides based on controller limitations)


The reason I added the logic in ufs-qcom.c is that even if it's placed in ufshcd-platform.c, the values get overridden in ufs-qcom.c.
If you prefer, I can move the parsing logic API to ufshcd-platform.c but still it needs to be called from ufs-qcom.c.

Thanks,
Ram.


> 
> Also, the patch with bindings should preceed driver and DT changes.

Hi Dmitry,

I have reordered the patch series to place the DT binding change as the first patch in latest patchset.

Thanks,
Ram.


> 
>> +		host_params->hs_tx_gear = hs_gear;
>> +		host_params->hs_rx_gear = hs_gear;
>> +		host->phy_gear = hs_gear;
>> +	}
>> +
>> +	if (!of_property_read_u32(np, "limit-rate", &hs_rate))
>> +		host_params->hs_rate = hs_rate;
>> +}
>> +
>>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
>>  {
>>  	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> @@ -1337,6 +1352,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(host);
>>  
>>  	err = ufs_qcom_ice_init(host);
>>  	if (err)
>> -- 
>> 2.50.1
>>
> 

Re: [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Dmitry Baryshkov]

On Thu, 24 Jul 2025 at 10:35, Ram Kumar Dwivedi
<quic_rdwivedi@quicinc.com> wrote:
>
>
>
> On 23-Jul-25 12:24 AM, Dmitry Baryshkov wrote:
> > On Tue, Jul 22, 2025 at 09:41:01PM +0530, Ram Kumar Dwivedi wrote:
> >> Add optional device tree properties to limit Tx/Rx gear and rate during UFS
> >> initialization. Parse these properties in ufs_qcom_init() and apply them to
> >> host->host_params to enforce platform-specific constraints.
> >>
> >> Use this mechanism to cap the maximum gear or rate on platforms with
> >> hardware limitations, such as those required by some automotive customers
> >> using SA8155. Preserve the default behavior if the properties are not
> >> specified in the device tree.
> >>
> >> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> >> ---
> >>  drivers/ufs/host/ufs-qcom.c | 28 ++++++++++++++++++++++------
> >>  1 file changed, 22 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> >> index 4bbe4de1679b..5e7fd3257aca 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;
> >
> > Why? This doesn't seem related.
>
> Hi Dmitry,
>
> I have refactored the patch to put this part in a separate patch in latest patchset.
>
> Thanks,
> Ram.
>
> >
> >>
> >>      mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A;
> >>
> >> @@ -1096,6 +1092,25 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
> >>      }
> >>  }
> >>
> >> +static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
> >> +{
> >> +    struct ufs_host_params *host_params = &host->host_params;
> >> +    struct device_node *np = host->hba->dev->of_node;
> >> +    u32 hs_gear, hs_rate = 0;
> >> +
> >> +    if (!np)
> >> +            return;
> >> +
> >> +    if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {
> >
> > These are generic properties, so they need to be handled in a generic
> > code path.
>
> Hi Dmitry,
>
>
> Below is the probe path for the UFS-QCOM platform driver:
>
> ufs_qcom_probe
>   └─ ufshcd_platform_init
>        └─ ufshcd_init
>             └─ ufs_qcom_init
>                  └─ ufs_qcom_set_host_params
>                       └─ ufshcd_init_host_params (initialized with default values)
>                            └─ ufs_qcom_get_hs_gear (overrides gear based on controller capability)
>                                 └─ ufs_qcom_set_phy_gear (further overrides based on controller limitations)
>
>
> The reason I added the logic in ufs-qcom.c is that even if it's placed in ufshcd-platform.c, the values get overridden in ufs-qcom.c.
> If you prefer, I can move the parsing logic API to ufshcd-platform.c but still it needs to be called from ufs-qcom.c.

I was thinking about ufshcd_init() or similar function.

>
> Thanks,
> Ram.
>
>
> >
> > Also, the patch with bindings should preceed driver and DT changes.
>
> Hi Dmitry,
>
> I have reordered the patch series to place the DT binding change as the first patch in latest patchset.
>
> Thanks,
> Ram.
>
>
> >
> >> +            host_params->hs_tx_gear = hs_gear;
> >> +            host_params->hs_rx_gear = hs_gear;
> >> +            host->phy_gear = hs_gear;
> >> +    }
> >> +
> >> +    if (!of_property_read_u32(np, "limit-rate", &hs_rate))
> >> +            host_params->hs_rate = hs_rate;
> >> +}
> >> +
> >>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
> >>  {
> >>      struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> >> @@ -1337,6 +1352,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(host);
> >>
> >>      err = ufs_qcom_ice_init(host);
> >>      if (err)
> >> --
> >> 2.50.1
> >>
> >
>


-- 
With best wishes
Dmitry

Re: [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Ram Kumar Dwivedi]

On 24-Jul-25 2:11 PM, Dmitry Baryshkov wrote:
> On Thu, 24 Jul 2025 at 10:35, Ram Kumar Dwivedi
> <quic_rdwivedi@quicinc.com> wrote:
>>
>>
>>
>> On 23-Jul-25 12:24 AM, Dmitry Baryshkov wrote:
>>> On Tue, Jul 22, 2025 at 09:41:01PM +0530, Ram Kumar Dwivedi wrote:
>>>> Add optional device tree properties to limit Tx/Rx gear and rate during UFS
>>>> initialization. Parse these properties in ufs_qcom_init() and apply them to
>>>> host->host_params to enforce platform-specific constraints.
>>>>
>>>> Use this mechanism to cap the maximum gear or rate on platforms with
>>>> hardware limitations, such as those required by some automotive customers
>>>> using SA8155. Preserve the default behavior if the properties are not
>>>> specified in the device tree.
>>>>
>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>>> ---
>>>>  drivers/ufs/host/ufs-qcom.c | 28 ++++++++++++++++++++++------
>>>>  1 file changed, 22 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>>>> index 4bbe4de1679b..5e7fd3257aca 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;
>>>
>>> Why? This doesn't seem related.
>>
>> Hi Dmitry,
>>
>> I have refactored the patch to put this part in a separate patch in latest patchset.
>>
>> Thanks,
>> Ram.
>>
>>>
>>>>
>>>>      mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A;
>>>>
>>>> @@ -1096,6 +1092,25 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
>>>>      }
>>>>  }
>>>>
>>>> +static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
>>>> +{
>>>> +    struct ufs_host_params *host_params = &host->host_params;
>>>> +    struct device_node *np = host->hba->dev->of_node;
>>>> +    u32 hs_gear, hs_rate = 0;
>>>> +
>>>> +    if (!np)
>>>> +            return;
>>>> +
>>>> +    if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {
>>>
>>> These are generic properties, so they need to be handled in a generic
>>> code path.
>>
>> Hi Dmitry,
>>
>>
>> Below is the probe path for the UFS-QCOM platform driver:
>>
>> ufs_qcom_probe
>>   └─ ufshcd_platform_init
>>        └─ ufshcd_init
>>             └─ ufs_qcom_init
>>                  └─ ufs_qcom_set_host_params
>>                       └─ ufshcd_init_host_params (initialized with default values)
>>                            └─ ufs_qcom_get_hs_gear (overrides gear based on controller capability)
>>                                 └─ ufs_qcom_set_phy_gear (further overrides based on controller limitations)
>>
>>
>> The reason I added the logic in ufs-qcom.c is that even if it's placed in ufshcd-platform.c, the values get overridden in ufs-qcom.c.
>> If you prefer, I can move the parsing logic API to ufshcd-platform.c but still it needs to be called from ufs-qcom.c.
> 
> I was thinking about ufshcd_init() or similar function.
Hi Dmitry,

It appears we can't move the logic to ufshcd.c because the PHY is initialized in ufs-qcom.c, and the gear must be set during that initialization.

Limiting the gear and lane in ufshcd.c won’t be effective, as qcom_init sets the PHY to the maximum supported gear by default. As a result, the PHY would still initialize with the max gear, defeating the purpose of the limits.

To ensure the PHY is initialized with the intended gear, the limits needs to be applied directly in ufs_qcom.c

Please let me know if you have any concerns.

Thanks,
Ram.


> 
>>
>> Thanks,
>> Ram.
>>
>>
>>>
>>> Also, the patch with bindings should preceed driver and DT changes.
>>
>> Hi Dmitry,
>>
>> I have reordered the patch series to place the DT binding change as the first patch in latest patchset.
>>
>> Thanks,
>> Ram.
>>
>>
>>>
>>>> +            host_params->hs_tx_gear = hs_gear;
>>>> +            host_params->hs_rx_gear = hs_gear;
>>>> +            host->phy_gear = hs_gear;
>>>> +    }
>>>> +
>>>> +    if (!of_property_read_u32(np, "limit-rate", &hs_rate))
>>>> +            host_params->hs_rate = hs_rate;
>>>> +}
>>>> +
>>>>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
>>>>  {
>>>>      struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>>>> @@ -1337,6 +1352,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(host);
>>>>
>>>>      err = ufs_qcom_ice_init(host);
>>>>      if (err)
>>>> --
>>>> 2.50.1
>>>>
>>>
>>
> 
> 

Re: [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Dmitry Baryshkov]

On Thu, Jul 31, 2025 at 09:58:47PM +0530, Ram Kumar Dwivedi wrote:
> 
> 
> On 24-Jul-25 2:11 PM, Dmitry Baryshkov wrote:
> > On Thu, 24 Jul 2025 at 10:35, Ram Kumar Dwivedi
> > <quic_rdwivedi@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 23-Jul-25 12:24 AM, Dmitry Baryshkov wrote:
> >>> On Tue, Jul 22, 2025 at 09:41:01PM +0530, Ram Kumar Dwivedi wrote:
> >>>> Add optional device tree properties to limit Tx/Rx gear and rate during UFS
> >>>> initialization. Parse these properties in ufs_qcom_init() and apply them to
> >>>> host->host_params to enforce platform-specific constraints.
> >>>>
> >>>> Use this mechanism to cap the maximum gear or rate on platforms with
> >>>> hardware limitations, such as those required by some automotive customers
> >>>> using SA8155. Preserve the default behavior if the properties are not
> >>>> specified in the device tree.
> >>>>
> >>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> >>>> ---
> >>>>  drivers/ufs/host/ufs-qcom.c | 28 ++++++++++++++++++++++------
> >>>>  1 file changed, 22 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> >>>> index 4bbe4de1679b..5e7fd3257aca 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;
> >>>
> >>> Why? This doesn't seem related.
> >>
> >> Hi Dmitry,
> >>
> >> I have refactored the patch to put this part in a separate patch in latest patchset.
> >>
> >> Thanks,
> >> Ram.
> >>
> >>>
> >>>>
> >>>>      mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A;
> >>>>
> >>>> @@ -1096,6 +1092,25 @@ static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
> >>>>      }
> >>>>  }
> >>>>
> >>>> +static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
> >>>> +{
> >>>> +    struct ufs_host_params *host_params = &host->host_params;
> >>>> +    struct device_node *np = host->hba->dev->of_node;
> >>>> +    u32 hs_gear, hs_rate = 0;
> >>>> +
> >>>> +    if (!np)
> >>>> +            return;
> >>>> +
> >>>> +    if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {
> >>>
> >>> These are generic properties, so they need to be handled in a generic
> >>> code path.
> >>
> >> Hi Dmitry,
> >>
> >>
> >> Below is the probe path for the UFS-QCOM platform driver:
> >>
> >> ufs_qcom_probe
> >>   └─ ufshcd_platform_init
> >>        └─ ufshcd_init
> >>             └─ ufs_qcom_init
> >>                  └─ ufs_qcom_set_host_params
> >>                       └─ ufshcd_init_host_params (initialized with default values)
> >>                            └─ ufs_qcom_get_hs_gear (overrides gear based on controller capability)
> >>                                 └─ ufs_qcom_set_phy_gear (further overrides based on controller limitations)
> >>
> >>
> >> The reason I added the logic in ufs-qcom.c is that even if it's placed in ufshcd-platform.c, the values get overridden in ufs-qcom.c.
> >> If you prefer, I can move the parsing logic API to ufshcd-platform.c but still it needs to be called from ufs-qcom.c.
> > 
> > I was thinking about ufshcd_init() or similar function.
> Hi Dmitry,
> 
> It appears we can't move the logic to ufshcd.c because the PHY is initialized in ufs-qcom.c, and the gear must be set during that initialization.
> 
> Limiting the gear and lane in ufshcd.c won’t be effective, as qcom_init sets the PHY to the maximum supported gear by default. As a result, the PHY would still initialize with the max gear, defeating the purpose of the limits.
> 
> To ensure the PHY is initialized with the intended gear, the limits needs to be applied directly in ufs_qcom.c

The limits are to be applied in ufs_qcom.c, but they can (and should) be
parsed in the generic code.

> 
> Please let me know if you have any concerns.
> 
> Thanks,
> Ram.
> 
> 
> > 
> >>
> >> Thanks,
> >> Ram.
> >>
> >>
> >>>
> >>> Also, the patch with bindings should preceed driver and DT changes.
> >>
> >> Hi Dmitry,
> >>
> >> I have reordered the patch series to place the DT binding change as the first patch in latest patchset.
> >>
> >> Thanks,
> >> Ram.
> >>
> >>
> >>>
> >>>> +            host_params->hs_tx_gear = hs_gear;
> >>>> +            host_params->hs_rx_gear = hs_gear;
> >>>> +            host->phy_gear = hs_gear;
> >>>> +    }
> >>>> +
> >>>> +    if (!of_property_read_u32(np, "limit-rate", &hs_rate))
> >>>> +            host_params->hs_rate = hs_rate;
> >>>> +}
> >>>> +
> >>>>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
> >>>>  {
> >>>>      struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> >>>> @@ -1337,6 +1352,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(host);
> >>>>
> >>>>      err = ufs_qcom_ice_init(host);
> >>>>      if (err)
> >>>> --
> >>>> 2.50.1
> >>>>
> >>>
> >>
> > 
> > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting

[Krzysztof Kozlowski]

On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> +static void ufs_qcom_parse_limits(struct ufs_qcom_host *host)
> +{
> +	struct ufs_host_params *host_params = &host->host_params;
> +	struct device_node *np = host->hba->dev->of_node;
> +	u32 hs_gear, hs_rate = 0;
> +
> +	if (!np)
> +		return;
> +
> +	if (!of_property_read_u32(np, "limit-hs-gear", &hs_gear)) {

You cannot use ABI before you document it. Read submitting patches in DT.

Best regards,
Krzysztof

[PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Ram Kumar Dwivedi]

Add optional limit-hs-gear and limit-rate properties to the UFS node to
support automotive use cases that require limiting the maximum Tx/Rx HS
gear and rate due to hardware constraints.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index b5494bcf5cff..87e8b60b3b2d 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -2082,6 +2082,9 @@ ufs_mem_hc: ufshc@1d84000 {
 			resets = <&gcc GCC_UFS_PHY_BCR>;
 			reset-names = "rst";
 
+			limit-hs-gear = <3>;
+			limit-rate = <1>;
+
 			iommus = <&apps_smmu 0x300 0>;
 
 			clock-names =
-- 
2.50.1

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Dmitry Baryshkov]

On Tue, Jul 22, 2025 at 09:41:02PM +0530, Ram Kumar Dwivedi wrote:
> Add optional limit-hs-gear and limit-rate properties to the UFS node to
> support automotive use cases that require limiting the maximum Tx/Rx HS
> gear and rate due to hardware constraints.


If they are optional and they are for automotive, then why are you
adding them to the SM8150 DTSi file, enforcing them for all SM8150
targets?

> 
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8150.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index b5494bcf5cff..87e8b60b3b2d 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -2082,6 +2082,9 @@ ufs_mem_hc: ufshc@1d84000 {
>  			resets = <&gcc GCC_UFS_PHY_BCR>;
>  			reset-names = "rst";
>  
> +			limit-hs-gear = <3>;
> +			limit-rate = <1>;
> +
>  			iommus = <&apps_smmu 0x300 0>;
>  
>  			clock-names =
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Ram Kumar Dwivedi]

On 23-Jul-25 12:25 AM, Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2025 at 09:41:02PM +0530, Ram Kumar Dwivedi wrote:
>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>> support automotive use cases that require limiting the maximum Tx/Rx HS
>> gear and rate due to hardware constraints.
> 
> 
> If they are optional and they are for automotive, then why are you
> adding them to the SM8150 DTSi file, enforcing them for all SM8150
> targets?
> 
Hi Dmitry,

I have moved them to board specific file sa8155p.dtsi in latest patchset.

Thanks,
Ram.



>>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sm8150.dtsi | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
>> index b5494bcf5cff..87e8b60b3b2d 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
>> @@ -2082,6 +2082,9 @@ ufs_mem_hc: ufshc@1d84000 {
>>  			resets = <&gcc GCC_UFS_PHY_BCR>;
>>  			reset-names = "rst";
>>  
>> +			limit-hs-gear = <3>;
>> +			limit-rate = <1>;
>> +
>>  			iommus = <&apps_smmu 0x300 0>;
>>  
>>  			clock-names =
>> -- 
>> 2.50.1
>>
> 

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Krzysztof Kozlowski]

On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> Add optional limit-hs-gear and limit-rate properties to the UFS node to
> support automotive use cases that require limiting the maximum Tx/Rx HS
> gear and rate due to hardware constraints.

What hardware constraints? This needs to be clearly documented.


Best regards,
Krzysztof

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Manivannan Sadhasivam]

On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> > Add optional limit-hs-gear and limit-rate properties to the UFS node to
> > support automotive use cases that require limiting the maximum Tx/Rx HS
> > gear and rate due to hardware constraints.
> 
> What hardware constraints? This needs to be clearly documented.
> 

Ram, both Krzysztof and I asked this question, but you never bothered to reply,
but keep on responding to other comments. This won't help you to get this series
merged in any form.

Please address *all* review comments before posting next iteration.

- Mani

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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Krzysztof Kozlowski]

On 01/08/2025 10:28, Manivannan Sadhasivam wrote:
> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>>> support automotive use cases that require limiting the maximum Tx/Rx HS
>>> gear and rate due to hardware constraints.
>>
>> What hardware constraints? This needs to be clearly documented.
>>
> 
> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
> but keep on responding to other comments. This won't help you to get this series
> merged in any form.
> 
> Please address *all* review comments before posting next iteration.

There was enough of time to respond to this, so I assume this patchset
is abandoned and there will be no new version.

Best regards,
Krzysztof

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Ram Kumar Dwivedi]

On 01-Aug-25 2:34 PM, Krzysztof Kozlowski wrote:
> On 01/08/2025 10:28, Manivannan Sadhasivam wrote:
>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>>>> support automotive use cases that require limiting the maximum Tx/Rx HS
>>>> gear and rate due to hardware constraints.
>>>
>>> What hardware constraints? This needs to be clearly documented.
>>>
>>
>> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
>> but keep on responding to other comments. This won't help you to get this series
>> merged in any form.
>>
>> Please address *all* review comments before posting next iteration.
> 
> There was enough of time to respond to this, so I assume this patchset
> is abandoned and there will be no new version.



Hi Krzysztof,

I was waiting for your DT binding bifurcation patch to be merged before posting the next version, which is why I didn’t respond earlier. 
I had planned to include the hardware constraints explanation in the next commit message.

I’ll make sure to address all pending comments in the upcoming revision.

Thanks,
Ram.

> 
> Best regards,
> Krzysztof

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Ram Kumar Dwivedi]

On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>>> support automotive use cases that require limiting the maximum Tx/Rx HS
>>> gear and rate due to hardware constraints.
>>
>> What hardware constraints? This needs to be clearly documented.
>>
> 
> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
> but keep on responding to other comments. This won't help you to get this series
> merged in any form.
> 
> Please address *all* review comments before posting next iteration.

Hi Mani,

Apologies for the delay in responding. 
I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 

To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.

I’ll ensure this is clearly documented in the next revision.


Thanks,
Ram.

> 
> - Mani
> 

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Krzysztof Kozlowski]

On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> 
> 
> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>>>> support automotive use cases that require limiting the maximum Tx/Rx HS
>>>> gear and rate due to hardware constraints.
>>>
>>> What hardware constraints? This needs to be clearly documented.
>>>
>>
>> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
>> but keep on responding to other comments. This won't help you to get this series
>> merged in any form.
>>
>> Please address *all* review comments before posting next iteration.
> 
> Hi Mani,
> 
> Apologies for the delay in responding. 
> I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 
> 
> To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.
> 

That's vague and does not justify the property. You need to document
instead hardware capabilities or characteristic. Or explain why they
cannot. With such form I will object to your next patch.

Best regards,
Krzysztof

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Manivannan Sadhasivam]

On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> > 
> > 
> > On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> >> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
> >>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> >>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
> >>>> support automotive use cases that require limiting the maximum Tx/Rx HS
> >>>> gear and rate due to hardware constraints.
> >>>
> >>> What hardware constraints? This needs to be clearly documented.
> >>>
> >>
> >> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
> >> but keep on responding to other comments. This won't help you to get this series
> >> merged in any form.
> >>
> >> Please address *all* review comments before posting next iteration.
> > 
> > Hi Mani,
> > 
> > Apologies for the delay in responding. 
> > I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 
> > 
> > To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.
> > 
> 
> That's vague and does not justify the property. You need to document
> instead hardware capabilities or characteristic. Or explain why they
> cannot. With such form I will object to your next patch.
> 

I had an offline chat with Ram and got clarified on what these properties are.
The problem here is not with the SoC, but with the board design. On some Qcom
customer designs, both the UFS controller in the SoC and the UFS device are
capable of operating at higher gears (say G5). But due to board constraints like
poor thermal dissipation, routing loss, the board cannot efficiently operate at
the higher speeds.

So the customers wanted a way to limit the gear speed (say G3) and rate
(say Mode-A) on the specific board DTS.

But this series ended up adding these properties in the SoC dtsi, which was
wrong in the first place. And the patch description also lacked the above
reasoning.

I hope Ram will fix these two things in the next version.

FWIW: The customer is using a DT overlay to add these properties to the base
DTS. So there would be no DTS change posted in the next version.

- Mani

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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Konrad Dybcio]

On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
>>>
>>>
>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>>>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>>>>>> support automotive use cases that require limiting the maximum Tx/Rx HS
>>>>>> gear and rate due to hardware constraints.
>>>>>
>>>>> What hardware constraints? This needs to be clearly documented.
>>>>>
>>>>
>>>> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
>>>> but keep on responding to other comments. This won't help you to get this series
>>>> merged in any form.
>>>>
>>>> Please address *all* review comments before posting next iteration.
>>>
>>> Hi Mani,
>>>
>>> Apologies for the delay in responding. 
>>> I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 
>>>
>>> To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.
>>>
>>
>> That's vague and does not justify the property. You need to document
>> instead hardware capabilities or characteristic. Or explain why they
>> cannot. With such form I will object to your next patch.
>>
> 
> I had an offline chat with Ram and got clarified on what these properties are.
> The problem here is not with the SoC, but with the board design. On some Qcom
> customer designs, both the UFS controller in the SoC and the UFS device are
> capable of operating at higher gears (say G5). But due to board constraints like
> poor thermal dissipation, routing loss, the board cannot efficiently operate at
> the higher speeds.
> 
> So the customers wanted a way to limit the gear speed (say G3) and rate
> (say Mode-A) on the specific board DTS.

I'm not necessarily saying no, but have you explored sysfs for this?

I suppose it may be too late (if the driver would e.g. init the UFS
at max gear/rate at probe time, it could cause havoc as it tries to
load the userland)..

Konrad

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Manivannan Sadhasivam]

On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> > On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
> >> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> >>>
> >>>
> >>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> >>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
> >>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> >>>>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
> >>>>>> support automotive use cases that require limiting the maximum Tx/Rx HS
> >>>>>> gear and rate due to hardware constraints.
> >>>>>
> >>>>> What hardware constraints? This needs to be clearly documented.
> >>>>>
> >>>>
> >>>> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
> >>>> but keep on responding to other comments. This won't help you to get this series
> >>>> merged in any form.
> >>>>
> >>>> Please address *all* review comments before posting next iteration.
> >>>
> >>> Hi Mani,
> >>>
> >>> Apologies for the delay in responding. 
> >>> I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 
> >>>
> >>> To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.
> >>>
> >>
> >> That's vague and does not justify the property. You need to document
> >> instead hardware capabilities or characteristic. Or explain why they
> >> cannot. With such form I will object to your next patch.
> >>
> > 
> > I had an offline chat with Ram and got clarified on what these properties are.
> > The problem here is not with the SoC, but with the board design. On some Qcom
> > customer designs, both the UFS controller in the SoC and the UFS device are
> > capable of operating at higher gears (say G5). But due to board constraints like
> > poor thermal dissipation, routing loss, the board cannot efficiently operate at
> > the higher speeds.
> > 
> > So the customers wanted a way to limit the gear speed (say G3) and rate
> > (say Mode-A) on the specific board DTS.
> 
> I'm not necessarily saying no, but have you explored sysfs for this?
> 
> I suppose it may be too late (if the driver would e.g. init the UFS
> at max gear/rate at probe time, it could cause havoc as it tries to
> load the userland)..
> 

If the driver tries to run with unsupported max gear speed/mode, it will just
crash with the error spit.

- Mani

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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Konrad Dybcio]

On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
>> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
>>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
>>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
>>>>>
>>>>>
>>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
>>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
>>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>>>>>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
>>>>>>>> support automotive use cases that require limiting the maximum Tx/Rx HS
>>>>>>>> gear and rate due to hardware constraints.
>>>>>>>
>>>>>>> What hardware constraints? This needs to be clearly documented.
>>>>>>>
>>>>>>
>>>>>> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
>>>>>> but keep on responding to other comments. This won't help you to get this series
>>>>>> merged in any form.
>>>>>>
>>>>>> Please address *all* review comments before posting next iteration.
>>>>>
>>>>> Hi Mani,
>>>>>
>>>>> Apologies for the delay in responding. 
>>>>> I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 
>>>>>
>>>>> To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.
>>>>>
>>>>
>>>> That's vague and does not justify the property. You need to document
>>>> instead hardware capabilities or characteristic. Or explain why they
>>>> cannot. With such form I will object to your next patch.
>>>>
>>>
>>> I had an offline chat with Ram and got clarified on what these properties are.
>>> The problem here is not with the SoC, but with the board design. On some Qcom
>>> customer designs, both the UFS controller in the SoC and the UFS device are
>>> capable of operating at higher gears (say G5). But due to board constraints like
>>> poor thermal dissipation, routing loss, the board cannot efficiently operate at
>>> the higher speeds.
>>>
>>> So the customers wanted a way to limit the gear speed (say G3) and rate
>>> (say Mode-A) on the specific board DTS.
>>
>> I'm not necessarily saying no, but have you explored sysfs for this?
>>
>> I suppose it may be too late (if the driver would e.g. init the UFS
>> at max gear/rate at probe time, it could cause havoc as it tries to
>> load the userland)..
>>
> 
> If the driver tries to run with unsupported max gear speed/mode, it will just
> crash with the error spit.

OK

just a couple related nits that I won't bother splitting into separate
emails

rate (mode? I'm seeing both names) should probably have dt-bindings defines
while gear doesn't have to since they're called G<number> anyway, with the
bindings description strongly discouraging use, unless absolutely necessary
(e.g. in the situation we have right there)

I'd also assume the code should be moved into the ufs-common code, rather
than making it ufs-qcom specific

Konrad

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Manivannan Sadhasivam]

On Tue, Aug 05, 2025 at 07:06:29PM GMT, Konrad Dybcio wrote:
> On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> > On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> >> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> >>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
> >>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> >>>>>
> >>>>>
> >>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> >>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
> >>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> >>>>>>>> Add optional limit-hs-gear and limit-rate properties to the UFS node to
> >>>>>>>> support automotive use cases that require limiting the maximum Tx/Rx HS
> >>>>>>>> gear and rate due to hardware constraints.
> >>>>>>>
> >>>>>>> What hardware constraints? This needs to be clearly documented.
> >>>>>>>
> >>>>>>
> >>>>>> Ram, both Krzysztof and I asked this question, but you never bothered to reply,
> >>>>>> but keep on responding to other comments. This won't help you to get this series
> >>>>>> merged in any form.
> >>>>>>
> >>>>>> Please address *all* review comments before posting next iteration.
> >>>>>
> >>>>> Hi Mani,
> >>>>>
> >>>>> Apologies for the delay in responding. 
> >>>>> I had planned to explain the hardware constraints in the next patchset’s commit message, which is why I didn’t reply earlier. 
> >>>>>
> >>>>> To clarify: the limitations are due to customer board designs, not our SoC. Some boards can't support higher gear operation, hence the need for optional limit-hs-gear and limit-rate properties.
> >>>>>
> >>>>
> >>>> That's vague and does not justify the property. You need to document
> >>>> instead hardware capabilities or characteristic. Or explain why they
> >>>> cannot. With such form I will object to your next patch.
> >>>>
> >>>
> >>> I had an offline chat with Ram and got clarified on what these properties are.
> >>> The problem here is not with the SoC, but with the board design. On some Qcom
> >>> customer designs, both the UFS controller in the SoC and the UFS device are
> >>> capable of operating at higher gears (say G5). But due to board constraints like
> >>> poor thermal dissipation, routing loss, the board cannot efficiently operate at
> >>> the higher speeds.
> >>>
> >>> So the customers wanted a way to limit the gear speed (say G3) and rate
> >>> (say Mode-A) on the specific board DTS.
> >>
> >> I'm not necessarily saying no, but have you explored sysfs for this?
> >>
> >> I suppose it may be too late (if the driver would e.g. init the UFS
> >> at max gear/rate at probe time, it could cause havoc as it tries to
> >> load the userland)..
> >>
> > 
> > If the driver tries to run with unsupported max gear speed/mode, it will just
> > crash with the error spit.
> 
> OK
> 
> just a couple related nits that I won't bother splitting into separate
> emails
> 
> rate (mode? I'm seeing both names) should probably have dt-bindings defines
> while gear doesn't have to since they're called G<number> anyway,

Yeah.

> with the
> bindings description strongly discouraging use, unless absolutely necessary
> (e.g. in the situation we have right there)
> 

There is no need to discourate its usage. But the description has to be clear in
such a way that the users should understand its purpose.

> I'd also assume the code should be moved into the ufs-common code, rather
> than making it ufs-qcom specific
> 

Both the dt-binding properties and relevant code should be moved to common
parts.

- Mani

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

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Sent: Tuesday, August 5, 2025 10:36 PM
> To: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
> <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
> avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
> konradybcio@kernel.org; James.Bottomley@hansenpartnership.com;
> martin.petersen@oracle.com; agross@kernel.org; linux-arm-
> msm@vger.kernel.org; linux-scsi@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> > On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> >> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> >>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
> >>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> >>>>>
> >>>>>
> >>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> >>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
> >>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> >>>>>>>> Add optional limit-hs-gear and limit-rate properties to the UFS
> >>>>>>>> node to support automotive use cases that require limiting the
> >>>>>>>> maximum Tx/Rx HS gear and rate due to hardware constraints.
> >>>>>>>
> >>>>>>> What hardware constraints? This needs to be clearly documented.
> >>>>>>>
> >>>>>>
> >>>>>> Ram, both Krzysztof and I asked this question, but you never
> >>>>>> bothered to reply, but keep on responding to other comments. This
> >>>>>> won't help you to get this series merged in any form.
> >>>>>>
> >>>>>> Please address *all* review comments before posting next iteration.
> >>>>>
> >>>>> Hi Mani,
> >>>>>
> >>>>> Apologies for the delay in responding.
> >>>>> I had planned to explain the hardware constraints in the next
> patchset’s commit message, which is why I didn’t reply earlier.
> >>>>>
> >>>>> To clarify: the limitations are due to customer board designs, not our
> SoC. Some boards can't support higher gear operation, hence the need for
> optional limit-hs-gear and limit-rate properties.
> >>>>>
> >>>>
> >>>> That's vague and does not justify the property. You need to
> >>>> document instead hardware capabilities or characteristic. Or
> >>>> explain why they cannot. With such form I will object to your next
> patch.
> >>>>
> >>>
> >>> I had an offline chat with Ram and got clarified on what these properties
> are.
> >>> The problem here is not with the SoC, but with the board design. On
> >>> some Qcom customer designs, both the UFS controller in the SoC and
> >>> the UFS device are capable of operating at higher gears (say G5).
> >>> But due to board constraints like poor thermal dissipation, routing
> >>> loss, the board cannot efficiently operate at the higher speeds.
> >>>
> >>> So the customers wanted a way to limit the gear speed (say G3) and
> >>> rate (say Mode-A) on the specific board DTS.
> >>
> >> I'm not necessarily saying no, but have you explored sysfs for this?
> >>
> >> I suppose it may be too late (if the driver would e.g. init the UFS
> >> at max gear/rate at probe time, it could cause havoc as it tries to
> >> load the userland)..
> >>
> >
> > If the driver tries to run with unsupported max gear speed/mode, it
> > will just crash with the error spit.
> 
> OK
> 
> just a couple related nits that I won't bother splitting into separate emails
> 
> rate (mode? I'm seeing both names) should probably have dt-bindings
> defines while gear doesn't have to since they're called G<number> anyway,
> with the bindings description strongly discouraging use, unless absolutely
> necessary (e.g. in the situation we have right there)
> 
> I'd also assume the code should be moved into the ufs-common code, rather
> than making it ufs-qcom specific
> 
> Konrad
Since this is a board specific constrains and not a SoC properties, have an option of handling this via bootloader is explored?
Or passing such properties via bootargs and handle the same in the driver?

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

['Manivannan Sadhasivam']

On Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
> 
> 
> > -----Original Message-----
> > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Sent: Tuesday, August 5, 2025 10:36 PM
> > To: Manivannan Sadhasivam <mani@kernel.org>
> > Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
> > <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
> > avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
> > krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
> > konradybcio@kernel.org; James.Bottomley@hansenpartnership.com;
> > martin.petersen@oracle.com; agross@kernel.org; linux-arm-
> > msm@vger.kernel.org; linux-scsi@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> > properties to UFS
> > 
> > On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> > > On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> > >> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> > >>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
> > >>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> > >>>>>
> > >>>>>
> > >>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> > >>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski wrote:
> > >>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> > >>>>>>>> Add optional limit-hs-gear and limit-rate properties to the UFS
> > >>>>>>>> node to support automotive use cases that require limiting the
> > >>>>>>>> maximum Tx/Rx HS gear and rate due to hardware constraints.
> > >>>>>>>
> > >>>>>>> What hardware constraints? This needs to be clearly documented.
> > >>>>>>>
> > >>>>>>
> > >>>>>> Ram, both Krzysztof and I asked this question, but you never
> > >>>>>> bothered to reply, but keep on responding to other comments. This
> > >>>>>> won't help you to get this series merged in any form.
> > >>>>>>
> > >>>>>> Please address *all* review comments before posting next iteration.
> > >>>>>
> > >>>>> Hi Mani,
> > >>>>>
> > >>>>> Apologies for the delay in responding.
> > >>>>> I had planned to explain the hardware constraints in the next
> > patchset’s commit message, which is why I didn’t reply earlier.
> > >>>>>
> > >>>>> To clarify: the limitations are due to customer board designs, not our
> > SoC. Some boards can't support higher gear operation, hence the need for
> > optional limit-hs-gear and limit-rate properties.
> > >>>>>
> > >>>>
> > >>>> That's vague and does not justify the property. You need to
> > >>>> document instead hardware capabilities or characteristic. Or
> > >>>> explain why they cannot. With such form I will object to your next
> > patch.
> > >>>>
> > >>>
> > >>> I had an offline chat with Ram and got clarified on what these properties
> > are.
> > >>> The problem here is not with the SoC, but with the board design. On
> > >>> some Qcom customer designs, both the UFS controller in the SoC and
> > >>> the UFS device are capable of operating at higher gears (say G5).
> > >>> But due to board constraints like poor thermal dissipation, routing
> > >>> loss, the board cannot efficiently operate at the higher speeds.
> > >>>
> > >>> So the customers wanted a way to limit the gear speed (say G3) and
> > >>> rate (say Mode-A) on the specific board DTS.
> > >>
> > >> I'm not necessarily saying no, but have you explored sysfs for this?
> > >>
> > >> I suppose it may be too late (if the driver would e.g. init the UFS
> > >> at max gear/rate at probe time, it could cause havoc as it tries to
> > >> load the userland)..
> > >>
> > >
> > > If the driver tries to run with unsupported max gear speed/mode, it
> > > will just crash with the error spit.
> > 
> > OK
> > 
> > just a couple related nits that I won't bother splitting into separate emails
> > 
> > rate (mode? I'm seeing both names) should probably have dt-bindings
> > defines while gear doesn't have to since they're called G<number> anyway,
> > with the bindings description strongly discouraging use, unless absolutely
> > necessary (e.g. in the situation we have right there)
> > 
> > I'd also assume the code should be moved into the ufs-common code, rather
> > than making it ufs-qcom specific
> > 
> > Konrad
> Since this is a board specific constrains and not a SoC properties, have an option of handling this via bootloader is explored?

Both board and SoC specific properties *should* be described in devicetree if
they are purely describing the hardware.

- Mani

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

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> Sent: Tuesday, August 5, 2025 10:52 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > > Sent: Tuesday, August 5, 2025 10:36 PM
> > > To: Manivannan Sadhasivam <mani@kernel.org>
> > > Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
> > > <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
> > > avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
> > > krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
> > > konradybcio@kernel.org; James.Bottomley@hansenpartnership.com;
> > > martin.petersen@oracle.com; agross@kernel.org; linux-arm-
> > > msm@vger.kernel.org; linux-scsi@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate
> > > limit properties to UFS
> > >
> > > On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> > > > On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> > > >> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> > > >>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
> > > >>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> > > >>>>>
> > > >>>>>
> > > >>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> > > >>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski
> wrote:
> > > >>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> > > >>>>>>>> Add optional limit-hs-gear and limit-rate properties to the
> > > >>>>>>>> UFS node to support automotive use cases that require
> > > >>>>>>>> limiting the maximum Tx/Rx HS gear and rate due to hardware
> constraints.
> > > >>>>>>>
> > > >>>>>>> What hardware constraints? This needs to be clearly
> documented.
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>> Ram, both Krzysztof and I asked this question, but you never
> > > >>>>>> bothered to reply, but keep on responding to other comments.
> > > >>>>>> This won't help you to get this series merged in any form.
> > > >>>>>>
> > > >>>>>> Please address *all* review comments before posting next
> iteration.
> > > >>>>>
> > > >>>>> Hi Mani,
> > > >>>>>
> > > >>>>> Apologies for the delay in responding.
> > > >>>>> I had planned to explain the hardware constraints in the next
> > > patchset’s commit message, which is why I didn’t reply earlier.
> > > >>>>>
> > > >>>>> To clarify: the limitations are due to customer board designs,
> > > >>>>> not our
> > > SoC. Some boards can't support higher gear operation, hence the need
> > > for optional limit-hs-gear and limit-rate properties.
> > > >>>>>
> > > >>>>
> > > >>>> That's vague and does not justify the property. You need to
> > > >>>> document instead hardware capabilities or characteristic. Or
> > > >>>> explain why they cannot. With such form I will object to your
> > > >>>> next
> > > patch.
> > > >>>>
> > > >>>
> > > >>> I had an offline chat with Ram and got clarified on what these
> > > >>> properties
> > > are.
> > > >>> The problem here is not with the SoC, but with the board design.
> > > >>> On some Qcom customer designs, both the UFS controller in the
> > > >>> SoC and the UFS device are capable of operating at higher gears (say
> G5).
> > > >>> But due to board constraints like poor thermal dissipation,
> > > >>> routing loss, the board cannot efficiently operate at the higher
> speeds.
> > > >>>
> > > >>> So the customers wanted a way to limit the gear speed (say G3)
> > > >>> and rate (say Mode-A) on the specific board DTS.
> > > >>
> > > >> I'm not necessarily saying no, but have you explored sysfs for this?
> > > >>
> > > >> I suppose it may be too late (if the driver would e.g. init the
> > > >> UFS at max gear/rate at probe time, it could cause havoc as it
> > > >> tries to load the userland)..
> > > >>
> > > >
> > > > If the driver tries to run with unsupported max gear speed/mode,
> > > > it will just crash with the error spit.
> > >
> > > OK
> > >
> > > just a couple related nits that I won't bother splitting into
> > > separate emails
> > >
> > > rate (mode? I'm seeing both names) should probably have dt-bindings
> > > defines while gear doesn't have to since they're called G<number>
> > > anyway, with the bindings description strongly discouraging use,
> > > unless absolutely necessary (e.g. in the situation we have right
> > > there)
> > >
> > > I'd also assume the code should be moved into the ufs-common code,
> > > rather than making it ufs-qcom specific
> > >
> > > Konrad
> > Since this is a board specific constrains and not a SoC properties, have an
> option of handling this via bootloader is explored?
> 
> Both board and SoC specific properties *should* be described in devicetree
> if they are purely describing the hardware.
> 
Agreed, what I understood from above conversation is that, we are try to solve a very *specific* board problem here, 
this does not looks like a generic problem to me and probably should be handled within the specific driver.

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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Konrad Dybcio]

On 8/5/25 7:36 PM, Alim Akhtar wrote:
> 
> 
>> -----Original Message-----
>> From: 'Manivannan Sadhasivam' <mani@kernel.org>
>> Sent: Tuesday, August 5, 2025 10:52 PM
>> To: Alim Akhtar <alim.akhtar@samsung.com>
>> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
>> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
>> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
>> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
>> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
>> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
>> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
>> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
>> properties to UFS
>>
>> On Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> Sent: Tuesday, August 5, 2025 10:36 PM
>>>> To: Manivannan Sadhasivam <mani@kernel.org>
>>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
>>>> <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
>>>> avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
>>>> krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
>>>> konradybcio@kernel.org; James.Bottomley@hansenpartnership.com;
>>>> martin.petersen@oracle.com; agross@kernel.org; linux-arm-
>>>> msm@vger.kernel.org; linux-scsi@vger.kernel.org;
>>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate
>>>> limit properties to UFS
>>>>
>>>> On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
>>>>> On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
>>>>>> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
>>>>>>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski wrote:
>>>>>>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
>>>>>>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski
>> wrote:
>>>>>>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>>>>>>>>>>> Add optional limit-hs-gear and limit-rate properties to the
>>>>>>>>>>>> UFS node to support automotive use cases that require
>>>>>>>>>>>> limiting the maximum Tx/Rx HS gear and rate due to hardware
>> constraints.
>>>>>>>>>>>
>>>>>>>>>>> What hardware constraints? This needs to be clearly
>> documented.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Ram, both Krzysztof and I asked this question, but you never
>>>>>>>>>> bothered to reply, but keep on responding to other comments.
>>>>>>>>>> This won't help you to get this series merged in any form.
>>>>>>>>>>
>>>>>>>>>> Please address *all* review comments before posting next
>> iteration.
>>>>>>>>>
>>>>>>>>> Hi Mani,
>>>>>>>>>
>>>>>>>>> Apologies for the delay in responding.
>>>>>>>>> I had planned to explain the hardware constraints in the next
>>>> patchset’s commit message, which is why I didn’t reply earlier.
>>>>>>>>>
>>>>>>>>> To clarify: the limitations are due to customer board designs,
>>>>>>>>> not our
>>>> SoC. Some boards can't support higher gear operation, hence the need
>>>> for optional limit-hs-gear and limit-rate properties.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That's vague and does not justify the property. You need to
>>>>>>>> document instead hardware capabilities or characteristic. Or
>>>>>>>> explain why they cannot. With such form I will object to your
>>>>>>>> next
>>>> patch.
>>>>>>>>
>>>>>>>
>>>>>>> I had an offline chat with Ram and got clarified on what these
>>>>>>> properties
>>>> are.
>>>>>>> The problem here is not with the SoC, but with the board design.
>>>>>>> On some Qcom customer designs, both the UFS controller in the
>>>>>>> SoC and the UFS device are capable of operating at higher gears (say
>> G5).
>>>>>>> But due to board constraints like poor thermal dissipation,
>>>>>>> routing loss, the board cannot efficiently operate at the higher
>> speeds.
>>>>>>>
>>>>>>> So the customers wanted a way to limit the gear speed (say G3)
>>>>>>> and rate (say Mode-A) on the specific board DTS.
>>>>>>
>>>>>> I'm not necessarily saying no, but have you explored sysfs for this?
>>>>>>
>>>>>> I suppose it may be too late (if the driver would e.g. init the
>>>>>> UFS at max gear/rate at probe time, it could cause havoc as it
>>>>>> tries to load the userland)..
>>>>>>
>>>>>
>>>>> If the driver tries to run with unsupported max gear speed/mode,
>>>>> it will just crash with the error spit.
>>>>
>>>> OK
>>>>
>>>> just a couple related nits that I won't bother splitting into
>>>> separate emails
>>>>
>>>> rate (mode? I'm seeing both names) should probably have dt-bindings
>>>> defines while gear doesn't have to since they're called G<number>
>>>> anyway, with the bindings description strongly discouraging use,
>>>> unless absolutely necessary (e.g. in the situation we have right
>>>> there)
>>>>
>>>> I'd also assume the code should be moved into the ufs-common code,
>>>> rather than making it ufs-qcom specific
>>>>
>>>> Konrad
>>> Since this is a board specific constrains and not a SoC properties, have an
>> option of handling this via bootloader is explored?
>>
>> Both board and SoC specific properties *should* be described in devicetree
>> if they are purely describing the hardware.
>>
> Agreed, what I understood from above conversation is that, we are try to solve a very *specific* board problem here, 
> this does not looks like a generic problem to me and probably should be handled within the specific driver.

Introducing generic solutions preemptively for problems that are simple
in concept and can occur widely is good practice (although it's sometimes
hard to gauge whether this is a one-off), as if the issue spreads a
generic solution will appear at some point, but we'll have to keep
supporting the odd ones as well

Konrad

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Sent: Tuesday, August 5, 2025 11:10 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>; 'Manivannan Sadhasivam'
> <mani@kernel.org>
> Cc: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On 8/5/25 7:36 PM, Alim Akhtar wrote:
> >
> >
> >> -----Original Message-----
> >> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> >> Sent: Tuesday, August 5, 2025 10:52 PM
> >> To: Alim Akhtar <alim.akhtar@samsung.com>
> >> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> >> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> >> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org;
> >> robh@kernel.org; krzk+dt@kernel.org;
> >> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> >> James.Bottomley@hansenpartnership.com;
> martin.petersen@oracle.com;
> >> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> >> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org
> >> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate
> >> limit properties to UFS
> >>
> >> On Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>> Sent: Tuesday, August 5, 2025 10:36 PM
> >>>> To: Manivannan Sadhasivam <mani@kernel.org>
> >>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
> >>>> <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
> >>>> avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
> >>>> krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
> >>>> konradybcio@kernel.org; James.Bottomley@hansenpartnership.com;
> >>>> martin.petersen@oracle.com; agross@kernel.org; linux-arm-
> >>>> msm@vger.kernel.org; linux-scsi@vger.kernel.org;
> >>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> >>>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and
> >>>> rate limit properties to UFS
> >>>>
> >>>> On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> >>>>> On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> >>>>>> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> >>>>>>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski
> wrote:
> >>>>>>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> >>>>>>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski
> >> wrote:
> >>>>>>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> >>>>>>>>>>>> Add optional limit-hs-gear and limit-rate properties to the
> >>>>>>>>>>>> UFS node to support automotive use cases that require
> >>>>>>>>>>>> limiting the maximum Tx/Rx HS gear and rate due to
> hardware
> >> constraints.
> >>>>>>>>>>>
> >>>>>>>>>>> What hardware constraints? This needs to be clearly
> >> documented.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Ram, both Krzysztof and I asked this question, but you never
> >>>>>>>>>> bothered to reply, but keep on responding to other comments.
> >>>>>>>>>> This won't help you to get this series merged in any form.
> >>>>>>>>>>
> >>>>>>>>>> Please address *all* review comments before posting next
> >> iteration.
> >>>>>>>>>
> >>>>>>>>> Hi Mani,
> >>>>>>>>>
> >>>>>>>>> Apologies for the delay in responding.
> >>>>>>>>> I had planned to explain the hardware constraints in the next
> >>>> patchset’s commit message, which is why I didn’t reply earlier.
> >>>>>>>>>
> >>>>>>>>> To clarify: the limitations are due to customer board designs,
> >>>>>>>>> not our
> >>>> SoC. Some boards can't support higher gear operation, hence the
> >>>> need for optional limit-hs-gear and limit-rate properties.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> That's vague and does not justify the property. You need to
> >>>>>>>> document instead hardware capabilities or characteristic. Or
> >>>>>>>> explain why they cannot. With such form I will object to your
> >>>>>>>> next
> >>>> patch.
> >>>>>>>>
> >>>>>>>
> >>>>>>> I had an offline chat with Ram and got clarified on what these
> >>>>>>> properties
> >>>> are.
> >>>>>>> The problem here is not with the SoC, but with the board design.
> >>>>>>> On some Qcom customer designs, both the UFS controller in the
> >>>>>>> SoC and the UFS device are capable of operating at higher gears
> >>>>>>> (say
> >> G5).
> >>>>>>> But due to board constraints like poor thermal dissipation,
> >>>>>>> routing loss, the board cannot efficiently operate at the higher
> >> speeds.
> >>>>>>>
> >>>>>>> So the customers wanted a way to limit the gear speed (say G3)
> >>>>>>> and rate (say Mode-A) on the specific board DTS.
> >>>>>>
> >>>>>> I'm not necessarily saying no, but have you explored sysfs for this?
> >>>>>>
> >>>>>> I suppose it may be too late (if the driver would e.g. init the
> >>>>>> UFS at max gear/rate at probe time, it could cause havoc as it
> >>>>>> tries to load the userland)..
> >>>>>>
> >>>>>
> >>>>> If the driver tries to run with unsupported max gear speed/mode,
> >>>>> it will just crash with the error spit.
> >>>>
> >>>> OK
> >>>>
> >>>> just a couple related nits that I won't bother splitting into
> >>>> separate emails
> >>>>
> >>>> rate (mode? I'm seeing both names) should probably have dt-bindings
> >>>> defines while gear doesn't have to since they're called G<number>
> >>>> anyway, with the bindings description strongly discouraging use,
> >>>> unless absolutely necessary (e.g. in the situation we have right
> >>>> there)
> >>>>
> >>>> I'd also assume the code should be moved into the ufs-common code,
> >>>> rather than making it ufs-qcom specific
> >>>>
> >>>> Konrad
> >>> Since this is a board specific constrains and not a SoC properties,
> >>> have an
> >> option of handling this via bootloader is explored?
> >>
> >> Both board and SoC specific properties *should* be described in
> >> devicetree if they are purely describing the hardware.
> >>
> > Agreed, what I understood from above conversation is that, we are try
> > to solve a very *specific* board problem here, this does not looks like a
> generic problem to me and probably should be handled within the specific
> driver.
> 
> Introducing generic solutions preemptively for problems that are simple in
> concept and can occur widely is good practice (although it's sometimes hard
> to gauge whether this is a one-off), as if the issue spreads a generic solution
> will appear at some point, but we'll have to keep supporting the odd ones as
> well
> 
Ok, 
I would prefer if we add a property which sounds like "poor thermal dissipation" or 
"routing channel loss" rather than adding limiting UFS gear properties. 
Poor thermal design or channel losses are generic enough and can happen on any board.

> Konrad

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Konrad Dybcio]

On 8/5/25 7:52 PM, Alim Akhtar wrote:
> 
> 
>> -----Original Message-----
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> Sent: Tuesday, August 5, 2025 11:10 PM
>> To: Alim Akhtar <alim.akhtar@samsung.com>; 'Manivannan Sadhasivam'
>> <mani@kernel.org>
>> Cc: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
>> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
>> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
>> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
>> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
>> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
>> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
>> properties to UFS
>>
>> On 8/5/25 7:36 PM, Alim Akhtar wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: 'Manivannan Sadhasivam' <mani@kernel.org>
>>>> Sent: Tuesday, August 5, 2025 10:52 PM
>>>> To: Alim Akhtar <alim.akhtar@samsung.com>
>>>> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
>>>> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
>>>> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
>> bvanassche@acm.org;
>>>> robh@kernel.org; krzk+dt@kernel.org;
>>>> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
>>>> James.Bottomley@hansenpartnership.com;
>> martin.petersen@oracle.com;
>>>> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
>>>> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
>>>> kernel@vger.kernel.org
>>>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate
>>>> limit properties to UFS
>>>>
>>>> On Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>> Sent: Tuesday, August 5, 2025 10:36 PM
>>>>>> To: Manivannan Sadhasivam <mani@kernel.org>
>>>>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
>>>>>> <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
>>>>>> avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
>>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
>>>>>> konradybcio@kernel.org; James.Bottomley@hansenpartnership.com;
>>>>>> martin.petersen@oracle.com; agross@kernel.org; linux-arm-
>>>>>> msm@vger.kernel.org; linux-scsi@vger.kernel.org;
>>>>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
>>>>>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and
>>>>>> rate limit properties to UFS
>>>>>>
>>>>>> On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
>>>>>>> On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
>>>>>>>> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
>>>>>>>>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski
>> wrote:
>>>>>>>>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
>>>>>>>>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski
>>>> wrote:
>>>>>>>>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
>>>>>>>>>>>>>> Add optional limit-hs-gear and limit-rate properties to the
>>>>>>>>>>>>>> UFS node to support automotive use cases that require
>>>>>>>>>>>>>> limiting the maximum Tx/Rx HS gear and rate due to
>> hardware
>>>> constraints.
>>>>>>>>>>>>>
>>>>>>>>>>>>> What hardware constraints? This needs to be clearly
>>>> documented.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Ram, both Krzysztof and I asked this question, but you never
>>>>>>>>>>>> bothered to reply, but keep on responding to other comments.
>>>>>>>>>>>> This won't help you to get this series merged in any form.
>>>>>>>>>>>>
>>>>>>>>>>>> Please address *all* review comments before posting next
>>>> iteration.
>>>>>>>>>>>
>>>>>>>>>>> Hi Mani,
>>>>>>>>>>>
>>>>>>>>>>> Apologies for the delay in responding.
>>>>>>>>>>> I had planned to explain the hardware constraints in the next
>>>>>> patchset’s commit message, which is why I didn’t reply earlier.
>>>>>>>>>>>
>>>>>>>>>>> To clarify: the limitations are due to customer board designs,
>>>>>>>>>>> not our
>>>>>> SoC. Some boards can't support higher gear operation, hence the
>>>>>> need for optional limit-hs-gear and limit-rate properties.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> That's vague and does not justify the property. You need to
>>>>>>>>>> document instead hardware capabilities or characteristic. Or
>>>>>>>>>> explain why they cannot. With such form I will object to your
>>>>>>>>>> next
>>>>>> patch.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I had an offline chat with Ram and got clarified on what these
>>>>>>>>> properties
>>>>>> are.
>>>>>>>>> The problem here is not with the SoC, but with the board design.
>>>>>>>>> On some Qcom customer designs, both the UFS controller in the
>>>>>>>>> SoC and the UFS device are capable of operating at higher gears
>>>>>>>>> (say
>>>> G5).
>>>>>>>>> But due to board constraints like poor thermal dissipation,
>>>>>>>>> routing loss, the board cannot efficiently operate at the higher
>>>> speeds.
>>>>>>>>>
>>>>>>>>> So the customers wanted a way to limit the gear speed (say G3)
>>>>>>>>> and rate (say Mode-A) on the specific board DTS.
>>>>>>>>
>>>>>>>> I'm not necessarily saying no, but have you explored sysfs for this?
>>>>>>>>
>>>>>>>> I suppose it may be too late (if the driver would e.g. init the
>>>>>>>> UFS at max gear/rate at probe time, it could cause havoc as it
>>>>>>>> tries to load the userland)..
>>>>>>>>
>>>>>>>
>>>>>>> If the driver tries to run with unsupported max gear speed/mode,
>>>>>>> it will just crash with the error spit.
>>>>>>
>>>>>> OK
>>>>>>
>>>>>> just a couple related nits that I won't bother splitting into
>>>>>> separate emails
>>>>>>
>>>>>> rate (mode? I'm seeing both names) should probably have dt-bindings
>>>>>> defines while gear doesn't have to since they're called G<number>
>>>>>> anyway, with the bindings description strongly discouraging use,
>>>>>> unless absolutely necessary (e.g. in the situation we have right
>>>>>> there)
>>>>>>
>>>>>> I'd also assume the code should be moved into the ufs-common code,
>>>>>> rather than making it ufs-qcom specific
>>>>>>
>>>>>> Konrad
>>>>> Since this is a board specific constrains and not a SoC properties,
>>>>> have an
>>>> option of handling this via bootloader is explored?
>>>>
>>>> Both board and SoC specific properties *should* be described in
>>>> devicetree if they are purely describing the hardware.
>>>>
>>> Agreed, what I understood from above conversation is that, we are try
>>> to solve a very *specific* board problem here, this does not looks like a
>> generic problem to me and probably should be handled within the specific
>> driver.
>>
>> Introducing generic solutions preemptively for problems that are simple in
>> concept and can occur widely is good practice (although it's sometimes hard
>> to gauge whether this is a one-off), as if the issue spreads a generic solution
>> will appear at some point, but we'll have to keep supporting the odd ones as
>> well
>>
> Ok, 
> I would prefer if we add a property which sounds like "poor thermal dissipation" or 
> "routing channel loss" rather than adding limiting UFS gear properties. 
> Poor thermal design or channel losses are generic enough and can happen on any board.

This is exactly what I'm trying to avoid through my suggestion - one
board may have poor thermal dissipation, another may have channel
losses, yet another one may feature a special batch of UFS chips that
will set the world on fire if instructed to attempt link training at
gear 7 - they all are causes, as opposed to describing what needs to
happen (i.e. what the hardware must be treated as - gear N incapable
despite what can be discovered at runtime), with perhaps a comment on
the side

Konrad

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Sent: Tuesday, August 5, 2025 11:57 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>; 'Manivannan Sadhasivam'
> <mani@kernel.org>
> Cc: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On 8/5/25 7:52 PM, Alim Akhtar wrote:
> >
> >
> >> -----Original Message-----
> >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >> Sent: Tuesday, August 5, 2025 11:10 PM
> >> To: Alim Akhtar <alim.akhtar@samsung.com>; 'Manivannan Sadhasivam'
> >> <mani@kernel.org>
> >> Cc: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> >> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org;
> >> robh@kernel.org; krzk+dt@kernel.org;
> >> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> >> James.Bottomley@hansenpartnership.com;
> martin.petersen@oracle.com;
> >> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> >> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org
> >> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate
> >> limit properties to UFS
> >>
> >> On 8/5/25 7:36 PM, Alim Akhtar wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> >>>> Sent: Tuesday, August 5, 2025 10:52 PM
> >>>> To: Alim Akhtar <alim.akhtar@samsung.com>
> >>>> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> >>>> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> >>>> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> >> bvanassche@acm.org;
> >>>> robh@kernel.org; krzk+dt@kernel.org;
> >>>> conor+dt@kernel.org; andersson@kernel.org;
> konradybcio@kernel.org;
> >>>> James.Bottomley@hansenpartnership.com;
> >> martin.petersen@oracle.com;
> >>>> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> >>>> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> >>>> kernel@vger.kernel.org
> >>>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and
> >>>> rate limit properties to UFS
> >>>>
> >>>> On Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>>>> Sent: Tuesday, August 5, 2025 10:36 PM
> >>>>>> To: Manivannan Sadhasivam <mani@kernel.org>
> >>>>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
> >>>>>> <quic_rdwivedi@quicinc.com>; alim.akhtar@samsung.com;
> >>>>>> avri.altman@wdc.com; bvanassche@acm.org; robh@kernel.org;
> >>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; andersson@kernel.org;
> >>>>>> konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com;
> >>>>>> martin.petersen@oracle.com; agross@kernel.org; linux-arm-
> >>>>>> msm@vger.kernel.org; linux-scsi@vger.kernel.org;
> >>>>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> >>>>>> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and
> >>>>>> rate limit properties to UFS
> >>>>>>
> >>>>>> On 8/5/25 6:55 PM, Manivannan Sadhasivam wrote:
> >>>>>>> On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
> >>>>>>>> On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
> >>>>>>>>> On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski
> >> wrote:
> >>>>>>>>>> On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
> >>>>>>>>>>>> On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski
> >>>> wrote:
> >>>>>>>>>>>>> On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
> >>>>>>>>>>>>>> Add optional limit-hs-gear and limit-rate properties to
> >>>>>>>>>>>>>> the UFS node to support automotive use cases that
> require
> >>>>>>>>>>>>>> limiting the maximum Tx/Rx HS gear and rate due to
> >> hardware
> >>>> constraints.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> What hardware constraints? This needs to be clearly
> >>>> documented.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Ram, both Krzysztof and I asked this question, but you
> >>>>>>>>>>>> never bothered to reply, but keep on responding to other
> comments.
> >>>>>>>>>>>> This won't help you to get this series merged in any form.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Please address *all* review comments before posting next
> >>>> iteration.
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Mani,
> >>>>>>>>>>>
> >>>>>>>>>>> Apologies for the delay in responding.
> >>>>>>>>>>> I had planned to explain the hardware constraints in the
> >>>>>>>>>>> next
> >>>>>> patchset’s commit message, which is why I didn’t reply earlier.
> >>>>>>>>>>>
> >>>>>>>>>>> To clarify: the limitations are due to customer board
> >>>>>>>>>>> designs, not our
> >>>>>> SoC. Some boards can't support higher gear operation, hence the
> >>>>>> need for optional limit-hs-gear and limit-rate properties.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> That's vague and does not justify the property. You need to
> >>>>>>>>>> document instead hardware capabilities or characteristic. Or
> >>>>>>>>>> explain why they cannot. With such form I will object to your
> >>>>>>>>>> next
> >>>>>> patch.
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I had an offline chat with Ram and got clarified on what these
> >>>>>>>>> properties
> >>>>>> are.
> >>>>>>>>> The problem here is not with the SoC, but with the board design.
> >>>>>>>>> On some Qcom customer designs, both the UFS controller in the
> >>>>>>>>> SoC and the UFS device are capable of operating at higher
> >>>>>>>>> gears (say
> >>>> G5).
> >>>>>>>>> But due to board constraints like poor thermal dissipation,
> >>>>>>>>> routing loss, the board cannot efficiently operate at the
> >>>>>>>>> higher
> >>>> speeds.
> >>>>>>>>>
> >>>>>>>>> So the customers wanted a way to limit the gear speed (say G3)
> >>>>>>>>> and rate (say Mode-A) on the specific board DTS.
> >>>>>>>>
> >>>>>>>> I'm not necessarily saying no, but have you explored sysfs for
> this?
> >>>>>>>>
> >>>>>>>> I suppose it may be too late (if the driver would e.g. init the
> >>>>>>>> UFS at max gear/rate at probe time, it could cause havoc as it
> >>>>>>>> tries to load the userland)..
> >>>>>>>>
> >>>>>>>
> >>>>>>> If the driver tries to run with unsupported max gear speed/mode,
> >>>>>>> it will just crash with the error spit.
> >>>>>>
> >>>>>> OK
> >>>>>>
> >>>>>> just a couple related nits that I won't bother splitting into
> >>>>>> separate emails
> >>>>>>
> >>>>>> rate (mode? I'm seeing both names) should probably have
> >>>>>> dt-bindings defines while gear doesn't have to since they're
> >>>>>> called G<number> anyway, with the bindings description strongly
> >>>>>> discouraging use, unless absolutely necessary (e.g. in the
> >>>>>> situation we have right
> >>>>>> there)
> >>>>>>
> >>>>>> I'd also assume the code should be moved into the ufs-common
> >>>>>> code, rather than making it ufs-qcom specific
> >>>>>>
> >>>>>> Konrad
> >>>>> Since this is a board specific constrains and not a SoC
> >>>>> properties, have an
> >>>> option of handling this via bootloader is explored?
> >>>>
> >>>> Both board and SoC specific properties *should* be described in
> >>>> devicetree if they are purely describing the hardware.
> >>>>
> >>> Agreed, what I understood from above conversation is that, we are
> >>> try to solve a very *specific* board problem here, this does not
> >>> looks like a
> >> generic problem to me and probably should be handled within the
> >> specific driver.
> >>
> >> Introducing generic solutions preemptively for problems that are
> >> simple in concept and can occur widely is good practice (although
> >> it's sometimes hard to gauge whether this is a one-off), as if the
> >> issue spreads a generic solution will appear at some point, but we'll
> >> have to keep supporting the odd ones as well
> >>
> > Ok,
> > I would prefer if we add a property which sounds like "poor thermal
> > dissipation" or "routing channel loss" rather than adding limiting UFS gear
> properties.
> > Poor thermal design or channel losses are generic enough and can happen
> on any board.
> 
> This is exactly what I'm trying to avoid through my suggestion - one board
> may have poor thermal dissipation, another may have channel losses, yet
> another one may feature a special batch of UFS chips that will set the world
> on fire if instructed to attempt link training at gear 7 - they all are causes, as
> opposed to describing what needs to happen (i.e. what the hardware must
> be treated as - gear N incapable despite what can be discovered at runtime),
> with perhaps a comment on the side
> 
But the solution for all possible board problems can't be by limiting Gear speed.
So it should be known why one particular board need to limit the gear.
I understand that this is a static configuration, where it is already known that board is broken for higher Gear.
Can this be achieved by limiting the clock? If not, can we add a board specific _quirk_ and let the _quirk_ to be enabled from vendor specific hooks? 

> Konrad

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

['Manivannan Sadhasivam']

On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:

[...]

> > >> Introducing generic solutions preemptively for problems that are
> > >> simple in concept and can occur widely is good practice (although
> > >> it's sometimes hard to gauge whether this is a one-off), as if the
> > >> issue spreads a generic solution will appear at some point, but we'll
> > >> have to keep supporting the odd ones as well
> > >>
> > > Ok,
> > > I would prefer if we add a property which sounds like "poor thermal
> > > dissipation" or "routing channel loss" rather than adding limiting UFS gear
> > properties.
> > > Poor thermal design or channel losses are generic enough and can happen
> > on any board.
> > 
> > This is exactly what I'm trying to avoid through my suggestion - one board
> > may have poor thermal dissipation, another may have channel losses, yet
> > another one may feature a special batch of UFS chips that will set the world
> > on fire if instructed to attempt link training at gear 7 - they all are causes, as
> > opposed to describing what needs to happen (i.e. what the hardware must
> > be treated as - gear N incapable despite what can be discovered at runtime),
> > with perhaps a comment on the side
> > 
> But the solution for all possible board problems can't be by limiting Gear speed.

Devicetree properties should precisely reflect how they are relevant to the
hardware. 'limiting-gear-speed' is self-explanatory that the gear speed is
getting limited (for a reason), but the devicetree doesn't need to describe the
*reason* itself.

> So it should be known why one particular board need to limit the gear.

That goes into the description, not in the property name.

> I understand that this is a static configuration, where it is already known that board is broken for higher Gear.
> Can this be achieved by limiting the clock? If not, can we add a board specific _quirk_ and let the _quirk_ to be enabled from vendor specific hooks? 
> 

How can we limit the clock without limiting the gears? When we limit the
gear/mode, both clock and power are implicitly limited.

- Mani

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

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> Sent: Wednesday, August 6, 2025 10:35 AM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> 
> [...]
> 
> > > >> Introducing generic solutions preemptively for problems that are
> > > >> simple in concept and can occur widely is good practice (although
> > > >> it's sometimes hard to gauge whether this is a one-off), as if
> > > >> the issue spreads a generic solution will appear at some point,
> > > >> but we'll have to keep supporting the odd ones as well
> > > >>
> > > > Ok,
> > > > I would prefer if we add a property which sounds like "poor
> > > > thermal dissipation" or "routing channel loss" rather than adding
> > > > limiting UFS gear
> > > properties.
> > > > Poor thermal design or channel losses are generic enough and can
> > > > happen
> > > on any board.
> > >
> > > This is exactly what I'm trying to avoid through my suggestion - one
> > > board may have poor thermal dissipation, another may have channel
> > > losses, yet another one may feature a special batch of UFS chips
> > > that will set the world on fire if instructed to attempt link
> > > training at gear 7 - they all are causes, as opposed to describing
> > > what needs to happen (i.e. what the hardware must be treated as -
> > > gear N incapable despite what can be discovered at runtime), with
> > > perhaps a comment on the side
> > >
> > But the solution for all possible board problems can't be by limiting Gear
> speed.
> 
> Devicetree properties should precisely reflect how they are relevant to the
> hardware. 'limiting-gear-speed' is self-explanatory that the gear speed is
> getting limited (for a reason), but the devicetree doesn't need to describe
> the
> *reason* itself.
> 
> > So it should be known why one particular board need to limit the gear.
> 
> That goes into the description, not in the property name.
> 
> > I understand that this is a static configuration, where it is already known
> that board is broken for higher Gear.
> > Can this be achieved by limiting the clock? If not, can we add a board
> specific _quirk_ and let the _quirk_ to be enabled from vendor specific
> hooks?
> >
> 
> How can we limit the clock without limiting the gears? When we limit the
> gear/mode, both clock and power are implicitly limited.
> 
Possibly someone need to check with designer of the SoC if that is possible or not.
Did we already tried _quirk_? If not, why not? 
If the board is so poorly designed and can't take care of the channel loses or heat dissipation etc,
Then I assumed the gear negotiation between host and device should fail for the higher gear 
and driver can have a re-try logic to re-init / re-try "power mode change" at the lower gear. Is that not possible / feasible?



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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

['Manivannan Sadhasivam']

On Wed, Aug 06, 2025 at 11:16:11AM GMT, Alim Akhtar wrote:
> 
> 
> > -----Original Message-----
> > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > Sent: Wednesday, August 6, 2025 10:35 AM
> > To: Alim Akhtar <alim.akhtar@samsung.com>
> > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> > Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> > <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> > bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> > conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> > James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> > agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> > scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> > properties to UFS
> > 
> > On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> > 
> > [...]
> > 
> > > > >> Introducing generic solutions preemptively for problems that are
> > > > >> simple in concept and can occur widely is good practice (although
> > > > >> it's sometimes hard to gauge whether this is a one-off), as if
> > > > >> the issue spreads a generic solution will appear at some point,
> > > > >> but we'll have to keep supporting the odd ones as well
> > > > >>
> > > > > Ok,
> > > > > I would prefer if we add a property which sounds like "poor
> > > > > thermal dissipation" or "routing channel loss" rather than adding
> > > > > limiting UFS gear
> > > > properties.
> > > > > Poor thermal design or channel losses are generic enough and can
> > > > > happen
> > > > on any board.
> > > >
> > > > This is exactly what I'm trying to avoid through my suggestion - one
> > > > board may have poor thermal dissipation, another may have channel
> > > > losses, yet another one may feature a special batch of UFS chips
> > > > that will set the world on fire if instructed to attempt link
> > > > training at gear 7 - they all are causes, as opposed to describing
> > > > what needs to happen (i.e. what the hardware must be treated as -
> > > > gear N incapable despite what can be discovered at runtime), with
> > > > perhaps a comment on the side
> > > >
> > > But the solution for all possible board problems can't be by limiting Gear
> > speed.
> > 
> > Devicetree properties should precisely reflect how they are relevant to the
> > hardware. 'limiting-gear-speed' is self-explanatory that the gear speed is
> > getting limited (for a reason), but the devicetree doesn't need to describe
> > the
> > *reason* itself.
> > 
> > > So it should be known why one particular board need to limit the gear.
> > 
> > That goes into the description, not in the property name.
> > 
> > > I understand that this is a static configuration, where it is already known
> > that board is broken for higher Gear.
> > > Can this be achieved by limiting the clock? If not, can we add a board
> > specific _quirk_ and let the _quirk_ to be enabled from vendor specific
> > hooks?
> > >
> > 
> > How can we limit the clock without limiting the gears? When we limit the
> > gear/mode, both clock and power are implicitly limited.
> > 
> Possibly someone need to check with designer of the SoC if that is possible or not.

It's not just clock. We need to consider reducing regulator, interconnect votes
also. But as I said above, limiting the gear/mode will take care of all these
parameters.

> Did we already tried _quirk_? If not, why not? 
> If the board is so poorly designed and can't take care of the channel loses or heat dissipation etc,
> Then I assumed the gear negotiation between host and device should fail for the higher gear 
> and driver can have a re-try logic to re-init / re-try "power mode change" at the lower gear. Is that not possible / feasible?
> 

I don't see why we need to add extra logic in the UFS driver if we can extract
that information from DT.

- Mani

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

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> Sent: Wednesday, August 6, 2025 4:56 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
[...]

> > >
> > > On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> > >
> > > [...]
> > >
> > > > > >> Introducing generic solutions preemptively for problems that
> > > > > >> are simple in concept and can occur widely is good practice
> > > > > >> (although it's sometimes hard to gauge whether this is a
> > > > > >> one-off), as if the issue spreads a generic solution will
> > > > > >> appear at some point, but we'll have to keep supporting the
> > > > > >> odd ones as well
> > > > > >>
> > > > > > Ok,
> > > > > > I would prefer if we add a property which sounds like "poor
> > > > > > thermal dissipation" or "routing channel loss" rather than
> > > > > > adding limiting UFS gear
> > > > > properties.
> > > > > > Poor thermal design or channel losses are generic enough and
> > > > > > can happen
> > > > > on any board.
> > > > >
> > > > > This is exactly what I'm trying to avoid through my suggestion -
> > > > > one board may have poor thermal dissipation, another may have
> > > > > channel losses, yet another one may feature a special batch of
> > > > > UFS chips that will set the world on fire if instructed to
> > > > > attempt link training at gear 7 - they all are causes, as
> > > > > opposed to describing what needs to happen (i.e. what the
> > > > > hardware must be treated as - gear N incapable despite what can
> > > > > be discovered at runtime), with perhaps a comment on the side
> > > > >
> > > > But the solution for all possible board problems can't be by
> > > > limiting Gear
> > > speed.
> > >
> > > Devicetree properties should precisely reflect how they are relevant
> > > to the hardware. 'limiting-gear-speed' is self-explanatory that the
> > > gear speed is getting limited (for a reason), but the devicetree
> > > doesn't need to describe the
> > > *reason* itself.
> > >
> > > > So it should be known why one particular board need to limit the gear.
> > >
> > > That goes into the description, not in the property name.
> > >
> > > > I understand that this is a static configuration, where it is
> > > > already known
> > > that board is broken for higher Gear.
> > > > Can this be achieved by limiting the clock? If not, can we add a
> > > > board
> > > specific _quirk_ and let the _quirk_ to be enabled from vendor
> > > specific hooks?
> > > >
> > >
> > > How can we limit the clock without limiting the gears? When we limit
> > > the gear/mode, both clock and power are implicitly limited.
> > >
> > Possibly someone need to check with designer of the SoC if that is possible
> or not.
> 
> It's not just clock. We need to consider reducing regulator, interconnect
> votes also. But as I said above, limiting the gear/mode will take care of all
> these parameters.
> 
> > Did we already tried _quirk_? If not, why not?
> > If the board is so poorly designed and can't take care of the channel
> > loses or heat dissipation etc, Then I assumed the gear negotiation
> > between host and device should fail for the higher gear and driver can have
> a re-try logic to re-init / re-try "power mode change" at the lower gear. Is
> that not possible / feasible?
> >
> 
> I don't see why we need to add extra logic in the UFS driver if we can extract
> that information from DT.
> 
You didn’t answer my question entirely, I am still not able to visualised how come Linkup is happening in higher gear and then 
Suddenly it is failing and we need to reduce the gear to solve that?
That's why my suggestion is to go for a re-try at lower gear when problem happens.
It is not that since adding DT property is simple to just go that path, that is solving _just_ this case, may be. 



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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

['Manivannan Sadhasivam']

On Thu, Aug 07, 2025 at 10:08:32PM GMT, Alim Akhtar wrote:
> 
> 
> > -----Original Message-----
> > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > Sent: Wednesday, August 6, 2025 4:56 PM
> > To: Alim Akhtar <alim.akhtar@samsung.com>
> > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> [...]
> 
> > > >
> > > > On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> > > >
> > > > [...]
> > > >
> > > > > > >> Introducing generic solutions preemptively for problems that
> > > > > > >> are simple in concept and can occur widely is good practice
> > > > > > >> (although it's sometimes hard to gauge whether this is a
> > > > > > >> one-off), as if the issue spreads a generic solution will
> > > > > > >> appear at some point, but we'll have to keep supporting the
> > > > > > >> odd ones as well
> > > > > > >>
> > > > > > > Ok,
> > > > > > > I would prefer if we add a property which sounds like "poor
> > > > > > > thermal dissipation" or "routing channel loss" rather than
> > > > > > > adding limiting UFS gear
> > > > > > properties.
> > > > > > > Poor thermal design or channel losses are generic enough and
> > > > > > > can happen
> > > > > > on any board.
> > > > > >
> > > > > > This is exactly what I'm trying to avoid through my suggestion -
> > > > > > one board may have poor thermal dissipation, another may have
> > > > > > channel losses, yet another one may feature a special batch of
> > > > > > UFS chips that will set the world on fire if instructed to
> > > > > > attempt link training at gear 7 - they all are causes, as
> > > > > > opposed to describing what needs to happen (i.e. what the
> > > > > > hardware must be treated as - gear N incapable despite what can
> > > > > > be discovered at runtime), with perhaps a comment on the side
> > > > > >
> > > > > But the solution for all possible board problems can't be by
> > > > > limiting Gear
> > > > speed.
> > > >
> > > > Devicetree properties should precisely reflect how they are relevant
> > > > to the hardware. 'limiting-gear-speed' is self-explanatory that the
> > > > gear speed is getting limited (for a reason), but the devicetree
> > > > doesn't need to describe the
> > > > *reason* itself.
> > > >
> > > > > So it should be known why one particular board need to limit the gear.
> > > >
> > > > That goes into the description, not in the property name.
> > > >
> > > > > I understand that this is a static configuration, where it is
> > > > > already known
> > > > that board is broken for higher Gear.
> > > > > Can this be achieved by limiting the clock? If not, can we add a
> > > > > board
> > > > specific _quirk_ and let the _quirk_ to be enabled from vendor
> > > > specific hooks?
> > > > >
> > > >
> > > > How can we limit the clock without limiting the gears? When we limit
> > > > the gear/mode, both clock and power are implicitly limited.
> > > >
> > > Possibly someone need to check with designer of the SoC if that is possible
> > or not.
> > 
> > It's not just clock. We need to consider reducing regulator, interconnect
> > votes also. But as I said above, limiting the gear/mode will take care of all
> > these parameters.
> > 
> > > Did we already tried _quirk_? If not, why not?
> > > If the board is so poorly designed and can't take care of the channel
> > > loses or heat dissipation etc, Then I assumed the gear negotiation
> > > between host and device should fail for the higher gear and driver can have
> > a re-try logic to re-init / re-try "power mode change" at the lower gear. Is
> > that not possible / feasible?
> > >
> > 
> > I don't see why we need to add extra logic in the UFS driver if we can extract
> > that information from DT.
> > 
> You didn’t answer my question entirely, I am still not able to visualised how come Linkup is happening in higher gear and then 
> Suddenly it is failing and we need to reduce the gear to solve that?

Oh well, this is the source of confusion here. I didn't (also the patch) claim
that the link up will happen with higher speed. It will most likely fail if it
couldn't operate at the higher speed and that's why we need to limit it to lower
gear/mode *before* bringing the link up.

As you can see, the driver patch is parsing the limits in its
ufs_hba_variant_ops::init() callback, which gets called during
ufshcd_hba_init().

- Mani

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

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> Sent: Friday, August 8, 2025 6:14 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On Thu, Aug 07, 2025 at 10:08:32PM GMT, Alim Akhtar wrote:
> >
> >
> > > -----Original Message-----
> > > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > > Sent: Wednesday, August 6, 2025 4:56 PM
> > > To: Alim Akhtar <alim.akhtar@samsung.com>
> > > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> > [...]
> >
> > > > >
> > > > > On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > > > >> Introducing generic solutions preemptively for problems
> > > > > > > >> that are simple in concept and can occur widely is good
> > > > > > > >> practice (although it's sometimes hard to gauge whether
> > > > > > > >> this is a one-off), as if the issue spreads a generic
> > > > > > > >> solution will appear at some point, but we'll have to
> > > > > > > >> keep supporting the odd ones as well
> > > > > > > >>
> > > > > > > > Ok,
> > > > > > > > I would prefer if we add a property which sounds like
> > > > > > > > "poor thermal dissipation" or "routing channel loss"
> > > > > > > > rather than adding limiting UFS gear
> > > > > > > properties.
> > > > > > > > Poor thermal design or channel losses are generic enough
> > > > > > > > and can happen
> > > > > > > on any board.
> > > > > > >
> > > > > > > This is exactly what I'm trying to avoid through my
> > > > > > > suggestion - one board may have poor thermal dissipation,
> > > > > > > another may have channel losses, yet another one may feature
> > > > > > > a special batch of UFS chips that will set the world on fire
> > > > > > > if instructed to attempt link training at gear 7 - they all
> > > > > > > are causes, as opposed to describing what needs to happen
> > > > > > > (i.e. what the hardware must be treated as - gear N
> > > > > > > incapable despite what can be discovered at runtime), with
> > > > > > > perhaps a comment on the side
> > > > > > >
> > > > > > But the solution for all possible board problems can't be by
> > > > > > limiting Gear
> > > > > speed.
> > > > >
> > > > > Devicetree properties should precisely reflect how they are
> > > > > relevant to the hardware. 'limiting-gear-speed' is
> > > > > self-explanatory that the gear speed is getting limited (for a
> > > > > reason), but the devicetree doesn't need to describe the
> > > > > *reason* itself.
> > > > >
> > > > > > So it should be known why one particular board need to limit the
> gear.
> > > > >
> > > > > That goes into the description, not in the property name.
> > > > >
> > > > > > I understand that this is a static configuration, where it is
> > > > > > already known
> > > > > that board is broken for higher Gear.
> > > > > > Can this be achieved by limiting the clock? If not, can we add
> > > > > > a board
> > > > > specific _quirk_ and let the _quirk_ to be enabled from vendor
> > > > > specific hooks?
> > > > > >
> > > > >
> > > > > How can we limit the clock without limiting the gears? When we
> > > > > limit the gear/mode, both clock and power are implicitly limited.
> > > > >
> > > > Possibly someone need to check with designer of the SoC if that is
> > > > possible
> > > or not.
> > >
> > > It's not just clock. We need to consider reducing regulator,
> > > interconnect votes also. But as I said above, limiting the gear/mode
> > > will take care of all these parameters.
> > >
> > > > Did we already tried _quirk_? If not, why not?
> > > > If the board is so poorly designed and can't take care of the
> > > > channel loses or heat dissipation etc, Then I assumed the gear
> > > > negotiation between host and device should fail for the higher
> > > > gear and driver can have
> > > a re-try logic to re-init / re-try "power mode change" at the lower
> > > gear. Is that not possible / feasible?
> > > >
> > >
> > > I don't see why we need to add extra logic in the UFS driver if we
> > > can extract that information from DT.
> > >
> > You didn’t answer my question entirely, I am still not able to
> > visualised how come Linkup is happening in higher gear and then Suddenly
> it is failing and we need to reduce the gear to solve that?
> 
> Oh well, this is the source of confusion here. I didn't (also the patch) claim
> that the link up will happen with higher speed. It will most likely fail if it
> couldn't operate at the higher speed and that's why we need to limit it to
> lower gear/mode *before* bringing the link up.
> 
Right, that's why a re-try logic to negotiate a __working__ power mode change can help, instead of introducing new binding for this case.
And that approach can be useful for many platforms.
Anyway coming back with the same point again and again is not productive.
I gave my opinion and suggestions. Rest is on the maintainers.

> As you can see, the driver patch is parsing the limits in its
> ufs_hba_variant_ops::init() callback, which gets called during
> ufshcd_hba_init().
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

['Manivannan Sadhasivam']

On Fri, Aug 08, 2025 at 08:38:09PM GMT, Alim Akhtar wrote:
> 
> 
> > -----Original Message-----
> > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > Sent: Friday, August 8, 2025 6:14 PM
> > To: Alim Akhtar <alim.akhtar@samsung.com>
> > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> > Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> > <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> > bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> > conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> > James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> > agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> > scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> > properties to UFS
> > 
> > On Thu, Aug 07, 2025 at 10:08:32PM GMT, Alim Akhtar wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > > > Sent: Wednesday, August 6, 2025 4:56 PM
> > > > To: Alim Akhtar <alim.akhtar@samsung.com>
> > > > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> > > [...]
> > >
> > > > > >
> > > > > > On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > > >> Introducing generic solutions preemptively for problems
> > > > > > > > >> that are simple in concept and can occur widely is good
> > > > > > > > >> practice (although it's sometimes hard to gauge whether
> > > > > > > > >> this is a one-off), as if the issue spreads a generic
> > > > > > > > >> solution will appear at some point, but we'll have to
> > > > > > > > >> keep supporting the odd ones as well
> > > > > > > > >>
> > > > > > > > > Ok,
> > > > > > > > > I would prefer if we add a property which sounds like
> > > > > > > > > "poor thermal dissipation" or "routing channel loss"
> > > > > > > > > rather than adding limiting UFS gear
> > > > > > > > properties.
> > > > > > > > > Poor thermal design or channel losses are generic enough
> > > > > > > > > and can happen
> > > > > > > > on any board.
> > > > > > > >
> > > > > > > > This is exactly what I'm trying to avoid through my
> > > > > > > > suggestion - one board may have poor thermal dissipation,
> > > > > > > > another may have channel losses, yet another one may feature
> > > > > > > > a special batch of UFS chips that will set the world on fire
> > > > > > > > if instructed to attempt link training at gear 7 - they all
> > > > > > > > are causes, as opposed to describing what needs to happen
> > > > > > > > (i.e. what the hardware must be treated as - gear N
> > > > > > > > incapable despite what can be discovered at runtime), with
> > > > > > > > perhaps a comment on the side
> > > > > > > >
> > > > > > > But the solution for all possible board problems can't be by
> > > > > > > limiting Gear
> > > > > > speed.
> > > > > >
> > > > > > Devicetree properties should precisely reflect how they are
> > > > > > relevant to the hardware. 'limiting-gear-speed' is
> > > > > > self-explanatory that the gear speed is getting limited (for a
> > > > > > reason), but the devicetree doesn't need to describe the
> > > > > > *reason* itself.
> > > > > >
> > > > > > > So it should be known why one particular board need to limit the
> > gear.
> > > > > >
> > > > > > That goes into the description, not in the property name.
> > > > > >
> > > > > > > I understand that this is a static configuration, where it is
> > > > > > > already known
> > > > > > that board is broken for higher Gear.
> > > > > > > Can this be achieved by limiting the clock? If not, can we add
> > > > > > > a board
> > > > > > specific _quirk_ and let the _quirk_ to be enabled from vendor
> > > > > > specific hooks?
> > > > > > >
> > > > > >
> > > > > > How can we limit the clock without limiting the gears? When we
> > > > > > limit the gear/mode, both clock and power are implicitly limited.
> > > > > >
> > > > > Possibly someone need to check with designer of the SoC if that is
> > > > > possible
> > > > or not.
> > > >
> > > > It's not just clock. We need to consider reducing regulator,
> > > > interconnect votes also. But as I said above, limiting the gear/mode
> > > > will take care of all these parameters.
> > > >
> > > > > Did we already tried _quirk_? If not, why not?
> > > > > If the board is so poorly designed and can't take care of the
> > > > > channel loses or heat dissipation etc, Then I assumed the gear
> > > > > negotiation between host and device should fail for the higher
> > > > > gear and driver can have
> > > > a re-try logic to re-init / re-try "power mode change" at the lower
> > > > gear. Is that not possible / feasible?
> > > > >
> > > >
> > > > I don't see why we need to add extra logic in the UFS driver if we
> > > > can extract that information from DT.
> > > >
> > > You didn’t answer my question entirely, I am still not able to
> > > visualised how come Linkup is happening in higher gear and then Suddenly
> > it is failing and we need to reduce the gear to solve that?
> > 
> > Oh well, this is the source of confusion here. I didn't (also the patch) claim
> > that the link up will happen with higher speed. It will most likely fail if it
> > couldn't operate at the higher speed and that's why we need to limit it to
> > lower gear/mode *before* bringing the link up.
> > 
> Right, that's why a re-try logic to negotiate a __working__ power mode change can help, instead of introducing new binding for this case.

Retry logic is already in place in the ufshcd core, but with this kind of signal
integrity issue, we cannot guarantee that it will gracefully fail and then we
could retry. The link up *may* succeed, then it could blow up later also (when
doing heavy I/O operations etc...). So with this non-deterministic behavior, we
cannot rely on this logic.

> And that approach can be useful for many platforms.

Other platforms could also reuse the same DT properties to workaround similar
issues.

> Anyway coming back with the same point again and again is not productive.
> I gave my opinion and suggestions. Rest is on the maintainers.

Suggestions are always welcomed. It is important to have comments to try out
different things instead of sticking to the proposed solution. But in my
opinion, the retry logic is not reliable in this case. Moreover, we do have
similar properties for other peripherals like PCIe, MMC, where the vendors would
use DT properties to limit the speed to workaround the board issues. So we are
not doing anything insane here.

If there are better solutions than what is proposed here, we would indeed like
to hear.

- Mani

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

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: 'Manivannan Sadhasivam' <mani@kernel.org>
> Sent: Friday, August 8, 2025 11:37 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> On Fri, Aug 08, 2025 at 08:38:09PM GMT, Alim Akhtar wrote:
> >
> >
> > > -----Original Message-----
> > > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > > Sent: Friday, August 8, 2025 6:14 PM
> > > To: Alim Akhtar <alim.akhtar@samsung.com>
> > > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> > > Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> > > <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> > > bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> > > conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> > > James.Bottomley@hansenpartnership.com;
> martin.petersen@oracle.com;
> > > agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> > > scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org
> > > Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate
> > > limit properties to UFS
> > >
> > > On Thu, Aug 07, 2025 at 10:08:32PM GMT, Alim Akhtar wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: 'Manivannan Sadhasivam' <mani@kernel.org>
> > > > > Sent: Wednesday, August 6, 2025 4:56 PM
> > > > > To: Alim Akhtar <alim.akhtar@samsung.com>
> > > > > Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> > > > [...]
> > > >
> > > > > > >
> > > > > > > On Wed, Aug 06, 2025 at 09:51:43AM GMT, Alim Akhtar wrote:
> > > > > > >
> > > > > > > [...]
> > > > > > >
> > > > > > > > > >> Introducing generic solutions preemptively for
> > > > > > > > > >> problems that are simple in concept and can occur
> > > > > > > > > >> widely is good practice (although it's sometimes hard
> > > > > > > > > >> to gauge whether this is a one-off), as if the issue
> > > > > > > > > >> spreads a generic solution will appear at some point,
> > > > > > > > > >> but we'll have to keep supporting the odd ones as
> > > > > > > > > >> well
> > > > > > > > > >>
> > > > > > > > > > Ok,
> > > > > > > > > > I would prefer if we add a property which sounds like
> > > > > > > > > > "poor thermal dissipation" or "routing channel loss"
> > > > > > > > > > rather than adding limiting UFS gear
> > > > > > > > > properties.
> > > > > > > > > > Poor thermal design or channel losses are generic
> > > > > > > > > > enough and can happen
> > > > > > > > > on any board.
> > > > > > > > >
> > > > > > > > > This is exactly what I'm trying to avoid through my
> > > > > > > > > suggestion - one board may have poor thermal
> > > > > > > > > dissipation, another may have channel losses, yet
> > > > > > > > > another one may feature a special batch of UFS chips
> > > > > > > > > that will set the world on fire if instructed to attempt
> > > > > > > > > link training at gear 7 - they all are causes, as
> > > > > > > > > opposed to describing what needs to happen (i.e. what
> > > > > > > > > the hardware must be treated as - gear N incapable
> > > > > > > > > despite what can be discovered at runtime), with perhaps
> > > > > > > > > a comment on the side
> > > > > > > > >
> > > > > > > > But the solution for all possible board problems can't be
> > > > > > > > by limiting Gear
> > > > > > > speed.
> > > > > > >
> > > > > > > Devicetree properties should precisely reflect how they are
> > > > > > > relevant to the hardware. 'limiting-gear-speed' is
> > > > > > > self-explanatory that the gear speed is getting limited (for
> > > > > > > a reason), but the devicetree doesn't need to describe the
> > > > > > > *reason* itself.
> > > > > > >
> > > > > > > > So it should be known why one particular board need to
> > > > > > > > limit the
> > > gear.
> > > > > > >
> > > > > > > That goes into the description, not in the property name.
> > > > > > >
> > > > > > > > I understand that this is a static configuration, where it
> > > > > > > > is already known
> > > > > > > that board is broken for higher Gear.
> > > > > > > > Can this be achieved by limiting the clock? If not, can we
> > > > > > > > add a board
> > > > > > > specific _quirk_ and let the _quirk_ to be enabled from
> > > > > > > vendor specific hooks?
> > > > > > > >
> > > > > > >
> > > > > > > How can we limit the clock without limiting the gears? When
> > > > > > > we limit the gear/mode, both clock and power are implicitly
> limited.
> > > > > > >
> > > > > > Possibly someone need to check with designer of the SoC if
> > > > > > that is possible
> > > > > or not.
> > > > >
> > > > > It's not just clock. We need to consider reducing regulator,
> > > > > interconnect votes also. But as I said above, limiting the
> > > > > gear/mode will take care of all these parameters.
> > > > >
> > > > > > Did we already tried _quirk_? If not, why not?
> > > > > > If the board is so poorly designed and can't take care of the
> > > > > > channel loses or heat dissipation etc, Then I assumed the gear
> > > > > > negotiation between host and device should fail for the higher
> > > > > > gear and driver can have
> > > > > a re-try logic to re-init / re-try "power mode change" at the
> > > > > lower gear. Is that not possible / feasible?
> > > > > >
> > > > >
> > > > > I don't see why we need to add extra logic in the UFS driver if
> > > > > we can extract that information from DT.
> > > > >
> > > > You didn’t answer my question entirely, I am still not able to
> > > > visualised how come Linkup is happening in higher gear and then
> > > > Suddenly
> > > it is failing and we need to reduce the gear to solve that?
> > >
> > > Oh well, this is the source of confusion here. I didn't (also the
> > > patch) claim that the link up will happen with higher speed. It will
> > > most likely fail if it couldn't operate at the higher speed and
> > > that's why we need to limit it to lower gear/mode *before* bringing the
> link up.
> > >
> > Right, that's why a re-try logic to negotiate a __working__ power mode
> change can help, instead of introducing new binding for this case.
> 
> Retry logic is already in place in the ufshcd core, but with this kind of signal
> integrity issue, we cannot guarantee that it will gracefully fail and then we
> could retry. The link up *may* succeed, then it could blow up later also
> (when doing heavy I/O operations etc...). So with this non-deterministic
> behavior, we cannot rely on this logic.
> 
I would image in that case , PHY tuning / programming is not proper.

> > And that approach can be useful for many platforms.
> 
> Other platforms could also reuse the same DT properties to workaround
> similar issues.
> 
> > Anyway coming back with the same point again and again is not productive.
> > I gave my opinion and suggestions. Rest is on the maintainers.
> 
> Suggestions are always welcomed. It is important to have comments to try
> out different things instead of sticking to the proposed solution. But in my
> opinion, the retry logic is not reliable in this case. Moreover, we do have
> similar properties for other peripherals like PCIe, MMC, where the vendors
> would use DT properties to limit the speed to workaround the board issues.
> So we are not doing anything insane here.
> 
> If there are better solutions than what is proposed here, we would indeed
> like to hear.
> 
For that, more _technical_ things need to be discussed (e.g. Is it the PHY which has problem, or problem is happening at unipro level or somewhere else), 
I didn't saw any technical backing from the patch Author/Submitter
(I assume Author should be knowing a bit more in-depth then what we are assuming and discussing here). 

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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

['Manivannan Sadhasivam']

On Sat, Aug 09, 2025 at 06:30:29AM GMT, Alim Akhtar wrote:

[...]

> > > > > > > > > I understand that this is a static configuration, where it
> > > > > > > > > is already known
> > > > > > > > that board is broken for higher Gear.
> > > > > > > > > Can this be achieved by limiting the clock? If not, can we
> > > > > > > > > add a board
> > > > > > > > specific _quirk_ and let the _quirk_ to be enabled from
> > > > > > > > vendor specific hooks?
> > > > > > > > >
> > > > > > > >
> > > > > > > > How can we limit the clock without limiting the gears? When
> > > > > > > > we limit the gear/mode, both clock and power are implicitly
> > limited.
> > > > > > > >
> > > > > > > Possibly someone need to check with designer of the SoC if
> > > > > > > that is possible
> > > > > > or not.
> > > > > >
> > > > > > It's not just clock. We need to consider reducing regulator,
> > > > > > interconnect votes also. But as I said above, limiting the
> > > > > > gear/mode will take care of all these parameters.
> > > > > >
> > > > > > > Did we already tried _quirk_? If not, why not?
> > > > > > > If the board is so poorly designed and can't take care of the
> > > > > > > channel loses or heat dissipation etc, Then I assumed the gear
> > > > > > > negotiation between host and device should fail for the higher
> > > > > > > gear and driver can have
> > > > > > a re-try logic to re-init / re-try "power mode change" at the
> > > > > > lower gear. Is that not possible / feasible?
> > > > > > >
> > > > > >
> > > > > > I don't see why we need to add extra logic in the UFS driver if
> > > > > > we can extract that information from DT.
> > > > > >
> > > > > You didn’t answer my question entirely, I am still not able to
> > > > > visualised how come Linkup is happening in higher gear and then
> > > > > Suddenly
> > > > it is failing and we need to reduce the gear to solve that?
> > > >
> > > > Oh well, this is the source of confusion here. I didn't (also the
> > > > patch) claim that the link up will happen with higher speed. It will
> > > > most likely fail if it couldn't operate at the higher speed and
> > > > that's why we need to limit it to lower gear/mode *before* bringing the
> > link up.
> > > >
> > > Right, that's why a re-try logic to negotiate a __working__ power mode
> > change can help, instead of introducing new binding for this case.
> > 
> > Retry logic is already in place in the ufshcd core, but with this kind of signal
> > integrity issue, we cannot guarantee that it will gracefully fail and then we
> > could retry. The link up *may* succeed, then it could blow up later also
> > (when doing heavy I/O operations etc...). So with this non-deterministic
> > behavior, we cannot rely on this logic.
> > 
> I would image in that case , PHY tuning / programming is not proper.

I don't have the insight into the PHY tuning to avoid this issue. Maybe Nitin or
Ram can comment here. But PHY tuning is mostly SoC specific in the PHY driver.
We don't have board level tuning sequence AFIAK.

> 
> > > And that approach can be useful for many platforms.
> > 
> > Other platforms could also reuse the same DT properties to workaround
> > similar issues.
> > 
> > > Anyway coming back with the same point again and again is not productive.
> > > I gave my opinion and suggestions. Rest is on the maintainers.
> > 
> > Suggestions are always welcomed. It is important to have comments to try
> > out different things instead of sticking to the proposed solution. But in my
> > opinion, the retry logic is not reliable in this case. Moreover, we do have
> > similar properties for other peripherals like PCIe, MMC, where the vendors
> > would use DT properties to limit the speed to workaround the board issues.
> > So we are not doing anything insane here.
> > 
> > If there are better solutions than what is proposed here, we would indeed
> > like to hear.
> > 
> For that, more _technical_ things need to be discussed (e.g. Is it the PHY which has problem, or problem is happening at unipro level or somewhere else), 
> I didn't saw any technical backing from the patch Author/Submitter
> (I assume Author should be knowing a bit more in-depth then what we are assuming and discussing here). 
> 

Nitin/Ram, please share more details on what level the customer is facing the
issue.

- Mani

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

Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Nitin Rawat]

On 8/9/2025 4:43 PM, 'Manivannan Sadhasivam' wrote:
> On Sat, Aug 09, 2025 at 06:30:29AM GMT, Alim Akhtar wrote:
> 
> [...]
> 
>>>>>>>>>> I understand that this is a static configuration, where it
>>>>>>>>>> is already known
>>>>>>>>> that board is broken for higher Gear.
>>>>>>>>>> Can this be achieved by limiting the clock? If not, can we
>>>>>>>>>> add a board
>>>>>>>>> specific _quirk_ and let the _quirk_ to be enabled from
>>>>>>>>> vendor specific hooks?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> How can we limit the clock without limiting the gears? When
>>>>>>>>> we limit the gear/mode, both clock and power are implicitly
>>> limited.
>>>>>>>>>
>>>>>>>> Possibly someone need to check with designer of the SoC if
>>>>>>>> that is possible
>>>>>>> or not.
>>>>>>>
>>>>>>> It's not just clock. We need to consider reducing regulator,
>>>>>>> interconnect votes also. But as I said above, limiting the
>>>>>>> gear/mode will take care of all these parameters.
>>>>>>>
>>>>>>>> Did we already tried _quirk_? If not, why not?
>>>>>>>> If the board is so poorly designed and can't take care of the
>>>>>>>> channel loses or heat dissipation etc, Then I assumed the gear
>>>>>>>> negotiation between host and device should fail for the higher
>>>>>>>> gear and driver can have
>>>>>>> a re-try logic to re-init / re-try "power mode change" at the
>>>>>>> lower gear. Is that not possible / feasible?
>>>>>>>>
>>>>>>>
>>>>>>> I don't see why we need to add extra logic in the UFS driver if
>>>>>>> we can extract that information from DT.
>>>>>>>
>>>>>> You didn’t answer my question entirely, I am still not able to
>>>>>> visualised how come Linkup is happening in higher gear and then
>>>>>> Suddenly
>>>>> it is failing and we need to reduce the gear to solve that?
>>>>>
>>>>> Oh well, this is the source of confusion here. I didn't (also the
>>>>> patch) claim that the link up will happen with higher speed. It will
>>>>> most likely fail if it couldn't operate at the higher speed and
>>>>> that's why we need to limit it to lower gear/mode *before* bringing the
>>> link up.
>>>>>
>>>> Right, that's why a re-try logic to negotiate a __working__ power mode
>>> change can help, instead of introducing new binding for this case.
>>>
>>> Retry logic is already in place in the ufshcd core, but with this kind of signal
>>> integrity issue, we cannot guarantee that it will gracefully fail and then we
>>> could retry. The link up *may* succeed, then it could blow up later also
>>> (when doing heavy I/O operations etc...). So with this non-deterministic
>>> behavior, we cannot rely on this logic.
>>>
>> I would image in that case , PHY tuning / programming is not proper.
> 
> I don't have the insight into the PHY tuning to avoid this issue. Maybe Nitin or
> Ram can comment here. But PHY tuning is mostly SoC specific in the PHY driver.
> We don't have board level tuning sequence AFIAK.

Hi Alim and Mani,

Here's my take:

There can be multiple reasons for limiting the gear/rate on a customer 
board beyond PHY tuning issues:

1. Board-level signal integrity concerns
2. Channel or reference clock configuration issues
3. Customer board layout not meeting layout design guidelines

This becomes especially critical in automotive platforms like the 
SA8155, as mentioned by Ram. In such safety-critical applications, 
customer prioritize reliability over peak performance, and hence 
customers are generally comfortable operating at lower gears if 
stability is ensured.

For the current case customer had some issue #1 at their end(though 
don't have complete details)

As Mani pointed out, issues are more likely to surface under stress 
conditions rather than during link startup. Therefore, IMHO if any 
limitations are known, it's advisable to restrict the gear/rate during 
initialization to avoid potential problems later.

Moreover, introducing quirks for such cases isn’t very effective, as it 
requires specifying the exact gear/rate to be limited—which can vary 
significantly across different targets.

Regards,
Nitin

> 
>>
>>>> And that approach can be useful for many platforms.
>>>
>>> Other platforms could also reuse the same DT properties to workaround
>>> similar issues.
>>>
>>>> Anyway coming back with the same point again and again is not productive.
>>>> I gave my opinion and suggestions. Rest is on the maintainers.
>>>
>>> Suggestions are always welcomed. It is important to have comments to try
>>> out different things instead of sticking to the proposed solution. But in my
>>> opinion, the retry logic is not reliable in this case. Moreover, we do have
>>> similar properties for other peripherals like PCIe, MMC, where the vendors
>>> would use DT properties to limit the speed to workaround the board issues.
>>> So we are not doing anything insane here.
>>>
>>> If there are better solutions than what is proposed here, we would indeed
>>> like to hear.
>>>
>> For that, more _technical_ things need to be discussed (e.g. Is it the PHY which has problem, or problem is happening at unipro level or somewhere else),
>> I didn't saw any technical backing from the patch Author/Submitter
>> (I assume Author should be knowing a bit more in-depth then what we are assuming and discussing here).
>>
> 
> Nitin/Ram, please share more details on what level the customer is facing the
> issue.
> 
> - Mani
> 

RE: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS

[Alim Akhtar]

> -----Original Message-----
> From: Nitin Rawat <quic_nitirawa@quicinc.com>
> Sent: Tuesday, August 12, 2025 3:16 AM
> To: 'Manivannan Sadhasivam' <mani@kernel.org>; Alim Akhtar
> <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.com>; avri.altman@wdc.com;
> bvanassche@acm.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; andersson@kernel.org; konradybcio@kernel.org;
> James.Bottomley@hansenpartnership.com; martin.petersen@oracle.com;
> agross@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> scsi@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
> properties to UFS
> 
> 
> 
> On 8/9/2025 4:43 PM, 'Manivannan Sadhasivam' wrote:
> > On Sat, Aug 09, 2025 at 06:30:29AM GMT, Alim Akhtar wrote:
> >
> > [...]
> >
> >>>>>>>>>> I understand that this is a static configuration, where it is
> >>>>>>>>>> already known
> >>>>>>>>> that board is broken for higher Gear.
> >>>>>>>>>> Can this be achieved by limiting the clock? If not, can we
> >>>>>>>>>> add a board
> >>>>>>>>> specific _quirk_ and let the _quirk_ to be enabled from vendor
> >>>>>>>>> specific hooks?
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> How can we limit the clock without limiting the gears? When we
> >>>>>>>>> limit the gear/mode, both clock and power are implicitly
> >>> limited.
> >>>>>>>>>
> >>>>>>>> Possibly someone need to check with designer of the SoC if that
> >>>>>>>> is possible
> >>>>>>> or not.
> >>>>>>>
> >>>>>>> It's not just clock. We need to consider reducing regulator,
> >>>>>>> interconnect votes also. But as I said above, limiting the
> >>>>>>> gear/mode will take care of all these parameters.
> >>>>>>>
> >>>>>>>> Did we already tried _quirk_? If not, why not?
> >>>>>>>> If the board is so poorly designed and can't take care of the
> >>>>>>>> channel loses or heat dissipation etc, Then I assumed the gear
> >>>>>>>> negotiation between host and device should fail for the higher
> >>>>>>>> gear and driver can have
> >>>>>>> a re-try logic to re-init / re-try "power mode change" at the
> >>>>>>> lower gear. Is that not possible / feasible?
> >>>>>>>>
> >>>>>>>
> >>>>>>> I don't see why we need to add extra logic in the UFS driver if
> >>>>>>> we can extract that information from DT.
> >>>>>>>
> >>>>>> You didn’t answer my question entirely, I am still not able to
> >>>>>> visualised how come Linkup is happening in higher gear and then
> >>>>>> Suddenly
> >>>>> it is failing and we need to reduce the gear to solve that?
> >>>>>
> >>>>> Oh well, this is the source of confusion here. I didn't (also the
> >>>>> patch) claim that the link up will happen with higher speed. It
> >>>>> will most likely fail if it couldn't operate at the higher speed
> >>>>> and that's why we need to limit it to lower gear/mode *before*
> >>>>> bringing the
> >>> link up.
> >>>>>
> >>>> Right, that's why a re-try logic to negotiate a __working__ power
> >>>> mode
> >>> change can help, instead of introducing new binding for this case.
> >>>
> >>> Retry logic is already in place in the ufshcd core, but with this
> >>> kind of signal integrity issue, we cannot guarantee that it will
> >>> gracefully fail and then we could retry. The link up *may* succeed,
> >>> then it could blow up later also (when doing heavy I/O operations
> >>> etc...). So with this non-deterministic behavior, we cannot rely on this
> logic.
> >>>
> >> I would image in that case , PHY tuning / programming is not proper.
> >
> > I don't have the insight into the PHY tuning to avoid this issue.
> > Maybe Nitin or Ram can comment here. But PHY tuning is mostly SoC
> specific in the PHY driver.
> > We don't have board level tuning sequence AFIAK.
> 
> Hi Alim and Mani,
> 
> Here's my take:
> 
> There can be multiple reasons for limiting the gear/rate on a customer board
> beyond PHY tuning issues:
> 
> 1. Board-level signal integrity concerns 2. Channel or reference clock
> configuration issues 3. Customer board layout not meeting layout design
> guidelines
> 
> This becomes especially critical in automotive platforms like the SA8155, as
> mentioned by Ram. In such safety-critical applications, customer prioritize
> reliability over peak performance, and hence customers are generally
> comfortable operating at lower gears if stability is ensured.
> 
Sorry for delay in reply (lost this email in my inbox), Thanks Nitin for detailed explanations 
Looks like board has too many issues for "safety-critical applications"
Anyway, looks like there is consensus to have this property in, as adopted by PCIe and other subsystem.

[PATCH 3/3] dt-bindings: ufs: qcom: Document HS gear and rate limit properties

[Ram Kumar Dwivedi]

Add documentation for two new optional properties:
  - limit-hs-gear
  - limit-rate

These properties allow platforms to restrict the maximum high-speed
gear and rate used by the UFS controller. This is required for
certain automotive platforms with hardware constraints.

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

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 6c6043d9809e..9dedd09df9e0 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -111,6 +111,16 @@ properties:
     description:
       GPIO connected to the RESET pin of the UFS memory device.
 
+  limit-hs-gear:
+    maxItems: 1
+    description:
+      Limit max phy hs gear
+
+  limit-rate:
+    maxItems: 1
+    description:
+      Limit max phy hs rate
+
 required:
   - compatible
   - reg
-- 
2.50.1

Re: [PATCH 3/3] dt-bindings: ufs: qcom: Document HS gear and rate limit properties

[Rob Herring (Arm)]

On Tue, 22 Jul 2025 21:41:03 +0530, Ram Kumar Dwivedi wrote:
> Add documentation for two new optional properties:
>   - limit-hs-gear
>   - limit-rate
> 
> These properties allow platforms to restrict the maximum high-speed
> gear and rate used by the UFS controller. This is required for
> certain automotive platforms with hardware constraints.
> 
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: limit-hs-gear: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: limit-rate: missing type definition

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250722161103.3938-4-quic_rdwivedi@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 3/3] dt-bindings: ufs: qcom: Document HS gear and rate limit properties

[Dmitry Baryshkov]

On Tue, Jul 22, 2025 at 09:41:03PM +0530, Ram Kumar Dwivedi wrote:
> Add documentation for two new optional properties:
>   - limit-hs-gear
>   - limit-rate
> 
> These properties allow platforms to restrict the maximum high-speed
> gear and rate used by the UFS controller. This is required for
> certain automotive platforms with hardware constraints.

Please reformat other way around: describe the actual problem (which
platforms, which constraints, what breaks, etc). Then describe your
solution.

> 
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> index 6c6043d9809e..9dedd09df9e0 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> @@ -111,6 +111,16 @@ properties:
>      description:
>        GPIO connected to the RESET pin of the UFS memory device.
>  
> +  limit-hs-gear:

If the properties are generic, they should go to the ufs-common.yaml. If
not (but why?), then they should be prefixed with 'qcom,' prefix, as
usual.

> +    maxItems: 1
> +    description:
> +      Limit max phy hs gear
> +
> +  limit-rate:
> +    maxItems: 1
> +    description:
> +      Limit max phy hs rate
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 3/3] dt-bindings: ufs: qcom: Document HS gear and rate limit properties

[Ram Kumar Dwivedi]

On 23-Jul-25 12:32 AM, Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2025 at 09:41:03PM +0530, Ram Kumar Dwivedi wrote:
>> Add documentation for two new optional properties:
>>   - limit-hs-gear
>>   - limit-rate
>>
>> These properties allow platforms to restrict the maximum high-speed
>> gear and rate used by the UFS controller. This is required for
>> certain automotive platforms with hardware constraints.
> 
> Please reformat other way around: describe the actual problem (which
> platforms, which constraints, what breaks, etc). Then describe your
> solution.
> 
Hi Dmitry,

I have addressed this in latest patchset.

Thanks,
Ram.


>>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> index 6c6043d9809e..9dedd09df9e0 100644
>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> @@ -111,6 +111,16 @@ properties:
>>      description:
>>        GPIO connected to the RESET pin of the UFS memory device.
>>  
>> +  limit-hs-gear:
> 
> If the properties are generic, they should go to the ufs-common.yaml. If
> not (but why?), then they should be prefixed with 'qcom,' prefix, as
> usual.

Hi Dmitry,

I have added qcom prefix in latest patchset.

Thanks,
Ram.


> 
>> +    maxItems: 1
>> +    description:
>> +      Limit max phy hs gear
>> +
>> +  limit-rate:
>> +    maxItems: 1
>> +    description:
>> +      Limit max phy hs rate
>> +
>>  required:
>>    - compatible
>>    - reg
>> -- 
>> 2.50.1
>>
> 

Re: [PATCH 3/3] dt-bindings: ufs: qcom: Document HS gear and rate limit properties

[Krzysztof Kozlowski]

On 24/07/2025 09:36, Ram Kumar Dwivedi wrote:
>>>        GPIO connected to the RESET pin of the UFS memory device.
>>>  
>>> +  limit-hs-gear:
>>
>> If the properties are generic, they should go to the ufs-common.yaml. If
>> not (but why?), then they should be prefixed with 'qcom,' prefix, as
>> usual.
> 
> Hi Dmitry,
> 
> I have added qcom prefix in latest patchset.

Unlike this patchset here, are you going to test before sending it?

Best regards,
Krzysztof

Re: [PATCH V1 0/3] Add DT-based gear and rate limiting support

[Manivannan Sadhasivam]

On Tue, Jul 22, 2025 at 09:41:00PM GMT, Ram Kumar Dwivedi wrote:
> This patch series adds support for limiting the maximum high-speed
> gear and rate used by the UFS controller on Qualcomm platforms via
> device tree.
> 
> Some automotive platforms, such as SA8155, require restricting the
> maximum gear or rate due to hardware limitations. To support this,

What do you mean by 'hardware limitation'? Please explain.

- Mani

> the driver is extended to parse two new optional DT properties and
> apply them during initialization. The default behavior remains
> unchanged if these properties are not specified.
> 
> Ram Kumar Dwivedi (3):
>   scsi: ufs: qcom: Add support for DT-based gear and rate limiting
>   arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS
>   dt-bindings: ufs: qcom: Document HS gear and rate limit properties
> 
>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 10 +++++++
>  arch/arm64/boot/dts/qcom/sm8150.dtsi          |  3 ++
>  drivers/ufs/host/ufs-qcom.c                   | 29 +++++++++++++++----
>  3 files changed, 36 insertions(+), 6 deletions(-)
> 
> -- 
> 2.50.1
> 

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