From: Eduardo Valentin <eduardo.valentin@ti.com> To: Durgadoss R <durgadoss.r@intel.com> Cc: <rui.zhang@intel.com>, <linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <hongbo.zhang@linaro.org>, <wni@nvidia.com> Subject: Re: [PATCH 3/8] Thermal: Add APIs to bind cdev to new zone structure Date: Thu, 28 Feb 2013 15:35:34 -0400 [thread overview] Message-ID: <512FB186.6010207@ti.com> (raw) In-Reply-To: <1360061183-14137-4-git-send-email-durgadoss.r@intel.com> Durga, On 05-02-2013 06:46, Durgadoss R wrote: > This patch creates new APIs to add/remove a > cdev to/from a zone. This patch does not change > the old cooling device implementation. Same comments on patch 02/08 I want to rise here: - Consider using linked list - You may have contention on your index/cdevs array - overflow on your buffer (carefully check your implementation) - zone removal condition. can we remove zones with cdevs registered? - get_by_name, why do we need it? (at least not on this patch) - Minors on strlcpy, snprintf, devm_ helpers - documentation in the code for these helper functions and also better naming.. > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > --- > drivers/thermal/thermal_sys.c | 80 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 9 +++++ > 2 files changed, 89 insertions(+) > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index 838d4fb..bf703b1 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -57,6 +57,7 @@ static LIST_HEAD(thermal_governor_list); > static DEFINE_MUTEX(thermal_list_lock); > static DEFINE_MUTEX(sensor_list_lock); > static DEFINE_MUTEX(zone_list_lock); > +static DEFINE_MUTEX(cdev_list_lock); > static DEFINE_MUTEX(thermal_governor_lock); > > #define for_each_thermal_sensor(pos) \ > @@ -83,6 +84,9 @@ static DEFINE_MUTEX(thermal_governor_lock); > ret; \ > }) > > +#define for_each_cdev(pos) \ > + list_for_each_entry(pos, &thermal_cdev_list, node) > + > static struct thermal_governor *__find_governor(const char *name) > { > struct thermal_governor *pos; > @@ -466,6 +470,24 @@ static void remove_sensor_from_zone(struct thermal_zone *tz, > tz->sensor_indx--; > } > > +static void remove_cdev_from_zone(struct thermal_zone *tz, > + struct thermal_cooling_device *cdev) > +{ > + int j, indx; > + > + indx = GET_INDEX(tz, cdev, cdev); > + if (indx < 0) > + return; > + > + sysfs_remove_link(&tz->device.kobj, kobject_name(&cdev->device.kobj)); > + > + /* Shift the entries in the tz->cdevs array */ > + for (j = indx; j < MAX_CDEVS_PER_ZONE - 1; j++) > + tz->cdevs[j] = tz->cdevs[j + 1]; > + > + tz->cdev_indx--; > +} > + > /* sys I/F for thermal zone */ > > #define to_thermal_zone(_dev) \ > @@ -1462,6 +1484,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev) > int i; > const struct thermal_zone_params *tzp; > struct thermal_zone_device *tz; > + struct thermal_zone *tmp_tz; > struct thermal_cooling_device *pos = NULL; > > if (!cdev) > @@ -1499,6 +1522,13 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev) > > mutex_unlock(&thermal_list_lock); > > + mutex_lock(&zone_list_lock); > + > + for_each_thermal_zone(tmp_tz) > + remove_cdev_from_zone(tmp_tz, cdev); > + > + mutex_unlock(&zone_list_lock); > + > if (cdev->type[0]) > device_remove_file(&cdev->device, &dev_attr_cdev_type); > device_remove_file(&cdev->device, &dev_attr_max_state); > @@ -1794,6 +1824,23 @@ exit: > } > EXPORT_SYMBOL(remove_thermal_zone); > > +struct thermal_cooling_device *get_cdev_by_name(const char *name) > +{ > + struct thermal_cooling_device *pos; > + struct thermal_cooling_device *cdev = NULL; > + > + mutex_lock(&cdev_list_lock); > + for_each_cdev(pos) { > + if (!strnicmp(pos->type, name, THERMAL_NAME_LENGTH)) { > + cdev = pos; > + break; > + } > + } > + mutex_unlock(&cdev_list_lock); > + return cdev; > +} > +EXPORT_SYMBOL(get_cdev_by_name); > + > struct thermal_sensor *get_sensor_by_name(const char *name) > { > struct thermal_sensor *pos; > @@ -1844,6 +1891,39 @@ exit_zone: > } > EXPORT_SYMBOL(add_sensor_to_zone); > > +int add_cdev_to_zone(struct thermal_zone *tz, > + struct thermal_cooling_device *cdev) > +{ > + int ret; > + > + if (!tz || !cdev) > + return -EINVAL; > + > + mutex_lock(&zone_list_lock); > + > + /* Ensure we are not adding the same cdev again!! */ > + ret = GET_INDEX(tz, cdev, cdev); > + if (ret >= 0) { > + ret = -EEXIST; > + goto exit_zone; > + } > + > + mutex_lock(&cdev_list_lock); > + ret = sysfs_create_link(&tz->device.kobj, &cdev->device.kobj, > + kobject_name(&cdev->device.kobj)); > + if (ret) > + goto exit_cdev; > + > + tz->cdevs[tz->cdev_indx++] = cdev; > + > +exit_cdev: > + mutex_unlock(&cdev_list_lock); > +exit_zone: > + mutex_unlock(&zone_list_lock); > + return ret; > +} > +EXPORT_SYMBOL(add_cdev_to_zone); > + > /** > * thermal_sensor_register - register a new thermal sensor > * @name: name of the thermal sensor > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 2194519..c841414 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -57,6 +57,8 @@ > > #define MAX_SENSORS_PER_ZONE 5 > > +#define MAX_CDEVS_PER_ZONE 5 > + > struct thermal_sensor; > struct thermal_zone_device; > struct thermal_cooling_device; > @@ -217,6 +219,10 @@ struct thermal_zone { > /* Sensor level information */ > int sensor_indx; /* index into 'sensors' array */ > struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE]; > + > + /* cdev level information */ > + int cdev_indx; /* index into 'cdevs' array */ > + struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE]; > }; > > /* Structure that holds thermal governor information */ > @@ -296,6 +302,9 @@ void remove_thermal_zone(struct thermal_zone *); > int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *); > struct thermal_sensor *get_sensor_by_name(const char *); > > +int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *); > +struct thermal_cooling_device *get_cdev_by_name(const char *); > + > #ifdef CONFIG_NET > extern int thermal_generate_netlink_event(struct thermal_zone_device *tz, > enum events event); >
WARNING: multiple messages have this Message-ID (diff)
From: Eduardo Valentin <eduardo.valentin@ti.com> To: Durgadoss R <durgadoss.r@intel.com> Cc: rui.zhang@intel.com, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, hongbo.zhang@linaro.org, wni@nvidia.com Subject: Re: [PATCH 3/8] Thermal: Add APIs to bind cdev to new zone structure Date: Thu, 28 Feb 2013 15:35:34 -0400 [thread overview] Message-ID: <512FB186.6010207@ti.com> (raw) In-Reply-To: <1360061183-14137-4-git-send-email-durgadoss.r@intel.com> Durga, On 05-02-2013 06:46, Durgadoss R wrote: > This patch creates new APIs to add/remove a > cdev to/from a zone. This patch does not change > the old cooling device implementation. Same comments on patch 02/08 I want to rise here: - Consider using linked list - You may have contention on your index/cdevs array - overflow on your buffer (carefully check your implementation) - zone removal condition. can we remove zones with cdevs registered? - get_by_name, why do we need it? (at least not on this patch) - Minors on strlcpy, snprintf, devm_ helpers - documentation in the code for these helper functions and also better naming.. > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > --- > drivers/thermal/thermal_sys.c | 80 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 9 +++++ > 2 files changed, 89 insertions(+) > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index 838d4fb..bf703b1 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -57,6 +57,7 @@ static LIST_HEAD(thermal_governor_list); > static DEFINE_MUTEX(thermal_list_lock); > static DEFINE_MUTEX(sensor_list_lock); > static DEFINE_MUTEX(zone_list_lock); > +static DEFINE_MUTEX(cdev_list_lock); > static DEFINE_MUTEX(thermal_governor_lock); > > #define for_each_thermal_sensor(pos) \ > @@ -83,6 +84,9 @@ static DEFINE_MUTEX(thermal_governor_lock); > ret; \ > }) > > +#define for_each_cdev(pos) \ > + list_for_each_entry(pos, &thermal_cdev_list, node) > + > static struct thermal_governor *__find_governor(const char *name) > { > struct thermal_governor *pos; > @@ -466,6 +470,24 @@ static void remove_sensor_from_zone(struct thermal_zone *tz, > tz->sensor_indx--; > } > > +static void remove_cdev_from_zone(struct thermal_zone *tz, > + struct thermal_cooling_device *cdev) > +{ > + int j, indx; > + > + indx = GET_INDEX(tz, cdev, cdev); > + if (indx < 0) > + return; > + > + sysfs_remove_link(&tz->device.kobj, kobject_name(&cdev->device.kobj)); > + > + /* Shift the entries in the tz->cdevs array */ > + for (j = indx; j < MAX_CDEVS_PER_ZONE - 1; j++) > + tz->cdevs[j] = tz->cdevs[j + 1]; > + > + tz->cdev_indx--; > +} > + > /* sys I/F for thermal zone */ > > #define to_thermal_zone(_dev) \ > @@ -1462,6 +1484,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev) > int i; > const struct thermal_zone_params *tzp; > struct thermal_zone_device *tz; > + struct thermal_zone *tmp_tz; > struct thermal_cooling_device *pos = NULL; > > if (!cdev) > @@ -1499,6 +1522,13 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev) > > mutex_unlock(&thermal_list_lock); > > + mutex_lock(&zone_list_lock); > + > + for_each_thermal_zone(tmp_tz) > + remove_cdev_from_zone(tmp_tz, cdev); > + > + mutex_unlock(&zone_list_lock); > + > if (cdev->type[0]) > device_remove_file(&cdev->device, &dev_attr_cdev_type); > device_remove_file(&cdev->device, &dev_attr_max_state); > @@ -1794,6 +1824,23 @@ exit: > } > EXPORT_SYMBOL(remove_thermal_zone); > > +struct thermal_cooling_device *get_cdev_by_name(const char *name) > +{ > + struct thermal_cooling_device *pos; > + struct thermal_cooling_device *cdev = NULL; > + > + mutex_lock(&cdev_list_lock); > + for_each_cdev(pos) { > + if (!strnicmp(pos->type, name, THERMAL_NAME_LENGTH)) { > + cdev = pos; > + break; > + } > + } > + mutex_unlock(&cdev_list_lock); > + return cdev; > +} > +EXPORT_SYMBOL(get_cdev_by_name); > + > struct thermal_sensor *get_sensor_by_name(const char *name) > { > struct thermal_sensor *pos; > @@ -1844,6 +1891,39 @@ exit_zone: > } > EXPORT_SYMBOL(add_sensor_to_zone); > > +int add_cdev_to_zone(struct thermal_zone *tz, > + struct thermal_cooling_device *cdev) > +{ > + int ret; > + > + if (!tz || !cdev) > + return -EINVAL; > + > + mutex_lock(&zone_list_lock); > + > + /* Ensure we are not adding the same cdev again!! */ > + ret = GET_INDEX(tz, cdev, cdev); > + if (ret >= 0) { > + ret = -EEXIST; > + goto exit_zone; > + } > + > + mutex_lock(&cdev_list_lock); > + ret = sysfs_create_link(&tz->device.kobj, &cdev->device.kobj, > + kobject_name(&cdev->device.kobj)); > + if (ret) > + goto exit_cdev; > + > + tz->cdevs[tz->cdev_indx++] = cdev; > + > +exit_cdev: > + mutex_unlock(&cdev_list_lock); > +exit_zone: > + mutex_unlock(&zone_list_lock); > + return ret; > +} > +EXPORT_SYMBOL(add_cdev_to_zone); > + > /** > * thermal_sensor_register - register a new thermal sensor > * @name: name of the thermal sensor > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 2194519..c841414 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -57,6 +57,8 @@ > > #define MAX_SENSORS_PER_ZONE 5 > > +#define MAX_CDEVS_PER_ZONE 5 > + > struct thermal_sensor; > struct thermal_zone_device; > struct thermal_cooling_device; > @@ -217,6 +219,10 @@ struct thermal_zone { > /* Sensor level information */ > int sensor_indx; /* index into 'sensors' array */ > struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE]; > + > + /* cdev level information */ > + int cdev_indx; /* index into 'cdevs' array */ > + struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE]; > }; > > /* Structure that holds thermal governor information */ > @@ -296,6 +302,9 @@ void remove_thermal_zone(struct thermal_zone *); > int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *); > struct thermal_sensor *get_sensor_by_name(const char *); > > +int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *); > +struct thermal_cooling_device *get_cdev_by_name(const char *); > + > #ifdef CONFIG_NET > extern int thermal_generate_netlink_event(struct thermal_zone_device *tz, > enum events event); >
next prev parent reply other threads:[~2013-02-28 19:35 UTC|newest] Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-02-05 10:46 [PATCHv3 0/8] Thermal Framework Enhancements Durgadoss R 2013-02-05 10:46 ` [PATCH 1/8] Thermal: Create sensor level APIs Durgadoss R 2013-02-08 7:53 ` Zhang Rui 2013-02-08 8:26 ` R, Durgadoss 2013-02-08 8:26 ` R, Durgadoss 2013-02-08 9:54 ` Zhang Rui 2013-02-28 18:58 ` Eduardo Valentin 2013-02-28 18:58 ` Eduardo Valentin 2013-03-01 5:08 ` R, Durgadoss 2013-02-05 10:46 ` [PATCH 2/8] Thermal: Create zone " Durgadoss R 2013-02-08 8:11 ` Zhang Rui 2013-02-08 8:54 ` R, Durgadoss 2013-02-08 8:54 ` R, Durgadoss 2013-02-08 9:54 ` Zhang Rui 2013-02-08 10:27 ` R, Durgadoss 2013-02-08 10:27 ` R, Durgadoss 2013-02-28 19:29 ` Eduardo Valentin 2013-02-28 19:29 ` Eduardo Valentin 2013-03-01 15:31 ` R, Durgadoss 2013-02-05 10:46 ` [PATCH 3/8] Thermal: Add APIs to bind cdev to new zone structure Durgadoss R 2013-02-08 8:28 ` Zhang Rui 2013-02-28 19:35 ` Eduardo Valentin [this message] 2013-02-28 19:35 ` Eduardo Valentin 2013-03-01 15:33 ` R, Durgadoss 2013-02-05 10:46 ` [PATCH 4/8] Thermal: Add trip point sysfs nodes for sensor Durgadoss R 2013-02-08 8:50 ` Zhang Rui 2013-02-28 19:51 ` Eduardo Valentin 2013-02-28 19:51 ` Eduardo Valentin 2013-03-01 15:34 ` R, Durgadoss 2013-02-05 10:46 ` [PATCH 5/8] Thermal: Create Thermal map sysfs attributes for a zone Durgadoss R 2013-02-08 9:04 ` Zhang Rui 2013-02-08 9:08 ` R, Durgadoss 2013-02-08 9:08 ` R, Durgadoss 2013-02-08 9:55 ` Zhang Rui 2013-02-28 21:30 ` Eduardo Valentin 2013-02-28 21:30 ` Eduardo Valentin 2013-02-05 10:46 ` [PATCH 6/8] Thermal: Add Documentation to new APIs Durgadoss R 2013-02-08 9:21 ` Zhang Rui 2013-02-05 10:46 ` [PATCH 7/8] Thermal: Add ABI Documentation for sysfs interfaces Durgadoss R 2013-02-05 10:46 ` [PATCH 8/8] Thermal: Dummy driver used for testing Durgadoss R 2013-02-08 7:53 ` [PATCHv3 0/8] Thermal Framework Enhancements Zhang Rui 2013-02-08 9:35 ` Zhang Rui 2013-02-28 21:33 ` Eduardo Valentin 2013-02-28 21:33 ` Eduardo Valentin 2013-03-01 5:12 ` R, Durgadoss 2013-08-29 19:38 ` Eduardo Valentin 2013-08-29 19:38 ` Eduardo Valentin 2013-08-30 9:20 ` R, Durgadoss 2013-08-30 12:34 ` Eduardo Valentin 2013-08-30 13:21 ` R, Durgadoss -- strict thread matches above, loose matches on Subject: below -- 2012-12-18 9:29 [PATCH " Durgadoss R 2012-12-18 9:29 ` [PATCH 3/8] Thermal: Add APIs to bind cdev to new zone structure Durgadoss R 2012-12-25 8:30 ` Wei Ni 2012-12-26 3:30 ` R, Durgadoss
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=512FB186.6010207@ti.com \ --to=eduardo.valentin@ti.com \ --cc=durgadoss.r@intel.com \ --cc=hongbo.zhang@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=rui.zhang@intel.com \ --cc=wni@nvidia.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.