diff mbox series

[v1,13/17] mlxsw: core_thermal: Use the .should_bind() thermal zone callback

Message ID 9002154.VV5PYv0bhD@rjwysocki.net
State New
Headers show
Series [v1,01/17] thermal: core: Fold two functions into their respective callers | expand

Commit Message

Rafael J. Wysocki July 30, 2024, 6:34 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make the mlxsw core_thermal driver use the .should_bind() thermal zone
callback to provide the thermal core with the information on whether or
not to bind the given cooling device to the given trip point in the
given thermal zone.  If it returns 'true', the thermal core will bind
the cooling device to the trip and the corresponding unbinding will be
taken care of automatically by the core on the removal of the involved
thermal zone or cooling device.

It replaces the .bind() and .unbind() thermal zone callbacks (in 3
places) which assumed the same trip points ordering in the driver
and in the thermal core (that may not be true any more in the
future).  The .bind() callbacks used loops over trip point indices
to call thermal_zone_bind_cooling_device() for the same cdev (once
it had been verified) and all of the trip points, but they passed
different 'upper' and 'lower' values to it for each trip.

To retain the original functionality, the .should_bind() callbacks
need to use the same 'upper' and 'lower' values that would be used
by the corresponding .bind() callbacks when they are about about to
return 'true'.  To that end, the 'priv' field of each trip is set
during the thermal zone initialization to point to the corresponding
'state' object containing the maximum and minimum cooling states of
the cooling device.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This patch only depends on patch [09/17].

---
 drivers/net/ethernet/mellanox/mlxsw/core_thermal.c |  121 +++++----------------
 1 file changed, 34 insertions(+), 87 deletions(-)

Comments

Ido Schimmel July 31, 2024, 12:43 p.m. UTC | #1
On Tue, Jul 30, 2024 at 08:34:45PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Make the mlxsw core_thermal driver use the .should_bind() thermal zone
> callback to provide the thermal core with the information on whether or
> not to bind the given cooling device to the given trip point in the
> given thermal zone.  If it returns 'true', the thermal core will bind
> the cooling device to the trip and the corresponding unbinding will be
> taken care of automatically by the core on the removal of the involved
> thermal zone or cooling device.
> 
> It replaces the .bind() and .unbind() thermal zone callbacks (in 3
> places) which assumed the same trip points ordering in the driver
> and in the thermal core (that may not be true any more in the
> future).  The .bind() callbacks used loops over trip point indices
> to call thermal_zone_bind_cooling_device() for the same cdev (once
> it had been verified) and all of the trip points, but they passed
> different 'upper' and 'lower' values to it for each trip.
> 
> To retain the original functionality, the .should_bind() callbacks
> need to use the same 'upper' and 'lower' values that would be used
> by the corresponding .bind() callbacks when they are about about to

Nit: s/about about/about/

> return 'true'.  To that end, the 'priv' field of each trip is set
> during the thermal zone initialization to point to the corresponding
> 'state' object containing the maximum and minimum cooling states of
> the cooling device.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Please see more comments below, but this patch is going to conflict with
the series at [1] which is currently under review. How do you want to
handle that?

https://lore.kernel.org/netdev/cover.1722345311.git.petrm@nvidia.com/

