2024-05-17 14:17:45

by Guilherme Giacomo Simoes

[permalink] [raw]
Subject: [PATCH] thermal: adding check if the thermal firmware is running

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 <[email protected]> for
your suggestions in mu first patch that results in this another patch.
---
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..68ab9966330c 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) {
+ const 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;
}
--
2.45.1



2024-05-17 14:56:35

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] thermal: adding check if the thermal firmware is running

Guilherme Giacomo Simoes <[email protected]> writes:

> 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 <[email protected]> for
> your suggestions in mu first patch that results in this another patch.
> ---
> drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)

Please read the wiki link below how to submit wireless patches. For
example, the title is wrong and you haven't signed the patch.

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

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

2024-05-17 16:38:08

by Guilherme Giacomo Simoes

[permalink] [raw]
Subject: Re: [PATCH] thermal: adding check if the thermal firmware is running

Kalle Valo <[email protected]> writes:
>
> Guilherme Giacomo Simoes <[email protected]> writes:
>
> > 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 <[email protected]> for
> > your suggestions in mu first patch that results in this another patch.
> > ---
> > drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Please read the wiki link below how to submit wireless patches. For
> example, the title is wrong and you haven't signed the patch.
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Thank you for your explanation Mr. Valo.
I will fix the patch and resend.

2024-05-17 17:26:37

by Guilherme Giacomo Simoes

[permalink] [raw]
Subject: Re: [PATCH] thermal: adding check if the thermal firmware is running

Em sex., 17 de mai. de 2024 às 13:57, Jonathan Bither
<[email protected]> escreveu:
>
>
> On 5/17/24 10:16, 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 <[email protected]> for
> > your suggestions in mu first patch that results in this another patch.
> > ---
> > 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..68ab9966330c 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) {
> > + const int res = iwl_mvm_firmware_running(mvm);
> > +
> > + if (!res) {
> > + ret = -EAGAIN;
> > + goto out;
> > + }
> > +
>
> You could skip using the res variable and move the mutex lock here and
> simplify the above a bit. Ex:
>
> int temp;
>
> - mutex_lock(&mvm->mutex);
> + if (!iwl_mvm_firmware_running(mvm))
> + return -EAGAIN;
>
> - if (!iwl_mvm_firmware_running(mvm) ||
> - mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
> + mutex_lock(&mvm->mutex);
> + if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
> ret = -ENODATA;
> goto out;
> }
>
> > + if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
> > ret = -ENODATA;
> > goto out;
> > }

Hey Jonathan, Thank you for your suggestion.
I sended a v2 patch of this patch
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

If you want, you can send this suggestion in this patch v2.

Thanks.