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

2012-10-09 Thread Jean Delvare 
On Sun, 7 Oct 2012 18:50:31 +0200 (CEST), Julia Lawall wrote:
> On Sun, 7 Oct 2012, walter harms wrote:
> > Am 07.10.2012 17:38, schrieb Julia Lawall:
> >> @@ -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 ? : , .len = 1 },
> >> +  I2C_MSG_WRITE(priv->addr, , sizeof(reg)),
> >> +  I2C_MSG_READ(priv->addr, val ? : , 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.

i2c_transfer() itself won't care, it just passes the request down to the
underlying i2c bus driver. Most driver implementations will assume
proper buffer addresses as soon as size > 0, so passing NULL instead
would crash them. In short, don't do that.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-09 Thread Jean Delvare 
On Sun, 7 Oct 2012 18:50:31 +0200 (CEST), Julia Lawall wrote:
 On Sun, 7 Oct 2012, walter harms wrote:
  Am 07.10.2012 17:38, schrieb Julia Lawall:
  @@ -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 = reg, .len = 1 },
  -  { .addr = priv-addr,
  -.flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
  +  I2C_MSG_WRITE(priv-addr, reg, 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.

i2c_transfer() itself won't care, it just passes the request down to the
underlying i2c bus driver. Most driver implementations will assume
proper buffer addresses as soon as size  0, so passing NULL instead
would crash them. In short, don't do that.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread Michael Büsch 
On Sun, 7 Oct 2012 18:50:31 +0200 (CEST)
Julia Lawall  wrote:

> >> @@ -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 ? : , .len = 1 },
> >> +  I2C_MSG_WRITE(priv->addr, , sizeof(reg)),
> >> +  I2C_MSG_READ(priv->addr, val ? : , 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.

Well its use case is if you only care about the side effects and not the actual 
data
returned by the read operation.

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E


signature.asc
Description: PGP signature

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

2012-10-07 Thread walter harms 


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 
>>>
>>> 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/)
>>>
>>> // 
>>> @@
>>> 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)
>>>  ;
>>> // 
>>>
>>> Signed-off-by: Julia Lawall 
>>>
>>> ---
>>>  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, , 1) != 1) {
>>>  dev_err(>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 ? : , .len = 1 },
>>> +I2C_MSG_WRITE(priv->addr, , sizeof(reg)),
>>> +I2C_MSG_READ(priv->addr, val ? : , 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
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread Julia Lawall 

On Sun, 7 Oct 2012, walter harms wrote:




Am 07.10.2012 17:38, schrieb Julia Lawall:

From: Julia Lawall 

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

// 
@@
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)
 ;
// 

Signed-off-by: Julia Lawall 

---
 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, , 1) != 1) {
dev_err(>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 ? : , .len = 1 },
+   I2C_MSG_WRITE(priv->addr, , sizeof(reg)),
+   I2C_MSG_READ(priv->addr, val ? : , 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.


julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread walter harms 


Am 07.10.2012 17:38, schrieb Julia Lawall:
> From: Julia Lawall 
> 
> 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/)
> 
> // 
> @@
> 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)
>  ;
> // 
> 
> Signed-off-by: Julia Lawall 
> 
> ---
>  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, , 1) != 1) {
>   dev_err(>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 ? : , .len = 1 },
> + I2C_MSG_WRITE(priv->addr, , sizeof(reg)),
> + I2C_MSG_READ(priv->addr, val ? : , sizeof(dummy)),
>   };
>  

This dummy looks strange, can it be that this is used uninitialised ?

re,
 wh


>   if (i2c_transfer(priv->i2c, msg, 2) != 2) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread walter harms 


Am 07.10.2012 17:38, schrieb Julia Lawall:
 From: Julia Lawall julia.law...@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.law...@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 = reg, .len = 1 },
 - { .addr = priv-addr,
 -   .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
 + I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
 + I2C_MSG_READ(priv-addr, val ? : dummy, sizeof(dummy)),
   };
  

This dummy looks strange, can it be that this is used uninitialised ?

re,
 wh


   if (i2c_transfer(priv-i2c, msg, 2) != 2) {
 
 --
 To unsubscribe from this list: send the line unsubscribe kernel-janitors in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread Julia Lawall 

On Sun, 7 Oct 2012, walter harms wrote:




Am 07.10.2012 17:38, schrieb Julia Lawall:

From: Julia Lawall julia.law...@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.law...@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 = reg, .len = 1 },
-   { .addr = priv-addr,
- .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
+   I2C_MSG_WRITE(priv-addr, reg, 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.


julia
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread walter harms 


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.law...@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.law...@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 = reg, .len = 1 },
 -{ .addr = priv-addr,
 -  .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
 +I2C_MSG_WRITE(priv-addr, reg, 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
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-10-07 Thread Michael Büsch 
On Sun, 7 Oct 2012 18:50:31 +0200 (CEST)
Julia Lawall julia.law...@lip6.fr wrote:

  @@ -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 = reg, .len = 1 },
  -  { .addr = priv-addr,
  -.flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
  +  I2C_MSG_WRITE(priv-addr, reg, 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.

Well its use case is if you only care about the side effects and not the actual 
data
returned by the read operation.

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E


signature.asc
Description: PGP signature