diff mbox series

[v1,6/6] thermal: netlink: Rework cdev-related notify API

Message ID 7628882.EvYhyI6sBW@kreacher (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series thermal: netlink: Redefine the API and drop unused code | expand

Commit Message

Rafael J. Wysocki Dec. 15, 2023, 8:02 p.m. UTC 
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(-)

Comments

Lukasz Luba Jan. 3, 2024, 8:10 p.m. UTC | #1 
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>
Daniel Lezcano Jan. 9, 2024, 11:36 a.m. UTC | #2 
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)
>   {
> 
> 
>
Rafael J. Wysocki Jan. 9, 2024, 12:23 p.m. UTC | #3 
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!
diff mbox series

Patch

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)
 {