[PATCH 0/4] thermal_sys: Cleanups

[PATCH 0/4] thermal_sys: Cleanups

[Joe Perches]

Joe Perches (4):
  thermal_sys: Remove unnecessary line continuations
  thermal_sys: Remove obfuscating used-once macros
  thermal_sys: Kernel style cleanups
  thermal_sys: Convert printks to pr_<level>

 drivers/thermal/thermal_sys.c |   87 +++++++++++++++++------------------------
 1 files changed, 36 insertions(+), 51 deletions(-)

-- 
1.7.8.111.gad25c.dirty

[PATCH 1/4] thermal_sys: Remove unnecessary line continuations

[Joe Perches]

Line continations are not necessary in function calls
or statements.  Remove them.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/thermal/thermal_sys.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index dd9a574..7326baa 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -283,8 +283,7 @@ passive_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR(type, 0444, type_show, NULL);
 static DEVICE_ATTR(temp, 0444, temp_show, NULL);
 static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
-static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
-		   passive_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),
@@ -1314,8 +1313,8 @@ int generate_netlink_event(u32 orig, enum events event)
 	int result;
 
 	/* allocate memory */
-	size = nla_total_size(sizeof(struct thermal_genl_event)) + \
-				nla_total_size(0);
+	size = nla_total_size(sizeof(struct thermal_genl_event)) +
+	       nla_total_size(0);
 
 	skb = genlmsg_new(size, GFP_ATOMIC);
 	if (!skb)
@@ -1331,8 +1330,8 @@ int generate_netlink_event(u32 orig, enum events event)
 	}
 
 	/* fill the data */
-	attr = nla_reserve(skb, THERMAL_GENL_ATTR_EVENT, \
-			sizeof(struct thermal_genl_event));
+	attr = nla_reserve(skb, THERMAL_GENL_ATTR_EVENT,
+			   sizeof(struct thermal_genl_event));
 
 	if (!attr) {
 		nlmsg_free(skb);
-- 
1.7.8.111.gad25c.dirty

Re: [PATCH 1/4] thermal_sys: Remove unnecessary line continuations

[Jesper Juhl]

On Tue, 24 Jan 2012, Joe Perches wrote:

> Line continations are not necessary in function calls
> or statements.  Remove them.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/thermal/thermal_sys.c |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index dd9a574..7326baa 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -283,8 +283,7 @@ passive_show(struct device *dev, struct device_attribute *attr,
>  static DEVICE_ATTR(type, 0444, type_show, NULL);
>  static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> -static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
> -		   passive_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),
> @@ -1314,8 +1313,8 @@ int generate_netlink_event(u32 orig, enum events event)
>  	int result;
>  
>  	/* allocate memory */
> -	size = nla_total_size(sizeof(struct thermal_genl_event)) + \
> -				nla_total_size(0);
> +	size = nla_total_size(sizeof(struct thermal_genl_event)) +
> +	       nla_total_size(0);
>  
>  	skb = genlmsg_new(size, GFP_ATOMIC);
>  	if (!skb)
> @@ -1331,8 +1330,8 @@ int generate_netlink_event(u32 orig, enum events event)
>  	}
>  
>  	/* fill the data */
> -	attr = nla_reserve(skb, THERMAL_GENL_ATTR_EVENT, \
> -			sizeof(struct thermal_genl_event));
> +	attr = nla_reserve(skb, THERMAL_GENL_ATTR_EVENT,
> +			   sizeof(struct thermal_genl_event));
>  
>  	if (!attr) {
>  		nlmsg_free(skb);
> 

Yes.
Reviewed-by: Jesper Juhl <jj@chaosbits.net>



-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

[PATCH 2/4] thermal_sys: Remove obfuscating used-once macros

[Joe Perches]

These don't add any value as they are used
only once and the surrounding code uses similar
variable.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/thermal/thermal_sys.c |   32 ++++++++++++--------------------
 1 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 7326baa..1c87167 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -312,22 +312,6 @@ static struct device_attribute trip_point_attrs[] = {
 	__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)
@@ -1198,7 +1182,12 @@ 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 = device_create_file(&tz->device,
+					    &trip_point_attrs[count * 2]);
+		if (result)
+			break;
+		result = device_create_file(&tz->device,
+					    &trip_point_attrs[count * 2 + 1]);
 		if (result)
 			goto unregister;
 		tz->ops->get_trip_type(tz, count, &trip_type);
@@ -1278,9 +1267,12 @@ 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,
+				   &trip_point_attrs[count * 2]);
+		device_remove_file(&tz->device,
+				   &trip_point_attrs[count * 2 + 1]);
+	}
 	thermal_remove_hwmon_sysfs(tz);
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 	idr_destroy(&tz->idr);
-- 
1.7.8.111.gad25c.dirty

Re: [PATCH 2/4] thermal_sys: Remove obfuscating used-once macros

[Andrew Morton]

On Tue, 24 Jan 2012 15:35:36 -0800
Joe Perches <joe@perches.com> wrote:

> These don't add any value as they are used
> only once and the surrounding code uses similar
> variable.

And they were buggy if passed an expression-with-side-effects.  Geeze
we make this mistake often, and it's C programming 101!  Or maybe 102.

> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -312,22 +312,6 @@ static struct device_attribute trip_point_attrs[] = {
>  	__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)

I don't forsee many tears being shed over the above ;)

[PATCH 3/4] thermal_sys: Kernel style cleanups

[Joe Perches]

Just a few tidies to make it more like most
kernel sources.

A couple of long lines still remain.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/thermal/thermal_sys.c |   30 +++++++++++++-----------------
 1 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 1c87167..68df616 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -66,7 +66,7 @@ static int get_idr(struct idr *idr, struct mutex *lock, int *id)
 {
 	int err;
 
-      again:
+again:
 	if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
 		return -ENOMEM;
 
@@ -818,15 +818,14 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 		return 0;
 
 	device_remove_file(&tz->device, &dev->attr);
-      remove_symbol_link:
+remove_symbol_link:
 	sysfs_remove_link(&tz->device.kobj, dev->name);
-      release_idr:
+release_idr:
 	release_idr(&tz->idr, &tz->lock, dev->id);
-      free_mem:
+free_mem:
 	kfree(dev);
 	return result;
 }
-
 EXPORT_SYMBOL(thermal_zone_bind_cooling_device);
 
 /**
@@ -856,14 +855,13 @@ int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
 
 	return -ENODEV;
 
-      unbind:
+unbind:
 	device_remove_file(&tz->device, &pos->attr);
 	sysfs_remove_link(&tz->device.kobj, pos->name);
 	release_idr(&tz->idr, &tz->lock, pos->id);
 	kfree(pos);
 	return 0;
 }
-
 EXPORT_SYMBOL(thermal_zone_unbind_cooling_device);
 
 static void thermal_release(struct device *dev)
@@ -871,7 +869,8 @@ static void thermal_release(struct device *dev)
 	struct thermal_zone_device *tz;
 	struct thermal_cooling_device *cdev;
 
-	if (!strncmp(dev_name(dev), "thermal_zone", sizeof "thermal_zone" - 1)) {
+	if (!strncmp(dev_name(dev), "thermal_zone",
+		     sizeof("thermal_zone") - 1)) {
 		tz = to_thermal_zone(dev);
 		kfree(tz);
 	} else {
@@ -891,8 +890,9 @@ static struct class thermal_class = {
  * @devdata:	device private data.
  * @ops:		standard thermal cooling devices callbacks.
  */
-struct thermal_cooling_device *thermal_cooling_device_register(
-     char *type, void *devdata, const struct thermal_cooling_device_ops *ops)
+struct thermal_cooling_device *
+thermal_cooling_device_register(char *type, void *devdata,
+				const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
 	struct thermal_zone_device *pos;
@@ -957,12 +957,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(
 	if (!result)
 		return cdev;
 
-      unregister:
+unregister:
 	release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id);
 	device_unregister(&cdev->device);
 	return ERR_PTR(result);
 }
