Re: [PATCH 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization

[Mauro Carvalho Chehab <mchehab@infradead.org>]

Em Mon, 8 Oct 2012 10:31:33 +0200 (CEST)
Julia Lawall <julia.lawall@lip6.fr> escreveu:

> I found only 15 uses of I2C_MSG_OP, out of 653 uses of one of the three
> macros.  Since I2C_MSG_OP has the complete set of flags, I think it should
> be OK?
> 
> One of the uses, in drivers/media/i2c/adv7604.c, is as follows:
> 
>        struct i2c_msg msg[2] = { { client->addr, 0, 1, msgbuf0 },
>                                  { client->addr, 0 | I2C_M_RD, len, msgbuf1 }
> 
> I'm not sure what was intended, but I guess the second structure is
> supposed to only do a read?

Yes, this is just typical I2C register read I2C messsage. The first line
specifies what register should be read (the content of msgbuf0), with is
a one char value, and the second line stores the registers contents at
msgbuf1.

This is exactly what I said before: this is a typical situation:

Just one macro could be used for that, with 4 parameters:

	I2C_MSG_READ_SUBADDR(addr, sub_addr, len, buf);

Almost all of those I2C messages will fall on 3 cases only:
	- a read msg (3 parameters, 1 msg);
	- a write message (3 parameters, 1 msg);
	- a write subaddr followed by a read (4 parameters, 2 msgs).

You'll find very few exceptions to it, where additional I2C flags are
needed, or several different transactions were grouped together, due
to the I2C locking or in order to use I2C repeat-start mode[1].

In a matter of fact, as the maintainer, I prefer to fully see the entire
I2C message for those exceptions, as those other usages require more care
while reviewing/merging.


[1] very, very few media i2c bus drivers implement any other flags except
for I2C_M_RD. That's why it is so rare to see them there.

> 
> julia
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html




Cheers,
Mauro