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
