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

'Manivannan Sadhasivam'
2025-08-06T11:25:39Z

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

--
மணிவண்ணன் சதாசிவம்