public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [thermal] adding check if the thermal firmware is running
@ 2024-05-16 20:05 Guilherme Giacomo Simoes
  2024-05-17 10:20 ` Wysocki, Rafael J
  0 siblings, 1 reply; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2024-05-16 20:05 UTC (permalink / raw)
  To: miriam.rachel.korenblit, kvalo, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov
  Cc: Guilherme Giacomo Simoes, linux-wireless, linux-kernel

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
occurrence because the thermal try read the temperature when is started,
but the firmware is not running yet.

For more legibiliti i create the NOTLOAD error code in the errno-base.h,
and in the iwl_mvm_tzone_get_temp() on tt.c i check if firmware is
running and I set the NOTLOAD code for ret variable and goto out.

After this, in the update_temperature() in the thermal_code.c i received
the return of thermal_zone_get_temp() and check if return is NOTLOAD,
because if it is, I print the warning message "firmware yet not load"
and return for caller

The thermal_core.c i think that is generic for any thermal drivers and
not only used for tt.c of course.
But if this ipotetic driver not check if firmware is running before read
the temperature, the thermal_code.c is work as a before this change.

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. In your place the warning messafe "Firmware yet not
load" is showing.

I would like to thank you in advance for any contribution, suggestion
or criticism of my patch suggestion.
---
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++--
 drivers/thermal/thermal_core.c              | 10 +++++++---
 include/uapi/asm-generic/errno-base.h       |  1 +
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index 8083c4b2ab6b..dd5725db06d2 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 = -NOTLOAD;
+		goto out;
+	}
+
+	if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
 		ret = -ENODATA;
 		goto out;
 	}
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 34a31bc72023..4116d312d4a1 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -414,10 +414,14 @@ static void handle_thermal_trip(struct thermal_zone_device *tz,
 
 static void update_temperature(struct thermal_zone_device *tz)
 {
-	int temp, ret;
-
-	ret = __thermal_zone_get_temp(tz, &temp);
+	int temp;
+	int ret = __thermal_zone_get_temp(tz, &temp);
 	if (ret) {
+		if (ret == -NOTLOAD) {
+			pr_warn("Firmware yet not load");
+			return;
+		}
+
 		if (ret != -EAGAIN)
 			dev_warn(&tz->device,
 				 "failed to read out thermal zone (%d)\n",
diff --git a/include/uapi/asm-generic/errno-base.h b/include/uapi/asm-generic/errno-base.h
index 9653140bff92..8b92c41f7993 100644
--- a/include/uapi/asm-generic/errno-base.h
+++ b/include/uapi/asm-generic/errno-base.h
@@ -36,5 +36,6 @@
 #define	EPIPE		32	/* Broken pipe */
 #define	EDOM		33	/* Math argument out of domain of func */
 #define	ERANGE		34	/* Math result not representable */
+#define	NOTLOAD		35	/* Firmware yet not load */
 
 #endif
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] [thermal] adding check if the thermal firmware is running
  2024-05-16 20:05 [PATCH] [thermal] " Guilherme Giacomo Simoes
@ 2024-05-17 10:20 ` Wysocki, Rafael J
  2024-05-17 14:17   ` Guilherme Giácomo Simões
  0 siblings, 1 reply; 10+ messages in thread
From: Wysocki, Rafael J @ 2024-05-17 10:20 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes, miriam.rachel.korenblit, kvalo,
	daniel.lezcano, johannes.berg, dmantipov
  Cc: linux-wireless, linux-kernel


On 5/16/2024 10:05 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
> occurrence because the thermal try read the temperature when is started,
> but the firmware is not running yet.
>
> For more legibiliti i create the NOTLOAD error code in the errno-base.h,
> and in the iwl_mvm_tzone_get_temp() on tt.c i check if firmware is
> running and I set the NOTLOAD code for ret variable and goto out.
>
> After this, in the update_temperature() in the thermal_code.c i received
> the return of thermal_zone_get_temp() and check if return is NOTLOAD,
> because if it is, I print the warning message "firmware yet not load"
> and return for caller
>
> The thermal_core.c i think that is generic for any thermal drivers and
> not only used for tt.c of course.
> But if this ipotetic driver not check if firmware is running before read
> the temperature, the thermal_code.c is work as a before this change.
>
> 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. In your place the warning messafe "Firmware yet not
> load" is showing.
>
> I would like to thank you in advance for any contribution, suggestion
> or criticism of my patch suggestion.
> ---
>   drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++--
>   drivers/thermal/thermal_core.c              | 10 +++++++---
>   include/uapi/asm-generic/errno-base.h       |  1 +
>   3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> index 8083c4b2ab6b..dd5725db06d2 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 = -NOTLOAD;
> +		goto out;
> +	}
> +
> +	if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
>   		ret = -ENODATA;
>   		goto out;
>   	}
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 34a31bc72023..4116d312d4a1 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -414,10 +414,14 @@ static void handle_thermal_trip(struct thermal_zone_device *tz,
>   
>   static void update_temperature(struct thermal_zone_device *tz)
>   {
> -	int temp, ret;
> -
> -	ret = __thermal_zone_get_temp(tz, &temp);
> +	int temp;
> +	int ret = __thermal_zone_get_temp(tz, &temp);
>   	if (ret) {
> +		if (ret == -NOTLOAD) {
> +			pr_warn("Firmware yet not load");
> +			return;
> +		}
> +

The thermal core doesn't need to be modified for this.

Please print the new message from the driver and you may as well return 
-EAGAIN from it in all cases when the issue is expected to be 
intermittent to prevent the core from printing the (existing) warning 
message.

Thanks!


>   		if (ret != -EAGAIN)
>   			dev_warn(&tz->device,
>   				 "failed to read out thermal zone (%d)\n",
> diff --git a/include/uapi/asm-generic/errno-base.h b/include/uapi/asm-generic/errno-base.h
> index 9653140bff92..8b92c41f7993 100644
> --- a/include/uapi/asm-generic/errno-base.h
> +++ b/include/uapi/asm-generic/errno-base.h
> @@ -36,5 +36,6 @@
>   #define	EPIPE		32	/* Broken pipe */
>   #define	EDOM		33	/* Math argument out of domain of func */
>   #define	ERANGE		34	/* Math result not representable */
> +#define	NOTLOAD		35	/* Firmware yet not load */
>   
>   #endif

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] thermal: adding check if the thermal firmware is running
@ 2024-05-17 14:16 Guilherme Giacomo Simoes
  2024-05-17 14:53 ` Kalle Valo
  2024-05-17 16:57 ` Jonathan Bither
  0 siblings, 2 replies; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2024-05-17 14:16 UTC (permalink / raw)
  To: miriam.rachel.korenblit, kvalo, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov
  Cc: Guilherme Giacomo Simoes, linux-wireless, linux-kernel

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] [thermal] adding check if the thermal firmware is running
  2024-05-17 10:20 ` Wysocki, Rafael J
