| Message ID | 1541352911-21343-1-git-send-email-rui.zhang@intel.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Zhang Rui |
| Headers | show |
| Series | [RFC,1/2] thermal: remove redundant polling timer update | expand |
On 04/11/2018 18:35, Zhang Rui wrote: > polling timer only needs to be updated once after all the trip points have > been updated. > Thus remove the redundant call of monitor_thermal_zone(). > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> Makes sense. Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/thermal_core.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index d6ebc1c..98f02b0 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -419,11 +419,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > handle_critical_trips(tz, trip, type); > else > handle_non_critical_trips(tz, trip, type); > - /* > - * Alright, we handled this trip successfully. > - * So, start monitoring again. > - */ > - monitor_thermal_zone(tz); > } > > static void update_temperature(struct thermal_zone_device *tz) > @@ -482,6 +477,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz, > > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > + monitor_thermal_zone(tz); > } > EXPORT_SYMBOL_GPL(thermal_zone_device_update); > >
[ reply to proper RFC mail.. ] Hi, On 11/04/2018 06:35 PM, Zhang Rui wrote: > polling timer only needs to be updated once after all the trip points have > been updated. > Thus remove the redundant call of monitor_thermal_zone(). > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/thermal/thermal_core.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index d6ebc1c..98f02b0 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -419,11 +419,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > handle_critical_trips(tz, trip, type); > else > handle_non_critical_trips(tz, trip, type); > - /* > - * Alright, we handled this trip successfully. > - * So, start monitoring again. > - */ > - monitor_thermal_zone(tz); > } > > static void update_temperature(struct thermal_zone_device *tz) > @@ -482,6 +477,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz, > > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > + monitor_thermal_zone(tz); > } > EXPORT_SYMBOL_GPL(thermal_zone_device_update); Shouldn't thermal_notify_framework() (the other handle_thermal_trip() user) be also updated now to call monitor_thermal_zone(tz) itself? drivers/net/wireless/intel/iwlwifi/mvm/tt.c is currently the only user of thermal_notify_framework() and it doesn't use polling so this patch doesn't really affect it but the patch description misses information whether lack of the update to thermal_notify_framework() is intentional or not.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hey, On Mon, Nov 05, 2018 at 01:35:10AM +0800, Zhang Rui wrote: > polling timer only needs to be updated once after all the trip points have > been updated. > Thus remove the redundant call of monitor_thermal_zone(). > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/thermal/thermal_core.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index d6ebc1c..98f02b0 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -419,11 +419,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > handle_critical_trips(tz, trip, type); > else > handle_non_critical_trips(tz, trip, type); > - /* > - * Alright, we handled this trip successfully. > - * So, start monitoring again. > - */ > - monitor_thermal_zone(tz); > } > > static void update_temperature(struct thermal_zone_device *tz) > @@ -482,6 +477,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz, > > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > + monitor_thermal_zone(tz); > } > EXPORT_SYMBOL_GPL(thermal_zone_device_update); The above change leaves thermal_notify_framework() calls with no updates to polling timer. Either leave it as is or call monitor_thermal_zone() into thermal_notify_framework() too. > > -- > 2.7.4 >
On 一, 2018-11-05 at 14:43 +0100, Bartlomiej Zolnierkiewicz wrote: > [ reply to proper RFC mail.. ] > > Hi, > > On 11/04/2018 06:35 PM, Zhang Rui wrote: > > > > polling timer only needs to be updated once after all the trip > > points have > > been updated. > > Thus remove the redundant call of monitor_thermal_zone(). > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > --- > > drivers/thermal/thermal_core.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/thermal/thermal_core.c > > b/drivers/thermal/thermal_core.c > > index d6ebc1c..98f02b0 100644 > > --- a/drivers/thermal/thermal_core.c > > +++ b/drivers/thermal/thermal_core.c > > @@ -419,11 +419,6 @@ static void handle_thermal_trip(struct > > thermal_zone_device *tz, int trip) > > handle_critical_trips(tz, trip, type); > > else > > handle_non_critical_trips(tz, trip, type); > > - /* > > - * Alright, we handled this trip successfully. > > - * So, start monitoring again. > > - */ > > - monitor_thermal_zone(tz); > > } > > > > static void update_temperature(struct thermal_zone_device *tz) > > @@ -482,6 +477,7 @@ void thermal_zone_device_update(struct > > thermal_zone_device *tz, > > > > for (count = 0; count < tz->trips; count++) > > handle_thermal_trip(tz, count); > > + monitor_thermal_zone(tz); > > } > > EXPORT_SYMBOL_GPL(thermal_zone_device_update); > Shouldn't thermal_notify_framework() (the other handle_thermal_trip() > user) be also updated now to call monitor_thermal_zone(tz) itself? > > drivers/net/wireless/intel/iwlwifi/mvm/tt.c > user of thermal_notify_framework() and it doesn't use polling so > this patch doesn't really affect it but the patch description misses > information whether lack of the update to thermal_notify_framework() > is intentional or not.. > I checked drivers/net/wireless/intel/iwlwifi/mvm/tt.c and I'm wondering if thermal_zone_device_update() can be used instead of thermal_notify_framework(). If yes, then there is no valid user of thermal_notify_framework() and we can probably remove it. But that's another topic, and yes, we should update the timer in thermal_notify_framwork() in this patch. thanks, rui > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics
On 一, 2018-11-05 at 16:17 -0800, Eduardo Valentin wrote: > Hey, > > On Mon, Nov 05, 2018 at 01:35:10AM +0800, Zhang Rui wrote: > > > > polling timer only needs to be updated once after all the trip > > points have > > been updated. > > Thus remove the redundant call of monitor_thermal_zone(). > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > --- > > drivers/thermal/thermal_core.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/thermal/thermal_core.c > > b/drivers/thermal/thermal_core.c > > index d6ebc1c..98f02b0 100644 > > --- a/drivers/thermal/thermal_core.c > > +++ b/drivers/thermal/thermal_core.c > > @@ -419,11 +419,6 @@ static void handle_thermal_trip(struct > > thermal_zone_device *tz, int trip) > > handle_critical_trips(tz, trip, type); > > else > > handle_non_critical_trips(tz, trip, type); > > - /* > > - * Alright, we handled this trip successfully. > > - * So, start monitoring again. > > - */ > > - monitor_thermal_zone(tz); > > } > > > > static void update_temperature(struct thermal_zone_device *tz) > > @@ -482,6 +477,7 @@ void thermal_zone_device_update(struct > > thermal_zone_device *tz, > > > > for (count = 0; count < tz->trips; count++) > > handle_thermal_trip(tz, count); > > + monitor_thermal_zone(tz); > > } > > EXPORT_SYMBOL_GPL(thermal_zone_device_update); > > The above change leaves thermal_notify_framework() calls with no > updates to polling timer. > > Either leave it as is or call monitor_thermal_zone() into > thermal_notify_framework() too. yes, Bartlomiej Zolnierkiewicz also pointed out this issue. thanks, rui
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index d6ebc1c..98f02b0 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -419,11 +419,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) handle_critical_trips(tz, trip, type); else handle_non_critical_trips(tz, trip, type); - /* - * Alright, we handled this trip successfully. - * So, start monitoring again. - */ - monitor_thermal_zone(tz); } static void update_temperature(struct thermal_zone_device *tz) @@ -482,6 +477,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz, for (count = 0; count < tz->trips; count++) handle_thermal_trip(tz, count); + monitor_thermal_zone(tz); } EXPORT_SYMBOL_GPL(thermal_zone_device_update);
polling timer only needs to be updated once after all the trip points have been updated. Thus remove the redundant call of monitor_thermal_zone(). Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/thermal/thermal_core.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)