Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-06-09 Thread Wolfram Sang
Hi Jean,

On Tue, Jun 08, 2010 at 10:01:00AM +0200, Jean Delvare wrote:
 Now that i2c-core offers the possibility to provide custom probing
 function for I2C devices, let's make use of it.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 Acked-by: Mauro Carvalho Chehab mche...@redhat.com

If this custom function is in i2c-core, maybe it should be documented?

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-06-09 Thread Jean Delvare
Hi Wolfram,

On Wed, 9 Jun 2010 17:05:40 +0200, Wolfram Sang wrote:
 Hi Jean,
 
 On Tue, Jun 08, 2010 at 10:01:00AM +0200, Jean Delvare wrote:
  Now that i2c-core offers the possibility to provide custom probing
  function for I2C devices, let's make use of it.
  
  Signed-off-by: Jean Delvare kh...@linux-fr.org
  Acked-by: Mauro Carvalho Chehab mche...@redhat.com
 
 If this custom function is in i2c-core, maybe it should be documented?

What kind of documentation would you expect for a one-line function?
Where, and aimed at who?

Patch welcome ;)

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


[PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-06-08 Thread Jean Delvare
Now that i2c-core offers the possibility to provide custom probing
function for I2C devices, let's make use of it.

Signed-off-by: Jean Delvare kh...@linux-fr.org
Acked-by: Mauro Carvalho Chehab mche...@redhat.com
---
 drivers/i2c/i2c-core.c|7 +++
 drivers/media/video/cx23885/cx23885-i2c.c |   15 ---
 drivers/media/video/cx88/cx88-i2c.c   |   19 ---
 include/linux/i2c.h   |3 +++
 4 files changed, 18 insertions(+), 26 deletions(-)

--- linux-2.6.35-rc2.orig/drivers/media/video/cx23885/cx23885-i2c.c 
2010-06-07 16:12:38.0 +0200
+++ linux-2.6.35-rc2/drivers/media/video/cx23885/cx23885-i2c.c  2010-06-08 
09:17:06.0 +0200
@@ -365,17 +365,10 @@ int cx23885_i2c_register(struct cx23885_
 
memset(info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, ir_video, I2C_NAME_SIZE);
-   /*
-* We can't call i2c_new_probed_device() because it uses
-* quick writes for probing and the IR receiver device only
-* replies to reads.
-*/
-   if (i2c_smbus_xfer(bus-i2c_adap, addr_list[0], 0,
-  I2C_SMBUS_READ, 0, I2C_SMBUS_QUICK,
-  NULL) = 0) {
-   info.addr = addr_list[0];
-   i2c_new_device(bus-i2c_adap, info);
-   }
+   /* Use quick read command for probe, some IR chips don't
+* support writes */
+   i2c_new_probed_device(bus-i2c_adap, info, addr_list,
+ i2c_probe_func_quick_read);
}
 
return bus-i2c_rc;
--- linux-2.6.35-rc2.orig/drivers/media/video/cx88/cx88-i2c.c   2010-06-07 
16:12:38.0 +0200
+++ linux-2.6.35-rc2/drivers/media/video/cx88/cx88-i2c.c2010-06-08 
09:17:06.0 +0200
@@ -188,24 +188,13 @@ int cx88_i2c_init(struct cx88_core *core
0x18, 0x6b, 0x71,
I2C_CLIENT_END
};
-   const unsigned short *addrp;
 
memset(info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, ir_video, I2C_NAME_SIZE);
-   /*
-* We can't call i2c_new_probed_device() because it uses
-* quick writes for probing and at least some R receiver
-* devices only reply to reads.
-*/
-   for (addrp = addr_list; *addrp != I2C_CLIENT_END; addrp++) {
-   if (i2c_smbus_xfer(core-i2c_adap, *addrp, 0,
-  I2C_SMBUS_READ, 0,
-  I2C_SMBUS_QUICK, NULL) = 0) {
-   info.addr = *addrp;
-   i2c_new_device(core-i2c_adap, info);
-   break;
-   }
-   }
+   /* Use quick read command for probe, some IR chips don't
+* support writes */
+   i2c_new_probed_device(core-i2c_adap, info, addr_list,
+ i2c_probe_func_quick_read);
}
return core-i2c_rc;
 }
--- linux-2.6.35-rc2.orig/drivers/i2c/i2c-core.c2010-06-07 
16:12:38.0 +0200
+++ linux-2.6.35-rc2/drivers/i2c/i2c-core.c 2010-06-08 09:17:06.0 
+0200
@@ -1453,6 +1453,13 @@ static int i2c_detect(struct i2c_adapter
return err;
 }
 
