Re: [PATCH 11/14] scsi: ufs: host: Add support for parsing OPP

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	vireshk@kernel.org, nm@ti.com, sboyd@kernel.org,
	myungjoo.ham@samsung.com, kyungmin.park@samsung.com,
	cw00.choi@samsung.com, andersson@kernel.org,
	konrad.dybcio@linaro.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	quic_asutoshd@quicinc.com, quic_cang@quicinc.com,
	quic_nitirawa@quicinc.com, quic_narepall@quicinc.com,
	quic_bhaskarv@quicinc.com, quic_richardp@quicinc.com,
	quic_nguyenb@quicinc.com, quic_ziqichen@quicinc.com,
	bmasney@redhat.com, krzysztof.kozlowski@linaro.org
Subject: Re: [PATCH 11/14] scsi: ufs: host: Add support for parsing OPP
Date: Thu, 13 Jul 2023 11:13:02 +0530	[thread overview]
Message-ID: <20230713054302.tu6fgd3meb5krsx5@vireshk-i7> (raw)
In-Reply-To: <20230713052843.GE3047@thinkpad>

Okay, sorry about missing one point first. I thought we are adding the
clk config callback (which neglects 0 frequencies) to a Qcom only
driver and so was okay-ish with that. But now that I realize that this
is a generic driver instead (my mistake here), I wonder if it is the
right thing to do anymore.

On 13-07-23, 10:58, Manivannan Sadhasivam wrote:
> On Thu, Jul 13, 2023 at 10:42:35AM +0530, Viresh Kumar wrote:
> > On 13-07-23, 10:35, Manivannan Sadhasivam wrote:
> > > We can settle with this custom callback for now. If there are drivers in the
> > > future trying to do the same (skipping 0 freq) then we can generalize.
> > 
> > Just for completeness, there isn't much to generalize here apart from
> > changing the DT order of clocks. Isn't it ?
> > 
> 
> Even with changing the order, driver has to know the "interesting" clocks
> beforehand. But that varies between platforms (this is a generic driver for
> ufshc platforms).
> 
> And I do not know if clocks have any dependency between them, atleast not in
> Qcom platforms. But not sure about others.

Maybe this requires some sort of callback, per-platform, which gets
you these details or the struct dev_pm_opp_config itself (so platforms
can choose the callback too, in case order is important).

> > The change require for the OPP core makes sense, I will probably just
> > push it anyway.

I tried to look at this code and I think it is doing the right thing
currently, i.e. it matches clk-count with the number of frequencies in
opp-hz, which should turn out to be the same in your case. So nothing
to change here I guess.

-- 
viresh