All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alim Akhtar" <alim.akhtar@samsung.com>
To: "'Nitin Rawat'" <quic_nitirawa@quicinc.com>,
	"'Manivannan Sadhasivam'" <mani@kernel.org>
Cc: "'Konrad Dybcio'" <konrad.dybcio@oss.qualcomm.com>,
	"'Krzysztof Kozlowski'" <krzk@kernel.org>,
	"'Ram Kumar Dwivedi'" <quic_rdwivedi@quicinc.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
Date: Wed, 3 Sep 2025 09:36:31 +0530	[thread overview]
Message-ID: <3a8a01dc1c88$23734880$6a59d980$@samsung.com> (raw)
In-Reply-To: <f5b4580c-4e68-405f-95fb-21fa1b105711@quicinc.com>



> -----Original Message-----
> From: Nitin Rawat <quic_nitirawa@quicinc.com>
> Sent: Tuesday, August 12, 2025 3:16 AM
> To: 'Manivannan Sadhasivam' <mani@kernel.org>; Alim Akhtar
> <alim.akhtar@samsung.com>
> Cc: 'Konrad Dybcio' <konrad.dybcio@oss.qualcomm.com>; 'Krzysztof
> Kozlowski' <krzk@kernel.org>; 'Ram Kumar Dwivedi'
> <quic_rdwivedi@quicinc.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/9/2025 4:43 PM, 'Manivannan Sadhasivam' wrote:
> > 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.
> 
> Hi Alim and Mani,
> 
> Here's my take:
> 
> There can be multiple reasons for limiting the gear/rate on a customer board
> beyond PHY tuning issues:
> 
> 1. Board-level signal integrity concerns 2. Channel or reference clock
> configuration issues 3. Customer board layout not meeting layout design
> guidelines
> 
> This becomes especially critical in automotive platforms like the SA8155, as
> mentioned by Ram. In such safety-critical applications, customer prioritize
> reliability over peak performance, and hence customers are generally
> comfortable operating at lower gears if stability is ensured.
> 
Sorry for delay in reply (lost this email in my inbox), Thanks Nitin for detailed explanations 
Looks like board has too many issues for "safety-critical applications"
Anyway, looks like there is consensus to have this property in, as adopted by PCIe and other subsystem.




  reply	other threads:[~2025-09-03  4:06 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-22 16:11 [PATCH V1 0/3] Add DT-based gear and rate limiting support Ram Kumar Dwivedi
2025-07-22 16:11 ` [PATCH 1/3] ufs: ufs-qcom: Add support for DT-based gear and rate limiting Ram Kumar Dwivedi
2025-07-22 18:54   ` Dmitry Baryshkov
2025-07-24  7:35     ` Ram Kumar Dwivedi
2025-07-24  8:41       ` Dmitry Baryshkov
2025-07-31 16:28         ` Ram Kumar Dwivedi
2025-07-31 17:43           ` Dmitry Baryshkov
2025-07-24  7:49   ` Krzysztof Kozlowski
2025-07-22 16:11 ` [PATCH 2/3] arm64: dts: qcom: sa8155: Add gear and rate limit properties to UFS Ram Kumar Dwivedi
2025-07-22 18:55   ` Dmitry Baryshkov
2025-07-24  7:36     ` Ram Kumar Dwivedi
2025-07-24  7:48   ` Krzysztof Kozlowski
2025-08-01  8:28     ` Manivannan Sadhasivam
2025-08-01  9:04       ` Krzysztof Kozlowski
2025-08-01  9:19         ` Ram Kumar Dwivedi
2025-08-01  9:10       ` Ram Kumar Dwivedi
2025-08-01  9:12         ` Krzysztof Kozlowski
2025-08-01 12:19           ` Manivannan Sadhasivam
2025-08-05 13:16             ` Konrad Dybcio
2025-08-05 16:55               ` Manivannan Sadhasivam
2025-08-05 17:06                 ` Konrad Dybcio
2025-08-05 17:19                   ` Manivannan Sadhasivam
2025-08-05 17:19                   ` Alim Akhtar
2025-08-05 17:22                     ` 'Manivannan Sadhasivam'
2025-08-05 17:36                       ` Alim Akhtar
2025-08-05 17:40                         ` Konrad Dybcio
2025-08-05 17:52                           ` Alim Akhtar
2025-08-05 18:26                             ` Konrad Dybcio
2025-08-06  4:21                               ` Alim Akhtar
2025-08-06  5:05                                 ` 'Manivannan Sadhasivam'
2025-08-06  5:46                                   ` Alim Akhtar
2025-08-06 11:25                                     ` 'Manivannan Sadhasivam'
2025-08-07 16:38                                       ` Alim Akhtar
2025-08-08 12:43                                         ` 'Manivannan Sadhasivam'
2025-08-08 15:08                                           ` Alim Akhtar
2025-08-08 18:06                                             ` 'Manivannan Sadhasivam'
2025-08-09  1:00                                               ` Alim Akhtar
2025-08-09 11:13                                                 ` 'Manivannan Sadhasivam'
2025-08-11 21:45                                                   ` Nitin Rawat
2025-09-03  4:06                                                     ` Alim Akhtar [this message]
2025-07-22 16:11 ` [PATCH 3/3] dt-bindings: ufs: qcom: Document HS gear and rate limit properties Ram Kumar Dwivedi
2025-07-22 18:31   ` Rob Herring (Arm)
2025-07-22 19:02   ` Dmitry Baryshkov
2025-07-24  7:36     ` Ram Kumar Dwivedi
2025-07-24  7:48       ` Krzysztof Kozlowski
2025-07-23 14:41 ` [PATCH V1 0/3] Add DT-based gear and rate limiting support Manivannan Sadhasivam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='3a8a01dc1c88$23734880$6a59d980$@samsung.com' \
    --to=alim.akhtar@samsung.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=agross@kernel.org \
    --cc=andersson@kernel.org \
    --cc=avri.altman@wdc.com \
    --cc=bvanassche@acm.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=quic_nitirawa@quicinc.com \
    --cc=quic_rdwivedi@quicinc.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.