On Wednesday, July 25, 2012, Zhang Rui wrote:
> 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.
I'm not talking about the callback, but of the static function with the
same name you've introduced into drivers/thermal/thermal_sys.c in the
previous patch. :-)
The only caller of this function seems to be thermal_zone_device_passive()
and my point is that it would be better to simply put the code from that
function into thermal_zone_device_passive() directly, unless there are
more callers added by the subsequent patches, which I'm not sure about.
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
