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: '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
> 
> --
> மணிவண்ணன் சதாசிவம்