thermal: gov_step_wide: Two cleanups
cover thermal: gov_step_wide: Two cleanups
Hi Everyone,
The patches in this series clean up the step-wise thermal governor and are not expected to alter its functionality.
Please see the changelogs of the patches for details.
The patches should apply directly on top of 6.9-rc2.
Thanks!
PATCH 1/2 thermal: gov_step_wise: Simplify get_target_state()
From: Rafael J. Wysocki <[email protected]>
The step-wise governor's get_target_state() function contains redundant braces, redundant parens and a redundant next_target local variable, so get rid of all that stuff.
No intentional functional impact.
--- drivers/thermal/gov_step_wise.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-)
drivers/thermal/gov_step_wise.c +10 -17
@@ -32,7 +32,6 @@ static unsigned long get_target_state(st {struct thermal_cooling_device *cdev = instance->cdev;unsigned long cur_state; -unsigned long next_target;/* * We keep this instance the way it is by default.
@@ -40,32 +39,26 @@ static unsigned long get_target_state(st* cdev in use to determine the next_target. */ cdev->ops->get_cur_state (cdev, &cur_state); - next_target = instance->target;dev_dbg (&cdev->device,"cur_state=%ld\n" , cur_state);if (!instance->initialized) { -if (throttle) { - next_target =clamp ((cur_state +1 ), instance->lower, instance->upper); - }else { - next_target =THERMAL_NO_TARGET ; - } +if (throttle) +return clamp (cur_state +1 , instance->lower, instance->upper); -return next_target; +return THERMAL_NO_TARGET ; }if (throttle) {if (trend ==THERMAL_TREND_RAISING ) - next_target =clamp ((cur_state +1 ), instance->lower, instance->upper); - }else { -if (trend ==THERMAL_TREND_DROPPING ) { -if (cur_state <= instance->lower) - next_target =THERMAL_NO_TARGET ; -else - next_target =clamp ((cur_state -1 ), instance->lower, instance->upper); - } +return clamp (cur_state +1 , instance->lower, instance->upper); + }else if (trend ==THERMAL_TREND_DROPPING ) { +if (cur_state <= instance->lower) +return THERMAL_NO_TARGET ; + +return clamp (cur_state -1 , instance->lower, instance->upper); } -return next_target; +return instance->target; } static void thermal_zone_trip_update(struct thermal_zone_device *tz,
Re: [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state()
Hi Rafael,
Quoted 61 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
The step-wise governor's
get_target_state()function contains redundant braces, redundant parens and a redundantnext_targetlocal variable, so get rid of all that stuff.No intentional functional impact.
Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/thermal/gov_step_wise.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-)Index:
linux-pm/drivers/thermal/gov_step_wise.c
===================================================================drivers/thermal/gov_step_wise.c +6
-16@@ -32,7 +32,6 @@ static unsigned long get_target_state(st {struct thermal_cooling_device *cdev = instance->cdev;unsigned long cur_state; -unsigned long next_target;/* * We keep this instance the way it is by default. @@ -40,32 +39,26 @@ static unsigned long get_target_state(st* cdev in use to determine the next_target. */ cdev->ops->get_cur_state (cdev, &cur_state); - next_target = instance->target;dev_dbg (&cdev->device,"cur_state=%ld\n" , cur_state);if (!instance->initialized) { -if (throttle) { - next_target =clamp ((cur_state +1 ), instance->lower, instance->upper); - }else { - next_target =THERMAL_NO_TARGET ; - } +if (throttle) +return clamp (cur_state +1 , instance->lower, instance->upper); -return next_target; +return THERMAL_NO_TARGET ; }if (throttle) {if (trend ==THERMAL_TREND_RAISING ) - next_target =clamp ((cur_state +1 ), instance->lower, instance->upper); - }else { -if (trend ==THERMAL_TREND_DROPPING ) { -if (cur_state <= instance->lower) - next_target =THERMAL_NO_TARGET ; -else - next_target =clamp ((cur_state -1 ), instance->lower, instance->upper); - } +return clamp (cur_state +1 , instance->lower, instance->upper); + }else if (trend ==THERMAL_TREND_DROPPING ) { +if (cur_state <= instance->lower)
drivers/thermal/gov_step_wise.c static unsigned long get_target_state(st · line 64
Quoted 10 lines
drivers/thermal/gov_step_wise.c +4
-1+return THERMAL_NO_TARGET ; + +return clamp (cur_state -1 , instance->lower, instance->upper); } -return next_target; +return instance->target; } static void thermal_zone_trip_update(struct thermal_zone_device *tz,
LGTM,
Regards,
Lukasz
PATCH 2/2 thermal: gov_step_wise: Simplify checks related to passive trips
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
Make it more clear from the code flow that the passive polling status updates only take place for passive trip points.
No intentional functional impact.
--- drivers/thermal/gov_step_wise.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/thermal/gov_step_wise.c +6 -8
@@ -92,15 +92,13 @@ static void thermal_zone_trip_update(strif (instance->initialized && old_target == instance->target)continue ; -if (old_target ==THERMAL_NO_TARGET && - instance->target !=THERMAL_NO_TARGET ) { -/* Activate a passive thermal instance */ -if (trip->type ==THERMAL_TRIP_PASSIVE ) +if (trip->type ==THERMAL_TRIP_PASSIVE ) { +/* If needed, update the status of passive polling. */ +if (old_target ==THERMAL_NO_TARGET && + instance->target !=THERMAL_NO_TARGET ) tz->passive++; - }else if (old_target !=THERMAL_NO_TARGET && - instance->target ==THERMAL_NO_TARGET ) { -/* Deactivate a passive thermal instance */ -if (trip->type ==THERMAL_TRIP_PASSIVE ) +else if (old_target !=THERMAL_NO_TARGET && + instance->target ==THERMAL_NO_TARGET ) tz->passive--; }
Re: [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips
Quoted 31 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
Make it more clear from the code flow that the passive polling status updates only take place for passive trip points.
No intentional functional impact.
Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/thermal/gov_step_wise.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)Index:
linux-pm/drivers/thermal/gov_step_wise.c
===================================================================drivers/thermal/gov_step_wise.c +4
-4@@ -92,15 +92,13 @@ static void thermal_zone_trip_update(strif (instance->initialized && old_target == instance->target)continue ; -if (old_target ==THERMAL_NO_TARGET && - instance->target !=THERMAL_NO_TARGET ) { -/* Activate a passive thermal instance */ -if (trip->type ==THERMAL_TRIP_PASSIVE ) +if (trip->type ==THERMAL_TRIP_PASSIVE ) { +/* If needed, update the status of passive polling. */ +if (old_target ==THERMAL_NO_TARGET && + instance->target !=THERMAL_NO_TARGET )
drivers/thermal/gov_step_wise.c static void thermal_zone_trip_update(str · line 104
Quoted 10 lines
drivers/thermal/gov_step_wise.c +2
-4tz->passive++; - }else if (old_target !=THERMAL_NO_TARGET && - instance->target ==THERMAL_NO_TARGET ) { -/* Deactivate a passive thermal instance */ -if (trip->type ==THERMAL_TRIP_PASSIVE ) + else if (old_target != THERMAL_NO_TARGET && + instance->target == THERMAL_NO_TARGET) tz->passive--; }
The patch looks good, although I got some warning while applying with my b4 tool:
BADSIG: DKIM/rjwysocki.net
Anyway, it looks like false warning IMO.
Rafael J. Wysocki
maintainerRe: [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips
Quoted 41 lines · Lukasz Luba
Quoted 37 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
Make it more clear from the code flow that the passive polling status updates only take place for passive trip points.
No intentional functional impact.
Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/thermal/gov_step_wise.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)Index:
linux-pm/drivers/thermal/gov_step_wise.c
===================================================================drivers/thermal/gov_step_wise.c +6
-8@@ -92,15 +92,13 @@ static void thermal_zone_trip_update(strif (instance->initialized && old_target == instance->target)continue ; -if (old_target ==THERMAL_NO_TARGET && - instance->target !=THERMAL_NO_TARGET ) { -/* Activate a passive thermal instance */ -if (trip->type ==THERMAL_TRIP_PASSIVE ) +if (trip->type ==THERMAL_TRIP_PASSIVE ) { +/* If needed, update the status of passive polling. */ +if (old_target ==THERMAL_NO_TARGET && + instance->target !=THERMAL_NO_TARGET ) tz->passive++; - }else if (old_target !=THERMAL_NO_TARGET && - instance->target ==THERMAL_NO_TARGET ) { -/* Deactivate a passive thermal instance */ -if (trip->type ==THERMAL_TRIP_PASSIVE ) +else if (old_target !=THERMAL_NO_TARGET && + instance->target ==THERMAL_NO_TARGET ) tz->passive--; }
Quoted 8 lines
Quoted 4 lines
The patch looks good, although I got some warning while applying with my b4 tool:
BADSIG: DKIM/rjwysocki.net
It's likely because it was sent from an address in the rjwysocki.net domain, but it is perfectly fine to send "somebody else's" patches if a replacement From: header is given.
Looks like a b4 issue to me.
Quoted 3 lines
Anyway, it looks like false warning IMO.
Reviewed-by Lukasz Luba <[email protected]>
Thank you!