On Tue, 9 Oct 2012, Jean Delvare wrote: > Hi Julia, > > On Sun, 7 Oct 2012 17:38:30 +0200, Julia Lawall wrote: > > This patch set introduces some macros for describing how an i2c_msg is > > being initialized. There are three macros: I2C_MSG_READ, for a read > > message, I2C_MSG_WRITE, for a write message, and I2C_MSG_OP, for some other > > kind of message, which is expected to be very rarely used. > > "Some other kind of message" is actually messages which need extra > flags. They are still read or write messages. I agree. We could also have a read with extra options macro and a write with extra options macro. That would give four macros, which is not too much more than three. > OK, I've read the whole series now and grepped the kernel tree so I > have a better overview. There are a lot more occurrences than what you > converted. I presume the conversions were just an example and you leave > the rest up to the relevant maintainers (e.g. me) if they are > interested? I would be happy to do the rest, or at least to do more. I just didn't want to do 600+ cases before knowing how others felt about the various changes. Actually, now that we seem to have decided to make fewer changes at once, I could probably work more quickly. So far, I have been comparing the results after running cpp, as well as checking that the sizeof transformation is correct, which is a bit slow. > Given the huge number of affected drivers (a quick grep suggests 230 > drivers and more than 300 occurrences), we'd better think twice before > going on as it will be intrusive and hard to change afterward. > > So my first question will be: what is your goal with this change? Are > you only trying to save a few lines of source code? Or do you expect to > actually fix/prevent bugs by introducing these macros? Or something > else? The main goal just seems to be to provide something that is more readable. > I admit I am not completely convinced by the benefit at the moment. A > number of these drivers should be using i2c_smbus_*() functions instead > of i2c_transfer() for improved compatibility, or i2c_master_send/recv() > for single message transfers (383 occurrences!), so making > i2c_transfer() easier to use isn't at the top of my priority list. And > I see the extra work for the pre-processor, so we need a good reason > for doing that. OK, if it doesn't seem like a good idea, it is no problem to drop the idea completely. It does seem a bit nicer to have writing indicated as WRITE rather than as 0, but that might not be a big enough benefit to justify making changes. julia -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