> ---
> 
> This patch only depends on patch [09/17].
> 
> ---
>  drivers/net/ethernet/mellanox/mlxsw/core_thermal.c |  121 +++++----------------
>  1 file changed, 34 insertions(+), 87 deletions(-)
> 
> Index: linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -165,52 +165,22 @@ static int mlxsw_get_cooling_device_idx(
>  	return -ENODEV;
>  }
>  
> -static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
> -			      struct thermal_cooling_device *cdev)
> +static bool mlxsw_thermal_should_bind(struct thermal_zone_device *tzdev,
> +				      const struct thermal_trip *trip,
> +				      struct thermal_cooling_device *cdev,
> +				      struct cooling_spec *c)
>  {
>  	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
> -	struct device *dev = thermal->bus_info->dev;
> -	int i, err;
> +	const struct mlxsw_cooling_states *state = trip->priv;
>  
>  	/* If the cooling device is one of ours bind it */
>  	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
> -		return 0;
> +		return false;
>  
> -	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
> -		const struct mlxsw_cooling_states *state = &thermal->cooling_states[i];
> +	c->upper = state->max_state;
> +	c->lower = state->min_state;
>  
> -		err = thermal_zone_bind_cooling_device(tzdev, i, cdev,
> -						       state->max_state,
> -						       state->min_state,
> -						       THERMAL_WEIGHT_DEFAULT);
> -		if (err < 0) {
> -			dev_err(dev, "Failed to bind cooling device to trip %d\n", i);
> -			return err;
> -		}
> -	}
> -	return 0;
> -}
> -
> -static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
> -				struct thermal_cooling_device *cdev)
> -{
> -	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
> -	struct device *dev = thermal->bus_info->dev;
> -	int i;
> -	int err;
> -
> -	/* If the cooling device is our one unbind it */
> -	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
> -		return 0;
> -
> -	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
> -		err = thermal_zone_unbind_cooling_device(tzdev, i, cdev);
> -		if (err < 0) {
> -			dev_err(dev, "Failed to unbind cooling device\n");
> -			return err;
> -		}
> -	}
> -	return 0;
> +	return true;
>  }
>  
>  static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
> @@ -239,59 +209,29 @@ static struct thermal_zone_params mlxsw_
>  	.no_hwmon = true,
>  };
>  
> -static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> -	.bind = mlxsw_thermal_bind,
> -	.unbind = mlxsw_thermal_unbind,
> -	.get_temp = mlxsw_thermal_get_temp,
> -};

Is there a reason to move 'mlxsw_thermal_ops' below?

