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.1On 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
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
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
>>
>
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
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
>>>>
>>>
>>
>
>
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
© 2016 - 2025 Red Hat, Inc.