@ 2024-05-17 14:17   ` Guilherme Giácomo Simões
  0 siblings, 0 replies; 10+ messages in thread
From: Guilherme Giácomo Simões @ 2024-05-17 14:17 UTC (permalink / raw)
  To: Wysocki, Rafael J
  Cc: miriam.rachel.korenblit, kvalo, daniel.lezcano, johannes.berg,
	dmantipov, linux-wireless, linux-kernel

Okay, this makes sense.
I will send a new patch with your suggestions.

Thanks


Em sex., 17 de mai. de 2024 às 07:20, Wysocki, Rafael J
<rafael.j.wysocki@intel.com> escreveu:
>
>
> On 5/16/2024 10:05 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
> > occurrence because the thermal try read the temperature when is started,
> > but the firmware is not running yet.
> >
> > For more legibiliti i create the NOTLOAD error code in the errno-base.h,
> > and in the iwl_mvm_tzone_get_temp() on tt.c i check if firmware is
> > running and I set the NOTLOAD code for ret variable and goto out.
> >
> > After this, in the update_temperature() in the thermal_code.c i received
> > the return of thermal_zone_get_temp() and check if return is NOTLOAD,
> > because if it is, I print the warning message "firmware yet not load"
> > and return for caller
> >
> > The thermal_core.c i think that is generic for any thermal drivers and
> > not only used for tt.c of course.
> > But if this ipotetic driver not check if firmware is running before read
> > the temperature, the thermal_code.c is work as a before this change.
> >
> > 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. In your place the warning messafe "Firmware yet not
> > load" is showing.
> >
> > I would like to thank you in advance for any contribution, suggestion
> > or criticism of my patch suggestion.
> > ---
> >   drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 10 ++++++++--
> >   drivers/thermal/thermal_core.c              | 10 +++++++---
> >   include/uapi/asm-generic/errno-base.h       |  1 +
> >   3 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> > index 8083c4b2ab6b..dd5725db06d2 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 = -NOTLOAD;
> > +             goto out;
> > +     }
> > +
> > +     if (mvm->fwrt.cur_fw_img != IWL_UCODE_REGULAR) {
> >               ret = -ENODATA;
> >               goto out;
> >       }
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 34a31bc72023..4116d312d4a1 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -414,10 +414,14 @@ static void handle_thermal_trip(struct thermal_zone_device *tz,
> >
> >   static void update_temperature(struct thermal_zone_device *tz)
> >   {
> > -     int temp, ret;
> > -
> > -     ret = __thermal_zone_get_temp(tz, &temp);
> > +     int temp;
> > +     int ret = __thermal_zone_get_temp(tz, &temp);
> >       if (ret) {
> > +             if (ret == -NOTLOAD) {
> > +                     pr_warn("Firmware yet not load");
> > +                     return;
> > +             }
> > +
>
> The thermal core doesn't need to be modified for this.
>
> Please print the new message from the driver and you may as well return
> -EAGAIN from it in all cases when the issue is expected to be
> intermittent to prevent the core from printing the (existing) warning
> message.
>
> Thanks!
>
>
> >               if (ret != -EAGAIN)
> >                       dev_warn(&tz->device,
> >                                "failed to read out thermal zone (%d)\n",
> > diff --git a/include/uapi/asm-generic/errno-base.h b/include/uapi/asm-generic/errno-base.h
> > index 9653140bff92..8b92c41f7993 100644
> > --- a/include/uapi/asm-generic/errno-base.h
> > +++ b/include/uapi/asm-generic/errno-base.h
> > @@ -36,5 +36,6 @@
> >   #define     EPIPE           32      /* Broken pipe */
> >   #define     EDOM            33      /* Math argument out of domain of func */
> >   #define     ERANGE          34      /* Math result not representable */
> > +#define      NOTLOAD         35      /* Firmware yet not load */
> >
> >   #endif

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] thermal: adding check if the thermal firmware is running
  2024-05-17 14:16 [PATCH] thermal: adding check if the thermal firmware is running Guilherme Giacomo Simoes
@ 2024-05-17 14:53 ` Kalle Valo
  2024-05-17 16:35   ` Guilherme Giácomo Simões
  2024-05-17 16:37   ` Guilherme Giácomo Simões
  2024-05-17 16:57 ` Jonathan Bither
  1 sibling, 2 replies; 10+ messages in thread
From: Kalle Valo @ 2024-05-17 14:53 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes
  Cc: miriam.rachel.korenblit, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov, linux-wireless, linux-kernel

Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> 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 <refael.j.wysocki@intel.com> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] thermal: adding check if the thermal firmware is running
  2024-05-17 14:53 ` Kalle Valo
