diff mbox series

[v3,09/12] thermal/core: Register with the trip points

Message ID 20220703183059.4133659-10-daniel.lezcano@linexp.org
State New
Headers show
Series thermal OF rework | expand

Commit Message

Daniel Lezcano July 3, 2022, 6:30 p.m. UTC
As we added the thermal trip points structure in the thermal zone,
let's extend the thermal zone register function to have the thermal
trip structures as a parameter and store it in the 'trips' field of
the thermal zone structure.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.c | 22 +++++++++++++++++-----
 drivers/thermal/thermal_core.h |  4 ++--
 include/linux/thermal.h        |  6 ++++++
 3 files changed, 25 insertions(+), 7 deletions(-)

Comments

Lukasz Luba July 4, 2022, 8:32 a.m. UTC | #1
On 7/3/22 19:30, Daniel Lezcano wrote:
> As we added the thermal trip points structure in the thermal zone,
> let's extend the thermal zone register function to have the thermal
> trip structures as a parameter and store it in the 'trips' field of
> the thermal zone structure.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.c | 22 +++++++++++++++++-----
>   drivers/thermal/thermal_core.h |  4 ++--
>   include/linux/thermal.h        |  6 ++++++
>   3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 434a675da245..e865c41d2320 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1167,8 +1167,9 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
>   }
>   
>   /**
> - * thermal_zone_device_register() - register a new thermal zone device
> + * thermal_zone_device_register_with_trips() - register a new thermal zone device
>    * @type:	the thermal zone device type
> + * @trips:	a pointer to an array of thermal trips
>    * @ntrips:	the number of trip points the thermal zone support
>    * @mask:	a bit string indicating the writeablility of trip points
>    * @devdata:	private device data
> @@ -1191,10 +1192,10 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
>    * IS_ERR*() helpers.
>    */
>   struct thermal_zone_device *
> -thermal_zone_device_register(const char *type, int ntrips, int mask,
> -			     void *devdata, struct thermal_zone_device_ops *ops,
> -			     struct thermal_zone_params *tzp, int passive_delay,
> -			     int polling_delay)
> +thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int ntrips, int mask,
> +					void *devdata, struct thermal_zone_device_ops *ops,
> +					struct thermal_zone_params *tzp, int passive_delay,
> +					int polling_delay)
>   {
>   	struct thermal_zone_device *tz;
>   	enum thermal_trip_type trip_type;
> @@ -1256,6 +1257,7 @@ thermal_zone_device_register(const char *type, int ntrips, int mask,
>   	tz->device.class = &thermal_class;
>   	tz->devdata = devdata;
>   	tz->ntrips = ntrips;
> +	tz->trips = trips;
>   
>   	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
>   	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> @@ -1331,6 +1333,16 @@ thermal_zone_device_register(const char *type, int ntrips, int mask,
>   	kfree(tz);
>   	return ERR_PTR(result);
>   }
> +
> +struct thermal_zone_device *thermal_zone_device_register(const char *type, int ntrips, int mask,
> +							 void *devdata, struct thermal_zone_device_ops *ops,
> +							 struct thermal_zone_params *tzp, int passive_delay,
> +							 int polling_delay)
> +{
> +	return thermal_zone_device_register_with_trips(type, NULL, ntrips, mask,
> +						       devdata, ops, tzp,
> +						       passive_delay, polling_delay);
> +}
>   EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>   
>   /**
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 84e341c1e0fc..bbe3ec26d12e 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -113,12 +113,12 @@ int thermal_build_list_of_policies(char *buf);
>   /* Helpers */
>   void thermal_zone_set_trips(struct thermal_zone_device *tz);
>   
> -static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone *tz)
> +static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone_device *tz)
>   {
>   	return tz->trips;
>   }
>   
> -static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
> +static inline int thermal_zone_get_ntrips(struct thermal_zone_device *tz)
>   {
>   	return tz->ntrips;
>   }
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 3733e23b6359..8cbe237a92d0 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -379,8 +379,14 @@ void devm_thermal_zone_of_sensor_unregister(struct device *dev,
>   struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
>   		void *, struct thermal_zone_device_ops *,
>   		struct thermal_zone_params *, int, int);
> +
>   void thermal_zone_device_unregister(struct thermal_zone_device *);
>   
> +struct thermal_zone_device *
> +thermal_zone_device_register_with_trips(const char *, struct thermal_trip *, int, int,
> +					void *, struct thermal_zone_device_ops *,
> +					struct thermal_zone_params *, int, int);
> +
>   int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>   				     struct thermal_cooling_device *,
>   				     unsigned long, unsigned long,

