Message ID | 7628882.EvYhyI6sBW@kreacher |
---|---|
State | New |
Headers | show |
Series | thermal: netlink: Redefine the API and drop unused code | expand |
On 12/15/23 20:02, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The only actually used thermal netlink notification routine related > to cooling devices is thermal_notify_cdev_state_update(). The other > two, thermal_notify_cdev_add() and thermal_notify_cdev_delete(), are > never used. > > So as to get rid of dead code, drop thermal_notify_cdev_add/delete(), > which can be added back if they turn out to be ever needed, along with > the related code. > > In analogy with the previous thermal netlink API changes, redefine > thermal_notify_cdev_state_update() to take a const cdev pointer as its > first argument and let it extract the requisite information from there > by itself. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/thermal_helpers.c | 2 - > drivers/thermal/thermal_netlink.c | 43 ++------------------------------------ > drivers/thermal/thermal_netlink.h | 19 +++------------- > 3 files changed, 8 insertions(+), 56 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_netlink.h > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_netlink.h > +++ linux-pm/drivers/thermal/thermal_netlink.h > @@ -24,9 +24,8 @@ int thermal_notify_tz_trip_up(const stru > const struct thermal_trip *trip); > int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz, > const struct thermal_trip *trip); > -int thermal_notify_cdev_state_update(int cdev_id, int state); > -int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state); > -int thermal_notify_cdev_delete(int cdev_id); > +int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, > + int state); > int thermal_notify_tz_gov_change(const struct thermal_zone_device *tz, > const char *name); > int thermal_genl_sampling_temp(int id, int temp); > @@ -76,18 +75,8 @@ static inline int thermal_notify_tz_trip > return 0; > } > > -static inline int thermal_notify_cdev_state_update(int cdev_id, int state) > -{ > - return 0; > -} > - > -static inline int thermal_notify_cdev_add(int cdev_id, const char *name, > - int max_state) > -{ > - return 0; > -} > - > -static inline int thermal_notify_cdev_delete(int cdev_id) > +static inline int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, > + int state) > { > return 0; > } > Index: linux-pm/drivers/thermal/thermal_helpers.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_helpers.c > +++ linux-pm/drivers/thermal/thermal_helpers.c > @@ -152,7 +152,7 @@ static void thermal_cdev_set_cur_state(s > if (cdev->ops->set_cur_state(cdev, target)) > return; > > - thermal_notify_cdev_state_update(cdev->id, target); > + thermal_notify_cdev_state_update(cdev, target); > thermal_cooling_device_stats_update(cdev, target); > } > > Index: linux-pm/drivers/thermal/thermal_netlink.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_netlink.c > +++ linux-pm/drivers/thermal/thermal_netlink.c > @@ -147,27 +147,6 @@ static int thermal_genl_event_tz_trip_ch > return 0; > } > > -static int thermal_genl_event_cdev_add(struct param *p) > -{ > - if (nla_put_string(p->msg, THERMAL_GENL_ATTR_CDEV_NAME, > - p->name) || > - nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, > - p->cdev_id) || > - nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_MAX_STATE, > - p->cdev_max_state)) > - return -EMSGSIZE; > - > - return 0; > -} > - > -static int thermal_genl_event_cdev_delete(struct param *p) > -{ > - if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, p->cdev_id)) > - return -EMSGSIZE; > - > - return 0; > -} > - > static int thermal_genl_event_cdev_state_update(struct param *p) > { > if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, > @@ -244,8 +223,6 @@ static cb_t event_cb[] = { > [THERMAL_GENL_EVENT_TZ_TRIP_UP] = thermal_genl_event_tz_trip_up, > [THERMAL_GENL_EVENT_TZ_TRIP_DOWN] = thermal_genl_event_tz_trip_down, > [THERMAL_GENL_EVENT_TZ_TRIP_CHANGE] = thermal_genl_event_tz_trip_change, > - [THERMAL_GENL_EVENT_CDEV_ADD] = thermal_genl_event_cdev_add, > - [THERMAL_GENL_EVENT_CDEV_DELETE] = thermal_genl_event_cdev_delete, > [THERMAL_GENL_EVENT_CDEV_STATE_UPDATE] = thermal_genl_event_cdev_state_update, > [THERMAL_GENL_EVENT_TZ_GOV_CHANGE] = thermal_genl_event_gov_change, > [THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE] = thermal_genl_event_cpu_capability_change, > @@ -348,28 +325,14 @@ int thermal_notify_tz_trip_change(const > return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p); > } > > -int thermal_notify_cdev_state_update(int cdev_id, int cdev_state) > +int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, > + int state) > { > - struct param p = { .cdev_id = cdev_id, .cdev_state = cdev_state }; > + struct param p = { .cdev_id = cdev->id, .cdev_state = state }; > > return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, &p); > } > > -int thermal_notify_cdev_add(int cdev_id, const char *name, int cdev_max_state) > -{ > - struct param p = { .cdev_id = cdev_id, .name = name, > - .cdev_max_state = cdev_max_state }; > - > - return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_ADD, &p); > -} > - > -int thermal_notify_cdev_delete(int cdev_id) > -{ > - struct param p = { .cdev_id = cdev_id }; > - > - return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_DELETE, &p); > -} > - > int thermal_notify_tz_gov_change(const struct thermal_zone_device *tz, > const char *name) > { > > > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On 15/12/2023 21:02, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The only actually used thermal netlink notification routine related > to cooling devices is thermal_notify_cdev_state_update(). The other > two, thermal_notify_cdev_add() and thermal_notify_cdev_delete(), are > never used. I think it is an oversight. These should be called in thermal_cooling_device_[un]register() > So as to get rid of dead code, drop thermal_notify_cdev_add/delete(), > which can be added back if they turn out to be ever needed, along with > the related code. > > In analogy with the previous thermal netlink API changes, redefine > thermal_notify_cdev_state_update() to take a const cdev pointer as its > first argument and let it extract the requisite information from there > by itself. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/thermal_helpers.c | 2 - > drivers/thermal/thermal_netlink.c | 43 ++------------------------------------ > drivers/thermal/thermal_netlink.h | 19 +++------------- > 3 files changed, 8 insertions(+), 56 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_netlink.h > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_netlink.h > +++ linux-pm/drivers/thermal/thermal_netlink.h > @@ -24,9 +24,8 @@ int thermal_notify_tz_trip_up(const stru > const struct thermal_trip *trip); > int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz, > const struct thermal_trip *trip); > -int thermal_notify_cdev_state_update(int cdev_id, int state); > -int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state); > -int thermal_notify_cdev_delete(int cdev_id); > +int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, > + int state); > int thermal_notify_tz_gov_change(const struct thermal_zone_device *tz, > const char *name); > int thermal_genl_sampling_temp(int id, int temp); > @@ -76,18 +75,8 @@ static inline int thermal_notify_tz_trip > return 0; > } > > -static inline int thermal_notify_cdev_state_update(int cdev_id, int state) > -{ > - return 0; > -} > - > -static inline int thermal_notify_cdev_add(int cdev_id, const char *name, > - int max_state) > -{ > - return 0; > -} > - > -static inline int thermal_notify_cdev_delete(int cdev_id) > +static inline int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, > + int state) > { > return 0; > } > Index: linux-pm/drivers/thermal/thermal_helpers.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_helpers.c > +++ linux-pm/drivers/thermal/thermal_helpers.c > @@ -152,7 +152,7 @@ static void thermal_cdev_set_cur_state(s > if (cdev->ops->set_cur_state(cdev, target)) > return; > > - thermal_notify_cdev_state_update(cdev->id, target); > + thermal_notify_cdev_state_update(cdev, target); > thermal_cooling_device_stats_update(cdev, target); > } > > Index: linux-pm/drivers/thermal/thermal_netlink.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_netlink.c > +++ linux-pm/drivers/thermal/thermal_netlink.c > @@ -147,27 +147,6 @@ static int thermal_genl_event_tz_trip_ch > return 0; > } > > -static int thermal_genl_event_cdev_add(struct param *p) > -{ > - if (nla_put_string(p->msg, THERMAL_GENL_ATTR_CDEV_NAME, > - p->name) || > - nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, > - p->cdev_id) || > - nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_MAX_STATE, > - p->cdev_max_state)) > - return -EMSGSIZE; > - > - return 0; > -} > - > -static int thermal_genl_event_cdev_delete(struct param *p) > -{ > - if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, p->cdev_id)) > - return -EMSGSIZE; > - > - return 0; > -} > - > static int thermal_genl_event_cdev_state_update(struct param *p) > { > if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, > @@ -244,8 +223,6 @@ static cb_t event_cb[] = { > [THERMAL_GENL_EVENT_TZ_TRIP_UP] = thermal_genl_event_tz_trip_up, > [THERMAL_GENL_EVENT_TZ_TRIP_DOWN] = thermal_genl_event_tz_trip_down, > [THERMAL_GENL_EVENT_TZ_TRIP_CHANGE] = thermal_genl_event_tz_trip_change, > - [THERMAL_GENL_EVENT_CDEV_ADD] = thermal_genl_event_cdev_add, > - [THERMAL_GENL_EVENT_CDEV_DELETE] = thermal_genl_event_cdev_delete, > [THERMAL_GENL_EVENT_CDEV_STATE_UPDATE] = thermal_genl_event_cdev_state_update, > [THERMAL_GENL_EVENT_TZ_GOV_CHANGE] = thermal_genl_event_gov_change, > [THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE] = thermal_genl_event_cpu_capability_change, > @@ -348,28 +325,14 @@ int thermal_notify_tz_trip_change(const > return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p); > } > > -int thermal_notify_cdev_state_update(int cdev_id, int cdev_state) > +int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, > + int state) > { > - struct param p = { .cdev_id = cdev_id, .cdev_state = cdev_state }; > + struct param p = { .cdev_id = cdev->id, .cdev_state = state }; > > return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, &p); > } > > -int thermal_notify_cdev_add(int cdev_id, const char *name, int cdev_max_state) > -{ > - struct param p = { .cdev_id = cdev_id, .name = name, > - .cdev_max_state = cdev_max_state }; > - > - return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_ADD, &p); > -} > - > -int thermal_notify_cdev_delete(int cdev_id) > -{ > - struct param p = { .cdev_id = cdev_id }; > - > - return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_DELETE, &p); > -} > - > int thermal_notify_tz_gov_change(const struct thermal_zone_device *tz, > const char *name) > { > > >
On Tue, Jan 9, 2024 at 12:36 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 15/12/2023 21:02, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > The only actually used thermal netlink notification routine related > > to cooling devices is thermal_notify_cdev_state_update(). The other > > two, thermal_notify_cdev_add() and thermal_notify_cdev_delete(), are > > never used. > > I think it is an oversight. These should be called in > thermal_cooling_device_[un]register() OK, so for now I'll just change them to take a (const) pointer to the cdev as the argument - the rest they can get from there. A patch adding them to thermal_cooling_device_[un]register() would be separate, for the next cycle. Where should they be called in there? Under thermal_list_lock I suppose? I'll send a replacement for the $subject patch later and I'll queue up the rest of the series. Thanks for the reviews!
Index: linux-pm/drivers/thermal/thermal_netlink.h =================================================================== --- linux-pm.orig/drivers/thermal/thermal_netlink.h +++ linux-pm/drivers/thermal/thermal_netlink.h @@ -24,9 +24,8 @@ int thermal_notify_tz_trip_up(const stru const struct thermal_trip *trip); int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz, const struct thermal_trip *trip); -int thermal_notify_cdev_state_update(int cdev_id, int state); -int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state); -int thermal_notify_cdev_delete(int cdev_id); +int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, + int state); int thermal_notify_tz_gov_change(const struct thermal_zone_device *tz, const char *name); int thermal_genl_sampling_temp(int id, int temp); @@ -76,18 +75,8 @@ static inline int thermal_notify_tz_trip return 0; } -static inline int thermal_notify_cdev_state_update(int cdev_id, int state) -{ - return 0; -} - -static inline int thermal_notify_cdev_add(int cdev_id, const char *name, - int max_state) -{ - return 0; -} - -static inline int thermal_notify_cdev_delete(int cdev_id) +static inline int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, + int state) { return 0; } Index: linux-pm/drivers/thermal/thermal_helpers.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_helpers.c +++ linux-pm/drivers/thermal/thermal_helpers.c @@ -152,7 +152,7 @@ static void thermal_cdev_set_cur_state(s if (cdev->ops->set_cur_state(cdev, target)) return; - thermal_notify_cdev_state_update(cdev->id, target); + thermal_notify_cdev_state_update(cdev, target); thermal_cooling_device_stats_update(cdev, target); } Index: linux-pm/drivers/thermal/thermal_netlink.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_netlink.c +++ linux-pm/drivers/thermal/thermal_netlink.c @@ -147,27 +147,6 @@ static int thermal_genl_event_tz_trip_ch return 0; } -static int thermal_genl_event_cdev_add(struct param *p) -{ - if (nla_put_string(p->msg, THERMAL_GENL_ATTR_CDEV_NAME, - p->name) || - nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, - p->cdev_id) || - nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_MAX_STATE, - p->cdev_max_state)) - return -EMSGSIZE; - - return 0; -} - -static int thermal_genl_event_cdev_delete(struct param *p) -{ - if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, p->cdev_id)) - return -EMSGSIZE; - - return 0; -} - static int thermal_genl_event_cdev_state_update(struct param *p) { if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, @@ -244,8 +223,6 @@ static cb_t event_cb[] = { [THERMAL_GENL_EVENT_TZ_TRIP_UP] = thermal_genl_event_tz_trip_up, [THERMAL_GENL_EVENT_TZ_TRIP_DOWN] = thermal_genl_event_tz_trip_down, [THERMAL_GENL_EVENT_TZ_TRIP_CHANGE] = thermal_genl_event_tz_trip_change, - [THERMAL_GENL_EVENT_CDEV_ADD] = thermal_genl_event_cdev_add, - [THERMAL_GENL_EVENT_CDEV_DELETE] = thermal_genl_event_cdev_delete, [THERMAL_GENL_EVENT_CDEV_STATE_UPDATE] = thermal_genl_event_cdev_state_update, [THERMAL_GENL_EVENT_TZ_GOV_CHANGE] = thermal_genl_event_gov_change, [THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE] = thermal_genl_event_cpu_capability_change, @@ -348,28 +325,14 @@ int thermal_notify_tz_trip_change(const return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p); } -int thermal_notify_cdev_state_update(int cdev_id, int cdev_state) +int thermal_notify_cdev_state_update(const struct thermal_cooling_device *cdev, + int state) { - struct param p = { .cdev_id = cdev_id, .cdev_state = cdev_state }; + struct param p = { .cdev_id = cdev->id, .cdev_state = state }; return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, &p); } -int thermal_notify_cdev_add(int cdev_id, const char *name, int cdev_max_state) -{ - struct param p = { .cdev_id = cdev_id, .name = name, - .cdev_max_state = cdev_max_state }; - - return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_ADD, &p); -} - -int thermal_notify_cdev_delete(int cdev_id) -{ - struct param p = { .cdev_id = cdev_id }; - - return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_DELETE, &p); -} - int thermal_notify_tz_gov_change(const struct thermal_zone_device *tz, const char *name) {