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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b235e338-8c16-439b-b7a5-24856893fb5d@oss.qualcomm.com>
Date: Tue, 5 Aug 2025 19:06:29 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>,
        Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>, alim.akhtar@...sung.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 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