[PATCH 1/2] Revert "mlxsw: core: Use different get_trend() callbacks for different thermal zones"

[PATCH 1/2] Revert "mlxsw: core: Use different get_trend() callbacks for different thermal zones"

[Daniel Lezcano]

This reverts commit 2dc2f760052da4925482ecdcdc5c94d4a599153c.

As discussed in the thread:

https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-bff366c8aeba@linaro.org/

the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
actually already handled by the thermal framework via the cooling
device state aggregation, thus all this code is pointless.

No conflict happened when reverting the patch.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 23 ++++---------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index 05f54bd982c0..f5751242653b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -345,7 +345,8 @@ static int mlxsw_thermal_set_trip_hyst(struct thermal_zone_device *tzdev,
 static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
 				   int trip, enum thermal_trend *trend)
 {
-	struct mlxsw_thermal *thermal = tzdev->devdata;
+	struct mlxsw_thermal_module *tz = tzdev->devdata;
+	struct mlxsw_thermal *thermal = tz->parent;
 
 	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
 		return -EINVAL;
@@ -537,22 +538,6 @@ mlxsw_thermal_module_trip_hyst_set(struct thermal_zone_device *tzdev, int trip,
 	return 0;
 }
 
-static int mlxsw_thermal_module_trend_get(struct thermal_zone_device *tzdev,
-					  int trip, enum thermal_trend *trend)
-{
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
-	struct mlxsw_thermal *thermal = tz->parent;
-
-	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
-		return -EINVAL;
-
-	if (tzdev == thermal->tz_highest_dev)
-		return 1;
-
-	*trend = THERMAL_TREND_STABLE;
-	return 0;
-}
-
 static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.bind		= mlxsw_thermal_module_bind,
 	.unbind		= mlxsw_thermal_module_unbind,
@@ -562,7 +547,7 @@ static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
 	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
 	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
-	.get_trend	= mlxsw_thermal_module_trend_get,
+	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
@@ -599,7 +584,7 @@ static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
 	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
 	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
 	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
-	.get_trend	= mlxsw_thermal_module_trend_get,
+	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_get_max_state(struct thermal_cooling_device *cdev,
-- 
2.25.1

[PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone detection"

[Daniel Lezcano]

This reverts commit 6f73862fabd93213de157d9cc6ef76084311c628.

As discussed in the thread:

https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-bff366c8aeba@linaro.org/

the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
actually already handled by the thermal framework via the cooling
device state aggregation, thus all this code is pointless.

The revert conflicts with the following changes:
 - 7f4957be0d5b8: thermal: Use mode helpers in drivers
 - 6a79507cfe94c: mlxsw: core: Extend thermal module with per QSFP module thermal zones

These conflicts were fixed and the resulting changes are in this patch.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 59 +------------------
 1 file changed, 2 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index f5751242653b..373a77c3da02 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -22,7 +22,6 @@
 #define MLXSW_THERMAL_HYSTERESIS_TEMP	5000	/* 5C */
 #define MLXSW_THERMAL_MODULE_TEMP_SHIFT	(MLXSW_THERMAL_HYSTERESIS_TEMP * 2)
 #define MLXSW_THERMAL_ZONE_MAX_NAME	16
-#define MLXSW_THERMAL_TEMP_SCORE_MAX	GENMASK(31, 0)
 #define MLXSW_THERMAL_MAX_STATE	10
 #define MLXSW_THERMAL_MIN_STATE	2
 #define MLXSW_THERMAL_MAX_DUTY	255
@@ -96,8 +95,6 @@ struct mlxsw_thermal {
 	u8 tz_module_num;
 	struct mlxsw_thermal_module *tz_gearbox_arr;
 	u8 tz_gearbox_num;
-	unsigned int tz_highest_score;
-	struct thermal_zone_device *tz_highest_dev;
 };
 
 static inline u8 mlxsw_state_to_duty(int state)
@@ -186,34 +183,6 @@ mlxsw_thermal_module_trips_update(struct device *dev, struct mlxsw_core *core,
 	return 0;
 }
 
-static void mlxsw_thermal_tz_score_update(struct mlxsw_thermal *thermal,
-					  struct thermal_zone_device *tzdev,
-					  struct mlxsw_thermal_trip *trips,
-					  int temp)
-{
-	struct mlxsw_thermal_trip *trip = trips;
-	unsigned int score, delta, i, shift = 1;
-
-	/* Calculate thermal zone score, if temperature is above the hot
-	 * threshold score is set to MLXSW_THERMAL_TEMP_SCORE_MAX.
-	 */
-	score = MLXSW_THERMAL_TEMP_SCORE_MAX;
-	for (i = MLXSW_THERMAL_TEMP_TRIP_NORM; i < MLXSW_THERMAL_NUM_TRIPS;
-	     i++, trip++) {
-		if (temp < trip->temp) {
-			delta = DIV_ROUND_CLOSEST(temp, trip->temp - temp);
-			score = delta * shift;
-			break;
-		}
-		shift *= 256;
-	}
-
-	if (score > thermal->tz_highest_score) {
-		thermal->tz_highest_score = score;
-		thermal->tz_highest_dev = tzdev;
-	}
-}
-
 static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
 			      struct thermal_cooling_device *cdev)
 {
@@ -278,10 +247,8 @@ static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
 		dev_err(dev, "Failed to query temp sensor\n");
 		return err;
 	}
+
 	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL, NULL, NULL);
-	if (temp > 0)
-		mlxsw_thermal_tz_score_update(thermal, tzdev, thermal->trips,
-					      temp);
 
 	*p_temp = temp;
 	return 0;
@@ -342,22 +309,6 @@ static int mlxsw_thermal_set_trip_hyst(struct thermal_zone_device *tzdev,
 	return 0;
 }
 
-static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
-				   int trip, enum thermal_trend *trend)
-{
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
-	struct mlxsw_thermal *thermal = tz->parent;
-
-	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
-		return -EINVAL;
-
-	if (tzdev == thermal->tz_highest_dev)
-		return 1;
-
-	*trend = THERMAL_TREND_STABLE;
-	return 0;
-}
-
 static struct thermal_zone_params mlxsw_thermal_params = {
 	.no_hwmon = true,
 };
