subject:"RE\: \[PATCH 4\/8\] Thermal\: Add trip point sysfs nodes for sensor"

RE: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor

2013-03-01 Thread R, Durgadoss

> -Original Message-
> From: linux-pm-ow...@vger.kernel.org [mailto:linux-pm-
> ow...@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Friday, March 01, 2013 1:21 AM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux...@vger.kernel.org; linux-kernel@vger.kernel.org;
> hongbo.zh...@linaro.org; w...@nvidia.com
> Subject: Re: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor
> 
> Durga,
> 
> 
> Same comments on patch 02/08 I want to rise here:
> - Minors on strlcpy, snprintf, devm_ helpers
> - documentation in the code for these helper functions and also better
> naming..
> 
> 
> On 05-02-2013 06:46, Durgadoss R wrote:
> > This patch adds a trip point related sysfs nodes
> > for each sensor under a zone in /sys/class/thermal/zoneX/.
> > The nodes will be named, sensorX_trip_active,
> > sensorX_trip_passive, sensorX_trip_hot, sensorX_trip_critical
> > for active, passive, hot and critical trip points
> > respectively for sensorX.
> >
> > Signed-off-by: Durgadoss R 
> > ---
> >   drivers/thermal/thermal_sys.c |  225
> -
> >   include/linux/thermal.h   |   38 ++-
> >   2 files changed, 260 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > index bf703b1..69a60a4 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -452,6 +452,37 @@ static void thermal_zone_device_check(struct
> work_struct *work)
> > thermal_zone_device_update(tz);
> >   }
> >
> > +static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char
> *name)
> > +{
> > +   int i, indx = -EINVAL;
> > +
> > +   mutex_lock(&sensor_list_lock);
> > +   for (i = 0; i < tz->sensor_indx; i++) {
> > +   if (!strnicmp(name, kobject_name(&tz->sensors[i]-
> >device.kobj),
> > +   THERMAL_NAME_LENGTH)) {
> > +   indx = i;
> > +   break;
> > +   }
> > +   }
> > +   mutex_unlock(&sensor_list_lock);
> > +   return indx;
> > +}
> > +
> > +static void inline __remove_trip_attr(struct thermal_zone *tz, int indx)
> I believe the preferred format would be:

Ok, will change..

> 
> static inline void __remove_trip_attr(struct thermal_zone *tz, int indx)
> 
> > +{
> > +   int i;
> > +   struct thermal_trip_attr *attr = tz->trip_attr[indx];
> > +
> > +   if (!attr)
> > +   return;
> > +
> > +   for (i = 0; i < NUM_TRIP_TYPES; i++)
> > +   device_remove_file(&tz->device, &attr->attrs[i].attr);
> > +
> > +   kfree(tz->trip_attr[indx]);
> > +   tz->trip_attr[indx] = NULL;
> > +}
> > +
> >   static void remove_sensor_from_zone(struct thermal_zone *tz,
> > struct thermal_sensor *ts)
> >   {
> > @@ -463,9 +494,15 @@ static void remove_sensor_from_zone(struct
> thermal_zone *tz,
> >
> > sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
> >device.kobj));
> >
> > +   /* Remove trip point attributes associated with this sensor */
> > +   __remove_trip_attr(tz, indx);
> > +
> > /* Shift the entries in the tz->sensors array */
> > -   for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> > +   for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
> > tz->sensors[j] = tz->sensors[j + 1];
> > +   tz->sensor_trip[j] = tz->sensor_trip[j + 1];
> > +   tz->trip_attr[j] = tz->trip_attr[j + 1];
> > +   }
> >
> > tz->sensor_indx--;
> >   }
> > @@ -879,6 +916,99 @@ policy_show(struct device *dev, struct
> device_attribute *devattr, char *buf)
> > return sprintf(buf, "%s\n", tz->governor->name);
> >   }
> >
> > +static ssize_t
> > +active_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +   int i, indx, ret = 0;
> > +   char kobj_name[THERMAL_NAME_LENGTH];
> > +   struct thermal_zone *tz = to_zone(dev);
> > +
> > +   if (!sscanf(attr->attr.name, "sensor%d_trip_active", &i))
> > +   return -EINVAL;
> > +
> > +   snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> > +   indx = get_sensor_indx_by_kobj(tz, kobj_name);
> > +   if (indx < 0)
> > +   return indx;
> > +
> > +   if (tz->sensor_trip[indx]->num_active_trips <= 0)
&

Re: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor

2013-02-28 Thread Eduardo Valentin


Durga,


