Thread (46 messages) 46 messages, 8 authors, 2025-09-03

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

From: Konrad Dybcio <hidden>
Date: 2025-08-05 18:27:03
Also in: linux-arm-msm, linux-scsi, lkml

On 8/5/25 7:52 PM, Alim Akhtar wrote:
quoted
-----Original Message-----
From: Konrad Dybcio <redacted>
Sent: Tuesday, August 5, 2025 11:10 PM
To: Alim Akhtar <alim.akhtar@samsung.com>; 'Manivannan Sadhasivam'
[off-list ref]
Cc: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
[off-list ref]; 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:
quoted
quoted
-----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' <redacted>; 'Krzysztof
Kozlowski' [off-list ref]; 'Ram Kumar Dwivedi'
[off-list ref]; avri.altman@wdc.com;
bvanassche@acm.org;
quoted
quoted
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;
quoted
quoted
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:
quoted
quoted
-----Original Message-----
From: Konrad Dybcio <redacted>
Sent: Tuesday, August 5, 2025 10:36 PM
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>; Ram Kumar Dwivedi
[off-list ref]; 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:
quoted
On Tue, Aug 05, 2025 at 03:16:33PM GMT, Konrad Dybcio wrote:
quoted
On 8/1/25 2:19 PM, Manivannan Sadhasivam wrote:
quoted
On Fri, Aug 01, 2025 at 11:12:42AM GMT, Krzysztof Kozlowski
wrote:
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
On 01/08/2025 11:10, Ram Kumar Dwivedi wrote:
quoted

On 01-Aug-25 1:58 PM, Manivannan Sadhasivam wrote:
quoted
On Thu, Jul 24, 2025 at 09:48:53AM GMT, Krzysztof Kozlowski
wrote:
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
On 22/07/2025 18:11, Ram Kumar Dwivedi wrote:
quoted
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
quoted
quoted
constraints.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
What hardware constraints? This needs to be clearly
documented.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
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.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
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.
quoted
quoted
quoted
quoted
quoted
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.
quoted
quoted
quoted
quoted
quoted
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.
quoted
quoted
quoted
quoted
I had an offline chat with Ram and got clarified on what these
properties
are.
quoted
quoted
quoted
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).
quoted
quoted
quoted
quoted
quoted
But due to board constraints like poor thermal dissipation,
routing loss, the board cannot efficiently operate at the higher
speeds.
quoted
quoted
quoted
quoted
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help