+int i2c_probe_func_quick_read(struct i2c_adapter *adap, unsigned short addr)
+{
+   return i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, 0,
+ I2C_SMBUS_QUICK, NULL) = 0;
+}
+EXPORT_SYMBOL_GPL(i2c_probe_func_quick_read);
+
 struct i2c_client *
 i2c_new_probed_device(struct i2c_adapter *adap,
  struct i2c_board_info *info,
--- linux-2.6.35-rc2.orig/include/linux/i2c.h   2010-06-07 16:15:10.0 
+0200
+++ linux-2.6.35-rc2/include/linux/i2c.h2010-06-08 09:19:07.0 
+0200
@@ -292,6 +292,9 @@ i2c_new_probed_device(struct i2c_adapter
  unsigned short const *addr_list,
  int (*probe)(struct i2c_adapter *, unsigned short addr));
 
+/* Common custom probe functions */
+extern int i2c_probe_func_quick_read(struct i2c_adapter *, unsigned short 
addr);
+
 /* For devices that use several addresses, use i2c_new_dummy() to make
  * client handles for the extra addresses.
  */


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


Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-09 Thread Jean Delvare
Hi Mauro,

On Fri, 09 Apr 2010 01:09:08 -0300, Mauro Carvalho Chehab wrote:
 Jean Delvare wrote:
  There are no other probing functions yet, this is the first one. I have
  added the mechanism to i2c-core for these very IR chips.
  
  Putting all probe functions together would mean moving them to
  i2c-core. This wasn't my original intent, but after all, it makes some
  sense. Would you be happy with the following?
 
 It seems fine for me. As you're touching on i2c core and on other drivers
 on this series, I think that the better is if you could apply it on your
 tree.

Yes, this is the plan. However, before I can add them to my tree, patch
named:
  [PATCH] FusionHDTV: Use quick reads for I2C IR device probing
which I posted to the linux-media mailing list some weeks ago must go
upstream. Otherwise these 2 patches do not apply cleanly.

 For both patches 1 and 2:
 
 Acked-by: Mauro Carvalho Chehab mche...@redhat.com

Thanks.

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


Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-08 Thread Mauro Carvalho Chehab
Jean Delvare wrote:
 Hi Mauro,
 
 On Tue, 06 Apr 2010 02:34:46 -0300, Mauro Carvalho Chehab wrote:
 Jean Delvare wrote:
 On Mon, 05 Apr 2010 15:26:32 -0300, Mauro Carvalho Chehab wrote:
 Please, don't add new things at ir-common module. It basically contains the
 decoding functions for RC5 and pulse/distance, plus several IR keymaps. 
 With
 the IR rework I'm doing, this module will go away, after having all the 
 current 
 IR decoders implemented via ir-raw-input binding. 

 The keymaps were already removed from it, on my experimental tree 
 (http://git.linuxtv.org/mchehab/ir.git), and rc5 decoder is already written
 (but still needs a few fixes). 

 The new ir-core is creating an abstract way to deal with Remote 
 Controllers,
 meant to be used not only by IR's, but also for other types of RC, like, 
 bluetooth and USB HID. It will also export a raw event interface, for use
 with lirc. As this is the core of the RC subsystem, a i2c-specific binding
 method also doesn't seem to belong there. SO, IMO, the better place is to 
 add 
 it as a static inline function at ir-kbd-i2c.h.
 Ever tried to pass the address of an inline function as another
 function's parameter? :)
 :) Never tried... maybe gcc would to the hard thing, de-inlining it ;)

 Well, we need to put this code somewhere. Where are the other probing
 codes? Probably the better is to put them together.
 
 There are no other probing functions yet, this is the first one. I have
 added the mechanism to i2c-core for these very IR chips.
 
 Putting all probe functions together would mean moving them to
 i2c-core. This wasn't my original intent, but after all, it makes some
 sense. Would you be happy with the following?

It seems fine for me. As you're touching on i2c core and on other drivers
on this series, I think that the better is if you could apply it on your
tree.

For both patches 1 and 2:

