[PATCH V2 0/3] Add support to disable UFS LPM

[PATCH V2 0/3] Add support to disable UFS LPM

[Nitin Rawat]

Add support to disable UFS Low Power Mode (LPM) for platforms
that doesn't support LPM.

Changes in v2:
1. Addressed peter wang's comment to exclude write booster from
   ufs lpm check.
2. Addressed rob herring's comment to remove ufs lpm debug print
   in ufshcd-pltr.c

Nitin Rawat (3):
  scsi: ufs: dt-bindings: Document UFS Disable LPM property
  scsi: ufs: pltfrm: Add parsing support for disable LPM property
  scsi: ufs: qcom: Add support to disable UFS LPM Feature

 .../devicetree/bindings/ufs/ufs-common.yaml         |  5 +++++
 drivers/ufs/host/ufs-qcom.c                         | 13 ++++++++-----
 drivers/ufs/host/ufshcd-pltfrm.c                    | 13 +++++++++++++
 include/ufs/ufshcd.h                                |  1 +
 4 files changed, 27 insertions(+), 5 deletions(-)

--
2.48.1

[PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Nitin Rawat]

Disable UFS low power mode on emulation FPGA platforms or other platforms
where it is either unsupported or power efficiency is not a critical
requirement.

Document the UFS Disable LPM property for such platforms.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 Documentation/devicetree/bindings/ufs/ufs-common.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
index 31fe7f30ff5b..eab28beb0e76 100644
--- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml
+++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
@@ -89,6 +89,11 @@ properties:
 
   msi-parent: true
 
+  disable-lpm:
+    type: boolean
+    description:
+      Disable UFS LPM features.
+
 dependencies:
   freq-table-hz: [ clocks ]
   operating-points-v2: [ clocks, clock-names ]
-- 
2.48.1

[PATCH V2 2/3] scsi: ufs: pltfrm: Add parsing support for disable LPM property

[Nitin Rawat]

There are emulation FPGA platforms or other platforms where UFS low
power mode is either unsupported or power efficiency is not a critical
requirement.

Add support for parsing disable LPM property from device tree . The
disable lpm support in devicetree is added through the "disable-lpm"
property for such platforms.

Disabling LPM ensure stable operation and compatibility with these
environments, where power management features might interfere with
performance or functionality.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufshcd-pltfrm.c | 13 +++++++++++++
 include/ufs/ufshcd.h             |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index ffe5d1d2b215..deed658fedad 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -210,6 +210,17 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
 	}
 }

