Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-05-11 Thread Mauro Carvalho Chehab 
Em Sat, 2 May 2009 09:12:11 +0200
Jean Delvare kh...@linux-fr.org escreveu:

 Mauro, please pull Mike's pvrusb2-dev work as soon as he asks you to do
 so. Then I'll rebase my own patch set and send it again.

Jean and Mike,

Any news about this subject?



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

Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-05-01 Thread Mike Isely 

Jean:

I have another idea that I think you'll like.  I'm putting the finishing 
touches on the patch right now.

What I have implements correct ir_video loading for the pvrusb2 driver.  
It also includes a lookup table (though with only 1 entry right now) to 
determine the proper I2C address and I use i2c_new_device() now instead 
of i2c_new_probed_device().  I've also renamed things to be ir_video 
everywhere instead of ir_kbd_i2c as was discussed.

In particular the disable option is there like before, but now it's 
called disable_autoload_ir_video.

So far this is exactly what I was saying before.  But I'm also making 
one more change: the disable_autoload_ir_video option default value will 
- for now - be 1, i.e. everything above I just described starts off 
disabled.  This means that the entire patch can be pulled _right_ _now_ 
without breaking anything at all, because the outward behavior is still 
unchanged.

Then, when you're ready to bring your stuff in, all you need to do is 
include a 1-line change to pvrusb2-i2c-core.c to switch the default 
value of disable_autoload_ir_video back to 0, which now enables all the 
corresponding pvrusb2 changes that you need to avoid any breakage inside 
my driver.  Just to be absolutely crystal clear, here's the change you 
will be able to do once these changes are in:

diff -r 8d2e1361520c linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
--- a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c  Fri May 01 
20:23:39 2009 -0500
+++ b/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c  Fri May 01 
20:24:23 2009 -0500
@@ -43,7 +43,7 @@
 module_param_array(ir_mode, int, NULL, 0444);
 MODULE_PARM_DESC(ir_mode,specify: 0=disable IR reception, 1=normal IR);
 
