From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752203AbcEVJqM (ORCPT ); Sun, 22 May 2016 05:46:12 -0400 Received: from piie.net ([80.82.223.85]:36324 "EHLO piie.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752065AbcEVJqH convert rfc822-to-8bit (ORCPT ); Sun, 22 May 2016 05:46:07 -0400 X-Greylist: delayed 366 seconds by postgrey-1.27 at vger.kernel.org; Sun, 22 May 2016 05:46:07 EDT Mime-Version: 1.0 Date: Sun, 22 May 2016 09:40:01 +0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Message-ID: X-Mailer: RainLoop/1.9.3.365 From: "Peter Feuerer" Subject: Re: [PATCH] thermal: check validity get_trip_hyst function pointer in bang-bang governor To: "Michele Di Giorgio" , linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org, "Javi Merino" , "Zhang Rui" , "Eduardo Valentin" In-Reply-To: <1462960147-9175-1-git-send-email-michele.digiorgio@arm.com> References: <1462960147-9175-1-git-send-email-michele.digiorgio@arm.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, 14. Mai 2016 13:47 Uhr, "Michele Di Giorgio" schrieb: > Bang-bang thermal governor uses trip point hysteresis to make decisions. > Hysteresis is a required property in the device tree for trip points, but it is > an optional thermal zone device operation. Hence, we need to check whether the > function pointer is valid or not. > > If it is not available, we assume the hysteresis to be zero. Consequently, a > highly varying temperature will make the governor continuosly switch a cooling > device ON and OFF. > > CC: Zhang Rui > CC: Eduardo Valentin > CC: Peter Feuerer > Signed-off-by: Michele Di Giorgio Acked-by: Peter Feuerer Thanks. -- kind regards, --peter; > --- > Using trip_hyst == 0 makes the governor work sensibly but may cause oscillations > of the control signals of a cooling device. An alternative to this could be to > fail the registration of the thermal governor. > > Another way would be to set the default value of the hysteresis to x% of the > trip temperature, to make the governor less sensitive to highly varying inputs. > > drivers/thermal/gov_bang_bang.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c > index 39d1519..bb118a1 100644 > --- a/drivers/thermal/gov_bang_bang.c > +++ b/drivers/thermal/gov_bang_bang.c > @@ -29,7 +29,13 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip) > struct thermal_instance *instance; > > tz->ops->get_trip_temp(tz, trip, &trip_temp); > - tz->ops->get_trip_hyst(tz, trip, &trip_hyst); > + > + if (!tz->ops->get_trip_hyst) { > + pr_warn_once("Undefined get_trip_hyst for thermal zone %s - " > + "running with default hysteresis zero\n", tz->type); > + trip_hyst = 0; > + } else > + tz->ops->get_trip_hyst(tz, trip, &trip_hyst); > > dev_dbg(&tz->device, "Trip%d[temp=%d]:temp=%d:hyst=%d\n", > trip, trip_temp, tz->temperature, > -- > 1.9.1