> -
> -static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
> -				     struct thermal_cooling_device *cdev)
> +static bool mlxsw_thermal_module_should_bind(struct thermal_zone_device *tzdev,
> +					     const struct thermal_trip *trip,
> +					     struct thermal_cooling_device *cdev,
> +					     struct cooling_spec *c)
>  {
>  	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
>  	struct mlxsw_thermal *thermal = tz->parent;
> -	int i, j, err;
> +	const struct mlxsw_cooling_states *state = trip->priv;

Please place it between 'tz' and 'thermal'. Networking code tries to
maintain reverse xmas tree ordering for local variables.

>  
>  	/* If the cooling device is one of ours bind it */
>  	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
> -		return 0;
> +		return false;
>  
> -	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
> -		const struct mlxsw_cooling_states *state = &tz->cooling_states[i];
> +	c->upper = state->max_state;
> +	c->lower = state->min_state;
>  
> -		err = thermal_zone_bind_cooling_device(tzdev, i, cdev,
> -						       state->max_state,
> -						       state->min_state,
> -						       THERMAL_WEIGHT_DEFAULT);
> -		if (err < 0)
> -			goto err_thermal_zone_bind_cooling_device;
> -	}
> -	return 0;
> -
> -err_thermal_zone_bind_cooling_device:
> -	for (j = i - 1; j >= 0; j--)
> -		thermal_zone_unbind_cooling_device(tzdev, j, cdev);
> -	return err;
> +	return true;
>  }
>  
> -static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
> -				       struct thermal_cooling_device *cdev)
> -{
> -	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
> -	struct mlxsw_thermal *thermal = tz->parent;
> -	int i;
> -	int err;
> -
> -	/* If the cooling device is one of ours unbind it */
> -	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
> -		return 0;
> -
> -	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
> -		err = thermal_zone_unbind_cooling_device(tzdev, i, cdev);
> -		WARN_ON(err);
> -	}
> -	return err;
> -}
> +static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> +	.should_bind = mlxsw_thermal_should_bind,
> +	.get_temp = mlxsw_thermal_get_temp,
> +};
>  
>  static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
>  					 int *p_temp)
> @@ -313,8 +253,7 @@ static int mlxsw_thermal_module_temp_get
>  }
>  
>  static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
> -	.bind		= mlxsw_thermal_module_bind,
> -	.unbind		= mlxsw_thermal_module_unbind,
> +	.should_bind	= mlxsw_thermal_module_should_bind,
>  	.get_temp	= mlxsw_thermal_module_temp_get,
>  };
>  
> @@ -342,8 +281,7 @@ static int mlxsw_thermal_gearbox_temp_ge
>  }
>  
>  static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
> -	.bind		= mlxsw_thermal_module_bind,
> -	.unbind		= mlxsw_thermal_module_unbind,
> +	.should_bind	= mlxsw_thermal_module_should_bind,
>  	.get_temp	= mlxsw_thermal_gearbox_temp_get,
>  };
>  
> @@ -451,6 +389,7 @@ mlxsw_thermal_module_init(struct device
>  			  struct mlxsw_thermal_area *area, u8 module)
>  {
>  	struct mlxsw_thermal_module *module_tz;
> +	int i;
>  
>  	module_tz = &area->tz_module_arr[module];
>  	/* Skip if parent is already set (case of port split). */
> @@ -465,6 +404,8 @@ mlxsw_thermal_module_init(struct device
>  	       sizeof(thermal->trips));
>  	memcpy(module_tz->cooling_states, default_cooling_states,
>  	       sizeof(thermal->cooling_states));
> +	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++)
> +		module_tz->trips[i].priv = &module_tz->cooling_states[i];
>  }
>  
>  static void mlxsw_thermal_module_fini(struct mlxsw_thermal_module *module_tz)
> @@ -579,7 +520,7 @@ mlxsw_thermal_gearboxes_init(struct devi
>  	struct mlxsw_thermal_module *gearbox_tz;
>  	char mgpir_pl[MLXSW_REG_MGPIR_LEN];
>  	u8 gbox_num;
> -	int i;
> +	int i, j;
>  	int err;
>  
>  	mlxsw_reg_mgpir_pack(mgpir_pl, area->slot_index);
> @@ -606,6 +547,9 @@ mlxsw_thermal_gearboxes_init(struct devi
>  		       sizeof(thermal->trips));
>  		memcpy(gearbox_tz->cooling_states, default_cooling_states,
>  		       sizeof(thermal->cooling_states));
> +		for (j = 0; j < MLXSW_THERMAL_NUM_TRIPS; j++)
> +			gearbox_tz->trips[j].priv = &gearbox_tz->cooling_states[j];
> +
>  		gearbox_tz->module = i;
>  		gearbox_tz->parent = thermal;
>  		gearbox_tz->slot_index = area->slot_index;
> @@ -722,6 +666,9 @@ int mlxsw_thermal_init(struct mlxsw_core
>  	thermal->bus_info = bus_info;
>  	memcpy(thermal->trips, default_thermal_trips, sizeof(thermal->trips));
>  	memcpy(thermal->cooling_states, default_cooling_states, sizeof(thermal->cooling_states));
> +	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++)
> +		thermal->trips[i].priv = &thermal->cooling_states[i];
> +
>  	thermal->line_cards[0].slot_index = 0;
>  
>  	err = mlxsw_reg_query(thermal->core, MLXSW_REG(mfcr), mfcr_pl);
> 
> 
>
Rafael J. Wysocki July 31, 2024, 1:01 p.m. UTC | #2
On Wed, Jul 31, 2024 at 2:43 PM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Tue, Jul 30, 2024 at 08:34:45PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Make the mlxsw core_thermal driver use the .should_bind() thermal zone
> > callback to provide the thermal core with the information on whether or
> > not to bind the given cooling device to the given trip point in the
> > given thermal zone.  If it returns 'true', the thermal core will bind
> > the cooling device to the trip and the corresponding unbinding will be
> > taken care of automatically by the core on the removal of the involved
> > thermal zone or cooling device.
> >
> > It replaces the .bind() and .unbind() thermal zone callbacks (in 3
> > places) which assumed the same trip points ordering in the driver
> > and in the thermal core (that may not be true any more in the
> > future).  The .bind() callbacks used loops over trip point indices
> > to call thermal_zone_bind_cooling_device() for the same cdev (once
> > it had been verified) and all of the trip points, but they passed
> > different 'upper' and 'lower' values to it for each trip.
> >
> > To retain the original functionality, the .should_bind() callbacks
> > need to use the same 'upper' and 'lower' values that would be used
> > by the corresponding .bind() callbacks when they are about about to
>
> Nit: s/about about/about/

Yes, thanks!

> > return 'true'.  To that end, the 'priv' field of each trip is set
> > during the thermal zone initialization to point to the corresponding
> > 'state' object containing the maximum and minimum cooling states of
> > the cooling device.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Please see more comments below, but this patch is going to conflict with
> the series at [1] which is currently under review. How do you want to
> handle that?
>
> https://lore.kernel.org/netdev/cover.1722345311.git.petrm@nvidia.com/

I may be missing something, but I don't see conflicts between this
patch and the series above that would be hard to resolve at merge
time.

Anyway, I'll try to apply the above series locally and merge it with
this patch, thanks for the heads up!

> > ---
> >
> > This patch only depends on patch [09/17].
> >
> > ---
> >  drivers/net/ethernet/mellanox/mlxsw/core_thermal.c |  121 +++++----------------
> >  1 file changed, 34 insertions(+), 87 deletions(-)
> >
> > Index: linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> > ===================================================================
> > --- linux-pm.orig/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> > +++ linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> > @@ -165,52 +165,22 @@ static int mlxsw_get_cooling_device_idx(
> >       return -ENODEV;
> >  }
> >
> > -static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
> > -                           struct thermal_cooling_device *cdev)
> > +static bool mlxsw_thermal_should_bind(struct thermal_zone_device *tzdev,
> > +                                   const struct thermal_trip *trip,
> > +                                   struct thermal_cooling_device *cdev,
> > +                                   struct cooling_spec *c)
> >  {
> >       struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
> > -     struct device *dev = thermal->bus_info->dev;
> > -     int i, err;
> > +     const struct mlxsw_cooling_states *state = trip->priv;
> >
> >       /* If the cooling device is one of ours bind it */
> >       if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
> > -             return 0;
> > +             return false;
> >
> > -     for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
> > -             const struct mlxsw_cooling_states *state = &thermal->cooling_states[i];
> > +     c->upper = state->max_state;
> > +     c->lower = state->min_state;
> >
> > -             err = thermal_zone_bind_cooling_device(tzdev, i, cdev,
> > -                                                    state->max_state,
> > -                                                    state->min_state,
> > -                                                    THERMAL_WEIGHT_DEFAULT);
> > -             if (err < 0) {
> > -                     dev_err(dev, "Failed to bind cooling device to trip %d\n", i);
> > -                     return err;
> > -             }
> > -     }
> > -     return 0;
> > -}
> > -
> > -static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
> > -                             struct thermal_cooling_device *cdev)
> > -{
> > -     struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
> > -     struct device *dev = thermal->bus_info->dev;
> > -     int i;
> > -     int err;
> > -
> > -     /* If the cooling device is our one unbind it */
> > -     if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
> > -             return 0;
> > -
> > -     for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
> > -             err = thermal_zone_unbind_cooling_device(tzdev, i, cdev);
> > -             if (err < 0) {
> > -                     dev_err(dev, "Failed to unbind cooling device\n");
> > -                     return err;
> > -             }
> > -     }
> > -     return 0;
> > +     return true;
> >  }
> >
> >  static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
> > @@ -239,59 +209,29 @@ static struct thermal_zone_params mlxsw_
> >       .no_hwmon = true,
> >  };
> >
> > -static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> > -     .bind = mlxsw_thermal_bind,
> > -     .unbind = mlxsw_thermal_unbind,
> > -     .get_temp = mlxsw_thermal_get_temp,
> > -};
>
> Is there a reason to move 'mlxsw_thermal_ops' below?

