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

["Alim Akhtar" <alim.akhtar@samsung.com>]

> -----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?