Sorry, I just read Durgadoss last comment. Please ignore mine.
On Fri, Apr 6, 2012 at 8:01 AM, Rob Lee <rob.lee@xxxxxxxxxx> wrote:
> On Thu, Apr 5, 2012 at 12:52 AM, Amit Daniel Kachhap
> <amit.kachhap@xxxxxxxxxx> wrote:
>> Hi Durgadoss,
>>
>> Instead of making all the trip-points of a thermal zone writeable we should
>> only allow non-critical trip points to be writeable.
>>
>> Thanks,
>> Amit Daniel
>>
>
> Agree, or even better, could the writeable attribute be made an
> optional for any trip point?
>
> Many SoCs now have built in thermal sensing and for cost reasons some
> device makers may want to rely on the kernel thermal handling
> capabilities as their primary thermal protection. Allowing userspace
> to modify the trip points may increase their risk/exposure to lawsuits
> or other undesirable results in some cases.
>
> Best Regards,
> Rob
>
>>> -----Original Message-----
>>> From: Durgadoss R <durgadoss.r@xxxxxxxxx>
>>> Sent: Friday, January 27, 2012 9:58 PM
>>> To: linux-acpi@xxxxxxxxxxxxxxx
>>> Cc: lenb@xxxxxxxxxx; rui.zhang@xxxxxxxxx; durgadoss.r@xxxxxxxxx
>>> Subject: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
>>
>>>Some of the thermal drivers using the Generic Thermal Framework
>>>require (all/some) trip points to be writeable. This patch makes
>>>the trip point temperatures writeable on a per-trip point basis,
>>>and modifies the required function call in thermal.c. This patch
>>>also updates the Documentation to reflect the new change.
>>>
>>>Signed-off-by: Durgadoss R <durgadoss.r@xxxxxxxxx>
>>>---
>>>v1
>>>* patch1/2: Code for making trip points writeable
>>>* patch2/2: Change the callee in thermal.c
>>>v2
>>>* Make both the changes in a single patch
>>>* Update Documentation/thermal/sysfs-api.txt
>>>---
>>> Documentation/thermal/sysfs-api.txt | 4 +-
>>> drivers/acpi/thermal.c | 4 +-
>>> drivers/thermal/thermal_sys.c | 127 ++++++++++++++++++++++-------------
>>> include/linux/thermal.h | 10 ++-
>>> 4 files changed, 91 insertions(+), 54 deletions(-)
>>>
>>>diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>>>index b61e46f..cd1476c 100644
>>>--- a/Documentation/thermal/sysfs-api.txt
>>>+++ b/Documentation/thermal/sysfs-api.txt
>>>@@ -32,7 +32,8 @@ temperature) and throttle appropriate devices.
>>>
>>> 1.1 thermal zone device interface
>>> 1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name,
>>>- int trips, void *devdata, struct thermal_zone_device_ops *ops)
>>>+ int trips, int flag, void *devdata,
>>>+ struct thermal_zone_device_ops *ops)
>>>
>>> This interface function adds a new thermal zone device (sensor) to
>>> /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
>>>@@ -40,6 +41,7 @@ temperature) and throttle appropriate devices.
>>>
>>> name: the thermal zone name.
>>> trips: the total number of trip points this thermal zone supports.
>>>+ flag: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
>>> devdata: device private data
>>> ops: thermal zone device call-backs.
>>> .bind: bind the thermal zone device with a thermal cooling device.
>>>diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>>>index 48fbc64..1c3133e 100644
>>>--- a/drivers/acpi/thermal.c
>>>+++ b/drivers/acpi/thermal.c
>>>@@ -845,7 +845,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>>>
>>> if (tz->trips.passive.flags.valid)
>>> tz->thermal_zone =
>>>- thermal_zone_device_register("acpitz", trips, tz,
>>>+ thermal_zone_device_register("acpitz", trips, 0, tz,
>>> &acpi_thermal_zone_ops,
>>> tz->trips.passive.tc1,
>>> tz->trips.passive.tc2,
>>>@@ -853,7 +853,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>>> tz->polling_frequency*100);
>>> else
>>> tz->thermal_zone =
>>>- thermal_zone_device_register("acpitz", trips, tz,
>>>+ thermal_zone_device_register("acpitz", trips, 0, tz,
>>> &acpi_thermal_zone_ops,
>>> 0, 0, 0,
>>> tz->polling_frequency*100);
>>>diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>>>index dd9a574..3a4341e 100644
>>>--- a/drivers/thermal/thermal_sys.c
>>>+++ b/drivers/thermal/thermal_sys.c
>>>@@ -220,6 +220,28 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>>> }
>>>
>>> static ssize_t
>>>+trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>>>+ const char *buf, size_t count)
>>>+{
>>>+ struct thermal_zone_device *tz = to_thermal_zone(dev);
>>>+ int trip, ret;
>>>+ unsigned long temperature;
>>>+
>>>+ if (!tz->ops->set_trip_temp)
>>>+ return -EPERM;
>>>+
>>>+ if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
>>>+ return -EINVAL;
>>>+
>>>+ if (kstrtoul(buf, 10, &temperature))
>>>+ return -EINVAL;
>>>+
>>>+ ret = tz->ops->set_trip_temp(tz, trip, temperature);
>>>+
>>>+ return ret ? ret : count;
>>>+}
>>>+
>>>+static ssize_t
>>> passive_store(struct device *dev, struct device_attribute *attr,
>>> const char *buf, size_t count)
>>> {
>>>@@ -286,49 +308,6 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>>> static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
>>> passive_store);
>>>
>>>-static struct device_attribute trip_point_attrs[] = {
>>>- __ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_0_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_1_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_1_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_2_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_2_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_3_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_3_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_4_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_4_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_5_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_5_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_6_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_6_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_7_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_7_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_8_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_8_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_9_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_9_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_10_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_10_temp, 0444, trip_point_temp_show, NULL),
>>>- __ATTR(trip_point_11_type, 0444, trip_point_type_show, NULL),
>>>- __ATTR(trip_point_11_temp, 0444, trip_point_temp_show, NULL),
>>>-};
>>>-
>>>-#define TRIP_POINT_ATTR_ADD(_dev, _index, result) \
>>>-do { \
>>>- result = device_create_file(_dev, \
>>>- &trip_point_attrs[_index * 2]); \
>>>- if (result) \
>>>- break; \
>>>- result = device_create_file(_dev, \
>>>- &trip_point_attrs[_index * 2 + 1]); \
>>>-} while (0)
>>>-
>>>-#define TRIP_POINT_ATTR_REMOVE(_dev, _index) \
>>>-do { \
>>>- device_remove_file(_dev, &trip_point_attrs[_index * 2]); \
>>>- device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]); \
>>>-} while (0)
>>>-
>>> /* sys I/F for cooling device */
>>> #define to_cooling_device(_dev) \
>>> container_of(_dev, struct thermal_cooling_device, device)
>>>@@ -1112,9 +1091,54 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>>> EXPORT_SYMBOL(thermal_zone_device_update);
>>>
>>> /**
>>>+ * create_trip_type_attr - creates a trip point type attribute
>>>+ * @tz: the thermal zone device
>>>+ * @indx: index into the trip_type_attrs array
>>>+ */
>>>+static int create_trip_type_attr(struct thermal_zone_device *tz, int indx)
>>>+{
>>>+ char attr_name[THERMAL_NAME_LENGTH];
>>>+
>>>+ snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_type", indx);
>>>+
>>>+ sysfs_attr_init(&tz->trip_type_attrs[indx].attr);
>>>+ tz->trip_type_attrs[indx].attr.name = attr_name;
>>>+ tz->trip_type_attrs[indx].attr.mode = S_IRUGO;
>>>+ tz->trip_type_attrs[indx].show = trip_point_type_show;
>>>+
>>>+ return device_create_file(&tz->device, &tz->trip_type_attrs[indx]);
>>>+}
>>>+
>>>+/**
>>>+ * create_trip_temp_attr - creates a trip point temp attribute
>>>+ * @tz: the thermal zone device
>>>+ * @indx: index into the trip_type_attrs array
>>>+ * @writeable: A flag: If 1, 'this' trip point is writeable; otherwise not
>>>+ */
>>>+static int create_trip_temp_attr(struct thermal_zone_device *tz,
>>>+ int indx, int writeable)
>>>+{
>>>+ char attr_name[THERMAL_NAME_LENGTH];
>>>+
>>>+ snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_temp", indx);
>>>+
>>>+ sysfs_attr_init(&tz->trip_temp_attrs[indx].attr);
>>>+ tz->trip_temp_attrs[indx].attr.name = attr_name;
>>>+ tz->trip_temp_attrs[indx].attr.mode = S_IRUGO;
>>>+ tz->trip_temp_attrs[indx].show = trip_point_temp_show;
>>>+ if (writeable) {
>>>+ tz->trip_temp_attrs[indx].attr.mode |= S_IWUSR;
>>>+ tz->trip_temp_attrs[indx].store = trip_point_temp_store;
>>>+ }
>>>+
>>>+ return device_create_file(&tz->device, &tz->trip_temp_attrs[indx]);
>>>+}
>>>+
>>>+/**
>>> * thermal_zone_device_register - register a new thermal zone device
>>> * @type: the thermal zone device type
>>> * @trips: the number of trip points the thermal zone support
>>>+ * @flag: a bit string indicating the writeablility of trip points
>>> * @devdata: private device data
>>> * @ops: standard thermal zone device callbacks
>>> * @tc1: thermal coefficient 1 for passive calculations
>>>@@ -1130,7 +1154,7 @@ EXPORT_SYMBOL(thermal_zone_device_update);
>>> * section 11.1.5.1 of the ACPI specification 3.0.
>>> */
>>> struct thermal_zone_device *thermal_zone_device_register(char *type,
>>>- int trips, void *devdata,
>>>+ int trips, int flag, void *devdata,
>>> const struct thermal_zone_device_ops *ops,
>>> int tc1, int tc2, int passive_delay, int polling_delay)
>>> {
>>>@@ -1199,9 +1223,15 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
>>> }
>>>
>>> for (count = 0; count < trips; count++) {
>>>- TRIP_POINT_ATTR_ADD(&tz->device, count, result);
>>>+ result = create_trip_type_attr(tz, count);
>>> if (result)
>>> goto unregister;
>>>+
>>>+ result = create_trip_temp_attr(tz, count,
>>>+ (flag & (1 << count)));
>>>+ if (result)
>>>+ goto unregister;
>>>+
>>> tz->ops->get_trip_type(tz, count, &trip_type);
>>> if (trip_type == THERMAL_TRIP_PASSIVE)
>>> passive = 1;
>>>@@ -1279,9 +1309,10 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>>> if (tz->ops->get_mode)
>>> device_remove_file(&tz->device, &dev_attr_mode);
>>>
>>>- for (count = 0; count < tz->trips; count++)
>>>- TRIP_POINT_ATTR_REMOVE(&tz->device, count);
>>>-
>>>+ for (count = 0; count < tz->trips; count++) {
>>>+ device_remove_file(&tz->device, &tz->trip_type_attrs[count]);
>>>+ device_remove_file(&tz->device, &tz->trip_temp_attrs[count]);
>>>+ }
>>> thermal_remove_hwmon_sysfs(tz);
>>> release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
>>> idr_destroy(&tz->idr);
>>>diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>>index 47b4a27..4aa5a45 100644
>>>--- a/include/linux/thermal.h
>>>+++ b/include/linux/thermal.h
>>>@@ -58,6 +58,8 @@ struct thermal_zone_device_ops {
>>> enum thermal_trip_type *);
>>> int (*get_trip_temp) (struct thermal_zone_device *, int,
>>> unsigned long *);
>>>+ int (*set_trip_temp) (struct thermal_zone_device *, int,
>>>+ unsigned long);
>>> int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
>>> int (*notify) (struct thermal_zone_device *, int,
>>> enum thermal_trip_type);
>>>@@ -89,6 +91,8 @@ struct thermal_zone_device {
>>> int id;
>>> char type[THERMAL_NAME_LENGTH];
>>> struct device device;
>>>+ struct device_attribute trip_temp_attrs[THERMAL_MAX_TRIPS];
>>>+ struct device_attribute trip_type_attrs[THERMAL_MAX_TRIPS];
>>> void *devdata;
>>> int trips;
>>> int tc1;
>>>@@ -137,9 +141,9 @@ enum {
>>> };
>>> #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
>>>
>>>-struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
>>>- const struct thermal_zone_device_ops *, int tc1, int tc2,
>>>- int passive_freq, int polling_freq);
>>>+struct thermal_zone_device *thermal_zone_device_register(char *, int, int,
>>>+ void *, const struct thermal_zone_device_ops *, int tc1,
>>>+ int tc2, int passive_freq, int polling_freq);
>>> void thermal_zone_device_unregister(struct thermal_zone_device *);
>>>
>>> int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>>>--
>>>1.7.1
>>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev@xxxxxxxxxxxxxxxx
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