Same comments on patch 02/08 I want to rise here:
- Minors on strlcpy, snprintf, devm_ helpers
- documentation in the code for these helper functions and also better 
naming..



On 05-02-2013 06:46, Durgadoss R wrote:

This patch adds a trip point related sysfs nodes
for each sensor under a zone in /sys/class/thermal/zoneX/.
The nodes will be named, sensorX_trip_active,
sensorX_trip_passive, sensorX_trip_hot, sensorX_trip_critical
for active, passive, hot and critical trip points
respectively for sensorX.

Signed-off-by: Durgadoss R 
---
  drivers/thermal/thermal_sys.c |  225 -
  include/linux/thermal.h   |   38 ++-
  2 files changed, 260 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index bf703b1..69a60a4 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -452,6 +452,37 @@ static void thermal_zone_device_check(struct work_struct 
*work)
thermal_zone_device_update(tz);
  }

+static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char *name)
+{
+   int i, indx = -EINVAL;
+
+   mutex_lock(&sensor_list_lock);
+   for (i = 0; i < tz->sensor_indx; i++) {
+   if (!strnicmp(name, kobject_name(&tz->sensors[i]->device.kobj),
+   THERMAL_NAME_LENGTH)) {
+   indx = i;
+   break;
+   }
+   }
+   mutex_unlock(&sensor_list_lock);
+   return indx;
+}
+
+static void inline __remove_trip_attr(struct thermal_zone *tz, int indx)

I believe the preferred format would be:

static inline void __remove_trip_attr(struct thermal_zone *tz, int indx)


+{
+   int i;
+   struct thermal_trip_attr *attr = tz->trip_attr[indx];
+
+   if (!attr)
+   return;
+
+   for (i = 0; i < NUM_TRIP_TYPES; i++)
+   device_remove_file(&tz->device, &attr->attrs[i].attr);
+
+   kfree(tz->trip_attr[indx]);
+   tz->trip_attr[indx] = NULL;
+}
+
  static void remove_sensor_from_zone(struct thermal_zone *tz,
struct thermal_sensor *ts)
  {
@@ -463,9 +494,15 @@ static void remove_sensor_from_zone(struct thermal_zone 
*tz,

sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));

+   /* Remove trip point attributes associated with this sensor */
+   __remove_trip_attr(tz, indx);
+
/* Shift the entries in the tz->sensors array */
-   for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
+   for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
tz->sensors[j] = tz->sensors[j + 1];
+   tz->sensor_trip[j] = tz->sensor_trip[j + 1];
+   tz->trip_attr[j] = tz->trip_attr[j + 1];
+   }

tz->sensor_indx--;
  }
@@ -879,6 +916,99 @@ policy_show(struct device *dev, struct device_attribute 
*devattr, char *buf)
return sprintf(buf, "%s\n", tz->governor->name);
  }

+static ssize_t
+active_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+   int i, indx, ret = 0;
+   char kobj_name[THERMAL_NAME_LENGTH];
+   struct thermal_zone *tz = to_zone(dev);
+
+   if (!sscanf(attr->attr.name, "sensor%d_trip_active", &i))
+   return -EINVAL;
+
+   snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
+   indx = get_sensor_indx_by_kobj(tz, kobj_name);
+   if (indx < 0)
+   return indx;
+
+   if (tz->sensor_trip[indx]->num_active_trips <= 0)
+   return sprintf(buf, "\n");
+
+   ret += sprintf(buf, "0x%x", tz->sensor_trip[indx]->active_trip_mask);
+   for (i = 0; i < tz->sensor_trip[indx]->num_active_trips; i++) {
+   ret += sprintf(buf + ret, " %d",
+   tz->sensor_trip[indx]->active_trips[i]);
+   }
+
+   ret += sprintf(buf + ret, "\n");
+   return ret;
+}
+


I believe the above function violates sysfs rules, no? Each and every 
file must contain only 1 value..



+static ssize_t
+ptrip_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+   int i, indx, ret = 0;
+   char kobj_name[THERMAL_NAME_LENGTH];
+   struct thermal_zone *tz = to_zone(dev);
+
+   if (!sscanf(attr->attr.name, "sensor%d_trip_passive", &i))
+   return -EINVAL;
+
+   snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
+   indx = get_sensor_indx_by_kobj(tz, kobj_name);
+   if (indx < 0)
+   return indx;
+
+   if (tz->sensor_trip[indx]->num_passive_trips <= 0)
+   return sprintf(buf, "\n");
+
+   for (i = 0; i < tz->sensor_trip[indx]->num_passive_trips; i++) {
+   ret += sprintf(buf + ret, "%d ",
+   tz->sensor_trip[indx]->passive_trips[i]);
+   }
+
+   ret += sprintf(buf + ret, "\n");
+   return ret;
+}



