Hallo Hans-Frieder, > attached is a small patch that removes unused code from i2c-nforce2 > and adds a single debug message. The patch is against 2.6.13-rc3-mm1. > I have tested the patch with 2.6.13-rc3: compiles cleanly and works > as without the patch (as expected). Thanks for this cleanup patch, I like it. However, I would have a few comments as I think we could clean up this driver even more. 1* The MAX_TIMEOUT constant doesn't seem to be used anywhere anymore, so you could rip it. 2* There are a few more lines of code that are commented out: 265 /* case I2C_SMBUS_PROC_CALL: not supported */ 270 /* case I2C_SMBUS_BLOCK_PROC_CALL: not supported */ 288 I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA /* | 289 I2C_FUNC_SMBUS_BLOCK_DATA */; Maybe you can rip them away as well (but see point 4*.) 3* This comment should be updated: 126 /* Return -1 on error. See smbus.h for more information */ There is no file named smbus.h. 4* The driver isn't very clear about what SMBus transactions it supports. nforce2_func reports that only quick, byte, byte data and word data transfers are supported, but nforce2_access seems to implement block and i2c block transfers. If the block implementations work, they should be advertised by nforce2_func. If not, they should be removed. 5* In nforce2_access, you have several cases for dealing with the various transfer types that the driver doesn't support. As users should check for advertised functionalities before they call nforce2_access, I wouldn't bother detailing the errors with different error messages. The default case should be enough. May we have a similar patch for the Linux 2.4 version of the driver as found in the lm_sensors CVS repository? Thanks, -- Jean Delvare