+static int pvr2_disable_ir_video;
-static int pvr2_disable_ir_video = 1;
 module_param_named(disable_autoload_ir_video, pvr2_disable_ir_video,
   int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(disable_autoload_ir_video,

So with all this, what I am setting up right now will cause no harm to 
the existing situation, requires no actual messing with module options, 
and once you're reading, just include the 1-line change above and you're 
set.  There's no race here, no gap in IR handling.

  -Mike


On Thu, 23 Apr 2009, Mike Isely wrote:

 
 Hi Jean,
 
 I had actually written out a longer, detailed, point-by-point reply 
 earlier today, but before I could finish it I got interrupted with a 
 crisis.  And then another.  And that's kind of how my day went.  Now I'm 
 finally back to this, but I have another e-mail debacle to immediately 
 deal with (thank you pobox.com, not!) so I'm tossing that unfinished 
 lengthy reply.  I think I can sum this whole thing up very simply.  
 Here's what I think should be happening:
 
 1a. Your existing IR changeset, minus the pvrusb2 changes, should be 
 merged.
 
 1b. In parallel with (1a) I modify my pvrusb2 changeset to use the right 
 module name and I adjust the driver option name to match.
 
 2. My pvrusb2 changeset, with changes from (1b) is merged.
 
 3. Andy's proposed change for ir_kbd_i2c to support additional IR 
 devices is investigated and probably merged.
 
 4. I test the changed ir_kbd_i2c against additional pvrusb2 devices 
 known not to work previously with ir_kbd_i2c.  If they work, then I will 
 create a pvrusb2 patch to load ir_video in those cases as well as the 
 cases already set up (which still won't cover all possible 
 pvrusb2-driven devices).
 
 I think this satisfies the remaining issues, except that from between 
 steps 1 and 2 ir_kbd_i2c won't work with the pvrusb2 driver.  But you 
 know, I'm OK with that.  I expect (2) to happen within a few days after 
 (1) so the impact should be minimal.  It certainly won't escape into the 
 kernel tree that way.  In addition, my impression from the community of 
 pvrusb2 users is that the preferred IR strategy is lirc anyway, and it's 
 a foregone conclusion that they are all going to be, uh, impacted once 
 your compatibility parts are gone from i2c.  From where I'm sitting the 
 gap from (1) to (2) is trivial compared to that impending mess - 
 realize I'm not complaining since after all (a) it really falls to the 
 lirc developers to fix that, (b) you do need to get your changes in, and 
 (c) there's little I can do for lirc there except to keep it working as 
 long as possible with the pvrusb2 driver.  I'm just pointing out that 
 I'm OK with the step 1 - 2 gap for the pvrusb2 driver because it's 
 small and will be nothing compared to what's about to happen with lirc.
 
 If you still don't like any of that, well, then I give up.  In that 
 case, then merge your changes with the pvrusb2 changes included.  I 
 won't ack them, but I'll just deal with it later.  Because while your 
 changes might keep ir_kbd_i2c going, they will also immediately break 
 the more-useful and apparently more-used lirc (by always binding 
 ir_kbd_i2c even in cases in the pvrusb2 driver where it's known

Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-23 Thread Jean Delvare 
Hi Mike,

Sorry for the late answer.

On Sat, 18 Apr 2009 08:53:35 -0500 (CDT), Mike Isely wrote:
 On Sat, 18 Apr 2009, Jean Delvare wrote:
  On Sat, 18 Apr 2009 11:25:19 +0200, Jean Delvare wrote:
   Hmm, I thought that our latest discussions had (at least partly)
   obsoleted your patches. Remember that we want to always instantiate
   ir_video I2C devices even when ir-kbd-i2c can't driver them, otherwise
   lirc won't be able to bind to the devices in question as soon as the
   legacy binding model is gone. So the conditionals in your second patch
   (which is all that makes it differ from mine) are no longer desirable.
 
 Jean:
 
 The differences between my patch(es) and yours are:
 
 1. My patch only attempts to bind the driver if the hardware actually 
 supports it.

That's not clear to me. I seem to understand that your patch
instantiates the ir_video device if and only if the ir-kbd-i2c driver
supports it. This is not what we want to do. We want to create the
ir_video device if an IR receiver exists, even if ir-kbd-i2c doesn't
support it. The reason is that ir-kbd-i2c could be extended to support
the IR receiver in question in the future, and that alternative drivers
(lirc_i2c comes to mind) could also be used.

I also don't understand why you use i2c_new_probed_device() rather than
i2c_new_device() if you already know for sure that the IR receiver
device is present?

 2. My patch selects the right I2C address for the case(s) where it makes 
 sense to bind.

This is a good thing, although your implementation isn't exactly what I
would expect. The address list should depend on the value of
hdw-ir_scheme_active. At the moment you support only 2 schemes and
they happen to use the same I2C address, but I presume this will change
in the future.

 3. My patch provides a module option to completely disable binding.

I agree this can be useful, as discussed earlier, although I still
object to the name you chose (disable_autoload_ir_kbd). This module
option is only remotely related to the i2c binding model change.

 You are probably thinking about (3) but you forgot that I had also taken 
 care of (1).  Difference (1) is why I don't want your patch.  If your 
 patch gets merged I will have to partially redo my patch to make (1) 
 work again.

This is correct. But if your second patch is merged before my own
changes, then IR support will be broken for all pvrusb2 users, unless
they temporarily load the driver with disable_autoload_ir_kbd=1. But
they will have to remove the parameter as soon as my own patches are
merged. This approach doesn't strike me as the best user experience.

If my patches are merged first and yours go second (which I admit means
you'll have to adjust your patches a bit) then everything will keep
working for the user. This is the reason why I was proposing this order.

 When I had issued my pull request to Mauro (which he didn't pull), there 
 were actually 2 patches.  The first one dealt with (1) and the second 
 dealt with (2) and (3), while taking advantage of (1).  Had Mauro pulled 
 those patches at that time then you could have made further changes on 
 top without losing (1).  But since he didn't, it's best just to leave 
 the pvrusb2 driver alone and I'll make the needed additional change(s) 
 there after your stuff is merged.

I can't leave the pvrusb2 driver alone, unless you consider it
acceptable that your users lose IR support between my patches and
yours. When ir-kbd-i2c is converted to the new-style i2c binding, all
bridge drivers must be updated too.

   I'll work on lirc patches today or tomorrow, so that lirc doesn't break
   when my patches hit mainline.
  
  Speaking of this: do you know all the I2C addresses that can host IR
  devices on pvrusb2 cards? I understand that the only address supported
  by ir-kbd-i2c is 0x18, but I also need to know the addresses supported
  by lirc_i2c and possibly lirc_zilog, if you happen to know this.
 
 Right now I only care about 0x18 (for 29xxx and early 24xxx devices).

I've seen this, but this isn't my question. If lirc_i2c supports other
IR devices that are present on pvrusb2 devices, we must declare them as
well so that we don't break lirc_i2c. So, once again: do you know all
the I2C addresses where pvrusb2 cards can have IR devices?

 I noticed the thread where Andy got IR reception to work with ir-kbd-i2c 
 using 0x71 (lirc_zilog type) and I suspect that same set of ir-kbd-i2c 
 changes will probably work with the pvrusb2 driver for MCE 24xxx and 
 HVR-1900/HVR-1950 devices.  But I figured once Andy's stuff gets into 
 ir-kbd-i2c I'd simply test for this and if it worked I would make the 
 appropriate adjustments in the pvrusb2 driver to enable ir-kbd-i2c 
 binding in those cases as well (an easy change).  However even with that 
 change, there are other pvrusb2-driven devices that cannot support 
 ir-kbd-i2c.

I'm worried that this means doing several changes first and only then
converting ir-kbd-i2c to the new i2c binding

Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-23 Thread Mike Isely 

Hi Jean,

I had actually written out a longer, detailed, point-by-point reply 
earlier today, but before I could finish it I got interrupted with a 
crisis.  And then another.  And that's kind of how my day went.  Now I'm 
finally back to this, but I have another e-mail debacle to immediately 
deal with (thank you pobox.com, not!) so I'm tossing that unfinished 
lengthy reply.  I think I can sum this whole thing up very simply.  
Here's what I think should be happening:

1a. Your existing IR changeset, minus the pvrusb2 changes, should be 
merged.

1b. In parallel with (1a) I modify my pvrusb2 changeset to use the right 
module name and I adjust the driver option name to match.

2. My pvrusb2 changeset, with changes from (1b) is merged.

3. Andy's proposed change for ir_kbd_i2c to support additional IR 
devices is investigated and probably merged.

4. I test the changed ir_kbd_i2c against additional pvrusb2 devices 
known not to work previously with ir_kbd_i2c.  If they work, then I will 
create a pvrusb2 patch to load ir_video in those cases as well as the 
cases already set up (which still won't cover all possible 
pvrusb2-driven devices).

I think this satisfies the remaining issues, except that from between 
steps 1 and 2 ir_kbd_i2c won't work with the pvrusb2 driver.  But you 
know, I'm OK with that.  I expect (2) to happen within a few days after 
(1) so the impact should be minimal.  It certainly won't escape into the 
kernel tree that way.  In addition, my impression from the community of 
pvrusb2 users is that the preferred IR strategy is lirc anyway, and it's 
a foregone conclusion that they are all going to be, uh, impacted once 
your compatibility parts are gone from i2c.  From where I'm sitting the 
gap from (1) to (2) is trivial compared to that impending mess - 
realize I'm not complaining since after all (a) it really falls to the 
lirc developers to fix that, (b) you do need to get your changes in, and 
(c) there's little I can do for lirc there except to keep it working as 
long as possible with the pvrusb2 driver.  I'm just pointing out that 
I'm OK with the step 1 - 2 gap for the pvrusb2 driver because it's 
small and will be nothing compared to what's about to happen with lirc.

If you still don't like any of that, well, then I give up.  In that 
case, then merge your changes with the pvrusb2 changes included.  I 
won't ack them, but I'll just deal with it later.  Because while your 
changes might keep ir_kbd_i2c going, they will also immediately break 
the more-useful and apparently more-used lirc (by always binding 
ir_kbd_i2c even in cases in the pvrusb2 driver where it's known that it 
can't even work but lirc would have) which as far as I'm concerned is 
far worse than the step 1 - 2 gap above.  But it's just another gap and 
I'll push another pvrusb2 changeset on top to clean it up.  So just do 
whatever you think is best right now, if you disagree with the sequence 
above.

Now I will go off and deal with the steamer that pobox.com has just 
handed me :-(

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
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/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-18 Thread Jean Delvare 
Hi again Mike,

On Sat, 18 Apr 2009 11:25:19 +0200, Jean Delvare wrote:
 On Fri, 17 Apr 2009 18:35:55 -0500 (CDT), Mike Isely wrote:
  I thought we were going to leave the pvrusb2 driver out of this since 
  I've already got a change ready that also includes additional logic to 
  take into account the properties of the hardware device (i.e. only 
  activate ir-kbd-i2c when we know it has a chance of working).
 
 Hmm, I thought that our latest discussions had (at least partly)
 obsoleted your patches. Remember that we want to always instantiate
 ir_video I2C devices even when ir-kbd-i2c can't driver them, otherwise
 lirc won't be able to bind to the devices in question as soon as the
 legacy binding model is gone. So the conditionals in your second patch
 (which is all that makes it differ from mine) are no longer desirable.
 
 I'll work on lirc patches today or tomorrow, so that lirc doesn't break
 when my patches hit mainline.

Speaking of this: do you know all the I2C addresses that can host IR
devices on pvrusb2 cards? I understand that the only address supported
by ir-kbd-i2c is 0x18, but I also need to know the addresses supported
by lirc_i2c and possibly lirc_zilog, if you happen to know this.

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

[PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-17 Thread Jean Delvare 
Let card drivers probe for IR receiver devices and instantiate them if
found. Ultimately it would be better if we could stop probing
completely, but I suspect this won't be possible for all card types.

There's certainly room for cleanups. For example, some drivers are
sharing I2C adapter IDs, so they also had to share the list of I2C
addresses being probed for an IR receiver. Now that each driver
explicitly says which addresses should be probed, maybe some addresses
can be dropped from some drivers.

Also, the special cases in saa7134-i2c should probably be handled on a
per-board basis. This would be more efficient and less risky than always
probing extra addresses on all boards. I'll give it a try later.

Signed-off-by: Jean Delvare kh...@linux-fr.org
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Hans Verkuil hverk...@xs4all.nl
Cc: Andy Walls awa...@radix.net
Cc: Mike Isely is...@pobox.com
---
 linux/drivers/media/video/bt8xx/bttv-i2c.c   |   21 +
 linux/drivers/media/video/cx231xx/cx231xx-cards.c|   11 
 linux/drivers/media/video/cx231xx/cx231xx-i2c.c  |3 
 linux/drivers/media/video/cx231xx/cx231xx.h  |1 
 linux/drivers/media/video/cx23885/cx23885-i2c.c  |   12 +
 linux/drivers/media/video/cx88/cx88-i2c.c|   13 +
 linux/drivers/media/video/em28xx/em28xx-cards.c  |   20 +
 linux/drivers/media/video/em28xx/em28xx-i2c.c|3 
 linux/drivers/media/video/em28xx/em28xx-input.c  |6 
 linux/drivers/media/video/em28xx/em28xx.h|1 
 linux/drivers/media/video/ir-kbd-i2c.c   |  200 +++---
 linux/drivers/media/video/ivtv/ivtv-i2c.c|   31 ++
 linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c |   24 ++
 linux/drivers/media/video/saa7134/saa7134-i2c.c  |3 
 linux/drivers/media/video/saa7134/saa7134-input.c|   86 ++-
 linux/drivers/media/video/saa7134/saa7134.h  |1 
 linux/include/media/ir-kbd-i2c.h |2 
 17 files changed, 244 insertions(+), 194 deletions(-)

--- v4l-dvb.orig/linux/drivers/media/video/bt8xx/bttv-i2c.c 2009-04-17 
14:36:51.0 +0200
+++ v4l-dvb/linux/drivers/media/video/bt8xx/bttv-i2c.c  2009-04-17 
14:37:54.0 +0200
@@ -405,6 +405,27 @@ int __devinit init_bttv_i2c(struct bttv
}
if (0 == btv-i2c_rc  i2c_scan)
do_i2c_scan(btv-c.v4l2_dev.name, btv-i2c_client);
+
+   /* Instantiate the IR receiver device, if present */
+   if (0 == btv-i2c_rc) {
+   struct i2c_board_info info;
+   /* The external IR receiver is at i2c address 0x34 (0x35 for
+  reads).  Future Hauppauge cards will have an internal
+  receiver at 0x30 (0x31 for reads).  In theory, both can be
+  fitted, and Hauppauge suggest an external overrides an
+  internal.
+
+  That's why we probe 0x1a (~0x34) first. CB
+   */
+   const unsigned short addr_list[] = {
+   0x1a, 0x18, 0x4b, 0x64, 0x30,
+   I2C_CLIENT_END
+   };
+
+   memset(info, 0, sizeof(struct i2c_board_info));
+   strlcpy(info.type, ir_video, I2C_NAME_SIZE);
+   i2c_new_probed_device(btv-c.i2c_adap, info, addr_list);
+   }
return btv-i2c_rc;
 }
 
--- v4l-dvb.orig/linux/drivers/media/video/cx231xx/cx231xx-cards.c  
2009-04-17 14:36:51.0 +0200
+++ v4l-dvb/linux/drivers/media/video/cx231xx/cx231xx-cards.c   2009-04-17 
14:37:54.0 +0200
@@ -282,13 +282,16 @@ static void cx231xx_config_tuner(struct
 }
 
 /* --- */
-void cx231xx_set_ir(struct cx231xx *dev, struct IR_i2c *ir)
+void cx231xx_register_i2c_ir(struct cx231xx *dev)
 {
-   if (disable_ir) {
-   ir-get_key = NULL;
+   if (disable_ir)
return;
-   }
 
+   /* REVISIT: instantiate IR device */
+}
+
+void cx231xx_set_ir(struct cx231xx *dev, struct IR_i2c *ir)
+{
/* detect  configure */
switch (dev-model) {
 
--- v4l-dvb.orig/linux/drivers/media/video/cx231xx/cx231xx-i2c.c
2009-04-17 14:36:51.0 +0200
+++ v4l-dvb/linux/drivers/media/video/cx231xx/cx231xx-i2c.c 2009-04-17 
14:37:54.0 +0200
@@ -540,6 +540,9 @@ int cx231xx_i2c_register(struct cx231xx_
if (0 == bus-i2c_rc) {
if (i2c_scan)
cx231xx_do_i2c_scan(dev, bus-i2c_client);
+
+   /* Instantiate the IR receiver device, if present */
+   cx231xx_register_i2c_ir(dev);
} else
cx231xx_warn(%s: i2c bus %d register FAILED\n,
 dev-name, bus-nr);
--- v4l-dvb.orig/linux/drivers/media/video/cx231xx/cx231xx.h2009-04-17 
14:36:51.0 +0200
+++ v4l-dvb/linux/drivers/media/video/cx231xx/cx231xx.h 2009-04-17 
14:37:54.0 +0200
@@

Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-17 Thread Mike Isely 

I thought we were going to leave the pvrusb2 driver out of this since 
I've already got a change ready that also includes additional logic to 
take into account the properties of the hardware device (i.e. only 
activate ir-kbd-i2c when we know it has a chance of working).

  -Mike


On Fri, 17 Apr 2009, Jean Delvare wrote:

 Let card drivers probe for IR receiver devices and instantiate them if
 found. Ultimately it would be better if we could stop probing
 completely, but I suspect this won't be possible for all card types.
 
 There's certainly room for cleanups. For example, some drivers are
 sharing I2C adapter IDs, so they also had to share the list of I2C
 addresses being probed for an IR receiver. Now that each driver
 explicitly says which addresses should be probed, maybe some addresses
 can be dropped from some drivers.
 
 Also, the special cases in saa7134-i2c should probably be handled on a
 per-board basis. This would be more efficient and less risky than always
 probing extra addresses on all boards. I'll give it a try later.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: Hans Verkuil hverk...@xs4all.nl
 Cc: Andy Walls awa...@radix.net
 Cc: Mike Isely is...@pobox.com
 ---
  linux/drivers/media/video/bt8xx/bttv-i2c.c   |   21 +
  linux/drivers/media/video/cx231xx/cx231xx-cards.c|   11 
  linux/drivers/media/video/cx231xx/cx231xx-i2c.c  |3 
  linux/drivers/media/video/cx231xx/cx231xx.h  |1 
  linux/drivers/media/video/cx23885/cx23885-i2c.c  |   12 +
  linux/drivers/media/video/cx88/cx88-i2c.c|   13 +
  linux/drivers/media/video/em28xx/em28xx-cards.c  |   20 +
  linux/drivers/media/video/em28xx/em28xx-i2c.c|3 
  linux/drivers/media/video/em28xx/em28xx-input.c  |6 
  linux/drivers/media/video/em28xx/em28xx.h|1 
  linux/drivers/media/video/ir-kbd-i2c.c   |  200 
 +++---
  linux/drivers/media/video/ivtv/ivtv-i2c.c|   31 ++
  linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c |   24 ++
  linux/drivers/media/video/saa7134/saa7134-i2c.c  |3 
  linux/drivers/media/video/saa7134/saa7134-input.c|   86 ++-
  linux/drivers/media/video/saa7134/saa7134.h  |1 
  linux/include/media/ir-kbd-i2c.h |2 
  17 files changed, 244 insertions(+), 194 deletions(-)

   [...]

-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
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