* [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute
@ 2023-02-20 16:24 Daniel Lezcano
2023-02-21 6:22 ` Zhang, Rui
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2023-02-20 16:24 UTC (permalink / raw)
To: daniel.lezcano, rafael
Cc: Sujith Thomas, Amit Kucheria, Zhang Rui,
open list:INTEL MENLOW THERMAL DRIVER, open list
The driver hooks the thermal framework sysfs to add some driver
specific information. A debatable approach as that may belong the
device sysfs directory, not the thermal zone directory.
As the driver is accessing the thermal internals, we should provide at
least an API to the thermal framework to add an attribute to the
existing sysfs thermal zone entry.
Before doing that and given the age of the driver (2008) may be it is
worth to double check if these attributes are really needed. So my
first proposal is to remove them if that does not hurt.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/intel/intel_menlow.c | 193 ---------------------------
1 file changed, 193 deletions(-)
diff --git a/drivers/thermal/intel/intel_menlow.c b/drivers/thermal/intel/intel_menlow.c
index 5a6ad0552311..5a9738a93083 100644
--- a/drivers/thermal/intel/intel_menlow.c
+++ b/drivers/thermal/intel/intel_menlow.c
@@ -230,174 +230,8 @@ struct intel_menlow_attribute {
static LIST_HEAD(intel_menlow_attr_list);
static DEFINE_MUTEX(intel_menlow_attr_lock);
-/*
- * sensor_get_auxtrip - get the current auxtrip value from sensor
- * @handle: Object handle
- * @index : GET_AUX1/GET_AUX0
- * @value : The address will be fill by the value
- */
-static int sensor_get_auxtrip(acpi_handle handle, int index,
- unsigned long long *value)
-{
- acpi_status status;
-
- if ((index != 0 && index != 1) || !value)
- return -EINVAL;
-
- status = acpi_evaluate_integer(handle, index ? GET_AUX1 : GET_AUX0,
- NULL, value);
- if (ACPI_FAILURE(status))
- return -EIO;
-
- return 0;
-}
-
-/*
- * sensor_set_auxtrip - set the new auxtrip value to sensor
- * @handle: Object handle
- * @index : GET_AUX1/GET_AUX0
- * @value : The value will be set
- */
-static int sensor_set_auxtrip(acpi_handle handle, int index, int value)
-{
- acpi_status status;
- union acpi_object arg = {
- ACPI_TYPE_INTEGER
- };
- struct acpi_object_list args = {
- 1, &arg
- };
- unsigned long long temp;
-
- if (index != 0 && index != 1)
- return -EINVAL;
-
- status = acpi_evaluate_integer(handle, index ? GET_AUX0 : GET_AUX1,
- NULL, &temp);
- if (ACPI_FAILURE(status))
- return -EIO;
- if ((index && value < temp) || (!index && value > temp))
- return -EINVAL;
-
- arg.integer.value = value;
- status = acpi_evaluate_integer(handle, index ? SET_AUX1 : SET_AUX0,
- &args, &temp);
- if (ACPI_FAILURE(status))
- return -EIO;
-
- /* do we need to check the return value of SAX0/SAX1 ? */
-
- return 0;
-}
-
-#define to_intel_menlow_attr(_attr) \
- container_of(_attr, struct intel_menlow_attribute, attr)
-
-static ssize_t aux_show(struct device *dev, struct device_attribute *dev_attr,
- char *buf, int idx)
-{
- struct intel_menlow_attribute *attr = to_intel_menlow_attr(dev_attr);
- unsigned long long value;
- int result;
-
- result = sensor_get_auxtrip(attr->handle, idx, &value);
- if (result)
- return result;
-
- return sprintf(buf, "%lu", deci_kelvin_to_celsius(value));
-}
-
-static ssize_t aux0_show(struct device *dev,
- struct device_attribute *dev_attr, char *buf)
-{
- return aux_show(dev, dev_attr, buf, 0);
-}
-
-static ssize_t aux1_show(struct device *dev,
- struct device_attribute *dev_attr, char *buf)
-{
- return aux_show(dev, dev_attr, buf, 1);
-}
-
-static ssize_t aux_store(struct device *dev, struct device_attribute *dev_attr,
- const char *buf, size_t count, int idx)
-{
- struct intel_menlow_attribute *attr = to_intel_menlow_attr(dev_attr);
- int value;
- int result;
-
- /*Sanity check; should be a positive integer */
- if (!sscanf(buf, "%d", &value))
- return -EINVAL;
-
- if (value < 0)
- return -EINVAL;
-
- result = sensor_set_auxtrip(attr->handle, idx,
- celsius_to_deci_kelvin(value));
- return result ? result : count;
-}
-
-static ssize_t aux0_store(struct device *dev,
- struct device_attribute *dev_attr,
- const char *buf, size_t count)
-{
- return aux_store(dev, dev_attr, buf, count, 0);
-}
-
-static ssize_t aux1_store(struct device *dev,
- struct device_attribute *dev_attr,
- const char *buf, size_t count)
-{
- return aux_store(dev, dev_attr, buf, count, 1);
-}
-
/* BIOS can enable/disable the thermal user application in dabney platform */
#define BIOS_ENABLED "\\_TZ.GSTS"
-static ssize_t bios_enabled_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- acpi_status status;
- unsigned long long bios_enabled;
-
- status = acpi_evaluate_integer(NULL, BIOS_ENABLED, NULL, &bios_enabled);
- if (ACPI_FAILURE(status))
- return -ENODEV;
-
- return sprintf(buf, "%s\n", bios_enabled ? "enabled" : "disabled");
-}
-
-static int intel_menlow_add_one_attribute(char *name, umode_t mode, void *show,
- void *store, struct device *dev,
- acpi_handle handle)
-{
- struct intel_menlow_attribute *attr;
- int result;
-
- attr = kzalloc(sizeof(struct intel_menlow_attribute), GFP_KERNEL);
- if (!attr)
- return -ENOMEM;
-
- sysfs_attr_init(&attr->attr.attr); /* That is consistent naming :D */
- attr->attr.attr.name = name;
- attr->attr.attr.mode = mode;
- attr->attr.show = show;
- attr->attr.store = store;
- attr->device = dev;
- attr->handle = handle;
-
- result = device_create_file(dev, &attr->attr);
- if (result) {
- kfree(attr);
- return result;
- }
-
- mutex_lock(&intel_menlow_attr_lock);
- list_add_tail(&attr->node, &intel_menlow_attr_list);
- mutex_unlock(&intel_menlow_attr_lock);
-
- return 0;
-}
static acpi_status intel_menlow_register_sensor(acpi_handle handle, u32 lvl,
void *context, void **rv)
@@ -420,12 +254,6 @@ static acpi_status intel_menlow_register_sensor(acpi_handle handle, u32 lvl,
if (ACPI_FAILURE(status))
return (status == AE_NOT_FOUND) ? AE_OK : status;
- result = intel_menlow_add_one_attribute("aux0", 0644,
- aux0_show, aux0_store,
- &thermal->device, handle);
- if (result)
- return AE_ERROR;
-
status = acpi_get_handle(handle, GET_AUX1, &dummy);
if (ACPI_FAILURE(status))
goto aux1_not_found;
@@ -434,27 +262,6 @@ static acpi_status intel_menlow_register_sensor(acpi_handle handle, u32 lvl,
if (ACPI_FAILURE(status))
goto aux1_not_found;
- result = intel_menlow_add_one_attribute("aux1", 0644,
- aux1_show, aux1_store,
- &thermal->device, handle);
- if (result) {
- intel_menlow_unregister_sensor();
- return AE_ERROR;
- }
-
- /*
- * create the "dabney_enabled" attribute which means the user app
- * should be loaded or not
- */
-
- result = intel_menlow_add_one_attribute("bios_enabled", 0444,
- bios_enabled_show, NULL,
- &thermal->device, handle);
- if (result) {
- intel_menlow_unregister_sensor();
- return AE_ERROR;
- }
-
return AE_OK;
aux1_not_found:
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute
2023-02-20 16:24 [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute Daniel Lezcano
@ 2023-02-21 6:22 ` Zhang, Rui
2023-02-21 11:30 ` Daniel Lezcano
0 siblings, 1 reply; 5+ messages in thread
From: Zhang, Rui @ 2023-02-21 6:22 UTC (permalink / raw)
To: rafael, daniel.lezcano; +Cc: linux-pm, linux-kernel, Thomas, Sujith, amitk
On Mon, 2023-02-20 at 17:24 +0100, Daniel Lezcano wrote:
> The driver hooks the thermal framework sysfs to add some driver
> specific information. A debatable approach as that may belong the
> device sysfs directory, not the thermal zone directory.
>
> As the driver is accessing the thermal internals, we should provide
> at
> least an API to the thermal framework to add an attribute to the
> existing sysfs thermal zone entry.
>
> Before doing that and given the age of the driver (2008) may be it is
> worth to double check if these attributes are really needed. So my
> first proposal is to remove them if that does not hurt.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
I don't have any device that uses this driver.
Let's see what Sujith says.
thanks,
rui
> ---
> drivers/thermal/intel/intel_menlow.c | 193 -----------------------
> ----
> 1 file changed, 193 deletions(-)
>
> diff --git a/drivers/thermal/intel/intel_menlow.c
> b/drivers/thermal/intel/intel_menlow.c
> index 5a6ad0552311..5a9738a93083 100644
> --- a/drivers/thermal/intel/intel_menlow.c
> +++ b/drivers/thermal/intel/intel_menlow.c
> @@ -230,174 +230,8 @@ struct intel_menlow_attribute {
> static LIST_HEAD(intel_menlow_attr_list);
> static DEFINE_MUTEX(intel_menlow_attr_lock);
>
> -/*
> - * sensor_get_auxtrip - get the current auxtrip value from sensor
> - * @handle: Object handle
> - * @index : GET_AUX1/GET_AUX0
> - * @value : The address will be fill by the value
> - */
> -static int sensor_get_auxtrip(acpi_handle handle, int index,
> - unsigned long
> long *value)
> -{
> - acpi_status status;
> -
> - if ((index != 0 && index != 1) || !value)
> - return -EINVAL;
> -
> - status = acpi_evaluate_integer(handle, index ? GET_AUX1 :
> GET_AUX0,
> - NULL, value);
> - if (ACPI_FAILURE(status))
> - return -EIO;
> -
> - return 0;
> -}
> -
> -/*
> - * sensor_set_auxtrip - set the new auxtrip value to sensor
> - * @handle: Object handle
> - * @index : GET_AUX1/GET_AUX0
> - * @value : The value will be set
> - */
> -static int sensor_set_auxtrip(acpi_handle handle, int index, int
> value)
> -{
> - acpi_status status;
> - union acpi_object arg = {
> - ACPI_TYPE_INTEGER
> - };
> - struct acpi_object_list args = {
> - 1, &arg
> - };
> - unsigned long long temp;
> -
> - if (index != 0 && index != 1)
> - return -EINVAL;
> -
> - status = acpi_evaluate_integer(handle, index ? GET_AUX0 :
> GET_AUX1,
> - NULL, &temp);
> - if (ACPI_FAILURE(status))
> - return -EIO;
> - if ((index && value < temp) || (!index && value > temp))
> - return -EINVAL;
> -
> - arg.integer.value = value;
> - status = acpi_evaluate_integer(handle, index ? SET_AUX1 :
> SET_AUX0,
> - &args, &temp);
> - if (ACPI_FAILURE(status))
> - return -EIO;
> -
> - /* do we need to check the return value of SAX0/SAX1 ? */
> -
> - return 0;
> -}
> -
> -#define to_intel_menlow_attr(_attr) \
> - container_of(_attr, struct intel_menlow_attribute, attr)
> -
> -static ssize_t aux_show(struct device *dev, struct device_attribute
> *dev_attr,
> - char *buf, int idx)
> -{
> - struct intel_menlow_attribute *attr =
> to_intel_menlow_attr(dev_attr);
> - unsigned long long value;
> - int result;
> -
> - result = sensor_get_auxtrip(attr->handle, idx, &value);
> - if (result)
> - return result;
> -
> - return sprintf(buf, "%lu", deci_kelvin_to_celsius(value));
> -}
> -
> -static ssize_t aux0_show(struct device *dev,
> - struct device_attribute *dev_attr, char *buf)
> -{
> - return aux_show(dev, dev_attr, buf, 0);
> -}
> -
> -static ssize_t aux1_show(struct device *dev,
> - struct device_attribute *dev_attr, char *buf)
> -{
> - return aux_show(dev, dev_attr, buf, 1);
> -}
> -
> -static ssize_t aux_store(struct device *dev, struct device_attribute
> *dev_attr,
> - const char *buf, size_t count, int idx)
> -{
> - struct intel_menlow_attribute *attr =
> to_intel_menlow_attr(dev_attr);
> - int value;
> - int result;
> -
> - /*Sanity check; should be a positive integer */
> - if (!sscanf(buf, "%d", &value))
> - return -EINVAL;
> -
> - if (value < 0)
> - return -EINVAL;
> -
> - result = sensor_set_auxtrip(attr->handle, idx,
> - celsius_to_deci_kelvin(value));
> - return result ? result : count;
> -}
> -
> -static ssize_t aux0_store(struct device *dev,
> - struct device_attribute *dev_attr,
> - const char *buf, size_t count)
> -{
> - return aux_store(dev, dev_attr, buf, count, 0);
> -}
> -
> -static ssize_t aux1_store(struct device *dev,
> - struct device_attribute *dev_attr,
> - const char *buf, size_t count)
> -{
> - return aux_store(dev, dev_attr, buf, count, 1);
> -}
> -
> /* BIOS can enable/disable the thermal user application in dabney
> platform */
> #define BIOS_ENABLED "\\_TZ.GSTS"
> -static ssize_t bios_enabled_show(struct device *dev,
> - struct device_attribute *attr, char
> *buf)
> -{
> - acpi_status status;
> - unsigned long long bios_enabled;
> -
> - status = acpi_evaluate_integer(NULL, BIOS_ENABLED, NULL,
> &bios_enabled);
> - if (ACPI_FAILURE(status))
> - return -ENODEV;
> -
> - return sprintf(buf, "%s\n", bios_enabled ? "enabled" :
> "disabled");
> -}
> -
> -static int intel_menlow_add_one_attribute(char *name, umode_t mode,
> void *show,
> - void *store, struct device
> *dev,
> - acpi_handle handle)
> -{
> - struct intel_menlow_attribute *attr;
> - int result;
> -
> - attr = kzalloc(sizeof(struct intel_menlow_attribute),
> GFP_KERNEL);
> - if (!attr)
> - return -ENOMEM;
> -
> - sysfs_attr_init(&attr->attr.attr); /* That is consistent naming
> :D */
> - attr->attr.attr.name = name;
> - attr->attr.attr.mode = mode;
> - attr->attr.show = show;
> - attr->attr.store = store;
> - attr->device = dev;
> - attr->handle = handle;
> -
> - result = device_create_file(dev, &attr->attr);
> - if (result) {
> - kfree(attr);
> - return result;
> - }
> -
> - mutex_lock(&intel_menlow_attr_lock);
> - list_add_tail(&attr->node, &intel_menlow_attr_list);
> - mutex_unlock(&intel_menlow_attr_lock);
> -
> - return 0;
> -}
>
> static acpi_status intel_menlow_register_sensor(acpi_handle handle,
> u32 lvl,
> void *context, void
> **rv)
> @@ -420,12 +254,6 @@ static acpi_status
> intel_menlow_register_sensor(acpi_handle handle, u32 lvl,
> if (ACPI_FAILURE(status))
> return (status == AE_NOT_FOUND) ? AE_OK : status;
>
> - result = intel_menlow_add_one_attribute("aux0", 0644,
> - aux0_show, aux0_store,
> - &thermal->device,
> handle);
> - if (result)
> - return AE_ERROR;
> -
> status = acpi_get_handle(handle, GET_AUX1, &dummy);
> if (ACPI_FAILURE(status))
> goto aux1_not_found;
> @@ -434,27 +262,6 @@ static acpi_status
> intel_menlow_register_sensor(acpi_handle handle, u32 lvl,
> if (ACPI_FAILURE(status))
> goto aux1_not_found;
>
> - result = intel_menlow_add_one_attribute("aux1", 0644,
> - aux1_show, aux1_store,
> - &thermal->device,
> handle);
> - if (result) {
> - intel_menlow_unregister_sensor();
> - return AE_ERROR;
> - }
> -
> - /*
> - * create the "dabney_enabled" attribute which means the user
> app
> - * should be loaded or not
> - */
> -
> - result = intel_menlow_add_one_attribute("bios_enabled", 0444,
> - bios_enabled_show,
> NULL,
> - &thermal->device,
> handle);
> - if (result) {
> - intel_menlow_unregister_sensor();
> - return AE_ERROR;
> - }
> -
> return AE_OK;
>
> aux1_not_found:
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute
2023-02-21 6:22 ` Zhang, Rui
@ 2023-02-21 11:30 ` Daniel Lezcano
2023-02-21 12:58 ` Zhang, Rui
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2023-02-21 11:30 UTC (permalink / raw)
To: Zhang, Rui, rafael; +Cc: linux-pm, linux-kernel, Thomas, Sujith, amitk
Hi Rui,
On 21/02/2023 07:22, Zhang, Rui wrote:
> On Mon, 2023-02-20 at 17:24 +0100, Daniel Lezcano wrote:
>> The driver hooks the thermal framework sysfs to add some driver
>> specific information. A debatable approach as that may belong the
>> device sysfs directory, not the thermal zone directory.
>>
>> As the driver is accessing the thermal internals, we should provide
>> at
>> least an API to the thermal framework to add an attribute to the
>> existing sysfs thermal zone entry.
>>
>> Before doing that and given the age of the driver (2008) may be it is
>> worth to double check if these attributes are really needed. So my
>> first proposal is to remove them if that does not hurt.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> I don't have any device that uses this driver.
> Let's see what Sujith says.
Thanks for your answer.
I take the opportunity to ask you for the ACPI thermal additional sysfs
entries.
The ACPI thermal driver adds a link:
/sys/class/thermal/thermal_zone0/device
which points to:
../../../LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00
And in this directory there is:
/sys/devices/LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00/thermal_zone
pointing to /sys/class/thermal/thermal_zone0
I was wondering if we have to keep it also? It is a cyclic description
and we can have the several thermal zones having a device link pointing
to the same location. So I'm not sure this is correct.
I can understand adding a link in the thermal zone pointing to the
device could make sense, and that could be generalized to all the
thermal zone creation, but the back pointer link seems strange.
Would it make sense to remove this second link ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute
2023-02-21 11:30 ` Daniel Lezcano
@ 2023-02-21 12:58 ` Zhang, Rui
2023-02-21 13:31 ` Daniel Lezcano
0 siblings, 1 reply; 5+ messages in thread
From: Zhang, Rui @ 2023-02-21 12:58 UTC (permalink / raw)
To: rafael, daniel.lezcano; +Cc: linux-pm, linux-kernel, Thomas, Sujith, amitk
Hi, Daniel,
On Tue, 2023-02-21 at 12:30 +0100, Daniel Lezcano wrote:
> Hi Rui,
>
>
> On 21/02/2023 07:22, Zhang, Rui wrote:
> > On Mon, 2023-02-20 at 17:24 +0100, Daniel Lezcano wrote:
> > > The driver hooks the thermal framework sysfs to add some driver
> > > specific information. A debatable approach as that may belong the
> > > device sysfs directory, not the thermal zone directory.
> > >
> > > As the driver is accessing the thermal internals, we should
> > > provide
> > > at
> > > least an API to the thermal framework to add an attribute to the
> > > existing sysfs thermal zone entry.
> > >
> > > Before doing that and given the age of the driver (2008) may be
> > > it is
> > > worth to double check if these attributes are really needed. So
> > > my
> > > first proposal is to remove them if that does not hurt.
> > >
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >
> > I don't have any device that uses this driver.
> > Let's see what Sujith says.
>
> Thanks for your answer.
>
> I take the opportunity to ask you for the ACPI thermal additional
> sysfs
> entries.
>
> The ACPI thermal driver adds a link:
>
> /sys/class/thermal/thermal_zone0/device
>
> which points to:
>
> ../../../LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00
>
>
> And in this directory there is:
>
> /sys/devices/LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00/thermal_zone
>
> pointing to /sys/class/thermal/thermal_zone0
>
>
> I was wondering if we have to keep it also? It is a cyclic
> description
> and we can have the several thermal zones having a device link
> pointing
> to the same location.
I don't think so. So far, ACPI Thermal object and the generic thermal
zone device are 1:1 match.
> So I'm not sure this is correct.
>
> I can understand adding a link in the thermal zone pointing to the
> device could make sense, and that could be generalized to all the
> thermal zone creation, but the back pointer link seems strange.
>
> Would it make sense to remove this second link ?
That was required by some userpsace tool running on menlow, similar to
this one. But TBH, I don't recall the userspace details.
thanks,
rui
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute
2023-02-21 12:58 ` Zhang, Rui
@ 2023-02-21 13:31 ` Daniel Lezcano
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Lezcano @ 2023-02-21 13:31 UTC (permalink / raw)
To: Zhang, Rui, rafael; +Cc: linux-pm, linux-kernel, Thomas, Sujith, amitk
On 21/02/2023 13:58, Zhang, Rui wrote:
> Hi, Daniel,
>
> On Tue, 2023-02-21 at 12:30 +0100, Daniel Lezcano wrote:
>> Hi Rui,
>>
>>
>> On 21/02/2023 07:22, Zhang, Rui wrote:
>>> On Mon, 2023-02-20 at 17:24 +0100, Daniel Lezcano wrote:
>>>> The driver hooks the thermal framework sysfs to add some driver
>>>> specific information. A debatable approach as that may belong the
>>>> device sysfs directory, not the thermal zone directory.
>>>>
>>>> As the driver is accessing the thermal internals, we should
>>>> provide
>>>> at
>>>> least an API to the thermal framework to add an attribute to the
>>>> existing sysfs thermal zone entry.
>>>>
>>>> Before doing that and given the age of the driver (2008) may be
>>>> it is
>>>> worth to double check if these attributes are really needed. So
>>>> my
>>>> first proposal is to remove them if that does not hurt.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>
>>> I don't have any device that uses this driver.
>>> Let's see what Sujith says.
>>
>> Thanks for your answer.
>>
>> I take the opportunity to ask you for the ACPI thermal additional
>> sysfs
>> entries.
>>
>> The ACPI thermal driver adds a link:
>>
>> /sys/class/thermal/thermal_zone0/device
>>
>> which points to:
>>
>> ../../../LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00
>>
>>
>> And in this directory there is:
>>
>> /sys/devices/LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00/thermal_zone
>>
>> pointing to /sys/class/thermal/thermal_zone0
>>
>>
>> I was wondering if we have to keep it also? It is a cyclic
>> description
>> and we can have the several thermal zones having a device link
>> pointing
>> to the same location.
>
> I don't think so. So far, ACPI Thermal object and the generic thermal
> zone device are 1:1 match.
>
>> So I'm not sure this is correct.
>>
>> I can understand adding a link in the thermal zone pointing to the
>> device could make sense, and that could be generalized to all the
>> thermal zone creation, but the back pointer link seems strange.
>>
>> Would it make sense to remove this second link ?
>
> That was required by some userpsace tool running on menlow, similar to
> this one. But TBH, I don't recall the userspace details.
If there is a 1:1 relationship, how can the userspace require the kernel
to describe an information it already has?
eg.
/sys/class/thermal/thermal_zone0/device points to
../../../LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00
So, userspace has already the opposite relationship as the 1:1 tells
thermal_zone0 -> LNXTHERM:00, then LNXTHERM:00 is associated to
thermal_zone0. It is a duplicate information in sysfs.
Anyway, I guess that now it is in sysfs, the removal will be very hard
to achieve *sigh*
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-21 13:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-20 16:24 [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute Daniel Lezcano
2023-02-21 6:22 ` Zhang, Rui
2023-02-21 11:30 ` Daniel Lezcano
2023-02-21 12:58 ` Zhang, Rui
2023-02-21 13:31 ` Daniel Lezcano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®