Hi Mike, Please note: I think the long post I just sent makes part of this discussion obsolete from a technical perspective. But still interesting from a functional perspective, which is why I am following up. On Mon, 6 Apr 2009 10:03:00 -0500 (CDT), Mike Isely wrote: > On Mon, 6 Apr 2009, Jean Delvare wrote: > > 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. > > The module parameter is named "disabled_autoload_ir_kbd", how is that > wrong? The name is based on the internal receiver name "ir-kbd". > There's no "[-_]i2c" in the name. The parameter's description also does > not reference ir-kbd-i2c by name either. I did it that way specifically > for the reason you point out here. I was perfectly fine with the "ir_kbd" part. The part I complain about is "autoload", because the original mechanism doesn't involve any autoloading. > > (...) > > 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. > > Well yes. I was saying "no harm" in the sense that everything that was > possible before is still possible now, though perhaps with the module > option set. We agree then. > (...) > This morning I actually realized another use-case that has been missed. > It was likely an issue before anyway, but it got me thinking about this: > If a user has multiple devices attached to one system, he probably won't > want all of the corresponding IR receivers enabled - that would just > trigger redundant input events. With a PCI-hosted ivtv device this is > not really an issue - because there one can just decline to plug in the > actual IR sensor. But with USB-hosted devices, the IR sensor is an > integral part of the device and can't be unplugged. OK, well such a > user could instead just choose to put a piece of tape over the window or > stick all but one device in a box (and causing potential heat problems > if it isn't ventilated), but that approach is well, inelegant. I think > this argues for a better solution in the bridge driver to selectively > disable IR on a per-device instance basis. There's already some logic > in the pvrusb2 driver to do this, but it's clumsy and wasn't intended to > solve that particular use-case. I need to consider this further and do > some cleanup. This use-case may actually suggest the disable option > should be expanded and possibly made permanent. I agree. I presume that this is one of the reasons why some bridge drivers have a disable_ir parameter (the other reason being lirc). It would probably make sense to not only keep these module parameters even after lirc is merged into the kernel tree, but turn them into arrays, so that IR receivers can be disabled selectively. This would address the problem you just raised. But all this can be done after the conversion work it finished. -- 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
