* [PATCH] thermal: cpufreq_cooling: Avoid all cluster using global cooling_ops
@ 2022-07-18 12:24 Di Shen
2022-07-19 4:14 ` Viresh Kumar
0 siblings, 1 reply; 3+ messages in thread
From: Di Shen @ 2022-07-18 12:24 UTC (permalink / raw)
To: lukasz.luba, amitk, rui.zhang
Cc: amit.kachhap, daniel.lezcano, viresh.kumar, rafael, linux-pm,
linux-kernel, xuewen.yan, xuewen.yan94
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) {
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal: cpufreq_cooling: Avoid all cluster using global cooling_ops
2022-07-18 12:24 [PATCH] thermal: cpufreq_cooling: Avoid all cluster using global cooling_ops Di Shen
@ 2022-07-19 4:14 ` Viresh Kumar
2022-07-19 9:24 ` Di Shen
0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2022-07-19 4:14 UTC (permalink / raw)
To: Di Shen
Cc: lukasz.luba, amitk, rui.zhang, amit.kachhap, daniel.lezcano,
rafael, linux-pm, linux-kernel, xuewen.yan, xuewen.yan94
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal: cpufreq_cooling: Avoid all cluster using global cooling_ops
2022-07-19 4:14 ` Viresh Kumar
@ 2022-07-19 9:24 ` Di Shen
0 siblings, 0 replies; 3+ messages in thread
From: Di Shen @ 2022-07-19 9:24 UTC (permalink / raw)
To: Viresh Kumar
Cc: Di Shen, Lukasz Luba, Amit Kucheria, Zhang, Rui, amit.kachhap,
Daniel Lezcano, Rafael J. Wysocki, Linux PM,
Linux Kernel Mailing List, xuewen.yan, xuewen.yan94
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-19 9:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 12:24 [PATCH] thermal: cpufreq_cooling: Avoid all cluster using global cooling_ops Di Shen
2022-07-19 4:14 ` Viresh Kumar
2022-07-19 9:24 ` Di Shen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.