Message-ID: <3cd33dce-f6b9-4f60-8cb2-a3bf2942a1e5@oss.qualcomm.com>
Date: Tue, 5 Aug 2025 20:26:57 +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:52 PM, Alim Akhtar wrote:
> 
> 
>> -----Original Message-----
>> From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
>> Sent: Tuesday, August 5, 2025 11:10 PM
>> 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
>>
> 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

Powered by blists - more mailing lists