* [PATCH v1 0/2] thermal: gov_step_wide: Two cleanups @ 2024-04-03 18:09 Rafael J. Wysocki 2024-04-03 18:11 ` [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state() Rafael J. Wysocki 2024-04-03 18:12 ` [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips Rafael J. Wysocki 0 siblings, 2 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2024-04-03 18:09 UTC (permalink / raw) To: Linux PM; +Cc: LKML, Lukasz Luba, Daniel Lezcano 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! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state() 2024-04-03 18:09 [PATCH v1 0/2] thermal: gov_step_wide: Two cleanups Rafael J. Wysocki @ 2024-04-03 18:11 ` Rafael J. Wysocki 2024-04-04 11:15 ` Lukasz Luba 2024-04-03 18:12 ` [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips Rafael J. Wysocki 1 sibling, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2024-04-03 18:11 UTC (permalink / raw) To: Linux PM; +Cc: LKML, Lukasz Luba, Daniel Lezcano From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> 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. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/thermal/gov_step_wise.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) Index: linux-pm/drivers/thermal/gov_step_wise.c =================================================================== --- linux-pm.orig/drivers/thermal/gov_step_wise.c +++ linux-pm/drivers/thermal/gov_step_wise.c @@ -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, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state() 2024-04-03 18:11 ` [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state() Rafael J. Wysocki @ 2024-04-04 11:15 ` Lukasz Luba 0 siblings, 0 replies; 6+ messages in thread From: Lukasz Luba @ 2024-04-04 11:15 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Daniel Lezcano, Linux PM Hi Rafael, On 4/3/24 19:11, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/gov_step_wise.c | 27 ++++++++++----------------- > 1 file changed, 10 insertions(+), 17 deletions(-) > > Index: linux-pm/drivers/thermal/gov_step_wise.c > =================================================================== > --- linux-pm.orig/drivers/thermal/gov_step_wise.c > +++ linux-pm/drivers/thermal/gov_step_wise.c > @@ -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, > > > LGTM, Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> Regards, Lukasz ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips 2024-04-03 18:09 [PATCH v1 0/2] thermal: gov_step_wide: Two cleanups Rafael J. Wysocki 2024-04-03 18:11 ` [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state() Rafael J. Wysocki @ 2024-04-03 18:12 ` Rafael J. Wysocki 2024-04-04 11:26 ` Lukasz Luba 1 sibling, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2024-04-03 18:12 UTC (permalink / raw) To: Linux PM; +Cc: LKML, Lukasz Luba, Daniel Lezcano From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> 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 <rafael.j.wysocki@intel.com> --- drivers/thermal/gov_step_wise.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) Index: linux-pm/drivers/thermal/gov_step_wise.c =================================================================== --- linux-pm.orig/drivers/thermal/gov_step_wise.c +++ linux-pm/drivers/thermal/gov_step_wise.c @@ -92,15 +92,13 @@ static void thermal_zone_trip_update(str if (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--; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips 2024-04-03 18:12 ` [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips Rafael J. Wysocki @ 2024-04-04 11:26 ` Lukasz Luba 2024-04-04 11:57 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Lukasz Luba @ 2024-04-04 11:26 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Daniel Lezcano, Linux PM On 4/3/24 19:12, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> > --- > drivers/thermal/gov_step_wise.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > Index: linux-pm/drivers/thermal/gov_step_wise.c > =================================================================== > --- linux-pm.orig/drivers/thermal/gov_step_wise.c > +++ linux-pm/drivers/thermal/gov_step_wise.c > @@ -92,15 +92,13 @@ static void thermal_zone_trip_update(str > if (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--; > } > > > > 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. Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips 2024-04-04 11:26 ` Lukasz Luba @ 2024-04-04 11:57 ` Rafael J. Wysocki 0 siblings, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2024-04-04 11:57 UTC (permalink / raw) To: Lukasz Luba; +Cc: Rafael J. Wysocki, LKML, Daniel Lezcano, Linux PM On Thu, Apr 4, 2024 at 1:26 PM Lukasz Luba <lukasz.luba@arm.com> wrote: > > > > On 4/3/24 19:12, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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 <rafael.j.wysocki@intel.com> > > --- > > drivers/thermal/gov_step_wise.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > Index: linux-pm/drivers/thermal/gov_step_wise.c > > =================================================================== > > --- linux-pm.orig/drivers/thermal/gov_step_wise.c > > +++ linux-pm/drivers/thermal/gov_step_wise.c > > @@ -92,15 +92,13 @@ static void thermal_zone_trip_update(str > > if (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--; > > } > > > > > > > > > > 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. > Anyway, it looks like false warning IMO. > > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> Thank you! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-04 11:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-03 18:09 [PATCH v1 0/2] thermal: gov_step_wide: Two cleanups Rafael J. Wysocki 2024-04-03 18:11 ` [PATCH v1 1/2] thermal: gov_step_wise: Simplify get_target_state() Rafael J. Wysocki 2024-04-04 11:15 ` Lukasz Luba 2024-04-03 18:12 ` [PATCH v1 2/2] thermal: gov_step_wise: Simplify checks related to passive trips Rafael J. Wysocki 2024-04-04 11:26 ` Lukasz Luba 2024-04-04 11:57 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox