diff mbox series

[v2,3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points

Message ID 20240403-gpa-no-cooling-devs-v2-3-79bdd8439449@trvn.ru (mailing list archive)
State Rejected, archived
Headers show
Series gov_power_allocator: Allow binding before cooling devices | expand

Commit Message

Nikita Travkin via B4 Relay April 3, 2024, 11:31 a.m. UTC 
From: Nikita Travkin <nikita@trvn.ru>

IPA warns if the thermal zone it was attached to doesn't define
sustainable_power value. In some cases though IPA may be bound to an
"empty" TZ, in which case the lack of sustainable_power doesn't matter.

Suppress the warning in case when IPA is bound to an empty TZ to make it
easier to see the warnings that actually matter.

Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---

I've decided to add this along to supress those warnings for some TZ on
sc7180. Feel free to drop this patch if you think the warning should
always appear.
---
 drivers/thermal/gov_power_allocator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lukasz Luba April 3, 2024, 12:52 p.m. UTC | #1 
On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@trvn.ru>
> 
> IPA warns if the thermal zone it was attached to doesn't define
> sustainable_power value. In some cases though IPA may be bound to an
> "empty" TZ, in which case the lack of sustainable_power doesn't matter.
> 
> Suppress the warning in case when IPA is bound to an empty TZ to make it
> easier to see the warnings that actually matter.
> 
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> 
> I've decided to add this along to supress those warnings for some TZ on
> sc7180. Feel free to drop this patch if you think the warning should
> always appear.

That warning should stay, since in the development or integration phase
quite a lot of stuff is missing. This will warn that there is an issue.
The case with 'empty' TZ is an exception only to 'work' with IPA.

Thanks for the patches!

Regards,
Lukasz


> ---
>   drivers/thermal/gov_power_allocator.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index e25e48d76aa7..05a40f6b5928 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -704,7 +704,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>   		params->allocated_tzp = true;
>   	}
>   
> -	if (!tz->tzp->sustainable_power)
> +	if (!tz->tzp->sustainable_power && params->trip_max)
>   		dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
>   	else
>   		params->sustainable_power = tz->tzp->sustainable_power;
>
Nikita Travkin April 3, 2024, 1:05 p.m. UTC | #2 
ср, 3 апр. 2024 г. в 17:52, Lukasz Luba <lukasz.luba@arm.com>:
>
>
>
> On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> > From: Nikita Travkin <nikita@trvn.ru>
> >
> > IPA warns if the thermal zone it was attached to doesn't define
> > sustainable_power value. In some cases though IPA may be bound to an
> > "empty" TZ, in which case the lack of sustainable_power doesn't matter.
> >
> > Suppress the warning in case when IPA is bound to an empty TZ to make it
> > easier to see the warnings that actually matter.
> >
> > Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> > ---
> >
> > I've decided to add this along to supress those warnings for some TZ on
> > sc7180. Feel free to drop this patch if you think the warning should
> > always appear.
>
> That warning should stay, since in the development or integration phase
> quite a lot of stuff is missing. This will warn that there is an issue.
> The case with 'empty' TZ is an exception only to 'work' with IPA.
>

Yes, that's understandable, though by suppressing those I could
actually see the few actual warnings for TZ with cooling devices
and no value, which I couldn't see before because it looked like
"all of them" have the warning.

In any case, as I said, I'm fine with this not being applied :)

Thanks for your review!
Nikita

> Thanks for the patches!
>
> Regards,
> Lukasz
>
>
> > ---
> >   drivers/thermal/gov_power_allocator.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> > index e25e48d76aa7..05a40f6b5928 100644
> > --- a/drivers/thermal/gov_power_allocator.c
> > +++ b/drivers/thermal/gov_power_allocator.c
> > @@ -704,7 +704,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> >               params->allocated_tzp = true;
> >       }
> >
> > -     if (!tz->tzp->sustainable_power)
> > +     if (!tz->tzp->sustainable_power && params->trip_max)
> >               dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
> >       else
> >               params->sustainable_power = tz->tzp->sustainable_power;
> >
diff mbox series

Patch

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index e25e48d76aa7..05a40f6b5928 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -704,7 +704,7 @@  static int power_allocator_bind(struct thermal_zone_device *tz)
 		params->allocated_tzp = true;
 	}
 
-	if (!tz->tzp->sustainable_power)
+	if (!tz->tzp->sustainable_power && params->trip_max)
 		dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
 	else
 		params->sustainable_power = tz->tzp->sustainable_power;