Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 6 Apr 2009, Jean Delvare wrote:

> 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.

I plan a reply to your RFC as well, probably later on tonight.


> 
> 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. 

But from the viewpoint of a user of the pvrusb2 driver, that is in fact 
what will appear to happen.  1. User plugs in device.  2. Driver 
(pvrusb2) instantiates self.  3. Driver automatically attempts to load 
and bind ir-kbd-i2c to the IR receiver, hence "autoload".  Yes I know 
ir-kbd-i2c no longer autoloads itself; that is a major goal of the 
conversion.  But with the pvrusb2 change to explicitly bind it, the 
behavior from the view of the user is still that of an autoload 
operation.  I think we're just arguing semantics, but it's why I put 
"autoload" in the name.

However given the realization below about multiple devices, I think I 
need to step back and look at this from a larger scope (e.g. make it an 
array, merge with the fairly clunky ir_mode switch already in the driver 
and clean that up, etc).

   [...]

> > 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.

I believe I can solve this for the pvrusb2 driver without entanglement 
with the conversion work underway.  Pieces of the solution are already 
in the driver; I just need to clean this up.  In any case, I'm not going 
to worry about it immediately.  I've been neglecting some non-Linux 
tasks, like filing bills and finishing my tax return :-)

  -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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux