Thread (55 messages) 55 messages, 9 authors, 2012-12-18

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

From: Julia Lawall <hidden>
Date: 2012-10-07 17:19:10
Also in: kernel-janitors, lkml

On Sun, 7 Oct 2012, walter harms wrote:

Am 07.10.2012 18:44, schrieb Julia Lawall:
quoted
On Sun, 7 Oct 2012, walter harms wrote:
quoted

Am 07.10.2012 17:38, schrieb Julia Lawall:
quoted
From: Julia Lawall <redacted>

Introduce use of I2c_MSG_READ/WRITE/OP, for readability.

In the second i2c_msg structure, a length expressed as an explicit
constant
is also re-expressed as the size of the buffer, reg.

A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression a,b,c;
identifier x;
@@

struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
 ;

@@
expression a,b,c;
identifier x;
@@

struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
 ;

@@
expression a,b,c,d;
identifier x;
@@

struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
 ;
// </smpl>

Signed-off-by: Julia Lawall <redacted>

---
 drivers/media/tuners/e4000.c |   20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv,
u8 reg, u8 *val, int len)
     int ret;
     u8 buf[1 + len];
     struct i2c_msg msg[1] = {
-        {
-            .addr = priv->cfg->i2c_addr,
-            .flags = 0,
-            .len = sizeof(buf),
-            .buf = buf,
-        }
+        I2C_MSG_WRITE(priv->cfg->i2c_addr, buf, sizeof(buf))
     };
Any reason why struct i2c_msg is an array ?
I assumed that it looked more harmonious with the other uses of
i2c_transfer, which takes as arguments an array and the number of elements.

But there are some files that instead use i2c_transfer(priv->i2c, &msg, 1).
I can change them all to do that if that is preferred.  But maybe I will
wait a little bit to see if there are other issues to address at the
same time.

thanks,
julia
Hi Julia,
please be aware i am not the maintainer only a distant watcher :)

do you really thing that a macro is appropriate here ? I feel uneasy about it
but i can not offer an other solution.
Some people thought that it would be nice to have the macros rather than 
the inlined field initializations, especially since there is no flag for 
write.  A separate question is whether an array of one element is useful, 
or whether one should systematically use & on a simple variable of the 
structure type.  I'm open to suggestions about either point.

julia
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help