Apart from 'ntrips', which might be 'num_trips', this looks OK.
Zhang Rui July 5, 2022, 2:03 a.m. UTC | #2
On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> As we added the thermal trip points structure in the thermal zone,
> let's extend the thermal zone register function to have the thermal
> trip structures as a parameter and store it in the 'trips' field of
> the thermal zone structure.

Just FYI.

I proposed a small topic for this year' LPC about the
thermal_zone_device_register() parameters.
We have more and more parameters introduced, IMO, it's better to use a
unified structure for registration phase configurations, or reuse
struct thermal_zone_params.
In this way, when a new parameter is needed, we only need to introduce
a new field in the structure, rather than update every caller of this
API, or introduce new wrapper functions like we did in this patch.

thanks,
rui
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>  drivers/thermal/thermal_core.c | 22 +++++++++++++++++-----
>  drivers/thermal/thermal_core.h |  4 ++--
>  include/linux/thermal.h        |  6 ++++++
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 434a675da245..e865c41d2320 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1167,8 +1167,9 @@ static void thermal_set_delay_jiffies(unsigned
> long *delay_jiffies, int delay_ms
>  }
>  
>  /**
> - * thermal_zone_device_register() - register a new thermal zone
> device
> + * thermal_zone_device_register_with_trips() - register a new
> thermal zone device
>   * @type:	the thermal zone device type
> + * @trips:	a pointer to an array of thermal trips
>   * @ntrips:	the number of trip points the thermal zone support
>   * @mask:	a bit string indicating the writeablility of trip
> points
>   * @devdata:	private device data
> @@ -1191,10 +1192,10 @@ static void
> thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
>   * IS_ERR*() helpers.
>   */
>  struct thermal_zone_device *
> -thermal_zone_device_register(const char *type, int ntrips, int mask,
> -			     void *devdata, struct
> thermal_zone_device_ops *ops,
> -			     struct thermal_zone_params *tzp, int
> passive_delay,
> -			     int polling_delay)
> +thermal_zone_device_register_with_trips(const char *type, struct
> thermal_trip *trips, int ntrips, int mask,
> +					void *devdata, struct
> thermal_zone_device_ops *ops,
> +					struct thermal_zone_params
> *tzp, int passive_delay,
> +					int polling_delay)
>  {
>  	struct thermal_zone_device *tz;
>  	enum thermal_trip_type trip_type;
> @@ -1256,6 +1257,7 @@ thermal_zone_device_register(const char *type,
> int ntrips, int mask,
>  	tz->device.class = &thermal_class;
>  	tz->devdata = devdata;
>  	tz->ntrips = ntrips;
> +	tz->trips = trips;
>  
>  	thermal_set_delay_jiffies(&tz->passive_delay_jiffies,
> passive_delay);
>  	thermal_set_delay_jiffies(&tz->polling_delay_jiffies,
> polling_delay);
> @@ -1331,6 +1333,16 @@ thermal_zone_device_register(const char *type,
> int ntrips, int mask,
>  	kfree(tz);
>  	return ERR_PTR(result);
>  }
> +
> +struct thermal_zone_device *thermal_zone_device_register(const char
> *type, int ntrips, int mask,
> +							 void *devdata,
> struct thermal_zone_device_ops *ops,
> +							 struct
> thermal_zone_params *tzp, int passive_delay,
> +							 int
> polling_delay)
> +{
> +	return thermal_zone_device_register_with_trips(type, NULL,
> ntrips, mask,
> +						       devdata, ops,
> tzp,
> +						       passive_delay,
> polling_delay);
> +}
>  EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>  
>  /**
> diff --git a/drivers/thermal/thermal_core.h
> b/drivers/thermal/thermal_core.h
> index 84e341c1e0fc..bbe3ec26d12e 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -113,12 +113,12 @@ int thermal_build_list_of_policies(char *buf);
>  /* Helpers */
>  void thermal_zone_set_trips(struct thermal_zone_device *tz);
>  
> -static inline struct thermal_trip *thermal_zone_get_trips(struct
> thermal_zone *tz)
> +static inline struct thermal_trip *thermal_zone_get_trips(struct
> thermal_zone_device *tz)
>  {
>  	return tz->trips;
>  }
>  
> -static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
> +static inline int thermal_zone_get_ntrips(struct thermal_zone_device
> *tz)
>  {
>  	return tz->ntrips;
>  }
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 3733e23b6359..8cbe237a92d0 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -379,8 +379,14 @@ void
> devm_thermal_zone_of_sensor_unregister(struct device *dev,
>  struct thermal_zone_device *thermal_zone_device_register(const char
> *, int, int,
>  		void *, struct thermal_zone_device_ops *,
>  		struct thermal_zone_params *, int, int);
> +
>  void thermal_zone_device_unregister(struct thermal_zone_device *);
>  
> +struct thermal_zone_device *
> +thermal_zone_device_register_with_trips(const char *, struct
> thermal_trip *, int, int,
> +					void *, struct
> thermal_zone_device_ops *,
> +					struct thermal_zone_params *,
> int, int);
> +
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *,
> int,
>  				     struct thermal_cooling_device *,
>  				     unsigned long, unsigned long,
Rafael J. Wysocki July 5, 2022, 1:59 p.m. UTC | #3
On Tue, Jul 5, 2022 at 4:03 AM Zhang Rui <rui.zhang@intel.com> wrote:
>
> On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> > As we added the thermal trip points structure in the thermal zone,
> > let's extend the thermal zone register function to have the thermal
> > trip structures as a parameter and store it in the 'trips' field of
> > the thermal zone structure.
>
> Just FYI.
>
> I proposed a small topic for this year' LPC about the
> thermal_zone_device_register() parameters.
> We have more and more parameters introduced, IMO, it's better to use a
> unified structure for registration phase configurations, or reuse
> struct thermal_zone_params.
> In this way, when a new parameter is needed, we only need to introduce
> a new field in the structure, rather than update every caller of this
> API, or introduce new wrapper functions like we did in this patch.

Sounds reasonable to me.
diff mbox series

Patch

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 434a675da245..e865c41d2320 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1167,8 +1167,9 @@  static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
 }
 
 /**
- * thermal_zone_device_register() - register a new thermal zone device
+ * thermal_zone_device_register_with_trips() - register a new thermal zone device
  * @type:	the thermal zone device type
+ * @trips:	a pointer to an array of thermal trips
  * @ntrips:	the number of trip points the thermal zone support
  * @mask:	a bit string indicating the writeablility of trip points
  * @devdata:	private device data
@@ -1191,10 +1192,10 @@  static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
  * IS_ERR*() helpers.
  */
 struct thermal_zone_device *
-thermal_zone_device_register(const char *type, int ntrips, int mask,
-			     void *devdata, struct thermal_zone_device_ops *ops,
-			     struct thermal_zone_params *tzp, int passive_delay,
-			     int polling_delay)
+thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int ntrips, int mask,
+					void *devdata, struct thermal_zone_device_ops *ops,
+					struct thermal_zone_params *tzp, int passive_delay,
+					int polling_delay)
 {
 	struct thermal_zone_device *tz;
 	enum thermal_trip_type trip_type;
@@ -1256,6 +1257,7 @@  thermal_zone_device_register(const char *type, int ntrips, int mask,
 	tz->device.class = &thermal_class;
 	tz->devdata = devdata;
 	tz->ntrips = ntrips;
+	tz->trips = trips;
 
 	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
 	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
@@ -1331,6 +1333,16 @@  thermal_zone_device_register(const char *type, int ntrips, int mask,
 	kfree(tz);
 	return ERR_PTR(result);
 }
+
+struct thermal_zone_device *thermal_zone_device_register(const char *type, int ntrips, int mask,
+							 void *devdata, struct thermal_zone_device_ops *ops,
+							 struct thermal_zone_params *tzp, int passive_delay,
+							 int polling_delay)
+{
+	return thermal_zone_device_register_with_trips(type, NULL, ntrips, mask,
+						       devdata, ops, tzp,
+						       passive_delay, polling_delay);
+}
 EXPORT_SYMBOL_GPL(thermal_zone_device_register);
 
 /**
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 84e341c1e0fc..bbe3ec26d12e 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -113,12 +113,12 @@  int thermal_build_list_of_policies(char *buf);
 /* Helpers */
 void thermal_zone_set_trips(struct thermal_zone_device *tz);
 
-static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone *tz)
+static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone_device *tz)
 {
 	return tz->trips;
 }
 
-static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
+static inline int thermal_zone_get_ntrips(struct thermal_zone_device *tz)
 {
 	return tz->ntrips;
 }
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 3733e23b6359..8cbe237a92d0 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -379,8 +379,14 @@  void devm_thermal_zone_of_sensor_unregister(struct device *dev,
 struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
 		void *, struct thermal_zone_device_ops *,
 		struct thermal_zone_params *, int, int);
+
 void thermal_zone_device_unregister(struct thermal_zone_device *);
 
+struct thermal_zone_device *
+thermal_zone_device_register_with_trips(const char *, struct thermal_trip *, int, int,
+					void *, struct thermal_zone_device_ops *,
+					struct thermal_zone_params *, int, int);
+
 int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
 				     struct thermal_cooling_device *,
 				     unsigned long, unsigned long,