Not really, it can stay here.

> > -
> > -static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
> > -                                  struct thermal_cooling_device *cdev)
> > +static bool mlxsw_thermal_module_should_bind(struct thermal_zone_device *tzdev,
> > +                                          const struct thermal_trip *trip,
> > +                                          struct thermal_cooling_device *cdev,
> > +                                          struct cooling_spec *c)
> >  {
> >       struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
> >       struct mlxsw_thermal *thermal = tz->parent;
> > -     int i, j, err;
> > +     const struct mlxsw_cooling_states *state = trip->priv;
>
> Please place it between 'tz' and 'thermal'. Networking code tries to
> maintain reverse xmas tree ordering for local variables.

I will.

Thanks for the comments!
Rafael J. Wysocki July 31, 2024, 2:32 p.m. UTC | #3
On Wed, Jul 31, 2024 at 3:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Jul 31, 2024 at 2:43 PM Ido Schimmel <idosch@nvidia.com> wrote:
> >
> > On Tue, Jul 30, 2024 at 08:34:45PM +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Make the mlxsw core_thermal driver use the .should_bind() thermal zone
> > > callback to provide the thermal core with the information on whether or
> > > not to bind the given cooling device to the given trip point in the
> > > given thermal zone.  If it returns 'true', the thermal core will bind
> > > the cooling device to the trip and the corresponding unbinding will be
> > > taken care of automatically by the core on the removal of the involved
> > > thermal zone or cooling device.
> > >
> > > It replaces the .bind() and .unbind() thermal zone callbacks (in 3
> > > places) which assumed the same trip points ordering in the driver
> > > and in the thermal core (that may not be true any more in the
> > > future).  The .bind() callbacks used loops over trip point indices
> > > to call thermal_zone_bind_cooling_device() for the same cdev (once
> > > it had been verified) and all of the trip points, but they passed
> > > different 'upper' and 'lower' values to it for each trip.
> > >
> > > To retain the original functionality, the .should_bind() callbacks
> > > need to use the same 'upper' and 'lower' values that would be used
> > > by the corresponding .bind() callbacks when they are about about to
> >
> > Nit: s/about about/about/
>
> Yes, thanks!
>
> > > return 'true'.  To that end, the 'priv' field of each trip is set
> > > during the thermal zone initialization to point to the corresponding
> > > 'state' object containing the maximum and minimum cooling states of
> > > the cooling device.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Please see more comments below, but this patch is going to conflict with
> > the series at [1] which is currently under review. How do you want to
> > handle that?
> >
> > https://lore.kernel.org/netdev/cover.1722345311.git.petrm@nvidia.com/
>
> I may be missing something, but I don't see conflicts between this
> patch and the series above that would be hard to resolve at merge
> time.
>
> Anyway, I'll try to apply the above series locally and merge it with
> this patch, thanks for the heads up!

So there is only one merge conflict that's straightforward to resolve
(as far as I'm concerned).  My resolution of it is attached, FWIW.

In my view the changes in the series above and this patch are mostly
independent of each other.

Thanks!
diff mbox series

Patch

Index: linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
===================================================================
--- linux-pm.orig/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -165,52 +165,22 @@  static int mlxsw_get_cooling_device_idx(
 	return -ENODEV;
 }
 
-static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
-			      struct thermal_cooling_device *cdev)
+static bool mlxsw_thermal_should_bind(struct thermal_zone_device *tzdev,
+				      const struct thermal_trip *trip,
+				      struct thermal_cooling_device *cdev,
+				      struct cooling_spec *c)
 {
 	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
-	struct device *dev = thermal->bus_info->dev;
-	int i, err;
+	const struct mlxsw_cooling_states *state = trip->priv;
 
 	/* If the cooling device is one of ours bind it */
 	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
-		return 0;
+		return false;
 
-	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
-		const struct mlxsw_cooling_states *state = &thermal->cooling_states[i];
+	c->upper = state->max_state;
+	c->lower = state->min_state;
 
-		err = thermal_zone_bind_cooling_device(tzdev, i, cdev,
-						       state->max_state,
-						       state->min_state,
-						       THERMAL_WEIGHT_DEFAULT);
-		if (err < 0) {
-			dev_err(dev, "Failed to bind cooling device to trip %d\n", i);
-			return err;
-		}
-	}
-	return 0;
-}
-
-static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
-				struct thermal_cooling_device *cdev)
-{
-	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
-	struct device *dev = thermal->bus_info->dev;
-	int i;
-	int err;
-
-	/* If the cooling device is our one unbind it */
-	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
-		return 0;
-
-	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
-		err = thermal_zone_unbind_cooling_device(tzdev, i, cdev);
-		if (err < 0) {
-			dev_err(dev, "Failed to unbind cooling device\n");
-			return err;
-		}
-	}
-	return 0;
+	return true;
 }
 
 static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