Acked-by: Mauro Carvalho Chehab mche...@redhat.com

 
 * * * * *
 
 From: Jean Delvare kh...@linux-fr.org
 Subject: V4L/DVB: Use custom I2C probing function mechanism
 
 Now that i2c-core offers the possibility to provide custom probing
 function for I2C devices, let's make use of it.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 ---
  drivers/i2c/i2c-core.c|7 +++
  drivers/media/video/cx23885/cx23885-i2c.c |   15 ---
  drivers/media/video/cx88/cx88-i2c.c   |   19 ---
  include/linux/i2c.h   |3 +++
  4 files changed, 18 insertions(+), 26 deletions(-)
 
 --- linux-2.6.34-rc3.orig/drivers/media/video/cx23885/cx23885-i2c.c   
 2010-04-06 11:31:20.0 +0200
 +++ linux-2.6.34-rc3/drivers/media/video/cx23885/cx23885-i2c.c
 2010-04-06 12:28:09.0 +0200
 @@ -365,17 +365,10 @@ int cx23885_i2c_register(struct cx23885_
  
   memset(info, 0, sizeof(struct i2c_board_info));
   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 - /*
 -  * We can't call i2c_new_probed_device() because it uses
 -  * quick writes for probing and the IR receiver device only
 -  * replies to reads.
 -  */
 - if (i2c_smbus_xfer(bus-i2c_adap, addr_list[0], 0,
 -I2C_SMBUS_READ, 0, I2C_SMBUS_QUICK,
 -NULL) = 0) {
 - info.addr = addr_list[0];
 - i2c_new_device(bus-i2c_adap, info);
 - }
 + /* Use quick read command for probe, some IR chips don't
 +  * support writes */
 + i2c_new_probed_device(bus-i2c_adap, info, addr_list,
 +   i2c_probe_func_quick_read);
   }
  
   return bus-i2c_rc;
 --- linux-2.6.34-rc3.orig/drivers/media/video/cx88/cx88-i2c.c 2010-04-06 
 11:31:20.0 +0200
 +++ linux-2.6.34-rc3/drivers/media/video/cx88/cx88-i2c.c  2010-04-06 
 12:28:06.0 +0200
 @@ -188,24 +188,13 @@ int cx88_i2c_init(struct cx88_core *core
   0x18, 0x6b, 0x71,
   I2C_CLIENT_END
   };
 - const unsigned short *addrp;
  
   memset(info, 0, sizeof(struct i2c_board_info));
   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 - /*
 -  * We can't call i2c_new_probed_device() because it uses
 -  * quick writes for probing and at least some R receiver
 -  * devices only reply to reads.
 -  */
 - for (addrp = addr_list; *addrp != I2C_CLIENT_END; addrp++) {
 - if (i2c_smbus_xfer(core-i2c_adap, *addrp, 0,
 -I2C_SMBUS_READ, 0,
 -I2C_SMBUS_QUICK, NULL) = 0) {
 - info.addr = *addrp;
 - i2c_new_device(core-i2c_adap, info);
 - break;
 - }
 - }
 + 

Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-06 Thread Jean Delvare
Hi Mauro,

On Tue, 06 Apr 2010 02:34:46 -0300, Mauro Carvalho Chehab wrote:
 Jean Delvare wrote:
  On Mon, 05 Apr 2010 15:26:32 -0300, Mauro Carvalho Chehab wrote:
  Please, don't add new things at ir-common module. It basically contains the
  decoding functions for RC5 and pulse/distance, plus several IR keymaps. 
  With
  the IR rework I'm doing, this module will go away, after having all the 
  current 
  IR decoders implemented via ir-raw-input binding. 
 
  The keymaps were already removed from it, on my experimental tree 
  (http://git.linuxtv.org/mchehab/ir.git), and rc5 decoder is already written
  (but still needs a few fixes). 
 
  The new ir-core is creating an abstract way to deal with Remote 
  Controllers,
  meant to be used not only by IR's, but also for other types of RC, like, 
  bluetooth and USB HID. It will also export a raw event interface, for use
  with lirc. As this is the core of the RC subsystem, a i2c-specific binding
  method also doesn't seem to belong there. SO, IMO, the better place is to 
  add 
  it as a static inline function at ir-kbd-i2c.h.
  
  Ever tried to pass the address of an inline function as another
  function's parameter? :)
 
 :) Never tried... maybe gcc would to the hard thing, de-inlining it ;)
 
 Well, we need to put this code somewhere. Where are the other probing
 codes? Probably the better is to put them together.

There are no other probing functions yet, this is the first one. I have
added the mechanism to i2c-core for these very IR chips.

Putting all probe functions together would mean moving them to
i2c-core. This wasn't my original intent, but after all, it makes some
sense. Would you be happy with the following?

* * * * *

From: Jean Delvare kh...@linux-fr.org
Subject: V4L/DVB: Use custom I2C probing function mechanism

Now that i2c-core offers the possibility to provide custom probing
function for I2C devices, let's make use of it.

Signed-off-by: Jean Delvare kh...@linux-fr.org
---
 drivers/i2c/i2c-core.c|7 +++
 drivers/media/video/cx23885/cx23885-i2c.c |   15 ---
 drivers/media/video/cx88/cx88-i2c.c   |   19 ---
 include/linux/i2c.h   |3 +++
 4 files changed, 18 insertions(+), 26 deletions(-)

--- linux-2.6.34-rc3.orig/drivers/media/video/cx23885/cx23885-i2c.c 
2010-04-06 11:31:20.0 +0200
+++ linux-2.6.34-rc3/drivers/media/video/cx23885/cx23885-i2c.c  2010-04-06 
12:28:09.0 +0200
@@ -365,17 +365,10 @@ int cx23885_i2c_register(struct cx23885_
 
memset(info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, ir_video, I2C_NAME_SIZE);
-   /*
-* We can't call i2c_new_probed_device() because it uses
-* quick writes for probing and the IR receiver device only
-* replies to reads.
-*/
-   if (i2c_smbus_xfer(bus-i2c_adap, addr_list[0], 0,
-  I2C_SMBUS_READ, 0, I2C_SMBUS_QUICK,
-  NULL) = 0) {
-   info.addr = addr_list[0];
-   i2c_new_device(bus-i2c_adap, info);
-   }
+   /* Use quick read command for probe, some IR chips don't
+* support writes */
+   i2c_new_probed_device(bus-i2c_adap, info, addr_list,
+ i2c_probe_func_quick_read);
}
 
return bus-i2c_rc;
--- linux-2.6.34-rc3.orig/drivers/media/video/cx88/cx88-i2c.c   2010-04-06 
11:31:20.0 +0200
+++ linux-2.6.34-rc3/drivers/media/video/cx88/cx88-i2c.c2010-04-06 
12:28:06.0 +0200
@@ -188,24 +188,13 @@ int cx88_i2c_init(struct cx88_core *core
0x18, 0x6b, 0x71,
I2C_CLIENT_END
};
-   const unsigned short *addrp;
 
memset(info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, ir_video, I2C_NAME_SIZE);
-   /*
-* We can't call i2c_new_probed_device() because it uses
-* quick writes for probing and at least some R receiver
-* devices only reply to reads.
-*/
-   for (addrp = addr_list; *addrp != I2C_CLIENT_END; addrp++) {
-   if (i2c_smbus_xfer(core-i2c_adap, *addrp, 0,
-  I2C_SMBUS_READ, 0,
-  I2C_SMBUS_QUICK, NULL) = 0) {
-   info.addr = *addrp;
-   i2c_new_device(core-i2c_adap, info);
-   break;
-   }
-   }
+   /* Use quick read command for probe, some IR chips don't
+* support writes */
+   i2c_new_probed_device(core-i2c_adap, info, addr_list,
+ 

Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-05 Thread Jean Delvare
Hi Andy,

On Sun, 04 Apr 2010 21:54:39 -0400, Andy Walls wrote:
 On Sun, 2010-04-04 at 16:14 +0200, Jean Delvare wrote:
  Now that i2c-core offers the possibility to provide custom probing
  function for I2C devices, let's make use of it.
  
  Signed-off-by: Jean Delvare kh...@linux-fr.org
  ---
  I wasn't too sure where to put the custom probe function: in each driver,
  in the ir-common module or in the v4l2-common module. I went for the
  second option as a middle ground, but am ready to discuss it if anyone
  objects.
 
 With respect to cx23885, could you comment on the interaction of this
 patch with some patches of yours that are not merged yet:
 
 http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/b39f8849a35b
 http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/3cf1ac545ca5
 http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/ef5d2c08106f
 
 Are they related to the IR microcontroller not being probed properly?

No, I don't expect any interaction between this new patch and the older
patchset. The older patchset would let the cx23885 I2C implementation
properly report slave nacks, but a successful IR device probing
wouldn't return a nack.

So, the patches can be merged in any order, nothing wrong will happen
either way.

 (I tried to get these patches merged, but didn't due to problems with
 other patches in my PULL request, and then a severe shortage of time.)

Thanks,
-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-05 Thread Mauro Carvalho Chehab
Hi Jean,

Jean Delvare wrote:
 Now that i2c-core offers the possibility to provide custom probing
 function for I2C devices, let's make use of it.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 ---
 I wasn't too sure where to put the custom probe function: in each driver,
 in the ir-common module or in the v4l2-common module. I went for the
 second option as a middle ground, but am ready to discuss it if anyone
 objects.

Please, don't add new things at ir-common module. It basically contains the
decoding functions for RC5 and pulse/distance, plus several IR keymaps. With
the IR rework I'm doing, this module will go away, after having all the current 
IR decoders implemented via ir-raw-input binding. 

The keymaps were already removed from it, on my experimental tree 
(http://git.linuxtv.org/mchehab/ir.git), and rc5 decoder is already written
(but still needs a few fixes). 

The new ir-core is creating an abstract way to deal with Remote Controllers,
meant to be used not only by IR's, but also for other types of RC, like, 
bluetooth and USB HID. It will also export a raw event interface, for use
with lirc. As this is the core of the RC subsystem, a i2c-specific binding
method also doesn't seem to belong there. SO, IMO, the better place is to add 
it as a static inline function at ir-kbd-i2c.h.


 
  drivers/media/IR/ir-functions.c   |   12 
  drivers/media/video/cx23885/cx23885-i2c.c |   14 +++---
  drivers/media/video/cx88/cx88-i2c.c   |   18 +++---
  include/media/ir-common.h |5 +
  4 files changed, 23 insertions(+), 26 deletions(-)
 
 --- linux-2.6.34-rc3.orig/drivers/media/video/cx23885/cx23885-i2c.c   
 2010-04-04 09:06:38.0 +0200
 +++ linux-2.6.34-rc3/drivers/media/video/cx23885/cx23885-i2c.c
 2010-04-04 13:34:34.0 +0200
 @@ -28,6 +28,7 @@
  #include cx23885.h
  
  #include media/v4l2-common.h
 +#include media/ir-common.h
  
  static unsigned int i2c_debug;
  module_param(i2c_debug, int, 0644);
 @@ -365,17 +366,8 @@ int cx23885_i2c_register(struct cx23885_
  
   memset(info, 0, sizeof(struct i2c_board_info));
   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 - /*
 -  * We can't call i2c_new_probed_device() because it uses
 -  * quick writes for probing and the IR receiver device only
 -  * replies to reads.
 -  */
 - if (i2c_smbus_xfer(bus-i2c_adap, addr_list[0], 0,
 -I2C_SMBUS_READ, 0, I2C_SMBUS_QUICK,
 -NULL) = 0) {
 - info.addr = addr_list[0];
 - i2c_new_device(bus-i2c_adap, info);
 - }
 + i2c_new_probed_device(bus-i2c_adap, info, addr_list,
 +   ir_i2c_probe);
   }
  
   return bus-i2c_rc;
 --- linux-2.6.34-rc3.orig/drivers/media/video/cx88/cx88-i2c.c 2010-04-04 
 09:06:38.0 +0200
 +++ linux-2.6.34-rc3/drivers/media/video/cx88/cx88-i2c.c  2010-04-04 
 13:34:34.0 +0200
 @@ -34,6 +34,7 @@
  
  #include cx88.h
  #include media/v4l2-common.h
 +#include media/ir-common.h
  
  static unsigned int i2c_debug;
  module_param(i2c_debug, int, 0644);
 @@ -188,24 +189,11 @@ int cx88_i2c_init(struct cx88_core *core
   0x18, 0x6b, 0x71,
   I2C_CLIENT_END
   };
 - const unsigned short *addrp;
  
   memset(info, 0, sizeof(struct i2c_board_info));
   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 - /*
 -  * We can't call i2c_new_probed_device() because it uses
 -  * quick writes for probing and at least some R receiver
 -  * devices only reply to reads.
 -  */
 - for (addrp = addr_list; *addrp != I2C_CLIENT_END; addrp++) {
 - if (i2c_smbus_xfer(core-i2c_adap, *addrp, 0,
 -I2C_SMBUS_READ, 0,
 -I2C_SMBUS_QUICK, NULL) = 0) {
 - info.addr = *addrp;
 - i2c_new_device(core-i2c_adap, info);
 - break;
 - }
 - }
 + i2c_new_probed_device(core-i2c_adap, info, addr_list,
 +   ir_i2c_probe);
   }
   return core-i2c_rc;
  }
 --- linux-2.6.34-rc3.orig/drivers/media/IR/ir-functions.c 2010-03-18 
 17:06:30.0 +0100
 +++ linux-2.6.34-rc3/drivers/media/IR/ir-functions.c  2010-04-04 
 14:30:29.0 +0200
 @@ -23,6 +23,7 @@
  #include linux/module.h
  #include linux/string.h
  #include linux/jiffies.h
 +#include linux/i2c.h
  #include media/ir-common.h
  
  /* 
 -- */
 @@ -353,3 +354,14 @@ void ir_rc5_timer_keyup(unsigned long da
   

Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-05 Thread Jean Delvare
Hi Mauro,

On Mon, 05 Apr 2010 15:26:32 -0300, Mauro Carvalho Chehab wrote:
 Jean Delvare wrote:
  Now that i2c-core offers the possibility to provide custom probing
  function for I2C devices, let's make use of it.
  
  Signed-off-by: Jean Delvare kh...@linux-fr.org
  ---
  I wasn't too sure where to put the custom probe function: in each driver,
  in the ir-common module or in the v4l2-common module. I went for the
  second option as a middle ground, but am ready to discuss it if anyone
  objects.
 
 Please, don't add new things at ir-common module. It basically contains the
 decoding functions for RC5 and pulse/distance, plus several IR keymaps. With
 the IR rework I'm doing, this module will go away, after having all the 
 current 
 IR decoders implemented via ir-raw-input binding. 
 
 The keymaps were already removed from it, on my experimental tree 
 (http://git.linuxtv.org/mchehab/ir.git), and rc5 decoder is already written
 (but still needs a few fixes). 
 
 The new ir-core is creating an abstract way to deal with Remote Controllers,
 meant to be used not only by IR's, but also for other types of RC, like, 
 bluetooth and USB HID. It will also export a raw event interface, for use
 with lirc. As this is the core of the RC subsystem, a i2c-specific binding
 method also doesn't seem to belong there. SO, IMO, the better place is to add 
 it as a static inline function at ir-kbd-i2c.h.

Ever tried to pass the address of an inline function as another
function's parameter? :)

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


Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-05 Thread Mauro Carvalho Chehab
Jean Delvare wrote:
 Hi Mauro,
 
 On Mon, 05 Apr 2010 15:26:32 -0300, Mauro Carvalho Chehab wrote:
 Jean Delvare wrote:
 Now that i2c-core offers the possibility to provide custom probing
 function for I2C devices, let's make use of it.

 Signed-off-by: Jean Delvare kh...@linux-fr.org
 ---
 I wasn't too sure where to put the custom probe function: in each driver,
 in the ir-common module or in the v4l2-common module. I went for the
 second option as a middle ground, but am ready to discuss it if anyone
 objects.
 Please, don't add new things at ir-common module. It basically contains the
 decoding functions for RC5 and pulse/distance, plus several IR keymaps. With
 the IR rework I'm doing, this module will go away, after having all the 
 current 
 IR decoders implemented via ir-raw-input binding. 

 The keymaps were already removed from it, on my experimental tree 
 (http://git.linuxtv.org/mchehab/ir.git), and rc5 decoder is already written
 (but still needs a few fixes). 

 The new ir-core is creating an abstract way to deal with Remote Controllers,
 meant to be used not only by IR's, but also for other types of RC, like, 
 bluetooth and USB HID. It will also export a raw event interface, for use
 with lirc. As this is the core of the RC subsystem, a i2c-specific binding
 method also doesn't seem to belong there. SO, IMO, the better place is to 
 add 
 it as a static inline function at ir-kbd-i2c.h.
 
 Ever tried to pass the address of an inline function as another
 function's parameter? :)

:) Never tried... maybe gcc would to the hard thing, de-inlining it ;)

Well, we need to put this code somewhere. Where are the other probing codes? 
Probably
the better is to put them together.


-- 

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


[PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-04 Thread Jean Delvare
Now that i2c-core offers the possibility to provide custom probing
function for I2C devices, let's make use of it.

Signed-off-by: Jean Delvare kh...@linux-fr.org
---
I wasn't too sure where to put the custom probe function: in each driver,
in the ir-common module or in the v4l2-common module. I went for the
second option as a middle ground, but am ready to discuss it if anyone
objects.

 drivers/media/IR/ir-functions.c   |   12 
 drivers/media/video/cx23885/cx23885-i2c.c |   14 +++---
 drivers/media/video/cx88/cx88-i2c.c   |   18 +++---
 include/media/ir-common.h |5 +
 4 files changed, 23 insertions(+), 26 deletions(-)

--- linux-2.6.34-rc3.orig/drivers/media/video/cx23885/cx23885-i2c.c 
2010-04-04 09:06:38.0 +0200
+++ linux-2.6.34-rc3/drivers/media/video/cx23885/cx23885-i2c.c  2010-04-04 
13:34:34.0 +0200
@@ -28,6 +28,7 @@
 #include cx23885.h
 
 #include media/v4l2-common.h
+#include media/ir-common.h
 
 static unsigned int i2c_debug;
 module_param(i2c_debug, int, 0644);
@@ -365,17 +366,8 @@ int cx23885_i2c_register(struct cx23885_
 
memset(info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, ir_video, I2C_NAME_SIZE);
-   /*
-* We can't call i2c_new_probed_device() because it uses
-* quick writes for probing and the IR receiver device only
-* replies to reads.
-*/
-   if (i2c_smbus_xfer(bus-i2c_adap, addr_list[0], 0,
-  I2C_SMBUS_READ, 0, I2C_SMBUS_QUICK,
-  NULL) = 0) {
-   info.addr = addr_list[0];
-   i2c_new_device(bus-i2c_adap, info);
-   }
+   i2c_new_probed_device(bus-i2c_adap, info, addr_list,
+ ir_i2c_probe);
}
 
return bus-i2c_rc;
--- linux-2.6.34-rc3.orig/drivers/media/video/cx88/cx88-i2c.c   2010-04-04 
09:06:38.0 +0200
+++ linux-2.6.34-rc3/drivers/media/video/cx88/cx88-i2c.c2010-04-04 
13:34:34.0 +0200
@@ -34,6 +34,7 @@
 
 #include cx88.h
 #include media/v4l2-common.h
+#include media/ir-common.h
 
 static unsigned int i2c_debug;
 module_param(i2c_debug, int, 0644);
@@ -188,24 +189,11 @@ int cx88_i2c_init(struct cx88_core *core
0x18, 0x6b, 0x71,
I2C_CLIENT_END
};
-   const unsigned short *addrp;
 
memset(info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, ir_video, I2C_NAME_SIZE);
-   /*
-* We can't call i2c_new_probed_device() because it uses
-* quick writes for probing and at least some R receiver
-* devices only reply to reads.
-*/
-   for (addrp = addr_list; *addrp != I2C_CLIENT_END; addrp++) {
-   if (i2c_smbus_xfer(core-i2c_adap, *addrp, 0,
-  I2C_SMBUS_READ, 0,
-  I2C_SMBUS_QUICK, NULL) = 0) {
-   info.addr = *addrp;
-   i2c_new_device(core-i2c_adap, info);
-   break;
-   }
-   }
+   i2c_new_probed_device(core-i2c_adap, info, addr_list,
+ ir_i2c_probe);
}
return core-i2c_rc;
 }
--- linux-2.6.34-rc3.orig/drivers/media/IR/ir-functions.c   2010-03-18 
17:06:30.0 +0100
+++ linux-2.6.34-rc3/drivers/media/IR/ir-functions.c2010-04-04 
14:30:29.0 +0200
@@ -23,6 +23,7 @@
 #include linux/module.h
 #include linux/string.h
 #include linux/jiffies.h
+#include linux/i2c.h
 #include media/ir-common.h
 
 /* -- 
*/
@@ -353,3 +354,14 @@ void ir_rc5_timer_keyup(unsigned long da
ir_input_nokey(ir-dev, ir-ir);
 }
 EXPORT_SYMBOL_GPL(ir_rc5_timer_keyup);
+
+/* Some functions only needed for I2C devices */
+#if defined CONFIG_I2C || defined CONFIG_I2C_MODULE
+/* use quick read command for probe, some IR chips don't support writes */
+int ir_i2c_probe(struct i2c_adapter *i2c, unsigned short addr)
+{
+   return i2c_smbus_xfer(i2c, addr, 0, I2C_SMBUS_READ, 0,
+ I2C_SMBUS_QUICK, NULL) = 0;
+}
+EXPORT_SYMBOL_GPL(ir_i2c_probe);
+#endif
--- linux-2.6.34-rc3.orig/include/media/ir-common.h 2010-03-18 
17:06:30.0 +0100
+++ linux-2.6.34-rc3/include/media/ir-common.h  2010-04-04 14:29:54.0 
+0200
@@ -97,6 +97,11 @@ u32  ir_rc5_decode(unsigned int code);
 void ir_rc5_timer_end(unsigned long data);
 void ir_rc5_timer_keyup(unsigned long data);
 
+#if defined CONFIG_I2C || defined CONFIG_I2C_MODULE
+struct i2c_adapter;
+int ir_i2c_probe(struct i2c_adapter *i2c, unsigned short addr);

Re: [PATCH 2/2] V4L/DVB: Use custom I2C probing function mechanism

2010-04-04 Thread Andy Walls
On Sun, 2010-04-04 at 16:14 +0200, Jean Delvare wrote:
 Now that i2c-core offers the possibility to provide custom probing
 function for I2C devices, let's make use of it.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 ---
 I wasn't too sure where to put the custom probe function: in each driver,
 in the ir-common module or in the v4l2-common module. I went for the
 second option as a middle ground, but am ready to discuss it if anyone
 objects.

Hi Jean,

With respect to cx23885, could you comment on the interaction of this
patch with some patches of yours that are not merged yet:

http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/b39f8849a35b
http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/3cf1ac545ca5
http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/ef5d2c08106f

Are they related to the IR microcontroller not being probed properly?

(I tried to get these patches merged, but didn't due to problems with
other patches in my PULL request, and then a severe shortage of time.)

Regards,
Andy

 
  drivers/media/IR/ir-functions.c   |   12 
  drivers/media/video/cx23885/cx23885-i2c.c |   14 +++---
  drivers/media/video/cx88/cx88-i2c.c   |   18 +++---
  include/media/ir-common.h |5 +
  4 files changed, 23 insertions(+), 26 deletions(-)
 
 --- linux-2.6.34-rc3.orig/drivers/media/video/cx23885/cx23885-i2c.c   
 2010-04-04 09:06:38.0 +0200
 +++ linux-2.6.34-rc3/drivers/media/video/cx23885/cx23885-i2c.c
 2010-04-04 13:34:34.0 +0200
 @@ -28,6 +28,7 @@
  #include cx23885.h
  
  #include media/v4l2-common.h
 +#include media/ir-common.h
  
  static unsigned int i2c_debug;
  module_param(i2c_debug, int, 0644);
 @@ -365,17 +366,8 @@ int cx23885_i2c_register(struct cx23885_
  
   memset(info, 0, sizeof(struct i2c_board_info));
   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 - /*
 -  * We can't call i2c_new_probed_device() because it uses
 -  * quick writes for probing and the IR receiver device only
 -  * replies to reads.
 -  */
 - if (i2c_smbus_xfer(bus-i2c_adap, addr_list[0], 0,
 -I2C_SMBUS_READ, 0, I2C_SMBUS_QUICK,
 -NULL) = 0) {
 - info.addr = addr_list[0];
 - i2c_new_device(bus-i2c_adap, info);
 - }
 + i2c_new_probed_device(bus-i2c_adap, info, addr_list,
 +   ir_i2c_probe);
   }
  
   return bus-i2c_rc;
 --- linux-2.6.34-rc3.orig/drivers/media/video/cx88/cx88-i2c.c 2010-04-04 
 09:06:38.0 +0200
 +++ linux-2.6.34-rc3/drivers/media/video/cx88/cx88-i2c.c  2010-04-04 
 13:34:34.0 +0200
 @@ -34,6 +34,7 @@
  
  #include cx88.h
  #include media/v4l2-common.h
 +#include media/ir-common.h
  
  static unsigned int i2c_debug;
  module_param(i2c_debug, int, 0644);
 @@ -188,24 +189,11 @@ int cx88_i2c_init(struct cx88_core *core
   0x18, 0x6b, 0x71,
   I2C_CLIENT_END
   };
 - const unsigned short *addrp;
  
   memset(info, 0, sizeof(struct i2c_board_info));
   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 - /*
 -  * We can't call i2c_new_probed_device() because it uses
 -  * quick writes for probing and at least some R receiver
 -  * devices only reply to reads.
 -  */
 - for (addrp = addr_list; *addrp != I2C_CLIENT_END; addrp++) {
 - if (i2c_smbus_xfer(core-i2c_adap, *addrp, 0,
 -I2C_SMBUS_READ, 0,
 -I2C_SMBUS_QUICK, NULL) = 0) {
 - info.addr = *addrp;
 - i2c_new_device(core-i2c_adap, info);
 - break;
 - }
 - }
 + i2c_new_probed_device(core-i2c_adap, info, addr_list,
 +   ir_i2c_probe);
   }
   return core-i2c_rc;
  }
 --- linux-2.6.34-rc3.orig/drivers/media/IR/ir-functions.c 2010-03-18 
 17:06:30.0 +0100
 +++ linux-2.6.34-rc3/drivers/media/IR/ir-functions.c  2010-04-04 
 14:30:29.0 +0200
 @@ -23,6 +23,7 @@
  #include linux/module.h
  #include linux/string.h
  #include linux/jiffies.h
 +#include linux/i2c.h
  #include media/ir-common.h
  
  /* 
 -- */
 @@ -353,3 +354,14 @@ void ir_rc5_timer_keyup(unsigned long da
   ir_input_nokey(ir-dev, ir-ir);
  }
  EXPORT_SYMBOL_GPL(ir_rc5_timer_keyup);
 +
 +/* Some functions only needed for I2C devices */
 +#if defined CONFIG_I2C || defined CONFIG_I2C_MODULE
 +/* use quick read command for probe, some IR chips don't support writes */
 +int ir_i2c_probe(struct i2c_adapter *i2c, unsigned short addr)
 +{
 +