@@ -371,7 +322,6 @@ static struct thermal_zone_device_ops mlxsw_thermal_ops = {
 	.set_trip_temp	= mlxsw_thermal_set_trip_temp,
 	.get_trip_hyst	= mlxsw_thermal_get_trip_hyst,
 	.set_trip_hyst	= mlxsw_thermal_set_trip_hyst,
-	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
@@ -473,8 +423,6 @@ static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
 	/* Update trip points. */
 	err = mlxsw_thermal_module_trips_update(dev, thermal->core, tz,
 						crit_temp, emerg_temp);
-	if (!err && temp > 0)
-		mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips, temp);
 
 	return 0;
 }
@@ -547,7 +495,6 @@ static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
 	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
 	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
-	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
@@ -568,8 +515,6 @@ static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
 		return err;
 
 	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL, NULL, NULL);
-	if (temp > 0)
-		mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips, temp);
 
 	*p_temp = temp;
 	return 0;
@@ -584,7 +529,6 @@ static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
 	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
 	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
 	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
-	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_get_max_state(struct thermal_cooling_device *cdev,
@@ -667,6 +611,7 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
 							MLXSW_THERMAL_TRIP_MASK,
 							module_tz,
 							&mlxsw_thermal_module_ops,
+
 							&mlxsw_thermal_params,
 							0,
 							module_tz->parent->polling_delay);
-- 
2.25.1

RE: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone detection"

[Vadim Pasternak]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Monday, August 1, 2022 12:56 PM
> To: daniel.lezcano@linaro.org; rafael@kernel.org
> Cc: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ido Schimmel
> <idosch@nvidia.com>; Petr Machata <petrm@nvidia.com>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>
> Subject: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone
> detection"
> 
> This reverts commit 6f73862fabd93213de157d9cc6ef76084311c628.
> 
> As discussed in the thread:
> 
> https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-
> bff366c8aeba@linaro.org/
> 
> the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
> actually already handled by the thermal framework via the cooling device
> state aggregation, thus all this code is pointless.
> 
> The revert conflicts with the following changes:
>  - 7f4957be0d5b8: thermal: Use mode helpers in drivers
>  - 6a79507cfe94c: mlxsw: core: Extend thermal module with per QSFP module
> thermal zones
> 
> These conflicts were fixed and the resulting changes are in this patch.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Tested-by: Vadim Pasternak <vadimp@nvidia.com>

Daniel,
Could you, please, re-base the patch on top of net-next as Jakub mentioned?
Or do you want me to do it?