@ 2024-05-17 16:35   ` Guilherme Giácomo Simões
  2024-05-17 16:37   ` Guilherme Giácomo Simões
  1 sibling, 0 replies; 10+ messages in thread
From: Guilherme Giácomo Simões @ 2024-05-17 16:35 UTC (permalink / raw)
  To: Kalle Valo
  Cc: miriam.rachel.korenblit, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov, linux-wireless, linux-kernel

Kalle Valo <kvalo@kernel.org> 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 <refael.j.wysocki@intel.com> 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.

Okay, thank you for your explanation Mr. Valo
I will fix the patch and resend this.

Em sex., 17 de mai. de 2024 às 11:53, Kalle Valo <kvalo@kernel.org> escreveu:
>
> Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> 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 <refael.j.wysocki@intel.com> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] thermal: adding check if the thermal firmware is running
  2024-05-17 14:53 ` Kalle Valo
  2024-05-17 16:35   ` Guilherme Giácomo Simões
@ 2024-05-17 16:37   ` Guilherme Giácomo Simões
  1 sibling, 0 replies; 10+ messages in thread
From: Guilherme Giácomo Simões @ 2024-05-17 16:37 UTC (permalink / raw)
  To: Kalle Valo
  Cc: miriam.rachel.korenblit, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov, linux-wireless, linux-kernel

Kalle Valo <kvalo@kernel.org> writes:
>
> Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> 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 <refael.j.wysocki@intel.com> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] thermal: adding check if the thermal firmware is running
  2024-05-17 14:16 [PATCH] thermal: adding check if the thermal firmware is running Guilherme Giacomo Simoes
  2024-05-17 14:53 ` Kalle Valo
