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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <o2lnzaxurshoyyxtdcyiyphprumisggd6m2qvcoeptvnkvh4ap@dm2nc4krinja>
Date: Sat, 9 Aug 2025 16:43:35 +0530
From: 'Manivannan Sadhasivam' <mani@...nel.org>
To: Alim Akhtar <alim.akhtar@...sung.com>
Cc: 'Konrad Dybcio' <konrad.dybcio@....qualcomm.com>, 
	'Krzysztof Kozlowski' <krzk@...nel.org>, 'Ram Kumar Dwivedi' <quic_rdwivedi@...cinc.com>, 
	avri.altman@....com, bvanassche@....org, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, andersson@...nel.org, konradybcio@...nel.org, 
	James.Bottomley@...senpartnership.com, martin.petersen@...cle.com, agross@...nel.org, 
	linux-arm-msm@...r.kernel.org, linux-scsi@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit
 properties to UFS

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

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