All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhang Rui <rui.zhang@intel.com>
To: linux-pm@vger.kernel.org
Cc: linux-wireless@vger.kernel.org, daniel.lezcano@linaro.org,
	andrzej.p@collabora.com, b.zolnierkie@samsung.com,
	luca@coelho.fi, rui.zhang@intel.com
Subject: [PATCH RFC 6/6] iwlwifi: enable thermal zone only when firmware loaded
Date: Thu, 30 Apr 2020 14:32:29 +0800	[thread overview]
Message-ID: <20200430063229.6182-7-rui.zhang@intel.com> (raw)
In-Reply-To: <20200430063229.6182-1-rui.zhang@intel.com>

in iwl_mvm_thermal_zone_register(), when registering a thermal zone, the
thermal subsystem will evaluate its temperature.
But iwl_mvm_tzone_get_temp() fails at this time because
iwl_mvm_firmware_running() returns false.
And that's why many users report that they see
"thermal thermal_zoneX: failed to read out thermal zone (-61)"
message during wifi driver probing.

With the changes made earlier in this patch series, the problem can be
solved by registering the iwlwifi thermal zone as disabled, and enable it
later when the firmware is ready.

*NOTE*
This is just a prototype patch to illustrate the problem and
how we can fix it based on the new thermal changes.

I'm not sure what is the proper place to enable/disable the thermal zone.
The solution in this patch causes a deadlock issue because
a) thermal_zone_device_enable() is invoked with mvm->mutex locked.
b) .get_temp() callback is invoked when enabling the thermal zone.
c) iwl_mvm_tzone_get_temp() also needs to hold mvm->mutex.
And that's why I hacked iwl_mvm_tzone_get_temp() to avoid this.

So if you have ideas on how to fix this, please feel free to propose it.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=201761
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c  |  2 ++
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h |  4 ++++
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c  | 12 +++++++++---
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index a4038f2..ff47fc6 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -1222,6 +1222,8 @@ int iwl_mvm_up(struct iwl_mvm *mvm)
 #ifdef CONFIG_THERMAL
 	/* TODO: read the budget from BIOS / Platform NVM */
 
+	if (mvm->tz_device.tzone && tz_disabled(mvm->tz_device.tzone))
+		thermal_zone_device_enable(mvm->tz_device.tzone);
 	/*
 	 * In case there is no budget from BIOS / Platform NVM the default
 	 * budget should be 2000mW (cooling state 0).
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index afcf2b9..b964c01 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -1946,6 +1946,10 @@ static inline u32 iwl_mvm_flushable_queues(struct iwl_mvm *mvm)
 
 static inline void iwl_mvm_stop_device(struct iwl_mvm *mvm)
 {
+#ifdef CONFIG_THERMAL
+	if (mvm->tz_device.tzone && !tz_disabled(mvm->tz_device.tzone))
+		thermal_zone_device_disable(mvm->tz_device.tzone);
+#endif
 	lockdep_assert_held(&mvm->mutex);
 	iwl_fw_cancel_timestamp(&mvm->fwrt);
 	clear_bit(IWL_MVM_STATUS_FIRMWARE_RUNNING, &mvm->status);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index 6344b6b..819dcaf 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -620,8 +620,9 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device,
 	struct iwl_mvm *mvm = (struct iwl_mvm *)device->devdata;
 	int ret;
 	int temp;
+	int locked;
 
-	mutex_lock(&mvm->mutex);
+	locked = mutex_trylock(&mvm->mutex);
 
 	if (!iwl_mvm_firmware_running(mvm) ||
 	    mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
@@ -636,7 +637,8 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device,
 	*temperature = temp * 1000;
 
 out:
-	mutex_unlock(&mvm->mutex);
+	if (locked)
+		mutex_unlock(&mvm->mutex);
 	return ret;
 }
 
@@ -730,6 +732,10 @@ static  struct thermal_zone_device_ops tzone_ops = {
 /* make all trips writable */
 #define IWL_WRITABLE_TRIPS_MSK (BIT(IWL_MAX_DTS_TRIPS) - 1)
 
+static struct thermal_zone_params tz_params = {
+	.initial_mode   = THERMAL_DEVICE_DISABLED,
+};
+
 static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
 {
 	int i;
@@ -749,7 +755,7 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
 							IWL_MAX_DTS_TRIPS,
 							IWL_WRITABLE_TRIPS_MSK,
 							mvm, &tzone_ops,
-							NULL, 0, 0);
+							&tz_params, 0, 0);
 	if (IS_ERR(mvm->tz_device.tzone)) {
 		IWL_DEBUG_TEMP(mvm,
 			       "Failed to register to thermal zone (err = %ld)\n",
-- 
2.7.4


      parent reply	other threads:[~2020-04-30  6:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30  6:32 [PATCH 0/6] thermal: improve handling of disabled thermal zone Zhang Rui
2020-04-30  6:32 ` [PATCH 1/6] iwlwifi: use thermal_zone_device_update() for temperature change Zhang Rui
2020-05-12  1:58   ` Zhang Rui
2020-05-18 11:21     ` Luca Coelho
2020-05-25  6:04       ` Zhang Rui
2020-05-25 13:33   ` Rafael J. Wysocki
2020-04-30  6:32 ` [PATCH 2/6] thermal: core: delete thermal_notify_framework() Zhang Rui
2020-04-30  8:47   ` Andrzej Pietrasiewicz
2020-04-30  8:58     ` Zhang, Rui
2020-04-30 11:32       ` Andrzej Pietrasiewicz
2020-04-30 11:33         ` Andrzej Pietrasiewicz
2020-05-04 10:18   ` Andrzej Pietrasiewicz
2020-04-30  6:32 ` [PATCH 3/6] thermal: core: update polling after all trips handled Zhang Rui
2020-05-04  7:01   ` Bartlomiej Zolnierkiewicz
2020-04-30  6:32 ` [PATCH 4/6] thermal: core: stop polling timer for disabled thermal zone Zhang Rui
2020-05-04  7:02   ` Bartlomiej Zolnierkiewicz
2020-04-30  6:32 ` [PATCH 5/6] thermal: core: introduce tz_disabled() helper function Zhang Rui
2020-05-04  7:09   ` Bartlomiej Zolnierkiewicz
2020-05-12  1:59     ` Zhang Rui
2020-04-30  6:32 ` Zhang Rui [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200430063229.6182-7-rui.zhang@intel.com \
    --to=rui.zhang@intel.com \
    --cc=andrzej.p@collabora.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luca@coelho.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.