[2/3] thermal: int340x: Indicate userspace usage
diff mbox series

Message ID 20201128175450.12456-2-kai.heng.feng@canonical.com
State New, archived
Headers show
Series
  • [1/3] thermal: core: Add indication for userspace usage
Related show

Commit Message

Kai-Heng Feng Nov. 28, 2020, 5:54 p.m. UTC
The device isn't present under ACPI ThermalZone, and there's a dedicated
userspace daemon for this thermal device.

Let thermal core know it shouldn't handle trips to avoid surprising
thermal shutdown.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/thermal/intel/int340x_thermal/int3400_thermal.c     | 1 +
 .../thermal/intel/int340x_thermal/int340x_thermal_zone.c    | 6 +-----
 2 files changed, 2 insertions(+), 5 deletions(-)

Comments

Srinivas Pandruvada Nov. 30, 2020, 5:29 a.m. UTC | #1
On Sun, 2020-11-29 at 01:54 +0800, Kai-Heng Feng wrote:
> The device isn't present under ACPI ThermalZone, and there's a
> dedicated
> userspace daemon for this thermal device.
> 
> Let thermal core know it shouldn't handle trips to avoid surprising
> thermal shutdown.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c     | 1 +
>  .../thermal/intel/int340x_thermal/int340x_thermal_zone.c    | 6 +---
> --
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 0966551cbaaa..2002bc96eb3c 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -439,6 +439,7 @@ static struct thermal_zone_device_ops
> int3400_thermal_ops = {
>  static struct thermal_zone_params int3400_thermal_params = {
>  	.governor_name = "user_space",
>  	.no_hwmon = true,
> +	.userspace = true,
I am copied on only this patch, so I don't know what is this attribute?
I think it is new.

>  };
>  
>  static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
> diff --git
> a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 6e479deff76b..a103eb42ef2d 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -208,6 +208,7 @@ EXPORT_SYMBOL_GPL(int340x_thermal_read_trips);
>  static struct thermal_zone_params int340x_thermal_params = {
>  	.governor_name = "user_space",
>  	.no_hwmon = true,
> +	.userspace = true,
>  };
>  
>  struct int34x_thermal_zone *int340x_thermal_zone_add(struct
> acpi_device *adev,
> @@ -259,14 +260,9 @@ struct int34x_thermal_zone
> *int340x_thermal_zone_add(struct acpi_device *adev,
>  		ret = PTR_ERR(int34x_thermal_zone->zone);
>  		goto err_thermal_zone;
>  	}
> -	ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
> -	if (ret)
> -		goto err_enable;

What is the effect of this?
The INT340X zones don't need to poll for temperature. When HW notifies
then user space gets notified via user space governor. Not sure if the
not enabling break that path.

Thanks,
Srinivas

>  
>  	return int34x_thermal_zone;
>  
> -err_enable:
> -	thermal_zone_device_unregister(int34x_thermal_zone->zone);
>  err_thermal_zone:
>  	acpi_lpat_free_conversion_table(int34x_thermal_zone-
> >lpat_table);
>  	kfree(int34x_thermal_zone->aux_trips);
Kai-Heng Feng Nov. 30, 2020, 5:46 a.m. UTC | #2
> On Nov 30, 2020, at 13:29, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> 
> On Sun, 2020-11-29 at 01:54 +0800, Kai-Heng Feng wrote:
>> The device isn't present under ACPI ThermalZone, and there's a
>> dedicated
>> userspace daemon for this thermal device.
>> 
>> Let thermal core know it shouldn't handle trips to avoid surprising
>> thermal shutdown.
>> 
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/thermal/intel/int340x_thermal/int3400_thermal.c     | 1 +
>> .../thermal/intel/int340x_thermal/int340x_thermal_zone.c    | 6 +---
>> --
>> 2 files changed, 2 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
>> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
>> index 0966551cbaaa..2002bc96eb3c 100644
>> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
>> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
>> @@ -439,6 +439,7 @@ static struct thermal_zone_device_ops
>> int3400_thermal_ops = {
>> static struct thermal_zone_params int3400_thermal_params = {
>> 	.governor_name = "user_space",
>> 	.no_hwmon = true,
>> +	.userspace = true,
> I am copied on only this patch, so I don't know what is this attribute?
> I think it is new.

Ok. The first one doesn't seem to be sent out correctly.

Series resent.

> 
>> };
>> 
>> static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
>> diff --git
>> a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
>> b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
>> index 6e479deff76b..a103eb42ef2d 100644
>> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
>> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
>> @@ -208,6 +208,7 @@ EXPORT_SYMBOL_GPL(int340x_thermal_read_trips);
>> static struct thermal_zone_params int340x_thermal_params = {
>> 	.governor_name = "user_space",
>> 	.no_hwmon = true,
>> +	.userspace = true,
>> };
>> 
>> struct int34x_thermal_zone *int340x_thermal_zone_add(struct
>> acpi_device *adev,
>> @@ -259,14 +260,9 @@ struct int34x_thermal_zone
>> *int340x_thermal_zone_add(struct acpi_device *adev,
>> 		ret = PTR_ERR(int34x_thermal_zone->zone);
>> 		goto err_thermal_zone;
>> 	}
>> -	ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
>> -	if (ret)
>> -		goto err_enable;
> 
> What is the effect of this?
> The INT340X zones don't need to poll for temperature. When HW notifies
> then user space gets notified via user space governor. Not sure if the
> not enabling break that path.

thermal_zone_device_disable()
  thermal_notify_tz_disable()
    thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_DISABLE, &p)

I think it doesn't affect user_space governor.

Kai-Heng

> 
> Thanks,
> Srinivas
> 
>> 
>> 	return int34x_thermal_zone;
>> 
>> -err_enable:
>> -	thermal_zone_device_unregister(int34x_thermal_zone->zone);
>> err_thermal_zone:
>> 	acpi_lpat_free_conversion_table(int34x_thermal_zone-
>>> lpat_table);
>> 	kfree(int34x_thermal_zone->aux_trips);

Patch
diff mbox series

diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 0966551cbaaa..2002bc96eb3c 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -439,6 +439,7 @@  static struct thermal_zone_device_ops int3400_thermal_ops = {
 static struct thermal_zone_params int3400_thermal_params = {
 	.governor_name = "user_space",
 	.no_hwmon = true,
+	.userspace = true,
 };
 
 static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 6e479deff76b..a103eb42ef2d 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -208,6 +208,7 @@  EXPORT_SYMBOL_GPL(int340x_thermal_read_trips);
 static struct thermal_zone_params int340x_thermal_params = {
 	.governor_name = "user_space",
 	.no_hwmon = true,
+	.userspace = true,
 };
 
 struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
@@ -259,14 +260,9 @@  struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 		ret = PTR_ERR(int34x_thermal_zone->zone);
 		goto err_thermal_zone;
 	}
-	ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
-	if (ret)
-		goto err_enable;
 
 	return int34x_thermal_zone;
 
-err_enable:
-	thermal_zone_device_unregister(int34x_thermal_zone->zone);
 err_thermal_zone:
 	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
 	kfree(int34x_thermal_zone->aux_trips);