| Message ID | 10527734.nUPlyArG6x@rjwysocki.net (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | thermal: Rework binding cooling devices to trip points | expand |
On Mon, 2024-08-19 at 17:52 +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Because the trip and cdev pointers are sufficient to identify a > thermal > instance holding them unambiguously, drop the additional thermal zone > checks from two loops walking the list of thermal instances in a > thermal zone. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Zhang Rui <rui.zhang@intel.com> thanks, rui > --- > > v1 -> v3: No changes > > --- > drivers/thermal/thermal_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -850,7 +850,7 @@ int thermal_bind_cdev_to_trip(struct the > mutex_lock(&tz->lock); > mutex_lock(&cdev->lock); > list_for_each_entry(pos, &tz->thermal_instances, tz_node) > - if (pos->tz == tz && pos->trip == trip && pos->cdev > == cdev) { > + if (pos->trip == trip && pos->cdev == cdev) { > result = -EEXIST; > break; > } > @@ -915,7 +915,7 @@ int thermal_unbind_cdev_from_trip(struct > mutex_lock(&tz->lock); > mutex_lock(&cdev->lock); > list_for_each_entry_safe(pos, next, &tz->thermal_instances, > tz_node) { > - if (pos->tz == tz && pos->trip == trip && pos->cdev > == cdev) { > + if (pos->trip == trip && pos->cdev == cdev) { > list_del(&pos->tz_node); > list_del(&pos->cdev_node); > > > >
On 19/08/2024 17:52, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Because the trip and cdev pointers are sufficient to identify a thermal > instance holding them unambiguously, drop the additional thermal zone > checks from two loops walking the list of thermal instances in a > thermal zone. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> I'm wondering if the thermal_instance 'tz' field could be removed too ? > --- > > v1 -> v3: No changes > > --- > drivers/thermal/thermal_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -850,7 +850,7 @@ int thermal_bind_cdev_to_trip(struct the > mutex_lock(&tz->lock); > mutex_lock(&cdev->lock); > list_for_each_entry(pos, &tz->thermal_instances, tz_node) > - if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) { > + if (pos->trip == trip && pos->cdev == cdev) { > result = -EEXIST; > break; > } > @@ -915,7 +915,7 @@ int thermal_unbind_cdev_from_trip(struct > mutex_lock(&tz->lock); > mutex_lock(&cdev->lock); > list_for_each_entry_safe(pos, next, &tz->thermal_instances, tz_node) { > - if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) { > + if (pos->trip == trip && pos->cdev == cdev) { > list_del(&pos->tz_node); > list_del(&pos->cdev_node); > > > >
On Wed, Aug 21, 2024 at 1:01 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 19/08/2024 17:52, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Because the trip and cdev pointers are sufficient to identify a thermal > > instance holding them unambiguously, drop the additional thermal zone > > checks from two loops walking the list of thermal instances in a > > thermal zone. > > > > No intentional functional impact. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > I'm wondering if the thermal_instance 'tz' field could be removed too ? It is used in a debug printk in __thermal_cdev_update(). If that message can be dropped, then yes, but that would be a separate patch anyway.
On 21/08/2024 13:11, Rafael J. Wysocki wrote: > On Wed, Aug 21, 2024 at 1:01 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> >> On 19/08/2024 17:52, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> Because the trip and cdev pointers are sufficient to identify a thermal >>> instance holding them unambiguously, drop the additional thermal zone >>> checks from two loops walking the list of thermal instances in a >>> thermal zone. >>> >>> No intentional functional impact. >>> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> >> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> >> I'm wondering if the thermal_instance 'tz' field could be removed too ? > > It is used in a debug printk in __thermal_cdev_update(). If that > message can be dropped, then yes, but that would be a separate patch > anyway. Yes, I don't think it is really worth the debug message here
On Wed, Aug 21, 2024 at 1:56 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 21/08/2024 13:11, Rafael J. Wysocki wrote: > > On Wed, Aug 21, 2024 at 1:01 PM Daniel Lezcano > > <daniel.lezcano@linaro.org> wrote: > >> > >> On 19/08/2024 17:52, Rafael J. Wysocki wrote: > >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >>> > >>> Because the trip and cdev pointers are sufficient to identify a thermal > >>> instance holding them unambiguously, drop the additional thermal zone > >>> checks from two loops walking the list of thermal instances in a > >>> thermal zone. > >>> > >>> No intentional functional impact. > >>> > >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >> > >> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> > >> > >> I'm wondering if the thermal_instance 'tz' field could be removed too ? > > > > It is used in a debug printk in __thermal_cdev_update(). If that > > message can be dropped, then yes, but that would be a separate patch > > anyway. > > Yes, I don't think it is really worth the debug message here OK
Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -850,7 +850,7 @@ int thermal_bind_cdev_to_trip(struct the mutex_lock(&tz->lock); mutex_lock(&cdev->lock); list_for_each_entry(pos, &tz->thermal_instances, tz_node) - if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) { + if (pos->trip == trip && pos->cdev == cdev) { result = -EEXIST; break; } @@ -915,7 +915,7 @@ int thermal_unbind_cdev_from_trip(struct mutex_lock(&tz->lock); mutex_lock(&cdev->lock); list_for_each_entry_safe(pos, next, &tz->thermal_instances, tz_node) { - if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) { + if (pos->trip == trip && pos->cdev == cdev) { list_del(&pos->tz_node); list_del(&pos->cdev_node);