* [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups
@ 2010-12-16 10:21 Juerg Haefliger
2010-12-16 15:32 ` Guenter Roeck
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Juerg Haefliger @ 2010-12-16 10:21 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1: Type: text/plain, Size: 148 bytes --]
Minor cleanups. Mostly removing assignments in if statements to get
rid of checkpatch errors.
Signed-off-by: Juerg Haefliger <juergh at gmail.com>
[-- Attachment #2: dme1737-cleanup.patch --]
[-- Type: text/x-patch, Size: 7855 bytes --]
Index: linux-2.6.36/drivers/hwmon/dme1737.c
===================================================================
--- linux-2.6.36.orig/drivers/hwmon/dme1737.c 2010-12-16 10:25:35.311669799 +0100
+++ linux-2.6.36/drivers/hwmon/dme1737.c 2010-12-16 11:11:02.423687592 +0100
@@ -17,7 +17,7 @@
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
@@ -1588,7 +1588,7 @@
* created unconditionally. The attributes that need modification of their
* permissions are created read-only and write permissions are added or removed
* on the fly when required */
-static struct attribute *dme1737_attr[] ={
+static struct attribute *dme1737_attr[] = {
/* Voltages */
&sensor_dev_attr_in0_input.dev_attr.attr,
&sensor_dev_attr_in0_min.dev_attr.attr,
@@ -1693,7 +1693,7 @@
};
-/* The following struct holds temp zone hysteresis related attributes, which
+/* The following struct holds temp zone hysteresis related attributes, which
* are not available in all chips. The following chips support them:
* DME1737, SCH311x */
static struct attribute *dme1737_zone_hyst_attr[] = {
@@ -2029,36 +2029,44 @@
int err, ix;
/* Create a name attribute for ISA devices */
- if (!data->client &&
- (err = sysfs_create_file(&dev->kobj, &dev_attr_name.attr))) {
- goto exit;
+ if (!data->client) {
+ err = sysfs_create_file(&dev->kobj, &dev_attr_name.attr);
+ if (err) {
+ goto exit;
+ }
}
/* Create standard sysfs attributes */
- if ((err = sysfs_create_group(&dev->kobj, &dme1737_group))) {
+ err = sysfs_create_group(&dev->kobj, &dme1737_group);
+ if (err) {
goto exit_remove;
}
/* Create chip-dependent sysfs attributes */
- if ((data->has_features & HAS_TEMP_OFFSET) &&
- (err = sysfs_create_group(&dev->kobj,
- &dme1737_temp_offset_group))) {
- goto exit_remove;
+ if (data->has_features & HAS_TEMP_OFFSET) {
+ err = sysfs_create_group(&dev->kobj,
+ &dme1737_temp_offset_group);
+ if (err) {
+ goto exit_remove;
+ }
}
- if ((data->has_features & HAS_VID) &&
- (err = sysfs_create_group(&dev->kobj,
- &dme1737_vid_group))) {
- goto exit_remove;
+ if (data->has_features & HAS_VID) {
+ err = sysfs_create_group(&dev->kobj, &dme1737_vid_group);
+ if (err) {
+ goto exit_remove;
+ }
}
- if ((data->has_features & HAS_ZONE3) &&
- (err = sysfs_create_group(&dev->kobj,
- &dme1737_zone3_group))) {
- goto exit_remove;
+ if (data->has_features & HAS_ZONE3) {
+ err = sysfs_create_group(&dev->kobj, &dme1737_zone3_group);
+ if (err) {
+ goto exit_remove;
+ }
}
- if ((data->has_features & HAS_ZONE_HYST) &&
- (err = sysfs_create_group(&dev->kobj,
- &dme1737_zone_hyst_group))) {
- goto exit_remove;
+ if (data->has_features & HAS_ZONE_HYST) {
+ err = sysfs_create_group(&dev->kobj, &dme1737_zone_hyst_group);
+ if (err) {
+ goto exit_remove;
+ }
}
if (data->has_features & HAS_IN7) {
err = sysfs_create_group(&dev->kobj, &dme1737_in7_group);
@@ -2070,8 +2078,9 @@
/* Create fan sysfs attributes */
for (ix = 0; ix < ARRAY_SIZE(dme1737_fan_group); ix++) {
if (data->has_features & HAS_FAN(ix)) {
- if ((err = sysfs_create_group(&dev->kobj,
- &dme1737_fan_group[ix]))) {
+ err = sysfs_create_group(&dev->kobj,
+ &dme1737_fan_group[ix]);
+ if (err) {
goto exit_remove;
}
}
@@ -2080,14 +2089,17 @@
/* Create PWM sysfs attributes */
for (ix = 0; ix < ARRAY_SIZE(dme1737_pwm_group); ix++) {
if (data->has_features & HAS_PWM(ix)) {
- if ((err = sysfs_create_group(&dev->kobj,
- &dme1737_pwm_group[ix]))) {
+ err = sysfs_create_group(&dev->kobj,
+ &dme1737_pwm_group[ix]);
+ if (err) {
goto exit_remove;
}
- if ((data->has_features & HAS_PWM_MIN) && ix < 3 &&
- (err = sysfs_create_file(&dev->kobj,
- dme1737_auto_pwm_min_attr[ix]))) {
- goto exit_remove;
+ if ((data->has_features & HAS_PWM_MIN) && (ix < 3)) {
+ err = sysfs_create_file(&dev->kobj,
+ dme1737_auto_pwm_min_attr[ix]);
+ if (err) {
+ goto exit_remove;
+ }
}
}
}
@@ -2317,8 +2329,9 @@
dme1737_sio_outb(sio_cip, 0x07, 0x0a);
/* Get the base address of the runtime registers */
- if (!(addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
- dme1737_sio_inb(sio_cip, 0x61))) {
+ addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
+ dme1737_sio_inb(sio_cip, 0x61);
+ if (!addr) {
err = -ENODEV;
goto exit;
}
@@ -2399,13 +2412,15 @@
mutex_init(&data->update_lock);
/* Initialize the DME1737 chip */
- if ((err = dme1737_init_device(dev))) {
+ err = dme1737_init_device(dev);
+ if (err) {
dev_err(dev, "Failed to initialize device.\n");
goto exit_kfree;
}
/* Create sysfs files */
- if ((err = dme1737_create_files(dev))) {
+ err = dme1737_create_files(dev);
+ if (err) {
dev_err(dev, "Failed to create sysfs files.\n");
goto exit_kfree;
}
@@ -2482,8 +2497,9 @@
dme1737_sio_outb(sio_cip, 0x07, 0x0a);
/* Get the base address of the runtime registers */
- if (!(base_addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
- dme1737_sio_inb(sio_cip, 0x61))) {
+ base_addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
+ dme1737_sio_inb(sio_cip, 0x61);
+ if (!base_addr) {
printk(KERN_ERR "dme1737: Base address not set.\n");
err = -ENODEV;
goto exit;
@@ -2512,19 +2528,22 @@
if (err)
goto exit;
- if (!(pdev = platform_device_alloc("dme1737", addr))) {
+ pdev = platform_device_alloc("dme1737", addr);
+ if (!pdev) {
printk(KERN_ERR "dme1737: Failed to allocate device.\n");
err = -ENOMEM;
goto exit;
}
- if ((err = platform_device_add_resources(pdev, &res, 1))) {
+ err = platform_device_add_resources(pdev, &res, 1);
+ if (err) {
printk(KERN_ERR "dme1737: Failed to add device resource "
"(err = %d).\n", err);
goto exit_device_put;
}
- if ((err = platform_device_add(pdev))) {
+ err = platform_device_add(pdev);
+ if (err) {
printk(KERN_ERR "dme1737: Failed to add device (err = %d).\n",
err);
goto exit_device_put;
@@ -2552,11 +2571,12 @@
dev_err(dev, "Failed to request region 0x%04x-0x%04x.\n",
(unsigned short)res->start,
(unsigned short)res->start + DME1737_EXTENT - 1);
- err = -EBUSY;
- goto exit;
- }
+ err = -EBUSY;
+ goto exit;
+ }
- if (!(data = kzalloc(sizeof(struct dme1737_data), GFP_KERNEL))) {
+ data = kzalloc(sizeof(struct dme1737_data), GFP_KERNEL);
+ if (!data) {
err = -ENOMEM;
goto exit_release_region;
}
@@ -2603,13 +2623,15 @@
data->type == sch5127 ? "SCH5127" : "SCH311x", data->addr);
/* Initialize the chip */
- if ((err = dme1737_init_device(dev))) {
+ err = dme1737_init_device(dev);
+ if (err) {
dev_err(dev, "Failed to initialize device.\n");
goto exit_kfree;
}
/* Create sysfs files */
- if ((err = dme1737_create_files(dev))) {
+ err = dme1737_create_files(dev);
+ if (err) {
dev_err(dev, "Failed to create sysfs files.\n");
goto exit_kfree;
}
@@ -2666,7 +2688,8 @@
int err;
unsigned short addr;
- if ((err = i2c_add_driver(&dme1737_i2c_driver))) {
+ err = i2c_add_driver(&dme1737_i2c_driver);
+ if (err) {
goto exit;
}
@@ -2679,12 +2702,14 @@
return 0;
}
- if ((err = platform_driver_register(&dme1737_isa_driver))) {
+ err = platform_driver_register(&dme1737_isa_driver);
+ if (err) {
goto exit_del_i2c_driver;
}
/* Sets global pdev as a side effect */
- if ((err = dme1737_isa_device_add(addr))) {
+ err = dme1737_isa_device_add(addr);
+ if (err) {
goto exit_del_isa_driver;
}
[-- Attachment #3: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups
2010-12-16 10:21 [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups Juerg Haefliger
@ 2010-12-16 15:32 ` Guenter Roeck
2010-12-16 15:46 ` Juerg Haefliger
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2010-12-16 15:32 UTC (permalink / raw)
To: lm-sensors
On Thu, Dec 16, 2010 at 05:21:40AM -0500, Juerg Haefliger wrote:
> Minor cleanups. Mostly removing assignments in if statements to get
> rid of checkpatch errors.
>
> Signed-off-by: Juerg Haefliger <juergh at gmail.com>
Just wondering ... did you run this patch through checkpatch ?
I did, and got 11 warnings for the unnecessary use of { }.
Not sure if getting rid of checkpatch errors is worth it if you just
replace the errors with warnings.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups
2010-12-16 10:21 [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups Juerg Haefliger
2010-12-16 15:32 ` Guenter Roeck
@ 2010-12-16 15:46 ` Juerg Haefliger
2011-01-09 21:02 ` Jean Delvare
2011-01-10 1:37 ` Guenter Roeck
3 siblings, 0 replies; 5+ messages in thread
From: Juerg Haefliger @ 2010-12-16 15:46 UTC (permalink / raw)
To: lm-sensors
Hi Guenter,
> On Thu, Dec 16, 2010 at 05:21:40AM -0500, Juerg Haefliger wrote:
>> Minor cleanups. Mostly removing assignments in if statements to get
>> rid of checkpatch errors.
>>
>> Signed-off-by: Juerg Haefliger <juergh at gmail.com>
>
> Just wondering ... did you run this patch through checkpatch ?
> I did, and got 11 warnings for the unnecessary use of { }.
>
> Not sure if getting rid of checkpatch errors is worth it if you just
> replace the errors with warnings.
Yes I did run it through checkpatch. I'll trade warnings for errors
any time :-) No seriously, I personally think it's bad to not have {}
around single statements. It's not only dangerous but also
inconsistent with the rest of the code. I refuse to remove them from
any of the drivers that I own. Just my personal preference.
...Juerg
> Guenter
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups
2010-12-16 10:21 [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups Juerg Haefliger
2010-12-16 15:32 ` Guenter Roeck
2010-12-16 15:46 ` Juerg Haefliger
@ 2011-01-09 21:02 ` Jean Delvare
2011-01-10 1:37 ` Guenter Roeck
3 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2011-01-09 21:02 UTC (permalink / raw)
To: lm-sensors
Hi Juerg,
Sorry for the late review.
On Thu, 16 Dec 2010 11:21:40 +0100, Juerg Haefliger wrote:
> Minor cleanups. Mostly removing assignments in if statements to get
> rid of checkpatch errors.
>
> Signed-off-by: Juerg Haefliger <juergh at gmail.com>
> Index: linux-2.6.36/drivers/hwmon/dme1737.c
> =================================> --- linux-2.6.36.orig/drivers/hwmon/dme1737.c 2010-12-16 10:25:35.311669799 +0100
> +++ linux-2.6.36/drivers/hwmon/dme1737.c 2010-12-16 11:11:02.423687592 +0100
> @@ -17,7 +17,7 @@
> *
> * This program is distributed in the hope that it will be useful,
> * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> * GNU General Public License for more details.
A double space after a dot is common in English typography. This isn't
a mistake, it is that way in the original GPL license, and 98% of the
source files in the kernel tree have it that way. So I've reverted it.
> *
> * You should have received a copy of the GNU General Public License
> (...)
> @@ -2080,14 +2089,17 @@
> /* Create PWM sysfs attributes */
> for (ix = 0; ix < ARRAY_SIZE(dme1737_pwm_group); ix++) {
> if (data->has_features & HAS_PWM(ix)) {
> - if ((err = sysfs_create_group(&dev->kobj,
> - &dme1737_pwm_group[ix]))) {
> + err = sysfs_create_group(&dev->kobj,
> + &dme1737_pwm_group[ix]);
> + if (err) {
> goto exit_remove;
> }
> - if ((data->has_features & HAS_PWM_MIN) && ix < 3 &&
> - (err = sysfs_create_file(&dev->kobj,
> - dme1737_auto_pwm_min_attr[ix]))) {
> - goto exit_remove;
> + if ((data->has_features & HAS_PWM_MIN) && (ix < 3)) {
Note that the parentheses around "ix < 3" aren't actually needed (but I
did not remove them.)
> + err = sysfs_create_file(&dev->kobj,
> + dme1737_auto_pwm_min_attr[ix]);
> + if (err) {
> + goto exit_remove;
> + }
> }
> }
> }
> (...)
> @@ -2482,8 +2497,9 @@
> dme1737_sio_outb(sio_cip, 0x07, 0x0a);
>
> /* Get the base address of the runtime registers */
> - if (!(base_addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
> - dme1737_sio_inb(sio_cip, 0x61))) {
> + base_addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
> + dme1737_sio_inb(sio_cip, 0x61);
We lost the nice alignment in the battle. Fixed.
Patch applied, thanks!
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups
2010-12-16 10:21 [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups Juerg Haefliger
` (2 preceding siblings ...)
2011-01-09 21:02 ` Jean Delvare
@ 2011-01-10 1:37 ` Guenter Roeck
3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2011-01-10 1:37 UTC (permalink / raw)
To: lm-sensors
On Sun, Jan 09, 2011 at 04:02:02PM -0500, Jean Delvare wrote:
> Hi Juerg,
>
> Sorry for the late review.
>
> On Thu, 16 Dec 2010 11:21:40 +0100, Juerg Haefliger wrote:
> > Minor cleanups. Mostly removing assignments in if statements to get
> > rid of checkpatch errors.
> >
> > Signed-off-by: Juerg Haefliger <juergh at gmail.com>
[ ... ]
>
> Patch applied, thanks!
>
Hope that won't conflict with Joe's pr_fmt changes. We'll see...
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-10 1:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-16 10:21 [lm-sensors] [PATCH 2/2] hwmon (dme1737): cleanups Juerg Haefliger
2010-12-16 15:32 ` Guenter Roeck
2010-12-16 15:46 ` Juerg Haefliger
2011-01-09 21:02 ` Jean Delvare
2011-01-10 1:37 ` Guenter Roeck
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.