Re: [PATCH V8 7/8] devfreq: mediatek: cci devfreq register opp notification for SVS support

[andrew-sh.cheng <andrew-sh.cheng@mediatek.com>]

On Thu, 2021-03-25 at 17:11 +0900, Chanwoo Choi wrote:
> Hi,
> 
> I think that you can squash this patch to patch4.

> On 3/23/21 8:34 PM, Andrew-sh.Cheng wrote:
> > From: "Andrew-sh.Cheng" <andrew-sh.cheng@mediatek.com>
> > 
> > SVS will change the voltage of opp item.
> 
> What it the full name of SVS?

Due to the content of this patch is for SVS, so I separate it from
patch4.
SVS is Smart-Voltage-Scaling.
It will check the IC quality, and then modify the voltage field of opp
table.
The required voltage will smaller than original signed-off voltage in
opp table.
This voltage will change when temperature is changed.
cci devfreq need to raise voltage when the required voltage raise.

> 
> > CCI devfreq need to react to change frequency.
> > 
> > Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> > ---
> >  drivers/devfreq/mt8183-cci-devfreq.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
> > index 018543db7bae..6942a48f3f4f 100644
> > --- a/drivers/devfreq/mt8183-cci-devfreq.c
> > +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> > @@ -21,6 +21,7 @@ struct cci_devfreq {
> >  	struct clk *cci_clk;
> >  	int old_vproc;
> >  	unsigned long old_freq;
> > +	struct notifier_block opp_nb;
> >  };
> >  
> >  static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
> > @@ -89,6 +90,26 @@ static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
> >  	return 0;
> >  }
> >  
> > +static int ccidevfreq_opp_notifier(struct notifier_block *nb,
> 
> I think that you better to change the function name as following:
> ccidevfreq_opp_notifier -> mtk_cci_devfreq_opp_notifier
I will change it on next patch

> 
> > +				   unsigned long event, void *data)
> > +{
> > +	struct dev_pm_opp *opp = data;
> > +	struct cci_devfreq *cci_df = container_of(nb, struct cci_devfreq,
> > +						  opp_nb);
> > +	unsigned long	freq, volt;
> > +
> > +	if (event == OPP_EVENT_ADJUST_VOLTAGE) {
> > +		freq = dev_pm_opp_get_freq(opp);
> > +		/* current opp item is changed */
> > +		if (freq == cci_df->old_freq) {
> > +			volt = dev_pm_opp_get_voltage(opp);
> > +			mtk_cci_set_voltage(cci_df, volt);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static struct devfreq_dev_profile cci_devfreq_profile = {
> >  	.target = mtk_cci_devfreq_target,
> >  };
> > @@ -98,12 +119,15 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> >  	struct device *cci_dev = &pdev->dev;
> >  	struct cci_devfreq *cci_df;
> >  	struct devfreq_passive_data *passive_data;
> > +	struct notifier_block *opp_nb;
> >  	int ret;
> >  
> >  	cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> >  	if (!cci_df)
> >  		return -ENOMEM;
> >  
> > +	opp_nb = &cci_df->opp_nb;
> 
> Just move this code at the neighborhood of 'opp_nb->notifier_call' init code.
I will modify it on next patch

> 
> > +
> >  	cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
> >  	ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
> >  	if (ret) {
> > @@ -152,6 +176,9 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> >  		goto err_opp;
> >  	}
> >  
> > +	opp_nb->notifier_call = ccidevfreq_opp_notifier;
> > +	dev_pm_opp_register_notifier(cci_dev, opp_nb);
> 
> Need to check whether return value is valid or not.
I will add the check for next patch


> 
> > +
> >  	return 0;
> >  
> >  err_opp:
> > 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel