Hi Mike, I'll answer all your questions and express my concerns in this reply, to avoid spreading the info all around the discussion thread. On Mon, 6 Apr 2009 00:19:23 -0500 (CDT), Mike Isely wrote: > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for two pvrusb2 > changesets. One sets up the ability to track what kind of IR receiver > is expected, Looks very good. The more we know about each board, the less probing we have to do, the better. > the other optionally autoloads ir-kbd-i2c based on that > result and a module option that can be used to disable that autoloading. Again, ir-kbd-i2c does _not_ auto-load. What my code (and now yours) does is instantiating an i2c device named "ir-kbd". _If_ the ir-kbd-i2c driver is later loaded, it will bind to that device. We might let udev load the ir-kbd-i2c driver at some point in the future, but clearly we need to sort out the lirc case beforehand, otherwise some users will hit the problems you have anticipated, and we don't want this to happen. So, your module parameter is improperly named. Setting it to 1 does prevent the pvrusb2 driver from instantiating the "ir-kbd" device, and that's all. Speaking of this module parameter, I was a little worried at first that you wanted an inverted logic for it in pvrusb2 compared to what some other bridge drivers (saa7134, em28xx, cx231xx), but thinking a bit more about I came to the conclusion that it was OK because it matched the history of the pvrusb2 driver. Now I see that you followed their logic (enabled is the default) but you use a different module parameter name (disable_autoload_ir_kbd vs. disable_ir). I think there would be some value in sticking to a common name in all bridge drivers (like we have the standard video_nr module parameter.) That being said, I will not insist on this. My feeling is that this is all temporary anyway, because the removal of the legacy i2c model will break lirc and the main point is to decide how we will fix it. I'll post a separate summary with proposals. Depending on what we do, the module parameter you added is likely to become obsolete. > This is the result of what I had posted about an hour ago. It looks > like a lot of lines, but it was all basically trivial stuff. > > Note that these changes are safe; nothing is regressed here and this > does not depend on anyone else's changes. Even though that second > changeset won't really do much until Jean's ir-kbd-i2c stuff is merged, > the fact is that it won't cause any harm either. Since the pvrusb2 > driver wasn't previously autoloading ir-kbd-i2c anyway, this change > therefore breaks nothing. For completeness, your second patch actually breaks the ir-kbd-i2c use case. Your code will instantiate a new-style "ir-kbd" device which the legacy ir-kbd-i2c can't use. As the instantiated device makes the address busy, the probing logic of legacy ir-kbd-i2c driver will skip it. Without my changes, all users will have to pass disable_autoload_ir_kbd=1, whether they want to use lirc or ir-kbd-i2c, otherwise they lose IR support. But honestly that doesn't matter much, I think, because the idea is to merge my changes quickly. > But it does have the desirable effect in that > it should take me out of the IR debate for now and I can stop being a > pain to Jean :-) I am totally fine removing the pvrusb2 parts from my patch set, and letting you deal with it. As you know, my main concern here is to get rid of the legacy i2c model. So I am virtually happy with any solution that leads there, and I am gladly accepting all the help I can get to go there. > Jean: I hope I didn't break any etiquette here. Though that second > change is "from" me, the majority of the changeset really is from your > patch to the pvrusb2 driver. I made changes to it so I didn't feel > right in still trying to blame you for it ;-) Plus, if I were to hand > it back to you for inclusion in your patch series then you would be > dependant upon the pvrusb2 change I made to drive the decision about > loading based on the device hardware. Since this change as a whole is > mergeable without the rest of your changeset I felt it most expedient > just to push this up now. That's alright. In this specific case I really don't care who gets the fame and flames, as longs as things get done quickly. Thank you for jumping in and helping me sorting it all out, this is very appreciated. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
