Re: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection

[Daniel Lezcano <daniel.lezcano@linaro.org>]

Hi Vadim,

On 29/05/2019 15:52, Vadim Pasternak wrote:
> When multiple sensors are mapped to the same cooling device, the
> cooling device should be set according the worst sensor from the
> sensors associated with this cooling device.
> 
> Provide the hottest thermal zone detection and enforce cooling device to
> follow the temperature trends the hottest zone only.
> Prevent competition for the cooling device control from others zones, by
> "stable trend" indication. A cooling device will not perform any
> actions associated with a zone with "stable trend".
> 
> When other thermal zone is detected as a hottest, a cooling device is to
> be switched to following temperature trends of new hottest zone.
> 
> Thermal zone score is represented by 32 bits unsigned integer and
> calculated according to the next formula:
> For T < TZ<t><i>, where t from {normal trip = 0, high trip = 1, hot
> trip = 2, critical = 3}:
> TZ<i> score = (T + (TZ<t><i> - T) / 2) / (TZ<t><i> - T) * 256 ** j;
> Highest thermal zone score s is set as MAX(TZ<i>score);
> Following this formula, if TZ<i> is in trip point higher than TZ<k>,
> the higher score is to be always assigned to TZ<i>.
> 
> For two thermal zones located at the same kind of trip point, the higher
> score will be assigned to the zone, which closer to the next trip point.
> Thus, the highest score will always be assigned objectively to the hottest
> thermal zone.

While reading the code I noticed this change and I was wondering why it 
was needed.

The thermal framework does already aggregates the mitigation decisions, 
taking the highest cooling state [1].

That allows for instance a spanning fan on a dual socket. Two thermal 
zones for one cooling device.

AFAICS, the code hijacked the get_trend function just for the sake of 
returning 1 for the hotter thermal zone leading to a computation of the 
trend in the thermal core code.

I would like to get rid of the get_trend ops in the thermal framework 
and the changes in this patch sounds like pointless as the aggregation 
of the cooling action is already handled in the thermal framework.

Given the above, it would make sense to revert commit 6f73862fabd93 and 
2dc2f760052da ?

Thanks

   -- Daniel

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_helpers.c#n190


[ ... ]


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