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

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 11:13:02AM +0530, Viresh Kumar wrote:
> 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.
> 

That's the pre-opp behavior as well. Reason is, most of the platforms have only
gate clocks supplied to the ufs controller and cannot change the frequency. Only
Qcom requires changing the frequency of _some_ clocks, so that's why we have to
use this hack of skipping 0 freq clocks.

> 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).
> 

Yeah but that seems overkill since the current config_clks helper satisfies the
requirement.

- Mani

> > > 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

-- 
மணிவண்ணன் சதாசிவம்