From: Andrew Morton on
On Wed, 3 Feb 2010 17:23:49 -0800
Steven King <sfking(a)fdwdc.com> wrote:

> The TI TMP102 is similar to the lm75. It differs from the lm75 by having a 16 bit conf register
> and the temp registers have a minimum resolution of 12bits; the extended conf register
> can select 13 bit resolution (which this driver does) and also change the update rate (which this
> driver currently doesn't use).
>

A neat little driver.

checkpatch spits this warning:

WARNING: struct dev_pm_ops should normally be const
#387: FILE: drivers/hwmon/tmp102.c:300:
+static struct dev_pm_ops tmp102_dev_pm_ops = {

which seems truthful enough.

--- a/drivers/hwmon/tmp102.c~hwmon-driver-for-ti-tmp102-temperature-sensor-checkpatch-fixes
+++ a/drivers/hwmon/tmp102.c
@@ -297,7 +297,7 @@ static int tmp102_resume(struct device *
return 0;
}

-static struct dev_pm_ops tmp102_dev_pm_ops = {
+static const struct dev_pm_ops tmp102_dev_pm_ops = {
.suspend = tmp102_suspend,
.resume = tmp102_resume,
};
_



And doing this will hurt readers' brains less:



Use conventional array-walk loop.

--- a/drivers/hwmon/tmp102.c~hwmon-driver-for-ti-tmp102-temperature-sensor-fix
+++ a/drivers/hwmon/tmp102.c
@@ -91,13 +91,14 @@ static struct tmp102 *tmp102_update_devi

mutex_lock(&tmp102->lock);
if (time_after(jiffies, tmp102->last_update + HZ / 4)) {
- int i = 0;
- do {
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tmp102->temp); i++) {
int status = tmp102_read_reg(client, tmp102_reg[i]);
if (status > -1)
tmp102->temp[i] = tmp102_reg_to_mC(status);

- } while (++i < ARRAY_SIZE(tmp102->temp));
+ }
tmp102->last_update = jiffies;
}
mutex_unlock(&tmp102->lock);
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jean Delvare on
Hi Steven,

On Wed, 3 Feb 2010 17:23:49 -0800, Steven King wrote:
> The TI TMP102 is similar to the lm75. It differs from the lm75 by having a 16 bit conf register
> and the temp registers have a minimum resolution of 12bits; the extended conf register
> can select 13 bit resolution (which this driver does) and also change the update rate (which this
> driver currently doesn't use).
>
> Signed-off-by: Steven King <sfking(a)fdwdc.com>
> ---
>
> Driver for the TI TMP102.

Could you please send a dump of your TMP102 chip, using i2cdump in word
mode?

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Steven King on
On Thursday 04 February 2010 10:22:03 Andrew Morton wrote:
> On Wed, 3 Feb 2010 17:23:49 -0800
>
> Steven King <sfking(a)fdwdc.com> wrote:
> > The TI TMP102 is similar to the lm75. It differs from the lm75 by having
> > a 16 bit conf register and the temp registers have a minimum resolution
> > of 12bits; the extended conf register can select 13 bit resolution (which
> > this driver does) and also change the update rate (which this driver
> > currently doesn't use).
>
> A neat little driver.

Thanks.

> checkpatch spits this warning:
>
> WARNING: struct dev_pm_ops should normally be const
> #387: FILE: drivers/hwmon/tmp102.c:300:
> +static struct dev_pm_ops tmp102_dev_pm_ops = {
>
> which seems truthful enough.

Indeed. I am, however, somewhat surprised since I ran the patch thru
checkpatch before posting it and no errors or warnings were reported. Is
there a version of checkpatch other than the one included in the tree that I
should be using?

>
> And doing this will hurt readers' brains less:
>
>
>
> Use conventional array-walk loop.

Ah yes, an idiosyncrasy of mine in preferring do while over for loops
especially when I 'know' the initial test will pass. Whatever is the
preferred idiom for the kernel is fine with me.

--
Steven King -- sfking at fdwdc dot com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on
On Thu, 4 Feb 2010 13:17:37 -0800
Steven King <sfking(a)fdwdc.com> wrote:

> > checkpatch spits this warning:
> >
> > WARNING: struct dev_pm_ops should normally be const
> > #387: FILE: drivers/hwmon/tmp102.c:300:
> > +static struct dev_pm_ops tmp102_dev_pm_ops = {
> >
> > which seems truthful enough.
>
> Indeed. I am, however, somewhat surprised since I ran the patch thru
> checkpatch before posting it and no errors or warnings were reported. Is
> there a version of checkpatch other than the one included in the tree that I
> should be using?

It was -mm's
checkpatchpl-extend-list-of-expected-to-be-const-structures.patch which
found this.


From: Emese Revfy <re.emese(a)gmail.com>

Based on Arjan's suggestion, extend the list of ops structures that should
be const.

Signed-off-by: Emese Revfy <re.emese(a)gmail.com>
Cc: Andy Whitcroft <apw(a)shadowen.org>
Cc: Arjan van de Ven <arjan(a)infradead.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---

scripts/checkpatch.pl | 41 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)

diff -puN scripts/checkpatch.pl~checkpatchpl-extend-list-of-expected-to-be-const-structures scripts/checkpatch.pl
--- a/scripts/checkpatch.pl~checkpatchpl-extend-list-of-expected-to-be-const-structures
+++ a/scripts/checkpatch.pl
@@ -2654,9 +2654,46 @@ sub process {
if ($line =~ /^.\s*__initcall\s*\(/) {
WARN("please use device_initcall() instead of __initcall()\n" . $herecurr);
}
-# check for struct file_operations, ensure they are const.
+# check for various ops structs, ensure they are const.
+ my $struct_ops = qr{acpi_dock_ops|
+ address_space_operations|
+ backlight_ops|
+ block_device_operations|
+ dentry_operations|
+ dev_pm_ops|
+ dma_map_ops|
+ extent_io_ops|
+ file_lock_operations|
+ file_operations|
+ hv_ops|
+ ide_dma_ops|
+ intel_dvo_dev_ops|
+ item_operations|
+ iwl_ops|
+ kgdb_arch|
+ kgdb_io|
+ kset_uevent_ops|
+ lock_manager_operations|
+ microcode_ops|
+ mtrr_ops|
+ neigh_ops|
+ nlmsvc_binding|
+ pci_raw_ops|
+ pipe_buf_operations|
+ platform_hibernation_ops|
+ platform_suspend_ops|
+ proto_ops|
+ rpc_pipe_ops|
+ seq_operations|
+ snd_ac97_build_ops|
+ soc_pcmcia_socket_ops|
+ stacktrace_ops|
+ sysfs_ops|
+ tty_operations|
+ usb_mon_operations|
+ wd_ops}x;
if ($line !~ /\bconst\b/ &&
- $line =~ /\bstruct\s+(file_operations|seq_operations)\b/) {
+ $line =~ /\bstruct\s+($struct_ops)\b/) {
WARN("struct $1 should normally be const\n" .
$herecurr);
}
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Steven King on
On Thursday 04 February 2010 10:37:12 Jean Delvare wrote:
> Hi Steven,
>
> On Wed, 3 Feb 2010 17:23:49 -0800, Steven King wrote:
> > The TI TMP102 is similar to the lm75. It differs from the lm75 by having
> > a 16 bit conf register and the temp registers have a minimum resolution
> > of 12bits; the extended conf register can select 13 bit resolution (which
> > this driver does) and also change the update rate (which this driver
> > currently doesn't use).
> >
> > Signed-off-by: Steven King <sfking(a)fdwdc.com>
> > ---
> >
> > Driver for the TI TMP102.
>
> Could you please send a dump of your TMP102 chip, using i2cdump in word
> mode?

sure, like so?

# ./i2cdump 0 0x4a w
WARNING! This program can confuse your I2C bus, cause data loss and worse!
I will probe file /dev/i2c-0, address 0x4a, mode word
Continue? [Y/n] y
0,8 1,9 2,a 3,b 4,c 5,d 6,e 7,f
00: d90c b062 002d 004b c01a d006 0000 0000
08: d90c b062 002d 004b c01a d006 0000 0000
10: d90c b062 002d 004b c01a d006 0000 0000
18: d90c b062 002d 004b c01a d006 0000 0000
20: d90c b062 002d 004b c01a d006 0000 0000
28: d90c b062 002d 004b c01a d006 0000 0000
30: d90c b062 002d 004b c01a d006 0000 0000
38: d90c b062 002d 004b c01a d006 0000 0000
40: d90c b062 002d 004b c01a d006 0000 0000
48: d90c b062 002d 004b c01a d006 0000 0000
50: d90c b062 002d 004b c01a d006 0000 0000
58: d90c b062 002d 004b c01a d006 0000 0000
60: d90c b062 002d 004b c01a d006 0000 0000
68: d90c b062 002d 004b c01a d006 0000 0000
70: d90c b062 002d 004b c01a d006 0000 0000
78: d90c b062 002d 004b c01a d006 0000 0000
80: d90c b062 002d 004b c01a d006 0000 0000
88: d90c b062 002d 004b c01a d006 0000 0000
90: d90c b062 002d 004b c01a d006 0000 0000
98: d90c b062 002d 004b c01a d006 0000 0000
a0: d90c b062 002d 004b c01a d006 0000 0000
a8: d90c b062 002d 004b c01a d006 0000 0000
b0: d90c b062 002d 004b c01a d006 0000 0000
b8: d90c b062 002d 004b c01a d006 0000 0000
c0: d90c b062 002d 004b c01a d006 0000 0000
c8: d90c b062 002d 004b c01a d006 0000 0000
d0: d90c b062 002d 004b c01a d006 0000 0000
d8: d90c b062 002d 004b c01a d006 0000 0000
e0: d10c b062 002d 004b c01a d006 0000 0000
e8: d10c b062 002d 004b c01a d006 0000 0000
f0: d10c b062 002d 004b c01a d006 0000 0000
f8: d10c b062 002d 004b c01a d006 0000 0000
#

--
Steven King -- sfking at fdwdc dot com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/