@@ -239,59 +209,29 @@  static struct thermal_zone_params mlxsw_
 	.no_hwmon = true,
 };
 
-static struct thermal_zone_device_ops mlxsw_thermal_ops = {
-	.bind = mlxsw_thermal_bind,
-	.unbind = mlxsw_thermal_unbind,
-	.get_temp = mlxsw_thermal_get_temp,
-};
-
-static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
-				     struct thermal_cooling_device *cdev)
+static bool mlxsw_thermal_module_should_bind(struct thermal_zone_device *tzdev,
+					     const struct thermal_trip *trip,
+					     struct thermal_cooling_device *cdev,
+					     struct cooling_spec *c)
 {
 	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
 	struct mlxsw_thermal *thermal = tz->parent;
-	int i, j, err;
+	const struct mlxsw_cooling_states *state = trip->priv;
 
 	/* If the cooling device is one of ours bind it */
 	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
-		return 0;
+		return false;
 
-	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
-		const struct mlxsw_cooling_states *state = &tz->cooling_states[i];
+	c->upper = state->max_state;
+	c->lower = state->min_state;
 
-		err = thermal_zone_bind_cooling_device(tzdev, i, cdev,
-						       state->max_state,
-						       state->min_state,
-						       THERMAL_WEIGHT_DEFAULT);
-		if (err < 0)
-			goto err_thermal_zone_bind_cooling_device;
-	}
-	return 0;
-
-err_thermal_zone_bind_cooling_device:
-	for (j = i - 1; j >= 0; j--)
-		thermal_zone_unbind_cooling_device(tzdev, j, cdev);
-	return err;
+	return true;
 }
 
