Hi Javi,
tiny nits again...
On Tue, Aug 11, 2015 at 6:21 PM, Javi Merino <javi.mer...@arm.com> wrote:
> The power allocator governor currently requires that the thermal zone
> has at least two passive trip points. If there aren't, the governor
> refuses to bind to the thermal zone.
>
> This commit relaxes that requirement. Now the governor will bind to all
> thermal zones regardless of how many trip points they have.
>
> Cc: Zhang Rui <rui.zh...@intel.com>
> Cc: Eduardo Valentin <edubez...@gmail.com>
> Signed-off-by: Javi Merino <javi.mer...@arm.com>
> ---
> Documentation/thermal/power_allocator.txt | 2 +-
> drivers/thermal/power_allocator.c | 91
> +++++++++++++++++--------------
> 2 files changed, 52 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/thermal/power_allocator.txt
> b/Documentation/thermal/power_allocator.txt
> index c3797b529991..a1ce2235f121 100644
> --- a/Documentation/thermal/power_allocator.txt
> +++ b/Documentation/thermal/power_allocator.txt
> @@ -4,7 +4,7 @@ Power allocator governor tunables
> Trip points
> -----------
>
> -The governor requires the following two passive trip points:
> +The governor works optimally with the following two passive trip points:
>
> 1. "switch on" trip point: temperature above which the governor
> control loop starts operating. This is the first passive trip
> diff --git a/drivers/thermal/power_allocator.c
> b/drivers/thermal/power_allocator.c
> index f78836c2da26..257c9af20f22 100644
> --- a/drivers/thermal/power_allocator.c
> +++ b/drivers/thermal/power_allocator.c
> @@ -24,6 +24,8 @@
>
> #include "thermal_core.h"
>
> +#define INVALID_TRIP -1
> +
> #define FRAC_BITS 10
> #define int_to_frac(x) ((x) << FRAC_BITS)
> #define frac_to_int(x) ((x) >> FRAC_BITS)
> @@ -61,6 +63,8 @@ static inline s64 div_frac(s64 x, s64 y)
> * Used to calculate the derivative term.
> * @trip_switch_on: first passive trip point of the thermal zone. The
> * governor switches on when this trip point is crossed.
> + * If the thermal zone only has one passive trip point,
> + * @trip_switch_on should be INVALID_TRIP.
> * @trip_max_desired_temperature: last passive trip point of the thermal
> * zone. The temperature we are
> * controlling for.
> @@ -378,43 +382,66 @@ unlock:
> return ret;
> }
>
> -static int get_governor_trips(struct thermal_zone_device *tz,
> - struct power_allocator_params *params)
> +/**
> + * get_governor_trips() - get the number of the two trip points that are key
> for this governor
> + * @tz: thermal zone to operate on
> + * @params: pointer the private data for this governor
^
pointer to private data
> + *
> + * The power allocator governor works optimally with two trips points:
> + * a "switch on" trip point and a "maximum desired temperature". These
> + * are defined as the first and last passive trip points.
> + *
> + * If there is only one trip point, then that's considered to be the
> + * "maximum desired temperature" trip point and the governor is always
> + * on. If there are no passive or active trip points, then the
> + * governor won't do anything. In fact, its throttle function
> + * shouldn't be called at all.
^
"shouldn't" or "won't" ?
[snip]
> static void power_allocator_unbind(struct thermal_zone_device *tz)
> @@ -544,13 +561,7 @@ static int power_allocator_throttle(struct
> thermal_zone_device *tz, int trip)
>
> ret = tz->ops->get_trip_temp(tz, params->trip_switch_on,
> &switch_on_temp);
> - if (ret) {
> - dev_warn(&tz->device,
> - "Failed to get switch on temperature: %d\n", ret);
> - return ret;
> - }
> -
> - if (current_temp < switch_on_temp) {
> + if ((!ret) && (current_temp < switch_on_temp)) {
nit: I think the inner pair of () are not necessary.
Thanks,
-Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