There is also redundant blank line in this patch:
							&mlxsw_thermal_module_ops,
+
 							&mlxsw_thermal_params,

> ---
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 59 +------------------
>  1 file changed, 2 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index f5751242653b..373a77c3da02 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -22,7 +22,6 @@
>  #define MLXSW_THERMAL_HYSTERESIS_TEMP	5000	/* 5C */
>  #define MLXSW_THERMAL_MODULE_TEMP_SHIFT
> 	(MLXSW_THERMAL_HYSTERESIS_TEMP * 2)
>  #define MLXSW_THERMAL_ZONE_MAX_NAME	16
> -#define MLXSW_THERMAL_TEMP_SCORE_MAX	GENMASK(31, 0)
>  #define MLXSW_THERMAL_MAX_STATE	10
>  #define MLXSW_THERMAL_MIN_STATE	2
>  #define MLXSW_THERMAL_MAX_DUTY	255
> @@ -96,8 +95,6 @@ struct mlxsw_thermal {
>  	u8 tz_module_num;
>  	struct mlxsw_thermal_module *tz_gearbox_arr;
>  	u8 tz_gearbox_num;
> -	unsigned int tz_highest_score;
> -	struct thermal_zone_device *tz_highest_dev;
>  };
> 
>  static inline u8 mlxsw_state_to_duty(int state) @@ -186,34 +183,6 @@
> mlxsw_thermal_module_trips_update(struct device *dev, struct
> mlxsw_core *core,
>  	return 0;
>  }
> 
> -static void mlxsw_thermal_tz_score_update(struct mlxsw_thermal
> *thermal,
> -					  struct thermal_zone_device *tzdev,
> -					  struct mlxsw_thermal_trip *trips,
> -					  int temp)
> -{
> -	struct mlxsw_thermal_trip *trip = trips;
> -	unsigned int score, delta, i, shift = 1;
> -
> -	/* Calculate thermal zone score, if temperature is above the hot
> -	 * threshold score is set to MLXSW_THERMAL_TEMP_SCORE_MAX.
> -	 */
> -	score = MLXSW_THERMAL_TEMP_SCORE_MAX;
> -	for (i = MLXSW_THERMAL_TEMP_TRIP_NORM; i <
> MLXSW_THERMAL_NUM_TRIPS;
> -	     i++, trip++) {
> -		if (temp < trip->temp) {
> -			delta = DIV_ROUND_CLOSEST(temp, trip->temp -
> temp);
> -			score = delta * shift;
> -			break;
> -		}
> -		shift *= 256;
> -	}
> -
> -	if (score > thermal->tz_highest_score) {
> -		thermal->tz_highest_score = score;
> -		thermal->tz_highest_dev = tzdev;
> -	}
> -}
> -
>  static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
>  			      struct thermal_cooling_device *cdev)  { @@ -
> 278,10 +247,8 @@ static int mlxsw_thermal_get_temp(struct
> thermal_zone_device *tzdev,
>  		dev_err(dev, "Failed to query temp sensor\n");
>  		return err;
>  	}
> +
>  	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL, NULL,
> NULL);
> -	if (temp > 0)
> -		mlxsw_thermal_tz_score_update(thermal, tzdev, thermal-
> >trips,
> -					      temp);
> 
>  	*p_temp = temp;
>  	return 0;
> @@ -342,22 +309,6 @@ static int mlxsw_thermal_set_trip_hyst(struct
> thermal_zone_device *tzdev,
>  	return 0;
>  }
> 
> -static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
> -				   int trip, enum thermal_trend *trend)
> -{
> -	struct mlxsw_thermal_module *tz = tzdev->devdata;
> -	struct mlxsw_thermal *thermal = tz->parent;
> -
> -	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
> -		return -EINVAL;
> -
> -	if (tzdev == thermal->tz_highest_dev)
> -		return 1;
> -
> -	*trend = THERMAL_TREND_STABLE;
> -	return 0;
> -}
> -
>  static struct thermal_zone_params mlxsw_thermal_params = {
>  	.no_hwmon = true,
>  };
> @@ -371,7 +322,6 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_ops = {
>  	.set_trip_temp	= mlxsw_thermal_set_trip_temp,
>  	.get_trip_hyst	= mlxsw_thermal_get_trip_hyst,
>  	.set_trip_hyst	= mlxsw_thermal_set_trip_hyst,
> -	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
> @@ -473,8 +423,6 @@ static int mlxsw_thermal_module_temp_get(struct
> thermal_zone_device *tzdev,
>  	/* Update trip points. */
>  	err = mlxsw_thermal_module_trips_update(dev, thermal->core, tz,
>  						crit_temp, emerg_temp);
> -	if (!err && temp > 0)
> -		mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips,
> temp);
> 
>  	return 0;
>  }
> @@ -547,7 +495,6 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_module_ops = {
>  	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
>  	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
>  	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
> -	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device
> *tzdev, @@ -568,8 +515,6 @@ static int
> mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
>  		return err;
> 
>  	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL, NULL,
> NULL);
> -	if (temp > 0)
> -		mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips,
> temp);
> 
>  	*p_temp = temp;
>  	return 0;
> @@ -584,7 +529,6 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_gearbox_ops = {
>  	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
>  	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
>  	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
> -	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_get_max_state(struct thermal_cooling_device
> *cdev, @@ -667,6 +611,7 @@ mlxsw_thermal_module_tz_init(struct
> mlxsw_thermal_module *module_tz)
> 
> 	MLXSW_THERMAL_TRIP_MASK,
>  							module_tz,
> 
> 	&mlxsw_thermal_module_ops,
> +
> 
> 	&mlxsw_thermal_params,
>  							0,
>  							module_tz->parent-
> >polling_delay);
> --
> 2.25.1

Re: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone detection"

[Daniel Lezcano]

Hi Vadim,


On 04/08/2022 14:21, Vadim Pasternak wrote:
> 
> 
>> -----Original Message-----
>> From: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Sent: Monday, August 1, 2022 12:56 PM
>> To: daniel.lezcano@linaro.org; rafael@kernel.org
>> Cc: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net;
>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ido Schimmel
>> <idosch@nvidia.com>; Petr Machata <petrm@nvidia.com>; Eric Dumazet
>> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
>> <pabeni@redhat.com>
>> Subject: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone
>> detection"
>>
>> This reverts commit 6f73862fabd93213de157d9cc6ef76084311c628.
>>
>> As discussed in the thread:
>>
>> https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-
>> bff366c8aeba@linaro.org/
>>
>> the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
>> actually already handled by the thermal framework via the cooling device
>> state aggregation, thus all this code is pointless.
>>
>> The revert conflicts with the following changes:
>>   - 7f4957be0d5b8: thermal: Use mode helpers in drivers
>>   - 6a79507cfe94c: mlxsw: core: Extend thermal module with per QSFP module
>> thermal zones
>>
>> These conflicts were fixed and the resulting changes are in this patch.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Tested-by: Vadim Pasternak <vadimp@nvidia.com>

Thanks for testing

> Daniel,
> Could you, please, re-base the patch on top of net-next as Jakub mentioned?
> Or do you want me to do it?

It is fine, I can do it. The conflict is trivial.

However, I would have preferred to have the patch in my tree so I can 
continue the consolidation work.

Is it ok if I pick the patch and the conflict being simple, that can be 
handle at merge time, no?

> There is also redundant blank line in this patch:
> 							&mlxsw_thermal_module_ops,
> +
>   							&mlxsw_thermal_params,

Yeah, thanks.

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

RE: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone detection"

[Vadim Pasternak]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Friday, August 5, 2022 7:07 PM
> To: Vadim Pasternak <vadimp@nvidia.com>; rafael@kernel.org
> Cc: davem@davemloft.net; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Ido Schimmel <idosch@nvidia.com>; Petr Machata
> <petrm@nvidia.com>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>
> Subject: Re: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal zone
> detection"
> 
> 
> Hi Vadim,
> 
> 
> On 04/08/2022 14:21, Vadim Pasternak wrote:
> >
> >
> >> -----Original Message-----
> >> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Sent: Monday, August 1, 2022 12:56 PM
> >> To: daniel.lezcano@linaro.org; rafael@kernel.org
> >> Cc: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net;
> >> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ido Schimmel
> >> <idosch@nvidia.com>; Petr Machata <petrm@nvidia.com>; Eric Dumazet
> >> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni
> >> <pabeni@redhat.com>
> >> Subject: [PATCH 2/2] Revert "mlxsw: core: Add the hottest thermal
> >> zone detection"
> >>
> >> This reverts commit 6f73862fabd93213de157d9cc6ef76084311c628.
> >>
> >> As discussed in the thread:
> >>
> >> https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-
> >> bff366c8aeba@linaro.org/
> >>
> >> the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
> >> actually already handled by the thermal framework via the cooling
> >> device state aggregation, thus all this code is pointless.
> >>
> >> The revert conflicts with the following changes:
> >>   - 7f4957be0d5b8: thermal: Use mode helpers in drivers
> >>   - 6a79507cfe94c: mlxsw: core: Extend thermal module with per QSFP
> >> module thermal zones
> >>
> >> These conflicts were fixed and the resulting changes are in this patch.
> >>
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Tested-by: Vadim Pasternak <vadimp@nvidia.com>
> 
> Thanks for testing
> 
> > Daniel,
> > Could you, please, re-base the patch on top of net-next as Jakub
> mentioned?
> > Or do you want me to do it?
> 
> It is fine, I can do it. The conflict is trivial.
> 
> However, I would have preferred to have the patch in my tree so I can
> continue the consolidation work.
> 
> Is it ok if I pick the patch and the conflict being simple, that can be handle at
> merge time, no?

Hi Daniel,

Sorry for the delay.

Yes, it is OK. There are no plans to make changes in 'core_thermal.c' module
In current cycle, so it should be fine.

Thanks,
Vadim.

> 
> > There is also redundant blank line in this patch:
> >
> 	&mlxsw_thermal_module_ops,
> > +
> >
> 	&mlxsw_thermal_params,
> 
> Yeah, thanks.
> 
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-
> blog/> Blog

RE: [PATCH 1/2] Revert "mlxsw: core: Use different get_trend() callbacks for different thermal zones"

[Vadim Pasternak]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Monday, August 1, 2022 12:56 PM
> To: daniel.lezcano@linaro.org; rafael@kernel.org
> Cc: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ido Schimmel
> <idosch@nvidia.com>; Petr Machata <petrm@nvidia.com>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>
> Subject: [PATCH 1/2] Revert "mlxsw: core: Use different get_trend()
> callbacks for different thermal zones"
> 
> This reverts commit 2dc2f760052da4925482ecdcdc5c94d4a599153c.
> 
> As discussed in the thread:
> 
> https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-
> bff366c8aeba@linaro.org/
> 
> the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
> actually already handled by the thermal framework via the cooling device
> state aggregation, thus all this code is pointless.
> 
> No conflict happened when reverting the patch.

Hi Daniel,


I am sorry, I didn't run emulation yet to validate this change.
Will do it in tomorrow and will send ACK if it is OK.

Thanks,
Vadim.

> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 23 ++++---------------
>  1 file changed, 4 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index 05f54bd982c0..f5751242653b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -345,7 +345,8 @@ static int mlxsw_thermal_set_trip_hyst(struct
> thermal_zone_device *tzdev,  static int mlxsw_thermal_trend_get(struct
> thermal_zone_device *tzdev,
>  				   int trip, enum thermal_trend *trend)  {
> -	struct mlxsw_thermal *thermal = tzdev->devdata;
> +	struct mlxsw_thermal_module *tz = tzdev->devdata;
> +	struct mlxsw_thermal *thermal = tz->parent;
> 
>  	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
>  		return -EINVAL;
> @@ -537,22 +538,6 @@ mlxsw_thermal_module_trip_hyst_set(struct
> thermal_zone_device *tzdev, int trip,
>  	return 0;
>  }
> 
> -static int mlxsw_thermal_module_trend_get(struct thermal_zone_device
> *tzdev,
> -					  int trip, enum thermal_trend
> *trend)
> -{
> -	struct mlxsw_thermal_module *tz = tzdev->devdata;
> -	struct mlxsw_thermal *thermal = tz->parent;
> -
> -	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
> -		return -EINVAL;
> -
> -	if (tzdev == thermal->tz_highest_dev)
> -		return 1;
> -
> -	*trend = THERMAL_TREND_STABLE;
> -	return 0;
> -}
> -
>  static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
>  	.bind		= mlxsw_thermal_module_bind,
>  	.unbind		= mlxsw_thermal_module_unbind,
> @@ -562,7 +547,7 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_module_ops = {
>  	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
>  	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
>  	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
> -	.get_trend	= mlxsw_thermal_module_trend_get,
> +	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device
> *tzdev, @@ -599,7 +584,7 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_gearbox_ops = {
>  	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
>  	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
>  	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
> -	.get_trend	= mlxsw_thermal_module_trend_get,
> +	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_get_max_state(struct thermal_cooling_device
> *cdev,
> --
> 2.25.1

Re: [PATCH 1/2] Revert "mlxsw: core: Use different get_trend() callbacks for different thermal zones"

[Jakub Kicinski]

On Mon, 1 Aug 2022 11:13:36 +0000 Vadim Pasternak wrote:
> > This reverts commit 2dc2f760052da4925482ecdcdc5c94d4a599153c.
> > 
> > As discussed in the thread:
> > 
> > https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-
> > bff366c8aeba@linaro.org/
> > 
> > the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
> > actually already handled by the thermal framework via the cooling device
> > state aggregation, thus all this code is pointless.
> > 
> > No conflict happened when reverting the patch.  
> 
> I am sorry, I didn't run emulation yet to validate this change.
> Will do it in tomorrow and will send ACK if it is OK.

We'll also need a rebase on top of net-next, the patch as is does not
apply to either of the networking trees.

RE: [PATCH 1/2] Revert "mlxsw: core: Use different get_trend() callbacks for different thermal zones"

[Vadim Pasternak]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Monday, August 1, 2022 12:56 PM
> To: daniel.lezcano@linaro.org; rafael@kernel.org
> Cc: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ido Schimmel
> <idosch@nvidia.com>; Petr Machata <petrm@nvidia.com>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>
> Subject: [PATCH 1/2] Revert "mlxsw: core: Use different get_trend()
> callbacks for different thermal zones"
> 
> This reverts commit 2dc2f760052da4925482ecdcdc5c94d4a599153c.
> 
> As discussed in the thread:
> 
> https://lore.kernel.org/all/f3c62ebe-7d59-c537-a010-
> bff366c8aeba@linaro.org/
> 
> the feature provided by commits 2dc2f760052da and 6f73862fabd93 is
> actually already handled by the thermal framework via the cooling device
> state aggregation, thus all this code is pointless.
> 
> No conflict happened when reverting the patch.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Tested-by: Vadim Pasternak <vadimp@nvidia.com>

> ---
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 23 ++++---------------
>  1 file changed, 4 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index 05f54bd982c0..f5751242653b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -345,7 +345,8 @@ static int mlxsw_thermal_set_trip_hyst(struct
> thermal_zone_device *tzdev,  static int mlxsw_thermal_trend_get(struct
> thermal_zone_device *tzdev,
>  				   int trip, enum thermal_trend *trend)  {
> -	struct mlxsw_thermal *thermal = tzdev->devdata;
> +	struct mlxsw_thermal_module *tz = tzdev->devdata;
> +	struct mlxsw_thermal *thermal = tz->parent;
> 
>  	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
>  		return -EINVAL;
> @@ -537,22 +538,6 @@ mlxsw_thermal_module_trip_hyst_set(struct
> thermal_zone_device *tzdev, int trip,
>  	return 0;
>  }
> 
> -static int mlxsw_thermal_module_trend_get(struct thermal_zone_device
> *tzdev,
> -					  int trip, enum thermal_trend
> *trend)
> -{
> -	struct mlxsw_thermal_module *tz = tzdev->devdata;
> -	struct mlxsw_thermal *thermal = tz->parent;
> -
> -	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
> -		return -EINVAL;
> -
> -	if (tzdev == thermal->tz_highest_dev)
> -		return 1;
> -
> -	*trend = THERMAL_TREND_STABLE;
> -	return 0;
> -}
> -
>  static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
>  	.bind		= mlxsw_thermal_module_bind,
>  	.unbind		= mlxsw_thermal_module_unbind,
> @@ -562,7 +547,7 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_module_ops = {
>  	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
>  	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
>  	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
> -	.get_trend	= mlxsw_thermal_module_trend_get,
> +	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device
> *tzdev, @@ -599,7 +584,7 @@ static struct thermal_zone_device_ops
> mlxsw_thermal_gearbox_ops = {
>  	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
>  	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
>  	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
> -	.get_trend	= mlxsw_thermal_module_trend_get,
> +	.get_trend	= mlxsw_thermal_trend_get,
>  };
> 
>  static int mlxsw_thermal_get_max_state(struct thermal_cooling_device
> *cdev,
> --
> 2.25.1