On 二, 2012-07-24 at 11:27 +0200, Rafael J. Wysocki wrote:
> On Tuesday, July 24, 2012, Zhang Rui wrote:
> > On 四, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote:
> > > On Thursday, July 19, 2012, Zhang Rui wrote:
> > > > This function is used to update the cooling state of
> > > > all the cooling devices that are binded to an active trip point.
> > >
> > > s/binded/bound/
> > >
> > got it.
> >
> > > > This will be used for passive cooling as well, in the future patches.
> > > > as both active and passive cooling can share the same algorithm,
> > > > which is
> > > >
> > > > 1. if the temperature is higher than a trip point,
> > > > a. if the trend is THERMAL_TREND_RAISING, use higher cooling
> > > > state for this trip point
> > > > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> > > > state for this trip point
> > > >
> > > > 2. if the temperature is lower than a trip point, use lower
> > > > cooling state for this trip point.
> > > >
> > > > Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
> > > > ---
> > > > drivers/acpi/thermal.c | 7 +++-
> > > > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------
> > > > 2 files changed, 71 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> > > > index 73e335f..14c4879 100644
> > > > --- a/drivers/acpi/thermal.c
> > > > +++ b/drivers/acpi/thermal.c
> > > > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
> > > > if (thermal_get_trip_type(thermal, trip, &type))
> > > > return -EINVAL;
> > > >
> > > > - /* Only PASSIVE trip points need TREND */
> > > > + if (type == THERMAL_TRIP_ACTIVE) {
> > > > + /* aggressive active cooling */
> > > > + *trend = THERMAL_TREND_RAISING;
> > > > + return 0;
> > >
> > > Please move that into thermal_zone_trip_update() directly, unless you
> > > need it elsewhere.
> > >
> > IMO, I can say that ACPI thermal wants aggressive active cooling, as it
> > will never spin down the fan when the temperature is higher than the
> > trip point.
>
> I meant the code organization, not the functionality. :-)
>
sure.
> Since thermal_get_trend() is static, it is not used outside of this file,
> so you can move the "if (type == THERMAL_TRIP_ACTIVE)" conditional from it
> directly into the caller (unless there are more callers, which I'm not sure
> about without reading the whole code again).
>
sorry I still do not get it.
the generic thermal layer is the caller of this callback.
so you mean in thermal_zone_trip_update(),
when updating an active trip point, we set the trend to
THERMAL_TREND_RAISING explicitly?
in this way, all the platform thermal drivers will spin up the fan step
by step when the temperature is higher than the trip point, even if the
temperature is dropping. (Note that, this is not a problem for ACPI, as
there is only two state for ACPI FAN, on/off, it must be always on when
the temperature is higher than the active trip point. but other platform
may want to spin down the fan a little even if the system is still warm)
> That would make the code cleaner, because right now the condition is
> hidden in thermal_get_trend() and it may not be clear to the reader of
> thermal_zone_trip_update() that it is actually checked.
>
why?
Readers should trust the value returned by
drivers/thermal/thermal_sys.c:thermal_get_trend().
And platform thermal drivers should provide the proper trend for active
trip points as well. If they don't, we will use the default trend by
comparing the current temperature and last temperature.
thanks,
rui
> Thanks,
> Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
