Prev: [12/74] ACPI: Add a generic API for _OSC -v2
Next: [PATCH] HPET: Drop WARN_ON for mismatch on HPET CMP readback II
From: Andrew Morton on 4 Feb 2010 13:30 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 4 Feb 2010 13:40 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 4 Feb 2010 16:20 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 4 Feb 2010 16:30 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 4 Feb 2010 17:00
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/ |