| Message ID | 20250506163705.31518-2-quic_nitirawa@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support to disable UFS LPM | expand |
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
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
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 > >
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
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.
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
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
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 >
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
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 ]
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(+)