Re: [PATCH 2/3] devfreq: mediatek: add mt8183 cci devfreq driver

[Johnson Wang]

On Thu, 2022-04-07 at 14:52 -0700, Kevin Hilman wrote:
> Hsin-Yi Wang <hsinyi@xxxxxxxxxx> writes:
> 
> > On Mon, Mar 7, 2022 at 8:32 PM Tim Chang <
> > jia-wei.chang@xxxxxxxxxxxx> wrote:
> > > 
> > > This adds a devfreq driver for the Cache Coherent Interconnect
> > > (CCI) of
> > > the Mediatek MT8183.
> > > 
> > > On mt8183 the cci is supplied by the same regulator as the little
> > > cores.
> > > The driver is notified when the regulator voltage changes (driven
> > > by
> > > cpufreq) and adjusts the cci frequency to the maximum possible
> > > value.
> > > 
> > > Add need_voltage_tracking variable to platforma data. if true, it
> > > indicates soc is required to realize the voltage tracking between
> > > voltage of sram and voltage of cci by software approach.
> > > otherwise, the
> > > voltage tracking is realized by hardware appraoch.
> > > 
> > > Add the notifier to cci so that it could react after svs driver
> > > changes
> > > opp table of cci.
> > > 
> > > Signed-off-by: Jia-Wei Chang <
> > > jia-wei.chang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> 
> [...]
> 
> > > +
> > > +       passive_data = devm_kzalloc(cci_dev, sizeof(struct
> > > devfreq_passive_data), GFP_KERNEL);
> > > +       if (!passive_data) {
> > > +               ret = -ENOMEM;
> > > +               goto out_remove_opp_table;
> > > +       }
> > > +
> > > +       passive_data->parent_type = CPUFREQ_PARENT_DEV;
> > 
> > It's better to add a note below commit message to state that this
> > series depends on
> > 
https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!3dQn_JiiD_cRiN-TmoNWXX7mD2cmF4M5elG4WabiS392Mg9S8wlsagK1o-n9beULMei4AAj9zsoPBElt9R4-UnAd1g$
> >  
> 
> Not only is this series dependent the previous series from Chanwoo,
> in
> also fails to compile if CONFIG_DEVFREQ_GOV_PASSIVE is not enabled,
> because CPUFREQ_PARENT_DEV defined inside of an #ifdef.
> 
> Please compile test this with and without CONFIG_DEVFREQ_GOV_PASSIVE
> enabled.
> 
> Kevin

Hi Kevin,

Thank you for review.
I will fix it in next verison.

BRs,
Johnson Wang