On 01/14/2014 05:29 AM, Eduardo Valentin wrote:

* PGP Signed by an unknown key

Wei,

On 06-01-2014 22:44, Wei Ni wrote:

quoted

On 01/06/2014 10:54 PM, Eduardo Valentin wrote:

quoted

quoted

Old Signed by an unknown key

On 06-01-2014 09:51, Mark Rutland wrote:

quoted

On Thu, Jan 02, 2014 at 05:50:06PM +0000, Matthew Longnecker wrote:

quoted

quoted

I think the platform driver may set governor for the thermal zone,
so how about to add a property named as "governor",
and parse it to tzp->governor_name,
something like:
                 ret = of_property_read_string(child, "governor", &str);
                 if (ret == 0)
                         if (strlen(str) < THERMAL_NAME_LENGTH)
                                 strcpy(tzp->governor_name, str);

Thanks.
Wei.

DT is supposed to describe the hardware, right? The governor isn't 
hardware -- it's a software control policy. On the other hand, that 
control policy must be tuned according to the behaviors of the platform 
hardware otherwise the system will be unstable.

Is it appropriate to be naming the governor in DT? If so, is it equally 
appropriate to describe any governor-specific parameters in DT (even 
though they are pure software constructs)?

The dt should be relatively static -- if the hardware doesn't change the
dt shouldn't have to.

The governers are not static. We can introduce new ones and throw away
old ones at any time. Tuning parameters can also change at any time.

I'd prefer to not have governer details described in the dt, and the
choice of governer and configuration of its tuning parameters should be
made at runtime somehow.

Agreed.

Yes, I think so, but the of-thermal driver handle the
thermal_zone_device_register, and pass the "tzp" without governor_name,

In fact, it will fall into the default governor, which is step_wise, by
default config.

In the thermal_zone_device_register(), it has following codes:
    if (tz->tzp)
            tz->governor = __find_governor(tz->tzp->governor_name);
    else
            tz->governor = __find_governor(DEFAULT_THERMAL_GOVERNOR);

It mean if the tz->tzp is not NULL, and the governor_name is NULL, then
the __find_governor() will return NULL, so the tz->governor is NULL, it
can't fall into the default governor. And in the of-thermal driver, it
call the thermal_zone_device_register(), and pass the "tzp" without
governor_name.
I think if we want to change the governor in user space, we need to fix
this first.

quoted

so the created thermal_zone's governor will be NULL, then it can't run
into the governor->throttle() if needed. And currently there have no

Actually, no, the tz will be set to default governor, and its throttle
call will be called.

quoted

interface to support updating governor and configuration at runtime.
I think it's better to initialize the governor_name when register the
thermal zone device in the of-thermal driver.

Still, why would you need to change the governor from a in kernel
decision? There is an ABI to change the thermal zone policy based on
user(land) request. If you need to change the policy from within the
kernel, which seams to be what you are trying to propose, you need to
explain why you need it, say, by giving at least one user of this API or
explaining its use case.

The thermal_zone_device_register() support to set the governor which you
want, but with the of-thermal framework, it only support to set to
default governor, if fix above issue. I think the driver which use the
of-thermal should be able to set to any governors which it want, in its
initialization. So I add the function thermal_update_governor().

quoted

Thanks.

quoted

quoted

Thanks,
Mark.