| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <27488f102c917ce1f6d24d30f801b1e1745674e1.camel@intel.com>
Date: Tue, 21 Feb 2023 06:22:38 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "rafael@...nel.org" <rafael@...nel.org>,
"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>
CC: "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Thomas, Sujith" <sujith.thomas@...el.com>,
"amitk@...nel.org" <amitk@...nel.org>
Subject: Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove
add_one_attribute
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@...aro.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:
Powered by blists - more mailing lists