ditto.


+
+static ssize_t
+hot_show(struct device *dev, struct device_attrib

Re: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor

2013-02-08 Thread Zhang Rui

On Tue, 2013-02-05 at 16:16 +0530, Durgadoss R wrote:
> This patch adds a trip point related sysfs nodes
> for each sensor under a zone in /sys/class/thermal/zoneX/.
> The nodes will be named, sensorX_trip_active,
> sensorX_trip_passive, sensorX_trip_hot, sensorX_trip_critical
> for active, passive, hot and critical trip points
> respectively for sensorX.
> 
> Signed-off-by: Durgadoss R 

this one looks good to me.

thanks,
rui
> ---
>  drivers/thermal/thermal_sys.c |  225 
> -
>  include/linux/thermal.h   |   38 ++-
>  2 files changed, 260 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index bf703b1..69a60a4 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -452,6 +452,37 @@ static void thermal_zone_device_check(struct work_struct 
> *work)
>   thermal_zone_device_update(tz);
>  }
>  
> +static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char *name)
> +{
> + int i, indx = -EINVAL;
> +
> + mutex_lock(&sensor_list_lock);
> + for (i = 0; i < tz->sensor_indx; i++) {
> + if (!strnicmp(name, kobject_name(&tz->sensors[i]->device.kobj),
> + THERMAL_NAME_LENGTH)) {
> + indx = i;
> + break;
> + }
> + }
> + mutex_unlock(&sensor_list_lock);
> + return indx;
> +}
> +
> +static void inline __remove_trip_attr(struct thermal_zone *tz, int indx)
> +{
> + int i;
> + struct thermal_trip_attr *attr = tz->trip_attr[indx];
> +
> + if (!attr)
> + return;
> +
> + for (i = 0; i < NUM_TRIP_TYPES; i++)
> + device_remove_file(&tz->device, &attr->attrs[i].attr);
> +
> + kfree(tz->trip_attr[indx]);
> + tz->trip_attr[indx] = NULL;
> +}
> +
>  static void remove_sensor_from_zone(struct thermal_zone *tz,
>   struct thermal_sensor *ts)
>  {
> @@ -463,9 +494,15 @@ static void remove_sensor_from_zone(struct thermal_zone 
> *tz,
>  
>   sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
>  
> + /* Remove trip point attributes associated with this sensor */
> + __remove_trip_attr(tz, indx);
> +
>   /* Shift the entries in the tz->sensors array */
> - for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
>   tz->sensors[j] = tz->sensors[j + 1];
> + tz->sensor_trip[j] = tz->sensor_trip[j + 1];
> + tz->trip_attr[j] = tz->trip_attr[j + 1];
> + }
>  
>   tz->sensor_indx--;
>  }
> @@ -879,6 +916,99 @@ policy_show(struct device *dev, struct device_attribute 
> *devattr, char *buf)
>   return sprintf(buf, "%s\n", tz->governor->name);
>  }
>  
> +static ssize_t
> +active_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int i, indx, ret = 0;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_active", &i))
> + return -EINVAL;
> +
> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> + if (indx < 0)
> + return indx;
> +
> + if (tz->sensor_trip[indx]->num_active_trips <= 0)
> + return sprintf(buf, "\n");
> +
> + ret += sprintf(buf, "0x%x", tz->sensor_trip[indx]->active_trip_mask);
> + for (i = 0; i < tz->sensor_trip[indx]->num_active_trips; i++) {
> + ret += sprintf(buf + ret, " %d",
> + tz->sensor_trip[indx]->active_trips[i]);
> + }
> +
> + ret += sprintf(buf + ret, "\n");
> + return ret;
> +}
> +
> +static ssize_t
> +ptrip_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int i, indx, ret = 0;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_passive", &i))
> + return -EINVAL;
> +
> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> + if (indx < 0)
> + return indx;
> +
> + if (tz->sensor_trip[indx]->num_passive_trips <= 0)
> + return sprintf(buf, "\n");
> +
> + for (i = 0; i < tz->sensor_trip[indx]->num_passive_trips; i++) {
> + ret += sprintf(buf + ret, "%d ",
> + tz->sensor_trip[indx]->passive_trips[i]);
> + }
> +
> + ret += sprintf(buf + ret, "\n");
> + return ret;
> +}
> +
> +static ssize_t
> +hot_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int indx;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_hot", &indx))
> + return -EINVAL

RE: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor

Re: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor

Re: [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor

3 matches

Site Navigation

Mail list logo

Footer information