Re: [PATCH 2/3] thermal: int340x: Indicate userspace usage

[Kai-Heng Feng <kai.heng.feng@canonical.com>]

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