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.1On 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
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 -- மணிவண்ணன் சதாசிவம்
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
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
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 >
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
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 -- மணிவண்ணன் சதாசிவம்
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
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 -- மணிவண்ணன் சதாசிவம்
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
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 -- மணிவண்ணன் சதாசிவம்
> -----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?
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 -- மணிவண்ணன் சதாசிவம்
> -----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 > > -- > மணிவண்ணன் சதாசிவம்
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
> -----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
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
> -----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
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 -- மணிவண்ணன் சதாசிவம்
> -----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 > > -- > மணிவண்ணன் சதாசிவம்
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 -- மணிவண்ணன் சதாசிவம்
> -----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 > > -- > மணிவண்ணன் சதாசிவம்
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 -- மணிவண்ணன் சதாசிவம்
> -----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 > > -- > மணிவண்ணன் சதாசிவம்
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 -- மணிவண்ணன் சதாசிவம்
> -----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 > > -- > மணிவண்ணன் சதாசிவம்
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 -- மணிவண்ணன் சதாசிவம்
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 >
> -----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.
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
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
>>
>
© 2016 - 2025 Red Hat, Inc.