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

[Hsin-Yi Wang]

On Thu, May 16, 2019 at 9:08 AM Andrew-sh.Cheng
<andrew-sh.cheng@xxxxxxxxxxxx> wrote:
>
> From: "Andrew-sh.Cheng" <andrew-sh.cheng@xxxxxxxxxxxx>
>
> SVS will change the voltage of opp item.
> CCI devfreq need to react to change frequency.
>
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@xxxxxxxxxxxx>
> ---
>  drivers/devfreq/mt8183-cci-devfreq.c | 63 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
> index 818a167c442f..250c963789f3 100644
> --- a/drivers/devfreq/mt8183-cci-devfreq.c
> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> @@ -19,7 +19,10 @@ struct cci_devfreq {
>         struct regulator *proc_reg;
>         unsigned long proc_reg_uV;
>         struct clk *cci_clk;
> +       unsigned long freq;
>         struct notifier_block nb;
> +       struct notifier_block opp_nb;
> +       int cci_min_freq;
>  };
>
>  static int cci_devfreq_regulator_notifier(struct notifier_block *nb,
> @@ -65,17 +68,62 @@ static int cci_devfreq_regulator_notifier(struct notifier_block *nb,
>         return 0;
>  }
>
> +static int ccidevfreq_opp_notifier(struct notifier_block *nb,
> +unsigned long event, void *data)
> +{
> +       int ret;
> +       struct dev_pm_opp *opp = data;
> +       struct cci_devfreq *cci_df = container_of(nb, struct cci_devfreq,
> +                                                 opp_nb);
> +       unsigned long   freq, volt, cur_volt;
> +
> +       if (event == OPP_EVENT_ADJUST_VOLTAGE) {
> +               freq = dev_pm_opp_get_freq(opp);
> +               /* current opp item is changed */
> +               if (freq == cci_df->freq) {
> +                       volt = dev_pm_opp_get_voltage(opp);
> +                       cur_volt = regulator_get_voltage(cci_df->proc_reg);
> +
> +                       if (volt > cur_volt) {
> +                               /* need reduce freq */
> +                               mutex_lock(&cci_df->devfreq->lock);
> +                               ret = update_devfreq(cci_df->devfreq);
> +                               if (ret)
> +                                       pr_err("Fail to reduce cci frequency by opp notification: %d\n",
This line is too long
> +                                              ret);
> +                               mutex_unlock(&cci_df->devfreq->lock);
> +                       }
> +               }
> +
> +               if (freq == cci_df->cci_min_freq) {
> +                       volt = dev_pm_opp_get_voltage(opp);
> +                       regulator_set_voltage(cci_df->proc_reg, volt, INT_MAX);
> +               }
> +       } else if (event == OPP_EVENT_DISABLE) {
Is this case handled? If not, is this branch needed?
> +       }
> +
> +       return 0;
> +}
> +
> +
>  static int mtk_cci_governor_get_target(struct devfreq *devfreq,
>                                        unsigned long *freq)
>  {
>         struct cci_devfreq *cci_df;
>         struct dev_pm_opp *opp;
> +       int ret;
>
>         cci_df = dev_get_drvdata(devfreq->dev.parent);
>
>         /* find available frequency */
>         opp = dev_pm_opp_find_freq_ceil_by_volt(devfreq->dev.parent,
>                                                 cci_df->proc_reg_uV);
> +       ret = PTR_ERR_OR_ZERO(opp);
> +       if (ret) {
> +               pr_err("%s[%d], cannot find opp with voltage=%d: %d\n",
> +                      __func__, __LINE__, cci_df->proc_reg_uV, ret);
> +               return ret;
> +       }
>         *freq = dev_pm_opp_get_freq(opp);
>
>         return 0;
> @@ -87,9 +135,11 @@ static int mtk_cci_governor_event_handler(struct devfreq *devfreq,
>         int ret;
>         struct cci_devfreq *cci_df;
>         struct notifier_block *nb;
> +       struct notifier_block *opp_nb;
>
>         cci_df = dev_get_drvdata(devfreq->dev.parent);
>         nb = &cci_df->nb;
> +       opp_nb = &cci_df->opp_nb;
>
>         switch (event) {
>         case DEVFREQ_GOV_START:
> @@ -100,6 +150,8 @@ static int mtk_cci_governor_event_handler(struct devfreq *devfreq,
>                 if (ret)
>                         pr_err("%s: failed to add governor: %d\n", __func__,
>                                ret);
> +               opp_nb->notifier_call = ccidevfreq_opp_notifier;
> +               dev_pm_opp_register_notifier(devfreq->dev.parent, opp_nb);
>                 break;
>
>         case DEVFREQ_GOV_STOP:
> @@ -141,6 +193,8 @@ static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
>                 return ret;
>         }
>
> +       cci_df->freq = *freq;
> +
>         return 0;
>  }
>
> @@ -152,6 +206,8 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
>  {
>         struct device *cci_dev = &pdev->dev;
>         struct cci_devfreq *cci_df;
> +       unsigned long freq, volt;
> +       struct dev_pm_opp *opp;
>         int ret;
>
>         cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> @@ -181,6 +237,13 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       /* set voltage lower bound */
> +       freq = 1;
> +       opp = dev_pm_opp_find_freq_ceil(cci_dev, &freq);
> +       cci_df->cci_min_freq = dev_pm_opp_get_freq(opp);
> +       volt = dev_pm_opp_get_voltage(opp);
> +       dev_pm_opp_put(opp);
> +
>         platform_set_drvdata(pdev, cci_df);
>
>         cci_df->devfreq = devm_devfreq_add_device(cci_dev,