-static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
-				       struct thermal_cooling_device *cdev)
-{
-	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
-	struct mlxsw_thermal *thermal = tz->parent;
-	int i;
-	int err;
-
-	/* If the cooling device is one of ours unbind it */
-	if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
-		return 0;
-
-	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
-		err = thermal_zone_unbind_cooling_device(tzdev, i, cdev);
-		WARN_ON(err);
-	}
-	return err;
-}
+static struct thermal_zone_device_ops mlxsw_thermal_ops = {
+	.should_bind = mlxsw_thermal_should_bind,
+	.get_temp = mlxsw_thermal_get_temp,
+};
 
 static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
 					 int *p_temp)
@@ -313,8 +253,7 @@  static int mlxsw_thermal_module_temp_get
 }
 
 static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
-	.bind		= mlxsw_thermal_module_bind,
-	.unbind		= mlxsw_thermal_module_unbind,
+	.should_bind	= mlxsw_thermal_module_should_bind,
 	.get_temp	= mlxsw_thermal_module_temp_get,
 };
 
@@ -342,8 +281,7 @@  static int mlxsw_thermal_gearbox_temp_ge
 }
 
 static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
-	.bind		= mlxsw_thermal_module_bind,
-	.unbind		= mlxsw_thermal_module_unbind,
+	.should_bind	= mlxsw_thermal_module_should_bind,
 	.get_temp	= mlxsw_thermal_gearbox_temp_get,
 };
 
@@ -451,6 +389,7 @@  mlxsw_thermal_module_init(struct device
 			  struct mlxsw_thermal_area *area, u8 module)
 {
 	struct mlxsw_thermal_module *module_tz;
+	int i;
 
 	module_tz = &area->tz_module_arr[module];
 	/* Skip if parent is already set (case of port split). */
@@ -465,6 +404,8 @@  mlxsw_thermal_module_init(struct device
 	       sizeof(thermal->trips));
 	memcpy(module_tz->cooling_states, default_cooling_states,
 	       sizeof(thermal->cooling_states));
+	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++)
+		module_tz->trips[i].priv = &module_tz->cooling_states[i];
 }
 
 static void mlxsw_thermal_module_fini(struct mlxsw_thermal_module *module_tz)
@@ -579,7 +520,7 @@  mlxsw_thermal_gearboxes_init(struct devi
 	struct mlxsw_thermal_module *gearbox_tz;
 	char mgpir_pl[MLXSW_REG_MGPIR_LEN];
 	u8 gbox_num;
-	int i;
+	int i, j;
 	int err;
 
 	mlxsw_reg_mgpir_pack(mgpir_pl, area->slot_index);
@@ -606,6 +547,9 @@  mlxsw_thermal_gearboxes_init(struct devi
 		       sizeof(thermal->trips));
 		memcpy(gearbox_tz->cooling_states, default_cooling_states,
 		       sizeof(thermal->cooling_states));
+		for (j = 0; j < MLXSW_THERMAL_NUM_TRIPS; j++)
+			gearbox_tz->trips[j].priv = &gearbox_tz->cooling_states[j];
+
 		gearbox_tz->module = i;
 		gearbox_tz->parent = thermal;
 		gearbox_tz->slot_index = area->slot_index;
@@ -722,6 +666,9 @@  int mlxsw_thermal_init(struct mlxsw_core
 	thermal->bus_info = bus_info;
 	memcpy(thermal->trips, default_thermal_trips, sizeof(thermal->trips));
 	memcpy(thermal->cooling_states, default_cooling_states, sizeof(thermal->cooling_states));
+	for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++)
+		thermal->trips[i].priv = &thermal->cooling_states[i];
+
 	thermal->line_cards[0].slot_index = 0;
 
 	err = mlxsw_reg_query(thermal->core, MLXSW_REG(mfcr), mfcr_pl);