-
 EXPORT_SYMBOL(thermal_cooling_device_register);
 
 /**
@@ -1007,7 +1006,6 @@ void thermal_cooling_device_unregister(struct
 	device_unregister(&cdev->device);
 	return;
 }
-
 EXPORT_SYMBOL(thermal_cooling_device_unregister);
 
 /**
@@ -1083,7 +1081,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 
 	tz->last_temperature = temp;
 
-      leave:
+leave:
 	if (tz->passive)
 		thermal_zone_device_set_polling(tz, tz->passive_delay);
 	else if (tz->polling_delay)
@@ -1223,12 +1221,11 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 	if (!result)
 		return tz;
 
-      unregister:
+unregister:
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 	device_unregister(&tz->device);
 	return ERR_PTR(result);
 }
-
 EXPORT_SYMBOL(thermal_zone_device_register);
 
 /**
@@ -1280,7 +1277,6 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 	device_unregister(&tz->device);
 	return;
 }
-
 EXPORT_SYMBOL(thermal_zone_device_unregister);
 
 #ifdef CONFIG_NET
-- 
1.7.8.111.gad25c.dirty

Re: [PATCH 3/4] thermal_sys: Kernel style cleanups

[Jesper Juhl]

On Tue, 24 Jan 2012, Joe Perches wrote:

> Just a few tidies to make it more like most
> kernel sources.
> 
> A couple of long lines still remain.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/thermal/thermal_sys.c |   30 +++++++++++++-----------------
>  1 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 1c87167..68df616 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -66,7 +66,7 @@ static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>  {
>  	int err;
>  
> -      again:
> +again:
>  	if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
>  		return -ENOMEM;
>  
> @@ -818,15 +818,14 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  		return 0;
>  
>  	device_remove_file(&tz->device, &dev->attr);
> -      remove_symbol_link:
> +remove_symbol_link:
>  	sysfs_remove_link(&tz->device.kobj, dev->name);
> -      release_idr:
> +release_idr:
>  	release_idr(&tz->idr, &tz->lock, dev->id);
> -      free_mem:
> +free_mem:
>  	kfree(dev);
>  	return result;
>  }
> -
>  EXPORT_SYMBOL(thermal_zone_bind_cooling_device);
>  
>  /**
> @@ -856,14 +855,13 @@ int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
>  
>  	return -ENODEV;
>  
> -      unbind:
> +unbind:
>  	device_remove_file(&tz->device, &pos->attr);
>  	sysfs_remove_link(&tz->device.kobj, pos->name);
>  	release_idr(&tz->idr, &tz->lock, pos->id);
>  	kfree(pos);
>  	return 0;
>  }
> -
>  EXPORT_SYMBOL(thermal_zone_unbind_cooling_device);
>  
>  static void thermal_release(struct device *dev)
> @@ -871,7 +869,8 @@ static void thermal_release(struct device *dev)
>  	struct thermal_zone_device *tz;
>  	struct thermal_cooling_device *cdev;
>  
> -	if (!strncmp(dev_name(dev), "thermal_zone", sizeof "thermal_zone" - 1)) {
> +	if (!strncmp(dev_name(dev), "thermal_zone",
> +		     sizeof("thermal_zone") - 1)) {
>  		tz = to_thermal_zone(dev);
>  		kfree(tz);
>  	} else {
> @@ -891,8 +890,9 @@ static struct class thermal_class = {
>   * @devdata:	device private data.
>   * @ops:		standard thermal cooling devices callbacks.
>   */
> -struct thermal_cooling_device *thermal_cooling_device_register(
> -     char *type, void *devdata, const struct thermal_cooling_device_ops *ops)
> +struct thermal_cooling_device *
> +thermal_cooling_device_register(char *type, void *devdata,
> +				const struct thermal_cooling_device_ops *ops)
>  {
>  	struct thermal_cooling_device *cdev;
>  	struct thermal_zone_device *pos;
> @@ -957,12 +957,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(
>  	if (!result)
>  		return cdev;
>  
> -      unregister:
> +unregister:
>  	release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id);
>  	device_unregister(&cdev->device);
>  	return ERR_PTR(result);
>  }
> -
>  EXPORT_SYMBOL(thermal_cooling_device_register);
>  
>  /**
> @@ -1007,7 +1006,6 @@ void thermal_cooling_device_unregister(struct
>  	device_unregister(&cdev->device);
>  	return;
>  }
> -
>  EXPORT_SYMBOL(thermal_cooling_device_unregister);
>  
>  /**
> @@ -1083,7 +1081,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>  
>  	tz->last_temperature = temp;
>  
> -      leave:
> +leave:
>  	if (tz->passive)
>  		thermal_zone_device_set_polling(tz, tz->passive_delay);
>  	else if (tz->polling_delay)
> @@ -1223,12 +1221,11 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
>  	if (!result)
>  		return tz;
>  
> -      unregister:
> +unregister:
>  	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
>  	device_unregister(&tz->device);
>  	return ERR_PTR(result);
>  }
> -
>  EXPORT_SYMBOL(thermal_zone_device_register);
>  
>  /**
> @@ -1280,7 +1277,6 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  	device_unregister(&tz->device);
>  	return;
>  }
> -
>  EXPORT_SYMBOL(thermal_zone_device_unregister);
>  
>  #ifdef CONFIG_NET
> 

Looks like a sane set of changes to me.
Reviewed-by: Jesper Juhl <jj@chaosbits.net>


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

[PATCH 4/4] thermal_sys: Convert printks to pr_<level>

[Joe Perches]

Use the current logging style.

Remove PREFIX, add pr_fmt, convert the printks.
All dmesg output now prefixed with "thermal_sys: ".

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/thermal/thermal_sys.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 68df616..9c69d8a 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -23,6 +23,8 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -39,8 +41,6 @@ MODULE_AUTHOR("Zhang Rui");
 MODULE_DESCRIPTION("Generic thermal management sysfs support");
 MODULE_LICENSE("GPL");
 
-#define PREFIX "Thermal: "
-
 struct thermal_cooling_device_instance {
 	int id;
 	char name[THERMAL_NAME_LENGTH];
@@ -1025,8 +1025,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 
 	if (tz->ops->get_temp(tz, &temp)) {
 		/* get_temp failed - retry it later */
-		printk(KERN_WARNING PREFIX "failed to read out thermal zone "
-		       "%d\n", tz->id);
+		pr_warn("failed to read out thermal zone %d\n", tz->id);
 		goto leave;
 	}
 
@@ -1041,9 +1040,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);
 				if (!ret) {
-					printk(KERN_EMERG
-					       "Critical temperature reached (%ld C), shutting down.\n",
-					       temp/1000);
+					pr_emerg("Critical temperature reached (%ld C), shutting down\n",
+						 temp/1000);
 					orderly_poweroff(true);
 				}
 			}
@@ -1346,7 +1344,7 @@ int generate_netlink_event(u32 orig, enum events event)
 
 	result = genlmsg_multicast(skb, 0, thermal_event_mcgrp.id, GFP_ATOMIC);
 	if (result)
-		printk(KERN_INFO "failed to send netlink event:%d", result);
+		pr_info("failed to send netlink event:%d\n", result);
 
 	return result;
 }
-- 
1.7.8.111.gad25c.dirty