@ 2024-05-17 16:57 ` Jonathan Bither
  2024-05-17 17:25   ` Guilherme Giácomo Simões
  1 sibling, 1 reply; 10+ messages in thread
From: Jonathan Bither @ 2024-05-17 16:57 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes, miriam.rachel.korenblit, kvalo,
	rafael.j.wysocki, daniel.lezcano, johannes.berg, dmantipov
  Cc: linux-wireless, linux-kernel


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 <refael.j.wysocki@intel.com> 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;
>   	}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] thermal: adding check if the thermal firmware is running
  2024-05-17 16:57 ` Jonathan Bither
@ 2024-05-17 17:25   ` Guilherme Giácomo Simões
  2024-05-17 17:43     ` Jonathan Bither
  0 siblings, 1 reply; 10+ messages in thread
From: Guilherme Giácomo Simões @ 2024-05-17 17:25 UTC (permalink / raw)
  To: Jonathan Bither
  Cc: miriam.rachel.korenblit, kvalo, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov, linux-wireless, linux-kernel

Em sex., 17 de mai. de 2024 às 13:57, Jonathan Bither
<jonbither@gmail.com> 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 <refael.j.wysocki@intel.com> 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/20240517171311.3705-1-trintaeoitogc@gmail.com/

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

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] thermal: adding check if the thermal firmware is running
  2024-05-17 17:25   ` Guilherme Giácomo Simões
@ 2024-05-17 17:43     ` Jonathan Bither
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Bither @ 2024-05-17 17:43 UTC (permalink / raw)
  To: Guilherme Giácomo Simões
  Cc: miriam.rachel.korenblit, kvalo, rafael.j.wysocki, daniel.lezcano,
	johannes.berg, dmantipov, linux-wireless, linux-kernel


On 5/17/24 13:25, Guilherme Giácomo Simões wrote:
> Em sex., 17 de mai. de 2024 às 13:57, Jonathan Bither
> <jonbither@gmail.com> 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 <refael.j.wysocki@intel.com> 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/20240517171311.3705-1-trintaeoitogc@gmail.com/
>
> If you want, you can send this suggestion in this patch v2.
Hey Guilherme, no worries.
>
> Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-05-17 17:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-17 14:16 [PATCH] thermal: adding check if the thermal firmware is running Guilherme Giacomo Simoes
2024-05-17 14:53 ` Kalle Valo
2024-05-17 16:35   ` Guilherme Giácomo Simões
2024-05-17 16:37   ` Guilherme Giácomo Simões
2024-05-17 16:57 ` Jonathan Bither
2024-05-17 17:25   ` Guilherme Giácomo Simões
2024-05-17 17:43     ` Jonathan Bither
  -- strict thread matches above, loose matches on Subject: below --
2024-05-16 20:05 [PATCH] [thermal] " Guilherme Giacomo Simoes
2024-05-17 10:20 ` Wysocki, Rafael J
2024-05-17 14:17   ` Guilherme Giácomo Simões

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox