Re: [PATCH 0/11] introduce macros for i2c_msg initialization

From: Jean Delvare
Date: Tue Oct 09 2012 - 11:33:03 EST


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.

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?

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?

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.

--
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/