| Message ID | 20240605195129.89353-1-trintaeoitogc@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [v3] iwlwifi: mvm: adding check if the thermal firmware is running | expand |
On 6/5/2024 9:51 PM, Guilherme Giacomo Simoes wrote: > In the dmesg is showing the message "failed to read out thermal zone" > as if the temperature read is failed by don't find the thermal zone. > > After researching and debugging, I see that this specific error is > occurrenced because the thermal try read the temperature when is started, > but the firmware is not running yet. > > For more legibiliti i change the tt.c for return EAGAIN when this was occurrence. > After this change, in my computer I compile and install kernel in /boot > and in my dmesg the message "failed to read out thermal zone" is not show > any more. > > I would like to thanks for Rafael Wysocki <refael.j.wysocki@intel.com> , > Kalle Valo <kvalo@kernel.org> and Johannes Berg <johannes@sipsolutions.net> > for your suggestions in my previous patch. > > Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> > --- > drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c > index 8083c4b2ab6b..9aa9e3be39b8 100644 > --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c > @@ -620,8 +620,14 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device, > > mutex_lock(&mvm->mutex); > > - if (!iwl_mvm_firmware_running(mvm) || > - mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { > + int res = iwl_mvm_firmware_running(mvm); Why do you need the res variable at all? You can just call the function directly in the if () statement. > + > + if (!res) { > + ret = -EAGAIN; > + goto out; > + } > + > + if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { > ret = -ENODATA; > goto out; > }
Wysocki, Rafael J <rafael.j.wysocki@intel.com> write: > > On 6/5/2024 9:51 PM, Guilherme Giacomo Simoes wrote: > > In the dmesg is showing the message "failed to read out thermal zone" > > as if the temperature read is failed by don't find the thermal zone. > > > > After researching and debugging, I see that this specific error is > > occurrenced because the thermal try read the temperature when is started, > > but the firmware is not running yet. > > > > For more legibiliti i change the tt.c for return EAGAIN when this was occurrence. > > After this change, in my computer I compile and install kernel in /boot > > and in my dmesg the message "failed to read out thermal zone" is not show > > any more. > > > > I would like to thanks for Rafael Wysocki <refael.j.wysocki@intel.com> , > > Kalle Valo <kvalo@kernel.org> and Johannes Berg <johannes@sipsolutions.net> > > for your suggestions in my previous patch. > > > > Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> > > --- > > drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c > > index 8083c4b2ab6b..9aa9e3be39b8 100644 > > --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c > > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c > > @@ -620,8 +620,14 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device, > > > > mutex_lock(&mvm->mutex); > > > > - if (!iwl_mvm_firmware_running(mvm) || > > - mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { > > + int res = iwl_mvm_firmware_running(mvm); > > Why do you need the res variable at all? You can just call the function > directly in the if () statement. > > > > + > > + if (!res) { > > + ret = -EAGAIN; > > + goto out; > > + } > > + > > + if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { > > ret = -ENODATA; > > goto out; > > } Yes, this makes sense, I only declare the "res" variable because seeing some files in the linux kernel, I see this standard practice. But I really don't need this variable. I will send a new patch v4 without this variable. Thank you for your suggestion on my code.
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c index 8083c4b2ab6b..9aa9e3be39b8 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c @@ -620,8 +620,14 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device, mutex_lock(&mvm->mutex); - if (!iwl_mvm_firmware_running(mvm) || - mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { + int res = iwl_mvm_firmware_running(mvm); + + if (!res) { + ret = -EAGAIN; + goto out; + } + + if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) { ret = -ENODATA; goto out; }
In the dmesg is showing the message "failed to read out thermal zone" as if the temperature read is failed by don't find the thermal zone. After researching and debugging, I see that this specific error is occurrenced because the thermal try read the temperature when is started, but the firmware is not running yet. For more legibiliti i change the tt.c for return EAGAIN when this was occurrence. After this change, in my computer I compile and install kernel in /boot and in my dmesg the message "failed to read out thermal zone" is not show any more. I would like to thanks for Rafael Wysocki <refael.j.wysocki@intel.com> , Kalle Valo <kvalo@kernel.org> and Johannes Berg <johannes@sipsolutions.net> for your suggestions in my previous patch. Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> --- drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)