On Wednesday 15 May 2019 at 11:56:30 (+0200), Daniel Lezcano wrote:

On 15/05/2019 11:17, Quentin Perret wrote:

quoted

Hi Daniel,

On Wednesday 15 May 2019 at 11:06:18 (+0200), Daniel Lezcano wrote:

quoted

On 15/05/2019 10:23, Quentin Perret wrote:

quoted

In the current state, the perf_domain struct is fully defined only when
CONFIG_ENERGY_MODEL=y. Since we need to write code that compiles both
with or without that option in the thermal framework, make sure to
actually define the struct regardless of the config option. That allows
to avoid using stubbed accessor functions all the time in code paths
that use the EM.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Quentin Perret <redacted>

This patch implies the cpu cooling device can be set without the energy
model.

Isn't it possible to make a strong dependency for the cpu cooling device
on the energy model option, add the energy model as default on arm arch
and drop this patch?

Right, that should work too.

quoted

After all, the cpu cooling is using the em framework.

The reason I did it that way is simply to keep things flexible. If you
don't compile in THERMAL_GOV_POWER_ALLOCATOR, you will never use the EM
for CPU thermal. So I thought it would be good to not mandate compiling
in ENERGY_MODEL in this case -- that should save a bit of space.

But TBH I don't have a strong opinion on this one, so if everybody
agrees it's fine to just make CPU_THERMAL depend on ENERGY_MODEL, I'm
happy to drop this patch and fix patch 3/3. That would indeed simplify
things a bit.

Ok in this case it will be better to drop the 2/3 and add a small series
doing for the cpu_cooling.c

#ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR

/* structure freq */

/* power2state */

/* state2power*/

/* getrequestedpower */

/* All functions needed for the above */

#endif

static struct thermal_cooling_device_ops cpufreq_cooling_ops = {
        .get_max_state          = cpufreq_get_max_state,
        .get_cur_state          = cpufreq_get_cur_state,
        .set_cur_state          = cpufreq_set_cur_state,
#ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
        .get_requested_power    = cpufreq_get_requested_power,
        .state2power            = cpufreq_state2power,
        .power2state            = cpufreq_power2state,
#endif
};

So you don't have to care about ENERGY_MODEL to be set as
THERMAL_GOV_POWER_ALLOCATOR depends on it.

I think the result for cpu_cooling.c will be even more cleaner with the
em change.

OK, that works for me. I'll give it a go in v5.

Thanks !
Quentin

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