2022-10-14 07:51:09

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 1/2] thermal/drivers/iwlwifi: Use generic thermal_zone_get_trip() function

The thermal framework gives the possibility to register the trip
points with the thermal zone. When that is done, no get_trip_* ops are
needed and they can be removed.

The get_trip_temp, get_trip_hyst and get_trip_type are handled by the
get_trip_point().

The set_trip_temp() generic function does some checks which are no
longer needed in the set_trip_point() ops.

Convert ops content logic into generic trip points and register them
with the thermal zone.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 71 ++++----------------
2 files changed, 13 insertions(+), 60 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index bf35e130c876..38e7426f98c0 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -501,7 +501,7 @@ struct iwl_mvm_tt_mgmt {
* @tzone: thermal zone device data
*/
struct iwl_mvm_thermal_device {
- s16 temp_trips[IWL_MAX_DTS_TRIPS];
+ struct thermal_trip trips[IWL_MAX_DTS_TRIPS];
u8 fw_trips_index[IWL_MAX_DTS_TRIPS];
struct thermal_zone_device *tzone;
};
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index 69cf3a372759..3da4af7bc1ca 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -573,11 +573,11 @@ int iwl_mvm_send_temp_report_ths_cmd(struct iwl_mvm *mvm)
* and uncompressed, the FW should get it compressed and sorted
*/

- /* compress temp_trips to cmd array, remove uninitialized values*/
+ /* compress trips to cmd array, remove uninitialized values*/
for (i = 0; i < IWL_MAX_DTS_TRIPS; i++) {
- if (mvm->tz_device.temp_trips[i] != S16_MIN) {
+ if (mvm->tz_device.trips[i].temperature != INT_MIN) {
cmd.thresholds[idx++] =
- cpu_to_le16(mvm->tz_device.temp_trips[i]);
+ cpu_to_le16((s16)(mvm->tz_device.trips[i].temperature / 1000));
}
}
cmd.num_temps = cpu_to_le32(idx);
@@ -593,8 +593,8 @@ int iwl_mvm_send_temp_report_ths_cmd(struct iwl_mvm *mvm)
*/
for (i = 0; i < idx; i++) {
for (j = 0; j < IWL_MAX_DTS_TRIPS; j++) {
- if (le16_to_cpu(cmd.thresholds[i]) ==
- mvm->tz_device.temp_trips[j])
+ if ((int)(le16_to_cpu(cmd.thresholds[i]) * 1000) ==
+ mvm->tz_device.trips[j].temperature)
mvm->tz_device.fw_trips_index[i] = j;
}
}
@@ -638,37 +638,12 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device,
return ret;
}

-static int iwl_mvm_tzone_get_trip_temp(struct thermal_zone_device *device,
- int trip, int *temp)
-{
- struct iwl_mvm *mvm = (struct iwl_mvm *)device->devdata;
-
- if (trip < 0 || trip >= IWL_MAX_DTS_TRIPS)
- return -EINVAL;
-
- *temp = mvm->tz_device.temp_trips[trip] * 1000;
-
- return 0;
-}
-
-static int iwl_mvm_tzone_get_trip_type(struct thermal_zone_device *device,
- int trip, enum thermal_trip_type *type)
-{
- if (trip < 0 || trip >= IWL_MAX_DTS_TRIPS)
- return -EINVAL;
-
- *type = THERMAL_TRIP_PASSIVE;
-
- return 0;
-}
-
static int iwl_mvm_tzone_set_trip_temp(struct thermal_zone_device *device,
int trip, int temp)
{
struct iwl_mvm *mvm = (struct iwl_mvm *)device->devdata;
struct iwl_mvm_thermal_device *tzone;
- int i, ret;
- s16 temperature;
+ int ret;

mutex_lock(&mvm->mutex);

@@ -678,40 +653,17 @@ static int iwl_mvm_tzone_set_trip_temp(struct thermal_zone_device *device,
goto out;
}

- if (trip < 0 || trip >= IWL_MAX_DTS_TRIPS) {
- ret = -EINVAL;
- goto out;
- }
-
if ((temp / 1000) > S16_MAX) {
ret = -EINVAL;
goto out;
}

- temperature = (s16)(temp / 1000);
tzone = &mvm->tz_device;
-
if (!tzone) {
ret = -EIO;
goto out;
}

- /* no updates*/
- if (tzone->temp_trips[trip] == temperature) {
- ret = 0;
- goto out;
- }
-
- /* already existing temperature */
- for (i = 0; i < IWL_MAX_DTS_TRIPS; i++) {
- if (tzone->temp_trips[i] == temperature) {
- ret = -EINVAL;
- goto out;
- }
- }
-
- tzone->temp_trips[trip] = temperature;
-
ret = iwl_mvm_send_temp_report_ths_cmd(mvm);
out:
mutex_unlock(&mvm->mutex);
@@ -720,8 +672,6 @@ static int iwl_mvm_tzone_set_trip_temp(struct thermal_zone_device *device,

static struct thermal_zone_device_ops tzone_ops = {
.get_temp = iwl_mvm_tzone_get_temp,
- .get_trip_temp = iwl_mvm_tzone_get_trip_temp,
- .get_trip_type = iwl_mvm_tzone_get_trip_type,
.set_trip_temp = iwl_mvm_tzone_set_trip_temp,
};

@@ -743,7 +693,8 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
BUILD_BUG_ON(ARRAY_SIZE(name) >= THERMAL_NAME_LENGTH);

sprintf(name, "iwlwifi_%u", atomic_inc_return(&counter) & 0xFF);
- mvm->tz_device.tzone = thermal_zone_device_register(name,
+ mvm->tz_device.tzone = thermal_zone_device_register_with_trips(name,
+ mvm->tz_device.trips,
IWL_MAX_DTS_TRIPS,
IWL_WRITABLE_TRIPS_MSK,
mvm, &tzone_ops,
@@ -766,8 +717,10 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
/* 0 is a valid temperature,
* so initialize the array with S16_MIN which invalid temperature
*/
- for (i = 0 ; i < IWL_MAX_DTS_TRIPS; i++)
- mvm->tz_device.temp_trips[i] = S16_MIN;
+ for (i = 0 ; i < IWL_MAX_DTS_TRIPS; i++) {
+ mvm->tz_device.trips[i].temperature = INT_MIN;
+ mvm->tz_device.trips[i].type = THERMAL_TRIP_PASSIVE;
+ }
}

static int iwl_mvm_tcool_get_max_state(struct thermal_cooling_device *cdev,
--
2.34.1


2022-10-14 10:20:03

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] thermal/drivers/iwlwifi: Use generic thermal_zone_get_trip() function

Daniel Lezcano <[email protected]> writes:

> The thermal framework gives the possibility to register the trip
> points with the thermal zone. When that is done, no get_trip_* ops are
> needed and they can be removed.
>
> The get_trip_temp, get_trip_hyst and get_trip_type are handled by the
> get_trip_point().
>
> The set_trip_temp() generic function does some checks which are no
> longer needed in the set_trip_point() ops.
>
> Convert ops content logic into generic trip points and register them
> with the thermal zone.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 2 +-
> drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 71 ++++----------------
> 2 files changed, 13 insertions(+), 60 deletions(-)

The subject should begin with "wifi: iwlwifi: ".

I don't see patch 2. Via which tree is the plan for this patch?

Gregory, please review this.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-10-14 10:21:56

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/2] thermal/drivers/iwlwifi: Use generic thermal_zone_get_trip() function

On 14/10/2022 12:15, Kalle Valo wrote:
> Daniel Lezcano <[email protected]> writes:
>
>> The thermal framework gives the possibility to register the trip
>> points with the thermal zone. When that is done, no get_trip_* ops are
>> needed and they can be removed.
>>
>> The get_trip_temp, get_trip_hyst and get_trip_type are handled by the
>> get_trip_point().
>>
>> The set_trip_temp() generic function does some checks which are no
>> longer needed in the set_trip_point() ops.
>>
>> Convert ops content logic into generic trip points and register them
>> with the thermal zone.
>>
>> Signed-off-by: Daniel Lezcano <[email protected]>
>> ---
>> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 2 +-
>> drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 71 ++++----------------
>> 2 files changed, 13 insertions(+), 60 deletions(-)
>
> The subject should begin with "wifi: iwlwifi: ".
>
> I don't see patch 2. Via which tree is the plan for this patch?

patch 2 are similar changes but related to the mellanox driver.

This is the continuation of the trip point rework:

https://lore.kernel.org/netdev/[email protected]/t/

This patch is planned to go through the thermal tree

Sorry I should have mentioned that.

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

2022-10-18 09:52:38

by Greenman, Gregory

[permalink] [raw]
Subject: Re: [PATCH 1/2] thermal/drivers/iwlwifi: Use generic thermal_zone_get_trip() function

On Fri, 2022-10-14 at 12:21 +0200, Daniel Lezcano wrote:
> On 14/10/2022 12:15, Kalle Valo wrote:
> > Daniel Lezcano <[email protected]> writes:
> >
> > > The thermal framework gives the possibility to register the trip
> > > points with the thermal zone. When that is done, no get_trip_* ops are
> > > needed and they can be removed.
> > >
> > > The get_trip_temp, get_trip_hyst and get_trip_type are handled by the
> > > get_trip_point().
> > >
> > > The set_trip_temp() generic function does some checks which are no
> > > longer needed in the set_trip_point() ops.
> > >
> > > Convert ops content logic into generic trip points and register them
> > > with the thermal zone.
> > >
> > > Signed-off-by: Daniel Lezcano <[email protected]>
> > > ---
> > >   drivers/net/wireless/intel/iwlwifi/mvm/mvm.h |  2 +-
> > >   drivers/net/wireless/intel/iwlwifi/mvm/tt.c  | 71 ++++----------------
> > >   2 files changed, 13 insertions(+), 60 deletions(-)
> >
> > The subject should begin with "wifi: iwlwifi: ".
> >
> > I don't see patch 2. Via which tree is the plan for this patch?
>
> patch 2 are similar changes but related to the mellanox driver.
>
> This is the continuation of the trip point rework:
>
> https://lore.kernel.org/netdev/[email protected]/t/
>
> This patch is planned to go through the thermal tree
>
> Sorry I should have mentioned that.
>

As Kalle commented above, the subject should start with wifi: iwlwifi:
The commit itself seems fine to me.

Acked-by: Gregory Greenman <[email protected]>

2022-10-19 06:18:11

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] thermal/drivers/iwlwifi: Use generic thermal_zone_get_trip() function

Daniel Lezcano <[email protected]> writes:

> On 14/10/2022 12:15, Kalle Valo wrote:
>
>> Daniel Lezcano <[email protected]> writes:
>>
>>> The thermal framework gives the possibility to register the trip
>>> points with the thermal zone. When that is done, no get_trip_* ops are
>>> needed and they can be removed.
>>>
>>> The get_trip_temp, get_trip_hyst and get_trip_type are handled by the
>>> get_trip_point().
>>>
>>> The set_trip_temp() generic function does some checks which are no
>>> longer needed in the set_trip_point() ops.
>>>
>>> Convert ops content logic into generic trip points and register them
>>> with the thermal zone.
>>>
>>> Signed-off-by: Daniel Lezcano <[email protected]>
>>> ---
>>> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 2 +-
>>> drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 71 ++++----------------
>>> 2 files changed, 13 insertions(+), 60 deletions(-)
>>
>> The subject should begin with "wifi: iwlwifi: ".
>>
>> I don't see patch 2. Via which tree is the plan for this patch?
>
> patch 2 are similar changes but related to the mellanox driver.
>
> This is the continuation of the trip point rework:
>
> https://lore.kernel.org/netdev/[email protected]/t/
>
> This patch is planned to go through the thermal tree

Ok, feel free to take this via the thermal tree:

Acked-by: Kalle Valo <[email protected]>

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches