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: <87c37d65-5ab1-4443-a428-dc3592062cdc@oss.qualcomm.com>
Date: Tue, 5 Aug 2025 19:40:29 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Alim Akhtar <alim.akhtar@...sung.com>,
        'Manivannan Sadhasivam' <mani@...nel.org>
Cc: '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 8/5/25 7:36 PM, Alim Akhtar wrote:
> 
> 
>> -----Original Message-----
>> From: 'Manivannan Sadhasivam' <mani@...nel.org>
>> Sent: Tuesday, August 5, 2025 10:52 PM
>> 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 Tue, Aug 05, 2025 at 10:49:45PM GMT, Alim Akhtar wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
>>>> Sent: Tuesday, August 5, 2025 10:36 PM
>>>> 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
>>> 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

Konrad