RE: [RFC PATCH 0/3] Add support of busfreq

[Anson Huang]

Hi, Alexandre

> -----Original Message-----
> From: Leonard Crestez
> Sent: Wednesday, May 15, 2019 3:34 AM
> To: Alexandre Bailon <abailon@xxxxxxxxxxxx>; Jacky Bai <ping.bai@xxxxxxx>
> Cc: Michael Turquette <mturquette@xxxxxxxxxxxx>; Linux PM list <linux-
> pm@xxxxxxxxxxxxxxx>; Georgi Djakov <georgi.djakov@xxxxxxxxxx>; Patrick
> Titiano <ptitiano@xxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
> kernel@xxxxxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxxxxxx>; Emilio
> Lopez <emilio@xxxxxxxxxxxxx>; Hans de Goede <hdegoede@xxxxxxxxxx>;
> linux-clk <linux-clk@xxxxxxxxxxxxxxx>; linux-arm-kernel <linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx>; Zening Wang <zening.wang@xxxxxxx>;
> Aisheng Dong <aisheng.dong@xxxxxxx>; Kevin Hilman
> <khilman@xxxxxxxxxxxx>; Carlo Caione <ccaione@xxxxxxxxxxxx>; dl-linux-
> imx <linux-imx@xxxxxxx>; Anson Huang <anson.huang@xxxxxxx>; Viresh
> Kumar <viresh.kumar@xxxxxxxxxx>
> Subject: Re: [RFC PATCH 0/3] Add support of busfreq
> 
> On 15.03.2019 18:55, Alexandre Bailon wrote:
> >> On Wed, Mar 13, 2019 at 12:33 PM Alexandre Bailon
> <abailon@xxxxxxxxxxxx> wrote:
> 
> >>> As exemple, this series implements busfreq for i.MX8MM whose
> >>> upstreaming is in progress. Because this relies on ATF to do the
> >>> frequency scaling, it won't be hard make it work.

I have similar question as previous reviewer, is there any branch that we can test
this series? 

And, from the patch, it has multiple levels description of fabric arch, while we ONLY
intend to scale "bus" frequency per devices' request, here "bus" includes DRAM, NOC and
AHB, AXI, should we make it more flatter, such as just a virtual fabric as a single provider, and then
all other devices as nodes under this provider?

Anson

> >>
> >> It's not clear to me whether this series actual scales the dram
> >> frequency based on what you said above. Is it just theoretical or do
> >> you have it working with a pile of out-of-tree patches? Would be good
> >> to include that pile of patches in your integration branch that I
> >> suggested above.
> 
> > The current series only introduce busfreq generic driver, and the
> > busfreq driver for the imx8mm.
> > As is, the imx8mm driver will just be loaded, but do nothing because
> > none of the drivers have been updated to request bandwidth using the
> > interconnect framework.
> >
> > My intent was to sent a first draft o busfreq, to get some feedback,
> > before to send a more complete, and fully functional series.
> 
> It's been a while since this was first posted and imx8mm now boots fine in
> linux-next. Is there a more up-to-date WIP branch somewhere?
> Otherwise I can try to hack this series into a bootable form.
> 
>  > In addition, the current clock driver of imx8mm doesn't allow dram  >
> frequency scaling, so if busfreq driver tries, it will fail (should be  > harmless
> because any other clocks should restored to their previous  > rate).
> 
> I'm confused about this. In NXP tree the actual DRAM switch is done inside
> ATF via SIP calls and involves corralling all CPUs. Do you want an "dram" clk
> which wraps the SIP calls required to changing dram frequency and root
> switching etc?
> 
> I've been looking at the busfreq implementation in the NXP tree and
> refactoring just the "dram freq switch" behind a clk might work nicely.
> 
> This would be similar to the imx_cpu clk used for cpufreq-dt and it might
> even be possible to upstream this separately from the rest of busfreq logic
> dealing with device requests.
> 
> 
> I haven't done a very careful review but I noticed you're not using the OPP
> framework and instead redefined everything? It's not clear why.
> 
> --
> Regards,
> Leonard