+/**
+ * ufshcd_parse_lpm_support - read from DT whether LPM modes should be disabled.
+ * @hba: host controller instance
+ */
+static void ufshcd_parse_lpm_support(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;
+
+	hba->disable_lpm = of_property_read_bool(dev->of_node, "disable-lpm");
+}
+
 /**
  * ufshcd_parse_clock_min_max_freq  - Parse MIN and MAX clocks freq
  * @hba: per adapter instance
@@ -495,6 +506,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,

 	ufshcd_init_lanes_per_dir(hba);

+	ufshcd_parse_lpm_support(hba);
+
 	err = ufshcd_parse_operating_points(hba);
 	if (err) {
 		dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index e928ed0265ff..5a3daed1f086 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1143,6 +1143,7 @@ struct ufs_hba {
 	int critical_health_count;
 	atomic_t dev_lvl_exception_count;
 	u64 dev_lvl_exception_id;
+	bool disable_lpm;
 };

 /**
--
2.48.1

[PATCH V2 3/3] scsi: ufs: qcom: Add support to disable UFS LPM Feature

[Nitin Rawat]

There are emulation FPGA platforms or other platforms where UFS low
power mode is either unsupported or power efficiency is not a critical
requirement.

Disable all low power mode UFS feature based on the "disable-lpm" device
tree property parsed in platform driver.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index c0761ccc1381..ad4974c6d08e 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1057,12 +1057,15 @@ static void ufs_qcom_set_host_caps(struct ufs_hba *hba)

 static void ufs_qcom_set_caps(struct ufs_hba *hba)
 {
-	hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
-	hba->caps |= UFSHCD_CAP_CLK_SCALING | UFSHCD_CAP_WB_WITH_CLK_SCALING;
-	hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
+	if (!hba->disable_lpm) {
+		hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
+		hba->caps |= UFSHCD_CAP_CLK_SCALING | UFSHCD_CAP_WB_WITH_CLK_SCALING;
+		hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
+		hba->caps |= UFSHCD_CAP_AGGR_POWER_COLLAPSE;
+		hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;
+	}
+
 	hba->caps |= UFSHCD_CAP_WB_EN;
-	hba->caps |= UFSHCD_CAP_AGGR_POWER_COLLAPSE;
-	hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;

 	ufs_qcom_set_host_caps(hba);
 }
--
2.48.1

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Krzysztof Kozlowski]

On 06/05/2025 18:37, Nitin Rawat wrote:
> Disable UFS low power mode on emulation FPGA platforms or other platforms

Why wouldn't you like to test LPM also on FPGA designs? I do not see
here correlation.

> where it is either unsupported or power efficiency is not a critical
> requirement.

That's a policy, not hardware, thus not suitable for DT.

Best regards,
Krzysztof

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Nitin Rawat]

On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
> On 06/05/2025 18:37, Nitin Rawat wrote:
>> Disable UFS low power mode on emulation FPGA platforms or other platforms
> 
> Why wouldn't you like to test LPM also on FPGA designs? I do not see
> here correlation.

Hi Krzysztof,

Since the FPGA platform doesn't support UFS Low Power Modes (such as the 
AutoHibern8 feature specified in the UFS specification), I have included 
this information in the hardware description (i.e dts).

Thanks,
Nitin

> 
>> where it is either unsupported or power efficiency is not a critical
>> requirement.
> 
> That's a policy, not hardware, thus not suitable for DT.
> 
> Best regards,
> Krzysztof

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Nitin Rawat]

On 5/7/2025 8:34 PM, Nitin Rawat wrote:
> 
> 
> On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
>> On 06/05/2025 18:37, Nitin Rawat wrote:
>>> Disable UFS low power mode on emulation FPGA platforms or other 
>>> platforms
>>
>> Why wouldn't you like to test LPM also on FPGA designs? I do not see
>> here correlation.
> 
> Hi Krzysztof,
> 
> Since the FPGA platform doesn't support UFS Low Power Modes (such as the 
> AutoHibern8 feature specified in the UFS specification), I have included 
> this information in the hardware description (i.e dts).


Hi Krzysztof,

Could you please share your thoughts on my above comment? If you still 
see concerns, I may need to consider other options like modparam.

Regards,
Nitin

> 
> Thanks,
> Nitin
> 
>>
>>> where it is either unsupported or power efficiency is not a critical
>>> requirement.
>>
>> That's a policy, not hardware, thus not suitable for DT.
>>
>> Best regards,
>> Krzysztof
> 
> 

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Pavan Kondeti]

On Mon, May 12, 2025 at 09:45:49AM +0530, Nitin Rawat wrote:
> 
> 
> On 5/7/2025 8:34 PM, Nitin Rawat wrote:
> > 
> > 
> > On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
> > > On 06/05/2025 18:37, Nitin Rawat wrote:
> > > > Disable UFS low power mode on emulation FPGA platforms or other
> > > > platforms
> > > 
> > > Why wouldn't you like to test LPM also on FPGA designs? I do not see
> > > here correlation.
> > 
> > Hi Krzysztof,
> > 
> > Since the FPGA platform doesn't support UFS Low Power Modes (such as the
> > AutoHibern8 feature specified in the UFS specification), I have included
> > this information in the hardware description (i.e dts).
> 
> 
> Hi Krzysztof,
> 
> Could you please share your thoughts on my above comment? If you still see
> concerns, I may need to consider other options like modparam.
> 

I understand why you are inclining towards the module param here. Before
we take that route,

Is it possible to use a different compatible (for ex: qcom,sm8650-emu-ufshc) for UFS controller
on the emulation platform and apply the quirk in the driver based on the device_get_match_data()
based detection?

Thanks,
Pavan

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Dmitry Baryshkov]

On Mon, May 12, 2025 at 01:11:08PM +0530, Pavan Kondeti wrote:
> On Mon, May 12, 2025 at 09:45:49AM +0530, Nitin Rawat wrote:
> > 
> > 
> > On 5/7/2025 8:34 PM, Nitin Rawat wrote:
> > > 
> > > 
> > > On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
> > > > On 06/05/2025 18:37, Nitin Rawat wrote:
> > > > > Disable UFS low power mode on emulation FPGA platforms or other
> > > > > platforms
> > > > 
> > > > Why wouldn't you like to test LPM also on FPGA designs? I do not see
> > > > here correlation.
> > > 
> > > Hi Krzysztof,
> > > 
> > > Since the FPGA platform doesn't support UFS Low Power Modes (such as the
> > > AutoHibern8 feature specified in the UFS specification), I have included
> > > this information in the hardware description (i.e dts).
> > 
> > 
> > Hi Krzysztof,
> > 
> > Could you please share your thoughts on my above comment? If you still see
> > concerns, I may need to consider other options like modparam.
> > 
> 
> I understand why you are inclining towards the module param here. Before
> we take that route,
> 
> Is it possible to use a different compatible (for ex: qcom,sm8650-emu-ufshc) for UFS controller
> on the emulation platform and apply the quirk in the driver based on the device_get_match_data()
> based detection?

Emulation platforms are generally not visible and not supported by the
upstream Linux kernel. During the bringup stage you can apply any kind
of quirks, but I don't think that FPGA or emulation are of concern to
the upstream kernel.

-- 
With best wishes
Dmitry

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Krzysztof Kozlowski]

On Mon, May 12, 2025 at 09:45:49AM GMT, Nitin Rawat wrote:
> 
> 
> On 5/7/2025 8:34 PM, Nitin Rawat wrote:
> > 
> > 
> > On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
> > > On 06/05/2025 18:37, Nitin Rawat wrote:
> > > > Disable UFS low power mode on emulation FPGA platforms or other
> > > > platforms
> > > 
> > > Why wouldn't you like to test LPM also on FPGA designs? I do not see
> > > here correlation.
> > 
> > Hi Krzysztof,
> > 
> > Since the FPGA platform doesn't support UFS Low Power Modes (such as the
> > AutoHibern8 feature specified in the UFS specification), I have included
> > this information in the hardware description (i.e dts).
> 
> 
> Hi Krzysztof,
> 
> Could you please share your thoughts on my above comment? If you still see
> concerns, I may need to consider other options like modparam.

It still looks like policy here. If this is soc specific, then I don't
get why SoC compatible would not be enough?

Best regards,
Krzysztof

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Krzysztof Kozlowski]

On 12/05/2025 09:41, Pavan Kondeti wrote:
> On Mon, May 12, 2025 at 09:45:49AM +0530, Nitin Rawat wrote:
>>
>>
>> On 5/7/2025 8:34 PM, Nitin Rawat wrote:
>>>
>>>
>>> On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
>>>> On 06/05/2025 18:37, Nitin Rawat wrote:
>>>>> Disable UFS low power mode on emulation FPGA platforms or other
>>>>> platforms
>>>>
>>>> Why wouldn't you like to test LPM also on FPGA designs? I do not see
>>>> here correlation.
>>>
>>> Hi Krzysztof,
>>>
>>> Since the FPGA platform doesn't support UFS Low Power Modes (such as the
>>> AutoHibern8 feature specified in the UFS specification), I have included
>>> this information in the hardware description (i.e dts).
>>
>>
>> Hi Krzysztof,
>>
>> Could you please share your thoughts on my above comment? If you still see
>> concerns, I may need to consider other options like modparam.
>>
> 
> I understand why you are inclining towards the module param here. Before
> we take that route,
> 
> Is it possible to use a different compatible (for ex: qcom,sm8650-emu-ufshc) for UFS controller
> on the emulation platform and apply the quirk in the driver based on the device_get_match_data()
> based detection?

I do not get what are the benefits of upstreaming such patches. It feels
like you have some internal product, which will never be released, no
one will ever use it and eventually will be obsolete even internally. We
don't want patches for every broken feature or every broken hardware.

Best regards,
Krzysztof

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Nitin Rawat]

On 5/20/2025 1:39 PM, Krzysztof Kozlowski wrote:
> On 12/05/2025 09:41, Pavan Kondeti wrote:
>> On Mon, May 12, 2025 at 09:45:49AM +0530, Nitin Rawat wrote:
>>>
>>>
>>> On 5/7/2025 8:34 PM, Nitin Rawat wrote:
>>>>
>>>>
>>>> On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
>>>>> On 06/05/2025 18:37, Nitin Rawat wrote:
>>>>>> Disable UFS low power mode on emulation FPGA platforms or other
>>>>>> platforms
>>>>>
>>>>> Why wouldn't you like to test LPM also on FPGA designs? I do not see
>>>>> here correlation.
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Since the FPGA platform doesn't support UFS Low Power Modes (such as the
>>>> AutoHibern8 feature specified in the UFS specification), I have included
>>>> this information in the hardware description (i.e dts).
>>>
>>>
>>> Hi Krzysztof,
>>>
>>> Could you please share your thoughts on my above comment? If you still see
>>> concerns, I may need to consider other options like modparam.
>>>
>>
>> I understand why you are inclining towards the module param here. Before
>> we take that route,
>>
>> Is it possible to use a different compatible (for ex: qcom,sm8650-emu-ufshc) for UFS controller
>> on the emulation platform and apply the quirk in the driver based on the device_get_match_data()
>> based detection?
> 
> I do not get what are the benefits of upstreaming such patches. It feels
> like you have some internal product, which will never be released, no
> one will ever use it and eventually will be obsolete even internally. We
> don't want patches for every broken feature or every broken hardware.

Hi Krzysztof,

Thank you for your review and opinions. I would like to clarify that 
this is a platform requirement rather than a broken feature. 
Additionally, there are few automotive targets, in addition to the FPGA 
platform, where Low Power Mode (LPM) is not a requirement. For these 
platforms, the LPM disable changes are currently maintained downstream.

My apology for not including the automotive requirements in my previous 
commit messages.

In my opinion, since these platforms do not support LPM, I requested 
that this be reflected in the hardware description (i.e. DTS)). However, 
I am open to suggestions and am willing to proceed with module 
parameters if you have concerns regarding the device tree.


Regards,
Nitin


> 
> Best regards,
> Krzysztof
> 

Re: [PATCH V2 1/3] scsi: ufs: dt-bindings: Document UFS Disable LPM property

[Krzysztof Kozlowski]

On 20/05/2025 20:49, Nitin Rawat wrote:
> 
> 
> On 5/20/2025 1:39 PM, Krzysztof Kozlowski wrote:
>> On 12/05/2025 09:41, Pavan Kondeti wrote:
>>> On Mon, May 12, 2025 at 09:45:49AM +0530, Nitin Rawat wrote:
>>>>
>>>>
>>>> On 5/7/2025 8:34 PM, Nitin Rawat wrote:
>>>>>
>>>>>
>>>>> On 5/6/2025 11:46 PM, Krzysztof Kozlowski wrote:
>>>>>> On 06/05/2025 18:37, Nitin Rawat wrote:
>>>>>>> Disable UFS low power mode on emulation FPGA platforms or other
>>>>>>> platforms
>>>>>>
>>>>>> Why wouldn't you like to test LPM also on FPGA designs? I do not see
>>>>>> here correlation.
>>>>>
>>>>> Hi Krzysztof,
>>>>>
>>>>> Since the FPGA platform doesn't support UFS Low Power Modes (such as the
>>>>> AutoHibern8 feature specified in the UFS specification), I have included
>>>>> this information in the hardware description (i.e dts).
>>>>
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Could you please share your thoughts on my above comment? If you still see
>>>> concerns, I may need to consider other options like modparam.
>>>>
>>>
>>> I understand why you are inclining towards the module param here. Before
>>> we take that route,
>>>
>>> Is it possible to use a different compatible (for ex: qcom,sm8650-emu-ufshc) for UFS controller
>>> on the emulation platform and apply the quirk in the driver based on the device_get_match_data()
>>> based detection?
>>
>> I do not get what are the benefits of upstreaming such patches. It feels
>> like you have some internal product, which will never be released, no
>> one will ever use it and eventually will be obsolete even internally. We
>> don't want patches for every broken feature or every broken hardware.
> 
> Hi Krzysztof,
> 
> Thank you for your review and opinions. I would like to clarify that 
> this is a platform requirement rather than a broken feature. 
> Additionally, there are few automotive targets, in addition to the FPGA 
> platform, where Low Power Mode (LPM) is not a requirement. For these 
> platforms, the LPM disable changes are currently maintained downstream.

And these platforms do not have their own SoC compatible? When you say
platforms, you mean SoC or boards? So many options... all this is so
unspecific, I need to dig every answer, every specific bit.

> 
> My apology for not including the automotive requirements in my previous 
> commit messages.
> 
> In my opinion, since these platforms do not support LPM, I requested 
> that this be reflected in the hardware description (i.e. DTS)). However, 
> I am open to suggestions and am willing to proceed with module 
> parameters if you have concerns regarding the device tree.

Module param will get other obstacles... We have lengthy discussion
because your commit msg explains nothing. Even now I still don't know
what do you mean by half of above statements. Use the wide upstream
terms, instead of ambiguous like "automotive platform".

Best regards,
Krzysztof