From: walter harms <wharms@bfs.de>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Michael Buesch <m@bues.ch>,
kernel-janitors@vger.kernel.org, rmallon@gmail.com,
shubhrajyoti@ti.com,
Mauro Carvalho Chehab <mchehab@infradead.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
Date: Sun, 07 Oct 2012 16:54:41 +0000 [thread overview]
Message-ID: <5071B3D1.6080004@bfs.de> (raw)
In-Reply-To: <alpine.DEB.2.02.1210071845030.2745@localhost6.localdomain6>
Am 07.10.2012 18:50, schrieb Julia Lawall:
> On Sun, 7 Oct 2012, walter harms wrote:
>
>>
>>
>> Am 07.10.2012 17:38, schrieb Julia Lawall:
>>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
>>>
>>> A length expressed as an explicit constant is also re-expressed as
>>> the size
>>> of the buffer in each case.
>>>
>>> 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 <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>> drivers/media/tuners/fc0011.c | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/tuners/fc0011.c
>>> b/drivers/media/tuners/fc0011.c
>>> index e488254..5dbba98 100644
>>> --- a/drivers/media/tuners/fc0011.c
>>> +++ b/drivers/media/tuners/fc0011.c
>>> @@ -80,8 +80,7 @@ struct fc0011_priv {
>>> static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
>>> {
>>> u8 buf[2] = { reg, val };
>>> - struct i2c_msg msg = { .addr = priv->addr,
>>> - .flags = 0, .buf = buf, .len = 2 };
>>> + struct i2c_msg msg = I2C_MSG_WRITE(priv->addr, buf, sizeof(buf));
>>>
>>> if (i2c_transfer(priv->i2c, &msg, 1) != 1) {
>>> dev_err(&priv->i2c->dev,
>>> @@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv
>>> *priv, u8 reg, u8 *val)
>>> {
>>> u8 dummy;
>>> struct i2c_msg msg[2] = {
>>> - { .addr = priv->addr,
>>> - .flags = 0, .buf = ®, .len = 1 },
>>> - { .addr = priv->addr,
>>> - .flags = I2C_M_RD, .buf = val ? : &dummy, .len = 1 },
>>> + I2C_MSG_WRITE(priv->addr, ®, sizeof(reg)),
>>> + I2C_MSG_READ(priv->addr, val ? : &dummy, sizeof(dummy)),
>>> };
>>>
>>
>> This dummy looks strange, can it be that this is used uninitialised ?
>
> I'm not sure to understand the question. The read, when it happens in
> i2c_transfer will initialize dummy. On the other hand, I don't know
> what i2c_transfer does when the buffer is NULL and the size is 1. It
> does not look very elegant at least.
>
mea culpa, i mixed read and write
re,
wh
> julia
>
>
WARNING: multiple messages have this Message-ID (diff)
From: walter harms <wharms@bfs.de>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Michael Buesch <m@bues.ch>,
kernel-janitors@vger.kernel.org, rmallon@gmail.com,
shubhrajyoti@ti.com,
Mauro Carvalho Chehab <mchehab@infradead.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
Date: Sun, 07 Oct 2012 18:54:41 +0200 [thread overview]
Message-ID: <5071B3D1.6080004@bfs.de> (raw)
In-Reply-To: <alpine.DEB.2.02.1210071845030.2745@localhost6.localdomain6>
Am 07.10.2012 18:50, schrieb Julia Lawall:
> On Sun, 7 Oct 2012, walter harms wrote:
>
>>
>>
>> Am 07.10.2012 17:38, schrieb Julia Lawall:
>>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
>>>
>>> A length expressed as an explicit constant is also re-expressed as
>>> the size
>>> of the buffer in each case.
>>>
>>> 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 <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>> drivers/media/tuners/fc0011.c | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/tuners/fc0011.c
>>> b/drivers/media/tuners/fc0011.c
>>> index e488254..5dbba98 100644
>>> --- a/drivers/media/tuners/fc0011.c
>>> +++ b/drivers/media/tuners/fc0011.c
>>> @@ -80,8 +80,7 @@ struct fc0011_priv {
>>> static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
>>> {
>>> u8 buf[2] = { reg, val };
>>> - struct i2c_msg msg = { .addr = priv->addr,
>>> - .flags = 0, .buf = buf, .len = 2 };
>>> + struct i2c_msg msg = I2C_MSG_WRITE(priv->addr, buf, sizeof(buf));
>>>
>>> if (i2c_transfer(priv->i2c, &msg, 1) != 1) {
>>> dev_err(&priv->i2c->dev,
>>> @@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv
>>> *priv, u8 reg, u8 *val)
>>> {
>>> u8 dummy;
>>> struct i2c_msg msg[2] = {
>>> - { .addr = priv->addr,
>>> - .flags = 0, .buf = ®, .len = 1 },
>>> - { .addr = priv->addr,
>>> - .flags = I2C_M_RD, .buf = val ? : &dummy, .len = 1 },
>>> + I2C_MSG_WRITE(priv->addr, ®, sizeof(reg)),
>>> + I2C_MSG_READ(priv->addr, val ? : &dummy, sizeof(dummy)),
>>> };
>>>
>>
>> This dummy looks strange, can it be that this is used uninitialised ?
>
> I'm not sure to understand the question. The read, when it happens in
> i2c_transfer will initialize dummy. On the other hand, I don't know
> what i2c_transfer does when the buffer is NULL and the size is 1. It
> does not look very elegant at least.
>
mea culpa, i mixed read and write
re,
wh
> julia
>
>
next prev parent reply other threads:[~2012-10-07 16:54 UTC|newest]
Thread overview: 122+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-07 15:38 [PATCH 0/11] introduce macros for i2c_msg initialization Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 13/13] drivers/media/tuners/e4000.c: use " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 16:33 ` walter harms
2012-10-07 16:33 ` walter harms
2012-10-07 16:44 ` Julia Lawall
2012-10-07 16:44 ` Julia Lawall
2012-10-07 17:13 ` walter harms
2012-10-07 17:13 ` walter harms
2012-10-07 17:18 ` Julia Lawall
2012-10-07 17:18 ` Julia Lawall
2012-10-07 18:16 ` Joe Perches
2012-10-07 18:16 ` Joe Perches
2012-10-07 18:56 ` Julia Lawall
2012-10-07 18:56 ` Julia Lawall
2012-10-07 21:39 ` Joe Perches
2012-10-07 21:39 ` Joe Perches
2012-10-07 21:43 ` Julia Lawall
2012-10-07 21:43 ` Julia Lawall
2012-10-07 21:51 ` Joe Perches
2012-10-07 21:51 ` Joe Perches
2012-10-08 1:56 ` Mauro Carvalho Chehab
2012-10-08 1:56 ` Mauro Carvalho Chehab
2012-10-08 2:11 ` Ryan Mallon
2012-10-08 2:11 ` Ryan Mallon
2012-10-08 7:54 ` Antti Palosaari
2012-10-08 7:54 ` Antti Palosaari
2012-10-08 8:31 ` Julia Lawall
2012-10-08 8:31 ` Julia Lawall
2012-10-09 23:32 ` Mauro Carvalho Chehab
2012-10-09 23:32 ` Mauro Carvalho Chehab
[not found] ` <20121009203238.63d2275f-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2012-10-11 6:45 ` Julia Lawall
2012-10-11 6:45 ` Julia Lawall
2012-10-11 6:45 ` Julia Lawall
2012-12-18 11:46 ` Jean Delvare
2012-12-18 11:46 ` Jean Delvare
2012-12-18 11:46 ` Jean Delvare
2012-12-18 11:46 ` Jean Delvare
[not found] ` <20121218124640.5b1e7176-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2012-12-18 12:36 ` Julia Lawall
2012-12-18 12:36 ` Julia Lawall
2012-12-18 12:36 ` Julia Lawall
2012-12-18 12:36 ` Julia Lawall
2012-12-18 13:13 ` Wolfram Sang
2012-12-18 13:13 ` Wolfram Sang
2012-12-18 13:13 ` Wolfram Sang
2012-10-09 23:50 ` Mauro Carvalho Chehab
2012-10-08 5:04 ` Julia Lawall
2012-10-08 5:04 ` Julia Lawall
2012-10-07 21:49 ` Ryan Mallon
2012-10-07 21:49 ` Ryan Mallon
2012-10-07 21:52 ` Ryan Mallon
2012-10-07 21:52 ` Ryan Mallon
2012-10-07 15:38 ` [PATCH 2/13] drivers/media/tuners/mxl5007t.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-09 12:30 ` Jean Delvare
2012-10-09 12:30 ` Jean Delvare
2012-10-09 12:50 ` Julia Lawall
2012-10-09 12:50 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 3/13] drivers/media/tuners/qt1010.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 21:55 ` Ryan Mallon
2012-10-07 21:55 ` Ryan Mallon
2012-10-08 5:05 ` Julia Lawall
2012-10-08 5:05 ` Julia Lawall
2012-10-08 5:13 ` Ryan Mallon
2012-10-08 5:13 ` Ryan Mallon
2012-10-08 5:24 ` Julia Lawall
2012-10-08 5:24 ` Julia Lawall
2012-10-09 12:12 ` Jean Delvare
2012-10-09 12:12 ` Jean Delvare
2012-10-09 12:51 ` Julia Lawall
2012-10-09 12:51 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 4/13] drivers/media/tuners/tda18212.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 5/13] drivers/media/tuners: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 22:05 ` Ryan Mallon
2012-10-07 22:05 ` Ryan Mallon
2012-10-08 5:12 ` Julia Lawall
2012-10-08 5:12 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 6/13] drivers/media/tuners/tda18271-common.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 7/13] drivers/media/tuners/tua9001.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 8/13] drivers/media/tuners/fc2580.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 9/13] drivers/media/tuners/fc0011.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 16:43 ` walter harms
2012-10-07 16:43 ` walter harms
2012-10-07 16:50 ` Julia Lawall
2012-10-07 16:50 ` Julia Lawall
2012-10-07 16:54 ` walter harms [this message]
2012-10-07 16:54 ` walter harms
2012-10-07 18:05 ` Michael Büsch
2012-10-07 18:05 ` Michael Büsch
2012-10-09 12:06 ` Jean Delvare
2012-10-09 12:06 ` Jean Delvare
2012-10-07 15:38 ` [PATCH 10/13] drivers/media/tuners/tda8290.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 11/13] drivers/media/tuners/tda18218.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` [PATCH 12/13] drivers/media/tuners/max2165.c: " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 22:14 ` Ryan Mallon
2012-10-07 22:14 ` Ryan Mallon
2012-10-08 5:13 ` Julia Lawall
2012-10-08 5:13 ` Julia Lawall
[not found] ` <1349624323-15584-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
2012-10-07 15:38 ` [PATCH 1/13] include/linux/i2c.h: introduce " Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-07 15:38 ` Julia Lawall
2012-10-09 15:32 ` [PATCH 0/11] " Jean Delvare
2012-10-09 15:32 ` Jean Delvare
2012-10-09 15:32 ` Jean Delvare
[not found] ` <20121009173237.7c1a49e9-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2012-10-09 15:43 ` Julia Lawall
2012-10-09 15:43 ` Julia Lawall
2012-10-09 15:43 ` Julia Lawall
2012-10-22 9:18 ` Julia Lawall
2012-10-22 9:18 ` Julia Lawall
2012-10-22 9:18 ` Julia Lawall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5071B3D1.6080004@bfs.de \
--to=wharms@bfs.de \
--cc=julia.lawall@lip6.fr \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=m@bues.ch \
--cc=mchehab@infradead.org \
--cc=rmallon@gmail.com \
--cc=shubhrajyoti@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.