Re: [PATCH 2.6.16-mm1 3/3] rtc: add support for m41t81 & m41t85 chips to m41t00 driver

Hi Mark,

Answering your questions before I forget about them and let a week pass
again...

Anything I don't comment on, means that you were right and I agree with
you.

May I ask why you define separate types for the M41T81 and the M41T85,
when you handle both exactly the same way?

The only reason is so that its obvious that both the t81 and t85 are
supported. If I make it M41T81 only then I can easily see someone
grep'ing around looking for M41T85, not finding it, and writing their
own driver. I don't see any harm in having both, do you?

It wastes some memory, and you may later fix something for the M41T81
and forget to fix it for the M41T85.

If your only concern is to help grepers, you can add a clear list of
supported chips either as a comment at the top of the source file, or
as Documentation/i2c/chips/m41t00. That's what we do for hardware
monitoring chips.

No big deal anyway, so the decision is up to you.

What you do here are basically SMBus Read Byte and SMBus Write Byte
transactions. The code would be much more simple if you were using the
i2c_smbus_read_byte_data and i2c_smbus_write_byte_data functions, which
take care of all the technical details.

That's true but I assumed that since I was using i2c_transfer
earlier, I should use it here. Is that a bad assumption?
I do see that ds1337.c uses both types.

Bad assumption, indeed. Nothing prevents you from using the smbus
functions and the i2c functions in the same driver, as long as your
call to i2c_check_functionality covers every function you use. So,
just use whatever makes your driver easier to write.

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

Relevant Pages

  • Linux 2.6.9-rc2
    ... ALSA update, and tons of small fixes pretty much ... work on trying to fix up the drivers as we go along, but they can be a bit ... Update unlink testing code in the usbtest driver ...
    (Linux-Kernel)
  • Linux 2.6.5-rc1
    ... Merging from Andrew, but also i2c updates, ALSA CVS merge, netconsole, ... prism54 driver merge, sata updates, carmel driver, pcmcia and nfs client ... fix missing include in include/asm-ia64/sn/router.h ... Fix CONFIG_DEBUG build on x86-64 & small cleanup ...
    (Linux-Kernel)
  • Linux 2.6.11-rc1
    ... small char/generic_serial.c cleanup ... lockd: fix two struct definitions ... serial driver for TI USB 3410/5052 chips ... Split ACPI boot table parsing ...
    (Linux-Kernel)
  • Linux 2.6.9-rc1
    ... cpufreq, agp, sata, network drivers - you name it. ... o cciss /proc dependency fix ... bus driver for multiple PowerPCs ... o USB: Make removable-LUN support a non-test option in the ...
    (Linux-Kernel)
  • Linux 2.6.7-rc2
    ... An ALSA update, and tons of sparse type-fixes from Al Viro. ... USB, firewire, network driver, XFS, CIFS updates. ... o Fix nodemask clearing bug in NUMA API ...
    (Linux-Kernel)