Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220718122419.9409-1-di.shen@unisoc.com> <20220719041448.iyavinsv3jzs3au4@vireshk-i7>
In-Reply-To: <20220719041448.iyavinsv3jzs3au4@vireshk-i7>
From:   Di Shen <cindygm567@gmail.com>
Date:   Tue, 19 Jul 2022 17:24:59 +0800
Message-ID: <CAHYJL4r4A5k=ztd+PavibF1xcMCiLdaCqZK6B0PwayaHH2210Q@mail.gmail.com>
Subject: Re: [PATCH] thermal: cpufreq_cooling: Avoid all cluster using global cooling_ops
To:     Viresh Kumar <viresh.kumar@linaro.org>
Cc:     Di Shen <di.shen@unisoc.com>, Lukasz Luba <lukasz.luba@arm.com>,
        Amit Kucheria <amitk@kernel.org>,
        "Zhang, Rui" <rui.zhang@intel.com>, amit.kachhap@gmail.com,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        xuewen.yan@unisoc.com, xuewen.yan94@gmail.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Yes, it fixes this problem! Thank you:)

BR
Di

On Tue, Jul 19, 2022 at 12:30 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 18-07-22, 20:24, Di Shen wrote:
> > Now, all the cooling device use the globle cpufreq_cooling_ops. When the
> > CONFIG_THERMAL_GOV_POWER_ALLOCATOR is enabled, once one cluster init the
> > cpufreq_cooling_ops, it would make all cooling device use the power allocator's
> > ops. If one's em is error because of the "em_is_sane", it would cause the
> > em NULL, but the cooling device's ops is exist, as a result, it would cause
> > panic because of the em.
> >
> > Add cpufreq_power_cooling_ops to avoid this case.
> >
> > Signed-off-by: Di Shen <di.shen@unisoc.com>
> > Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> > ---
> >  drivers/thermal/cpufreq_cooling.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
> > index b8151d95a806..af5cfb458370 100644
> > --- a/drivers/thermal/cpufreq_cooling.c
> > +++ b/drivers/thermal/cpufreq_cooling.c
> > @@ -493,6 +493,17 @@ static struct thermal_cooling_device_ops cpufreq_cooling_ops = {
> >       .set_cur_state          = cpufreq_set_cur_state,
> >  };
> >
> > +#ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
> > +static struct thermal_cooling_device_ops cpufreq_power_cooling_ops = {
> > +     .get_max_state          = cpufreq_get_max_state,
> > +     .get_cur_state          = cpufreq_get_cur_state,
> > +     .set_cur_state          = cpufreq_set_cur_state,
> > +     .get_requested_power    = cpufreq_get_requested_power,
> > +     .state2power            = cpufreq_state2power,
> > +     .power2state            = cpufreq_power2state,
> > +};
> > +#endif
> > +
> >  /**
> >   * __cpufreq_cooling_register - helper function to create cpufreq cooling device
> >   * @np: a valid struct device_node to the cooling device device tree node
> > @@ -559,9 +570,7 @@ __cpufreq_cooling_register(struct device_node *np,
> >  #ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
> >       if (em_is_sane(cpufreq_cdev, em)) {
> >               cpufreq_cdev->em = em;
> > -             cooling_ops->get_requested_power = cpufreq_get_requested_power;
> > -             cooling_ops->state2power = cpufreq_state2power;
> > -             cooling_ops->power2state = cpufreq_power2state;
> > +             cooling_ops = &cpufreq_power_cooling_ops;
> >       } else
> >  #endif
> >       if (policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED) {
>
> Please have a look at this patch in linux-next.
>
> commit 6ee324afdf30 ("drivers/thermal/cpufreq_cooling: Use private callback ops for each cooling device")
>
> This already fixes the problem, right ?
>
> --
> viresh