| Message ID | 1302267045.1749.38.camel@gagarin (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > This patch restores remote control input for cx2388x based boards on > Linux kernels >= 2.6.38. > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > control input of my Hauppauge Nova-S plus was no longer functioning. > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > some helpful pointers as to the likely cause. > > Turns out that there are 2 problems: > > 1. In the IR interrupt handler of cx88-input.c there's a 32-bit multiply > overflow which causes IR pulse durations to be incorrectly calculated. > > 2. The RC5 decoder appends the system code to the scancode and passes > the combination to rc_keydown(). Unfortunately, the combined value is > then forwarded to input_event() which then fails to recognise a valid > scancode and hence no input events are generated. > > I note that in commit 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d, which > introduced these changes, David Härdeman changed the IR sample frequency > to a supposed 4kHz. However, the registers dealing with IR input are > undocumented in the cx2388x datasheets and there's no publicly available > information on them. I have to ask the question why this change was > made as it is of no apparent benefit and could have unanticipated > consequences. IMHO that change should also be reverted unless there is > evidence to substantiate it. > > Signed off by: Lawrence Rust <lvr at softsystem dot co dot uk> Nacked-by: Jarod Wilson <jarod@redhat.com> > diff --git a/drivers/media/rc/ir-rc5-decoder.c b/drivers/media/rc/ir-rc5-decoder.c > index ebdba55..c4052da 100644 > --- a/drivers/media/rc/ir-rc5-decoder.c > +++ b/drivers/media/rc/ir-rc5-decoder.c > @@ -144,10 +144,15 @@ again: > system = (data->bits & 0x007C0) >> 6; > toggle = (data->bits & 0x00800) ? 1 : 0; > command += (data->bits & 0x01000) ? 0 : 0x40; > - scancode = system << 8 | command; > - > - IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n", > - scancode, toggle); > + /* Notes > + * 1. Should filter unknown systems e.g Hauppauge use 0x1e or 0x1f > + * 2. Don't include system in the scancode otherwise input_event() > + * doesn't recognise the scancode > + */ > + scancode = command; > + > + IR_dprintk(1, "RC5 scancode 0x%02x (system: 0x%02x toggle: %u)\n", > + scancode, system, toggle); > } > > rc_keydown(dev, scancode, toggle); This part is so very very wrong. We should NOT filter here. Filtering can be achieved on the keymap side, and you *do* include the system here. The fix for your issue is an update to the relevant keymap so that its matching the system byte as well. The divide fix looks sane though.
On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > This patch restores remote control input for cx2388x based boards on > Linux kernels >= 2.6.38. > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > control input of my Hauppauge Nova-S plus was no longer functioning. > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > some helpful pointers as to the likely cause. > > Turns out that there are 2 problems: ... > 2. The RC5 decoder appends the system code to the scancode and passes > the combination to rc_keydown(). Unfortunately, the combined value is > then forwarded to input_event() which then fails to recognise a valid > scancode and hence no input events are generated. Just to clarify on this one, you're missing a step. We get the scancode, and its passed to rc_keydown. rc_keydown then looks for a match in the loaded keytable, then passes the *keycode* that matches the scancode along to input_event. If you fix the keytable to contain system and command, everything should work just fine again. Throwing away data is a no-no though -- take a look at recent changes to ir-kdb-i2c, which actually just recently made it start *including* system. :)
On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: > On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > > > This patch restores remote control input for cx2388x based boards on > > Linux kernels >= 2.6.38. > > > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > > control input of my Hauppauge Nova-S plus was no longer functioning. > > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > > some helpful pointers as to the likely cause. > > > > Turns out that there are 2 problems: > ... > > 2. The RC5 decoder appends the system code to the scancode and passes > > the combination to rc_keydown(). Unfortunately, the combined value is > > then forwarded to input_event() which then fails to recognise a valid > > scancode and hence no input events are generated. > > Just to clarify on this one, you're missing a step. We get the scancode, > and its passed to rc_keydown. rc_keydown then looks for a match in the > loaded keytable, then passes the *keycode* that matches the scancode > along to input_event. If you fix the keytable to contain system and > command, everything should work just fine again. Throwing away data is > a no-no though -- take a look at recent changes to ir-kdb-i2c, which > actually just recently made it start *including* system. :) Don't shoot the messenger. I'm just reporting what I had to do to fix a clumsy hack by someone 6 months ago who didn't test their changes. This patch _restores_ the operation of a subsystem broken by those changes Perhaps those responsible for commit 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: Signed-off-by: David Härdeman <david@hardeman.nu> Acked-by: Jarod Wilson <jarod@redhat.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> should fix the keytable. In the meantime (next year) I'll be using this patch.
On Apr 8, 2011, at 11:22 AM, Lawrence Rust wrote: > On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: >> On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: >> >>> This patch restores remote control input for cx2388x based boards on >>> Linux kernels >= 2.6.38. >>> >>> After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote >>> control input of my Hauppauge Nova-S plus was no longer functioning. >>> I posted a question on this newsgroup and Mauro Carvalho Chehab gave >>> some helpful pointers as to the likely cause. >>> >>> Turns out that there are 2 problems: >> ... >>> 2. The RC5 decoder appends the system code to the scancode and passes >>> the combination to rc_keydown(). Unfortunately, the combined value is >>> then forwarded to input_event() which then fails to recognise a valid >>> scancode and hence no input events are generated. >> >> Just to clarify on this one, you're missing a step. We get the scancode, >> and its passed to rc_keydown. rc_keydown then looks for a match in the >> loaded keytable, then passes the *keycode* that matches the scancode >> along to input_event. If you fix the keytable to contain system and >> command, everything should work just fine again. Throwing away data is >> a no-no though -- take a look at recent changes to ir-kdb-i2c, which >> actually just recently made it start *including* system. :) > > Don't shoot the messenger. Wasn't my intention. I was simply trying to explain why your "fix" isn't correct. > I'm just reporting what I had to do to fix a clumsy hack by someone 6 > months ago who didn't test their changes. This patch _restores_ the > operation of a subsystem broken by those changes > > Perhaps those responsible for commit > 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: > > Signed-off-by: David Härdeman <david@hardeman.nu> > Acked-by: Jarod Wilson <jarod@redhat.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > should fix the keytable. In the meantime (next year) I'll be using this > patch. The entire commit message: [media] ir-core: convert drivers/media/video/cx88 to ir-core This patch converts the cx88 driver (for sampling hw) to use the decoders provided by ir-core instead of the separate ones provided by ir-functions (and gets rid of those). The value for MO_DDS_IO had a comment saying it corresponded to a 4kHz samplerate. That comment was unfortunately misleading. The actual samplerate was something like 3250Hz. The current value has been derived by analyzing the elapsed time between interrupts for different values (knowing that each interrupt corresponds to 32 samples). Thanks to Mariusz Bialonczyk <manio@skyboo.net> for testing my patches (about one a day for two weeks!) on actual hardware. Please note the part about how it *was* tested. And this certainly was not a "clumsy hack", I'd actually call it a fairly skilled bit of code de-duplication by David. Anyway... The problem is that there isn't a "the keytable". There are many many keytables. And a lot of different hardware. Testing all possible combinations of hardware (both receiver side and remote side) is next to impossible. We do what we can. Its unfortunate that your hardware regressed in functionality. It happens, but it *can* be fixed. The fix you provided just wasn't correct. The correct fix is trivially updating drivers/media/rc/keymaps/<insert-your-keymap-here>.
On Fri, 2011-04-08 at 12:21 -0400, Jarod Wilson wrote: > On Apr 8, 2011, at 11:22 AM, Lawrence Rust wrote: > > > On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: > >> On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > >> > >>> This patch restores remote control input for cx2388x based boards on > >>> Linux kernels >= 2.6.38. > >>> > >>> After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > >>> control input of my Hauppauge Nova-S plus was no longer functioning. > >>> I posted a question on this newsgroup and Mauro Carvalho Chehab gave > >>> some helpful pointers as to the likely cause. > >>> > >>> Turns out that there are 2 problems: > >> ... > >>> 2. The RC5 decoder appends the system code to the scancode and passes > >>> the combination to rc_keydown(). Unfortunately, the combined value is > >>> then forwarded to input_event() which then fails to recognise a valid > >>> scancode and hence no input events are generated. > >> > >> Just to clarify on this one, you're missing a step. We get the scancode, > >> and its passed to rc_keydown. rc_keydown then looks for a match in the > >> loaded keytable, then passes the *keycode* that matches the scancode > >> along to input_event. If you fix the keytable to contain system and > >> command, everything should work just fine again. Throwing away data is > >> a no-no though -- take a look at recent changes to ir-kdb-i2c, which > >> actually just recently made it start *including* system. :) > > > > Don't shoot the messenger. > > Wasn't my intention. I was simply trying to explain why your "fix" > isn't correct. > > > > I'm just reporting what I had to do to fix a clumsy hack by someone 6 > > months ago who didn't test their changes. This patch _restores_ the > > operation of a subsystem broken by those changes > > > > Perhaps those responsible for commit > > 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: > > > > Signed-off-by: David Härdeman <david@hardeman.nu> > > Acked-by: Jarod Wilson <jarod@redhat.com> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > > > should fix the keytable. In the meantime (next year) I'll be using this > > patch. > > The entire commit message: > > [media] ir-core: convert drivers/media/video/cx88 to ir-core > > This patch converts the cx88 driver (for sampling hw) to use the > decoders provided by ir-core instead of the separate ones provided > by ir-functions (and gets rid of those). > > The value for MO_DDS_IO had a comment saying it corresponded to > a 4kHz samplerate. That comment was unfortunately misleading. The > actual samplerate was something like 3250Hz. > > The current value has been derived by analyzing the elapsed time > between interrupts for different values (knowing that each interrupt > corresponds to 32 samples). > > Thanks to Mariusz Bialonczyk <manio@skyboo.net> for testing my patches > (about one a day for two weeks!) on actual hardware. > > Please note the part about how it *was* tested. And this certainly > was not a "clumsy hack", I'd actually call it a fairly skilled bit > of code de-duplication by David. Anyway... > > The problem is that there isn't a "the keytable". There are many > many keytables. And a lot of different hardware. Testing all possible > combinations of hardware (both receiver side and remote side) is > next to impossible. Hauppauge have the lion's share of the DVB card market so this mod should have been tested with a Haupauge RC, but clearly wasn't. It also clearly wasn't tested with a cx2388x card because the interrupt handler has an obvious overflow. > We do what we can. Its unfortunate that your > hardware regressed in functionality. It happens, but it *can* be > fixed. The fix you provided just wasn't correct. It is correct for 2.6.38, which is what was described. I haven't checked all the keymaps but a sample of 5 show that none include a system ID. > The correct fix is > trivially updating drivers/media/rc/keymaps/<insert-your-keymap-here>. So, cherrypick the Hauppauge keytable from 2.6.39 and apply it to 2.6.38 together with the 2nd part of this patch. It would be real nice if that was in 2.6.38.3
On Fri, Apr 8, 2011 at 12:21 PM, Jarod Wilson <jarod@wilsonet.com> wrote: > The problem is that there isn't a "the keytable". There are many > many keytables. And a lot of different hardware. Testing all possible > combinations of hardware (both receiver side and remote side) is > next to impossible. We do what we can. Its unfortunate that your > hardware regressed in functionality. It happens, but it *can* be > fixed. The fix you provided just wasn't correct. The correct fix is > trivially updating drivers/media/rc/keymaps/<insert-your-keymap-here>. I think the fundamental failure here was avoidable. We introduced a new requirement that keytables included system codes, knowing full well that the vast majority of them did not meet the requirement. In fact, a quick scan through the first 20 or so keymaps show that even today only *HALF* of them are populated today. That means that half of the remote keymaps are also completely broken. This decision was doomed to fail. It basically said, "Yes, I know full well that I'm breaking most of the keymaps currently supported, but maybe some of those users will eventually report the issue and I'll make them provide an updated keymap which will eventually be merged upstream for others so that their remotes are no longer broken." We should have introduced a RC profile property indicating how many bits were "significant". Then for those remotes which didn't have the system code, we could have continued to match against only the key code. Then over time, as keymaps improved, those keymaps could be updated and the number of significant bits could be adjusted to indicate that the system code was present. This was a crappy call, and it was completely foreseeable. Devin
On Apr 8, 2011, at 1:07 PM, Devin Heitmueller wrote: > On Fri, Apr 8, 2011 at 12:21 PM, Jarod Wilson <jarod@wilsonet.com> wrote: >> The problem is that there isn't a "the keytable". There are many >> many keytables. And a lot of different hardware. Testing all possible >> combinations of hardware (both receiver side and remote side) is >> next to impossible. We do what we can. Its unfortunate that your >> hardware regressed in functionality. It happens, but it *can* be >> fixed. The fix you provided just wasn't correct. The correct fix is >> trivially updating drivers/media/rc/keymaps/<insert-your-keymap-here>. > > I think the fundamental failure here was avoidable. We introduced a > new requirement that keytables included system codes, knowing full > well that the vast majority of them did not meet the requirement. In > fact, a quick scan through the first 20 or so keymaps show that even > today only *HALF* of them are populated today. That means that half > of the remote keymaps are also completely broken. Have to admit that I don't think it ever registered in my head that we were going to break that many existing keymaps. But something to consider: how many of those are *raw* rc-5 scancode keymaps, vs. cooked scancodes from drivers that only provide command? It may well be that we should have been more discriminating when building those keymaps, to distinguish which were truly raw IR scancodes that the in-kernel decoders ascertained, and which were just scancodes handed to us directly from the IR hardware. > This decision was doomed to fail. It basically said, "Yes, I know > full well that I'm breaking most of the keymaps currently supported, > but maybe some of those users will eventually report the issue and > I'll make them provide an updated keymap which will eventually be > merged upstream for others so that their remotes are no longer > broken." Well, ir-keytable -w is also an option, though that does kill the "Just Works"-ness when you first have to come up with the new map. > We should have introduced a RC profile property indicating how many > bits were "significant". Then for those remotes which didn't have the > system code, we could have continued to match against only the key > code. This was a change in the raw rc-5 IR decoder. There's *always* going to be a system code (or at least, a resulting byte), isn't there? Otherwise, its simply not an rc-5 signal. The "no system code" case should really only apply to hardware decoders that strip it off internally. Speaking of which, something just occurred to me. Functionality of Hauppauge receivers and remotes *was* tested. With ir-kbd-i2c. Which was stripping off the system code at the time. It just wasn't tested with Hauppauge hardware that actually passed along raw IR. In 2.6.39, ir-kbd-i2c has been fixed so that it passes along system code and the Hauppauge keymaps were updated accordingly, which also happens to fix the cx88 raw IR case. > Then over time, as keymaps improved, those keymaps could be > updated and the number of significant bits could be adjusted to > indicate that the system code was present. > > This was a crappy call, and it was completely foreseeable. Possibly. I think too many of us are only hacking on this in their limited free time though, so things like this may get overlooked. I have quite a few pieces of Hauppauge hardware, several with IR receivers and remotes, but all of which use ir-kbd-i2c (or lirc_zilog), i.e., none of which pass along raw IR.
On Apr 8, 2011, at 12:50 PM, Lawrence Rust wrote: > On Fri, 2011-04-08 at 12:21 -0400, Jarod Wilson wrote: >> On Apr 8, 2011, at 11:22 AM, Lawrence Rust wrote: >> >>> On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: >>>> On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: >>>> >>>>> This patch restores remote control input for cx2388x based boards on >>>>> Linux kernels >= 2.6.38. >>>>> >>>>> After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote >>>>> control input of my Hauppauge Nova-S plus was no longer functioning. >>>>> I posted a question on this newsgroup and Mauro Carvalho Chehab gave >>>>> some helpful pointers as to the likely cause. >>>>> >>>>> Turns out that there are 2 problems: >>>> ... >>>>> 2. The RC5 decoder appends the system code to the scancode and passes >>>>> the combination to rc_keydown(). Unfortunately, the combined value is >>>>> then forwarded to input_event() which then fails to recognise a valid >>>>> scancode and hence no input events are generated. >>>> >>>> Just to clarify on this one, you're missing a step. We get the scancode, >>>> and its passed to rc_keydown. rc_keydown then looks for a match in the >>>> loaded keytable, then passes the *keycode* that matches the scancode >>>> along to input_event. If you fix the keytable to contain system and >>>> command, everything should work just fine again. Throwing away data is >>>> a no-no though -- take a look at recent changes to ir-kdb-i2c, which >>>> actually just recently made it start *including* system. :) >>> >>> Don't shoot the messenger. >> >> Wasn't my intention. I was simply trying to explain why your "fix" >> isn't correct. >> >> >>> I'm just reporting what I had to do to fix a clumsy hack by someone 6 >>> months ago who didn't test their changes. This patch _restores_ the >>> operation of a subsystem broken by those changes >>> >>> Perhaps those responsible for commit >>> 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: >>> >>> Signed-off-by: David Härdeman <david@hardeman.nu> >>> Acked-by: Jarod Wilson <jarod@redhat.com> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> >>> >>> should fix the keytable. In the meantime (next year) I'll be using this >>> patch. >> >> The entire commit message: >> >> [media] ir-core: convert drivers/media/video/cx88 to ir-core >> >> This patch converts the cx88 driver (for sampling hw) to use the >> decoders provided by ir-core instead of the separate ones provided >> by ir-functions (and gets rid of those). >> >> The value for MO_DDS_IO had a comment saying it corresponded to >> a 4kHz samplerate. That comment was unfortunately misleading. The >> actual samplerate was something like 3250Hz. >> >> The current value has been derived by analyzing the elapsed time >> between interrupts for different values (knowing that each interrupt >> corresponds to 32 samples). >> >> Thanks to Mariusz Bialonczyk <manio@skyboo.net> for testing my patches >> (about one a day for two weeks!) on actual hardware. >> >> Please note the part about how it *was* tested. And this certainly >> was not a "clumsy hack", I'd actually call it a fairly skilled bit >> of code de-duplication by David. Anyway... >> >> The problem is that there isn't a "the keytable". There are many >> many keytables. And a lot of different hardware. Testing all possible >> combinations of hardware (both receiver side and remote side) is >> next to impossible. > > Hauppauge have the lion's share of the DVB card market so this mod > should have been tested with a Haupauge RC, but clearly wasn't. Its actually not that clear. It clearly wasn't tested with *your* receiver hardware, but a Hauppauge RC *was* tested using other receiver hardware, most notably, ir-kbd-i2c-driven IR hardware in the PVR-250, HVR-1950 and HD-PVR. Umpteen different combinations of bridge drivers, demod drivers, IR chips, etc., make it rather hard to test every possible combination, even when looking solely at products from a single company. :\ > It also > clearly wasn't tested with a cx2388x card because the interrupt handler > has an obvious overflow. I'll give you that one. But look how many different devices are listed in cx88_ir_init(), and you can see how it might be hard to be able to test every single combination. >> We do what we can. Its unfortunate that your >> hardware regressed in functionality. It happens, but it *can* be >> fixed. The fix you provided just wasn't correct. > > It is correct for 2.6.38, which is what was described. I haven't > checked all the keymaps but a sample of 5 show that none include a > system ID. Likely because they're primarily used by non-raw IR drivers and/or ir-kbd-i2c, which until 2.6.39, was stripping out system codes (which made it impossible to use a universal remote programmed for rc-5 signals that aren't using Hauppauge's system codes of choice, despite it being entirely possible for the hardware to handle it). >> The correct fix is >> trivially updating drivers/media/rc/keymaps/<insert-your-keymap-here>. > > So, cherrypick the Hauppauge keytable from 2.6.39 and apply it to 2.6.38 > together with the 2nd part of this patch. It would be real nice if that > was in 2.6.38.3 See my reply to Devin's mail, re: ir-kbd-i2c. We're sort of in a crappy situation here. Changing the keymaps to include system code in 2.6.38 means breaking every other Hauppauge card that uses ir-kbd-i2c, unless that *also* gets updated in 2.6.38. I'm fairly certain that Hauppauge hardware with ir-kbd-i2c-driven IR is actually more common in the field than that which actually passes raw IR. However, I think by simply adding a temporarily redundant keymap that gets used by raw IR hardware, or by increasing the size of the single Hauppauge keymap by way of multiple mappings for each key (both with and without system codes), we can satisfy both cases in 2.6.38.
On Fri, Apr 8, 2011 at 2:00 PM, Jarod Wilson <jarod@wilsonet.com> wrote: > Have to admit that I don't think it ever registered in my head that > we were going to break that many existing keymaps. But something > to consider: how many of those are *raw* rc-5 scancode keymaps, vs. > cooked scancodes from drivers that only provide command? It may > well be that we should have been more discriminating when building > those keymaps, to distinguish which were truly raw IR scancodes that > the in-kernel decoders ascertained, and which were just scancodes > handed to us directly from the IR hardware. As far as I know, keymaps didn't even support more than eight bit codes until recently. Even if the hardware had supported system codes, there was nothing to compare it against since the keymaps were limited to eight entries. >> This decision was doomed to fail. It basically said, "Yes, I know >> full well that I'm breaking most of the keymaps currently supported, >> but maybe some of those users will eventually report the issue and >> I'll make them provide an updated keymap which will eventually be >> merged upstream for others so that their remotes are no longer >> broken." > > Well, ir-keytable -w is also an option, though that does kill the > "Just Works"-ness when you first have to come up with the new map. Agreed. >> We should have introduced a RC profile property indicating how many >> bits were "significant". Then for those remotes which didn't have the >> system code, we could have continued to match against only the key >> code. > > This was a change in the raw rc-5 IR decoder. There's *always* going > to be a system code (or at least, a resulting byte), isn't there? > Otherwise, its simply not an rc-5 signal. The "no system code" case > should really only apply to hardware decoders that strip it off > internally. I realize that the actual remote controls do have system codes, but our remote control keymaps are missing the info. Wouldn't it have been better if for those cases we only compared against the key code, thereby not resulting in those keymaps being broken? IMHO, it would be better to have had the relatively low risk of the receiver responding to keypresses from an incorrect remote because the keymap wasn't explicit enough than have those remote controls stop working entirely. > Speaking of which, something just occurred to me. Functionality of > Hauppauge receivers and remotes *was* tested. With ir-kbd-i2c. Which > was stripping off the system code at the time. It just wasn't tested > with Hauppauge hardware that actually passed along raw IR. In 2.6.39, > ir-kbd-i2c has been fixed so that it passes along system code and > the Hauppauge keymaps were updated accordingly, which also happens > to fix the cx88 raw IR case. I don't doubt that the Hauppauge remote worked against ir-kbd-i2c because of the deficiency you described in the ir-kbd-i2c driver. I question the notion of introducing the requirement that all keymap definitions must have system codes without having really thought through the notion that it would result in breaking every existing keymap which hadn't been updated. >> Then over time, as keymaps improved, those keymaps could be >> updated and the number of significant bits could be adjusted to >> indicate that the system code was present. >> >> This was a crappy call, and it was completely foreseeable. > > Possibly. I think too many of us are only hacking on this in their > limited free time though, so things like this may get overlooked. > I have quite a few pieces of Hauppauge hardware, several with IR > receivers and remotes, but all of which use ir-kbd-i2c (or > lirc_zilog), i.e., none of which pass along raw IR. You don't have an HVR-950 or some other stick which announces RC5 codes? If not, let me know and I will send you something. It's kind of silly for someone doing that sort of work to not have at least one product in each category of receiver. Devin
On Apr 8, 2011, at 2:38 PM, Devin Heitmueller wrote: ... > I question the notion of introducing the requirement that all keymap > definitions must have system codes without having really thought > through the notion that it would result in breaking every existing > keymap which hadn't been updated. Speaks the the "too many of us are only hacking on this in their limited free time" point I raised. Hack, hack, hack, test with the hardware available on hand (which is actually quite a bit in my case, I think I have upwards of 35 receivers and even more remotes now), see that it works, move on to the next issue. I'm certainly guilty of not looking at the bigger picture and thinking about possible ramifications more than once. :) ... >> I have quite a few pieces of Hauppauge hardware, several with IR >> receivers and remotes, but all of which use ir-kbd-i2c (or >> lirc_zilog), i.e., none of which pass along raw IR. > > You don't have an HVR-950 or some other stick which announces RC5 > codes? If not, let me know and I will send you something. It's kind > of silly for someone doing that sort of work to not have at least one > product in each category of receiver. I don't think I even fully realized before today that there was Hauppauge hardware shipping with the grey remotes and a raw IR receiver. All the Hauppauge stuff I've got is either i2c IR (PVR-250, PVR-350, HVR-1950, HD-PVR) or came with a bundled mceusb transceiver (HVR-1500Q, HVR-1800, HVR-950Q -- model 72241, iirc), and its all working these days (modulo some quirks with the HD-PVR that still need sorting, but they're not regressions, its actually better now than it used to be). So yeah, I guess I have a gap in my IR hardware collection here, and would be happy to have something to fill it.
Jarod Wilson <jarod@wilsonet.com> wrote: >On Apr 8, 2011, at 2:38 PM, Devin Heitmueller wrote: >... >> I question the notion of introducing the requirement that all keymap >> definitions must have system codes without having really thought >> through the notion that it would result in breaking every existing >> keymap which hadn't been updated. > >Speaks the the "too many of us are only hacking on this in their >limited free time" point I raised. Hack, hack, hack, test with the >hardware available on hand (which is actually quite a bit in my case, >I think I have upwards of 35 receivers and even more remotes now), >see that it works, move on to the next issue. I'm certainly guilty of >not looking at the bigger picture and thinking about possible >ramifications more than once. :) > >... >>> I have quite a few pieces of Hauppauge hardware, several with IR >>> receivers and remotes, but all of which use ir-kbd-i2c (or >>> lirc_zilog), i.e., none of which pass along raw IR. >> >> You don't have an HVR-950 or some other stick which announces RC5 >> codes? If not, let me know and I will send you something. It's kind >> of silly for someone doing that sort of work to not have at least one >> product in each category of receiver. > >I don't think I even fully realized before today that there was >Hauppauge hardware shipping with the grey remotes and a raw IR >receiver. All the Hauppauge stuff I've got is either i2c IR >(PVR-250, PVR-350, HVR-1950, HD-PVR) or came with a bundled mceusb >transceiver (HVR-1500Q, HVR-1800, HVR-950Q -- model 72241, iirc), >and its all working these days (modulo some quirks with the HD-PVR >that still need sorting, but they're not regressions, its actually >better now than it used to be). > >So yeah, I guess I have a gap in my IR hardware collection here, >and would be happy to have something to fill it. > >-- >Jarod Wilson >jarod@wilsonet.com > > > >-- >To unsubscribe from this list: send the line "unsubscribe linux-media" >in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html Jarod, The HVR-1850 uses a raw IR receiver in the CX23888 and older HVR-1250s use the raw IR receiver in the CX23885. They both work for Rx (I need to tweak the Cx23885 rx watermark though), but I never found time to finish Tx (lack of kernel interface when I had time). If you obtain one of these I can answer any driver questions. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Apr 8, 2011, at 4:50 PM, Andy Walls wrote: > Jarod Wilson <jarod@wilsonet.com> wrote: ... >>>> I have quite a few pieces of Hauppauge hardware, several with IR >>>> receivers and remotes, but all of which use ir-kbd-i2c (or >>>> lirc_zilog), i.e., none of which pass along raw IR. >>> >>> You don't have an HVR-950 or some other stick which announces RC5 >>> codes? If not, let me know and I will send you something. It's kind >>> of silly for someone doing that sort of work to not have at least one >>> product in each category of receiver. >> >> I don't think I even fully realized before today that there was >> Hauppauge hardware shipping with the grey remotes and a raw IR >> receiver. All the Hauppauge stuff I've got is either i2c IR >> (PVR-250, PVR-350, HVR-1950, HD-PVR) or came with a bundled mceusb >> transceiver (HVR-1500Q, HVR-1800, HVR-950Q -- model 72241, iirc), >> and its all working these days (modulo some quirks with the HD-PVR >> that still need sorting, but they're not regressions, its actually >> better now than it used to be). >> >> So yeah, I guess I have a gap in my IR hardware collection here, >> and would be happy to have something to fill it. >> > > Jarod, > > The HVR-1850 uses a raw IR receiver in the CX23888 and older HVR-1250s use the raw IR receiver in the CX23885. They both work for Rx (I need to tweak the Cx23885 rx watermark though), but I never found time to finish Tx (lack of kernel interface when I had time). > > If you obtain one of these I can answer any driver questions. Quite some time back, I bought an HVR-1800 and an HVR-1250. I know one of them came with an mceusb transceiver and remote, as was pretty sure it was the 1800. For some reason, I didn't recall the 1250 coming with anything at all, but looking at dmesg output for it: cx23885 driver version 0.0.2 loaded cx23885 0000:03:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 CORE cx23885[0]: subsystem: 0070:7911, board: Hauppauge WinTV-HVR1250 [card=3,autodetected] tveeprom 0-0050: Hauppauge model 79001, rev E3D9, serial# 4904656 tveeprom 0-0050: MAC address is 00:0d:fe:4a:d6:d0 tveeprom 0-0050: tuner model is Microtune MT2131 (idx 139, type 4) tveeprom 0-0050: TV standards NTSC(M) ATSC/DVB Digital (eeprom 0x88) tveeprom 0-0050: audio processor is CX23885 (idx 39) tveeprom 0-0050: decoder processor is CX23885 (idx 33) tveeprom 0-0050: has no radio, has IR receiver, has no IR transmitter So it seems I do have hardware. However, its one of the two tuner cards in my "production" mythtv backend right now, making it a bit hard to do any experimenting with. If I can get it out of there, it looks like I just add an enable_885_ir=1, and I should be able to poke at it...
On Sat, 2011-04-09 at 21:39 -0400, Jarod Wilson wrote: > > Jarod, > > > > The HVR-1850 uses a raw IR receiver in the CX23888 and older > HVR-1250s use the raw IR receiver in the CX23885. They both work for > Rx (I need to tweak the Cx23885 rx watermark though), but I never > found time to finish Tx (lack of kernel interface when I had time). > > > > If you obtain one of these I can answer any driver questions. > > Quite some time back, I bought an HVR-1800 and an HVR-1250. I know one of > them came with an mceusb transceiver and remote, as was pretty sure it was > the 1800. For some reason, I didn't recall the 1250 coming with anything at > all, but looking at dmesg output for it: > > cx23885 driver version 0.0.2 loaded > cx23885 0000:03:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 > CORE cx23885[0]: subsystem: 0070:7911, board: Hauppauge WinTV-HVR1250 [card=3,autodetected] > tveeprom 0-0050: Hauppauge model 79001, rev E3D9, serial# 4904656 > tveeprom 0-0050: MAC address is 00:0d:fe:4a:d6:d0 > tveeprom 0-0050: tuner model is Microtune MT2131 (idx 139, type 4) > tveeprom 0-0050: TV standards NTSC(M) ATSC/DVB Digital (eeprom 0x88) > tveeprom 0-0050: audio processor is CX23885 (idx 39) > tveeprom 0-0050: decoder processor is CX23885 (idx 33) > tveeprom 0-0050: has no radio, has IR receiver, has no IR transmitter > > So it seems I do have hardware. However, its one of the two tuner cards in > my "production" mythtv backend right now, making it a bit hard to do any > experimenting with. If I can get it out of there, it looks like I just add > an enable_885_ir=1, and I should be able to poke at it... Yeah. Igor's TeVii S470 CX23885 based card had interrupt storms when enabled, so IR for '885 chips is disabled by default. To investigate, I tried to by an HVR-1250 with a CX23885, but instead got an HVR-1275 with a CX23888. dandel, on IRC, did a pretty decent job in testing HVR-1250 operation and finding it works, despite climbing kernel build/development learning curve at the time. You may also want to set ir_debug=2 for the *cx25840* module, if you want to see the raw pulse/space measurements in the logs. The cx25840 module handles the CX23885 IR controller. When testing, you may want to add pci=nomsi to your kernel comandline. Unless Igor has submitted his fix to reset some CX23885 hardware on module unload or reload, CX23885 interrupts won't work right after unloading and reloading the cx23885 module with MSI enabled. :( In the cx25840 module you may also want to change the call in drivers/media/video/cx25840/cx25840-ir.c: control_rx_irq_watermark(c, RX_FIFO_HALF_FULL); to control_rx_irq_watermark(c, RX_FIFO_NOT_EMPTY); That's a design bug on my part. The CX23885 IR controller is I2C connected. Waiting unitl the RX FIFO is half full risks losing some pulse measurements. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Apr 10, 2011, at 7:08 PM, Andy Walls wrote: > On Sat, 2011-04-09 at 21:39 -0400, Jarod Wilson wrote: > >>> Jarod, >>> >>> The HVR-1850 uses a raw IR receiver in the CX23888 and older >> HVR-1250s use the raw IR receiver in the CX23885. They both work for >> Rx (I need to tweak the Cx23885 rx watermark though), but I never >> found time to finish Tx (lack of kernel interface when I had time). >>> >>> If you obtain one of these I can answer any driver questions. >> >> Quite some time back, I bought an HVR-1800 and an HVR-1250. I know one of >> them came with an mceusb transceiver and remote, as was pretty sure it was >> the 1800. For some reason, I didn't recall the 1250 coming with anything at >> all, but looking at dmesg output for it: >> >> cx23885 driver version 0.0.2 loaded >> cx23885 0000:03:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 >> CORE cx23885[0]: subsystem: 0070:7911, board: Hauppauge WinTV-HVR1250 [card=3,autodetected] >> tveeprom 0-0050: Hauppauge model 79001, rev E3D9, serial# 4904656 >> tveeprom 0-0050: MAC address is 00:0d:fe:4a:d6:d0 >> tveeprom 0-0050: tuner model is Microtune MT2131 (idx 139, type 4) >> tveeprom 0-0050: TV standards NTSC(M) ATSC/DVB Digital (eeprom 0x88) >> tveeprom 0-0050: audio processor is CX23885 (idx 39) >> tveeprom 0-0050: decoder processor is CX23885 (idx 33) >> tveeprom 0-0050: has no radio, has IR receiver, has no IR transmitter >> >> So it seems I do have hardware. However, its one of the two tuner cards in >> my "production" mythtv backend right now, making it a bit hard to do any >> experimenting with. If I can get it out of there, it looks like I just add >> an enable_885_ir=1, and I should be able to poke at it... > > Yeah. Igor's TeVii S470 CX23885 based card had interrupt storms when > enabled, so IR for '885 chips is disabled by default. To investigate, I > tried to by an HVR-1250 with a CX23885, but instead got an HVR-1275 with > a CX23888. dandel, on IRC, did a pretty decent job in testing HVR-1250 > operation and finding it works, despite climbing kernel > build/development learning curve at the time. ... Finally got some time to play with my 1250, dug out its rx cable, turns out to be the same one I special-ordered to work on the 1150 Devin sent me. Oops. Anyway. First impressions, not so good. Got a panic on boot, somewhere in cx23885_video_irq, iirc, when booting with enable_885_ir=1 set. However, dmesg was somewhere in the middle of cx18 init of the HVR-1600 in the box. Dunno if there's any way that's actually directly related, but I yanked the 1600. After doing that, the box managed to boot fine, but then while testing w/ir-keytable and an RC-6 remote, I got what I think was the same panic -- definitely the cx23885_video_irq bit in the trace, something about sleeping while atomic, IP at mwait_idle. (On the plus side, IR did seem to be working before that). Note also that this is a 2.6.32-based kernel with a 2.6.38-era backport of the driver code, because that's what was on this box. Was about to put it back into "production" use, but hey, its summer, there's nothing really for it to record for another few months, so I'll keep it out and throw it into another box with a newer kernel and serial console, etc., so I can further debug...
Jarod Wilson <jarod@wilsonet.com> wrote: >On Apr 10, 2011, at 7:08 PM, Andy Walls wrote: > >> On Sat, 2011-04-09 at 21:39 -0400, Jarod Wilson wrote: >> >>>> Jarod, >>>> >>>> The HVR-1850 uses a raw IR receiver in the CX23888 and older >>> HVR-1250s use the raw IR receiver in the CX23885. They both work >for >>> Rx (I need to tweak the Cx23885 rx watermark though), but I never >>> found time to finish Tx (lack of kernel interface when I had time). >>>> >>>> If you obtain one of these I can answer any driver questions. >>> >>> Quite some time back, I bought an HVR-1800 and an HVR-1250. I know >one of >>> them came with an mceusb transceiver and remote, as was pretty sure >it was >>> the 1800. For some reason, I didn't recall the 1250 coming with >anything at >>> all, but looking at dmesg output for it: >>> >>> cx23885 driver version 0.0.2 loaded >>> cx23885 0000:03:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 >>> CORE cx23885[0]: subsystem: 0070:7911, board: Hauppauge >WinTV-HVR1250 [card=3,autodetected] >>> tveeprom 0-0050: Hauppauge model 79001, rev E3D9, serial# 4904656 >>> tveeprom 0-0050: MAC address is 00:0d:fe:4a:d6:d0 >>> tveeprom 0-0050: tuner model is Microtune MT2131 (idx 139, type 4) >>> tveeprom 0-0050: TV standards NTSC(M) ATSC/DVB Digital (eeprom 0x88) >>> tveeprom 0-0050: audio processor is CX23885 (idx 39) >>> tveeprom 0-0050: decoder processor is CX23885 (idx 33) >>> tveeprom 0-0050: has no radio, has IR receiver, has no IR >transmitter >>> >>> So it seems I do have hardware. However, its one of the two tuner >cards in >>> my "production" mythtv backend right now, making it a bit hard to do >any >>> experimenting with. If I can get it out of there, it looks like I >just add >>> an enable_885_ir=1, and I should be able to poke at it... >> >> Yeah. Igor's TeVii S470 CX23885 based card had interrupt storms when >> enabled, so IR for '885 chips is disabled by default. To >investigate, I >> tried to by an HVR-1250 with a CX23885, but instead got an HVR-1275 >with >> a CX23888. dandel, on IRC, did a pretty decent job in testing >HVR-1250 >> operation and finding it works, despite climbing kernel >> build/development learning curve at the time. >... > >Finally got some time to play with my 1250, dug out its rx cable, turns >out to >be the same one I special-ordered to work on the 1150 Devin sent me. >Oops. >Anyway. First impressions, not so good. Got a panic on boot, somewhere >in >cx23885_video_irq, iirc, when booting with enable_885_ir=1 set. >However, dmesg >was somewhere in the middle of cx18 init of the HVR-1600 in the box. >Dunno if >there's any way that's actually directly related, but I yanked the >1600. After >doing that, the box managed to boot fine, but then while testing >w/ir-keytable >and an RC-6 remote, I got what I think was the same panic -- definitely >the >cx23885_video_irq bit in the trace, something about sleeping while >atomic, IP >at mwait_idle. (On the plus side, IR did seem to be working before >that). > >Note also that this is a 2.6.32-based kernel with a 2.6.38-era backport >of the >driver code, because that's what was on this box. Was about to put it >back into >"production" use, but hey, its summer, there's nothing really for it to >record >for another few months, so I'll keep it out and throw it into another >box with >a newer kernel and serial console, etc., so I can further debug... > > >-- >Jarod Wilson >jarod@wilsonet.com In a very early version of the CX23885 IR changes I made the mistake of performing I2C transactions in an interrupt handler. That has been fixed in cx23885 for quite some time though. It also required some I2C fixes in cx25840-core.c IIRC, which again, has been fixed for some time. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 28, 2011, at 6:30 AM, Andy Walls wrote: > Jarod Wilson <jarod@wilsonet.com> wrote: > >> On Apr 10, 2011, at 7:08 PM, Andy Walls wrote: >> >>> On Sat, 2011-04-09 at 21:39 -0400, Jarod Wilson wrote: >>> >>>>> Jarod, >>>>> >>>>> The HVR-1850 uses a raw IR receiver in the CX23888 and older >>>> HVR-1250s use the raw IR receiver in the CX23885. They both work >> for >>>> Rx (I need to tweak the Cx23885 rx watermark though), but I never >>>> found time to finish Tx (lack of kernel interface when I had time). >>>>> >>>>> If you obtain one of these I can answer any driver questions. >>>> >>>> Quite some time back, I bought an HVR-1800 and an HVR-1250. I know >> one of >>>> them came with an mceusb transceiver and remote, as was pretty sure >> it was >>>> the 1800. For some reason, I didn't recall the 1250 coming with >> anything at >>>> all, but looking at dmesg output for it: >>>> >>>> cx23885 driver version 0.0.2 loaded >>>> cx23885 0000:03:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 >>>> CORE cx23885[0]: subsystem: 0070:7911, board: Hauppauge >> WinTV-HVR1250 [card=3,autodetected] >>>> tveeprom 0-0050: Hauppauge model 79001, rev E3D9, serial# 4904656 >>>> tveeprom 0-0050: MAC address is 00:0d:fe:4a:d6:d0 >>>> tveeprom 0-0050: tuner model is Microtune MT2131 (idx 139, type 4) >>>> tveeprom 0-0050: TV standards NTSC(M) ATSC/DVB Digital (eeprom 0x88) >>>> tveeprom 0-0050: audio processor is CX23885 (idx 39) >>>> tveeprom 0-0050: decoder processor is CX23885 (idx 33) >>>> tveeprom 0-0050: has no radio, has IR receiver, has no IR >> transmitter >>>> >>>> So it seems I do have hardware. However, its one of the two tuner >> cards in >>>> my "production" mythtv backend right now, making it a bit hard to do >> any >>>> experimenting with. If I can get it out of there, it looks like I >> just add >>>> an enable_885_ir=1, and I should be able to poke at it... >>> >>> Yeah. Igor's TeVii S470 CX23885 based card had interrupt storms when >>> enabled, so IR for '885 chips is disabled by default. To >> investigate, I >>> tried to by an HVR-1250 with a CX23885, but instead got an HVR-1275 >> with >>> a CX23888. dandel, on IRC, did a pretty decent job in testing >> HVR-1250 >>> operation and finding it works, despite climbing kernel >>> build/development learning curve at the time. >> ... >> >> Finally got some time to play with my 1250, dug out its rx cable, turns >> out to >> be the same one I special-ordered to work on the 1150 Devin sent me. >> Oops. >> Anyway. First impressions, not so good. Got a panic on boot, somewhere >> in >> cx23885_video_irq, iirc, when booting with enable_885_ir=1 set. >> However, dmesg >> was somewhere in the middle of cx18 init of the HVR-1600 in the box. >> Dunno if >> there's any way that's actually directly related, but I yanked the >> 1600. After >> doing that, the box managed to boot fine, but then while testing >> w/ir-keytable >> and an RC-6 remote, I got what I think was the same panic -- definitely >> the >> cx23885_video_irq bit in the trace, something about sleeping while >> atomic, IP >> at mwait_idle. (On the plus side, IR did seem to be working before >> that). >> >> Note also that this is a 2.6.32-based kernel with a 2.6.38-era backport >> of the >> driver code, because that's what was on this box. Was about to put it >> back into >> "production" use, but hey, its summer, there's nothing really for it to >> record >> for another few months, so I'll keep it out and throw it into another >> box with >> a newer kernel and serial console, etc., so I can further debug... > > In a very early version of the CX23885 IR changes I made the mistake of performing I2C transactions in an interrupt handler. That has been fixed in cx23885 for quite some time though. It also required some I2C fixes in cx25840-core.c IIRC, which again, has been fixed for some time. Up and running on 3.0-rc5 now, and I'm not seeing the panic, but the box keeps hard-locking after some number of keypresses. Can't get a peep out of it with sysrq, nmi watchdog doesn't seem to fire, etc. At the suggestion of "Dark Shadow", I've also tried booting the box with pci=nomsi. Works a treat then. Since his HVR-1270 and my HVR-1250 both behave much better with pci=nomsi, I'm thinking that in the short-term, we should probably make sure msi doesn't get enabled in the cx23885 driver, and longer-term, we can look at fixing it.
On Tue, 2011-06-28 at 17:39 -0400, Jarod Wilson wrote: > Up and running on 3.0-rc5 now, and I'm not seeing the panic, but the > box keeps hard-locking after some number of keypresses. Can't get a > peep out of it with sysrq, nmi watchdog doesn't seem to fire, etc. > > At the suggestion of "Dark Shadow", I've also tried booting the box > with pci=nomsi. Works a treat then. Since his HVR-1270 and my HVR-1250 > both behave much better with pci=nomsi, I'm thinking that in the > short-term, we should probably make sure msi doesn't get enabled in > the cx23885 driver, and longer-term, we can look at fixing it. Sounds fine. But fixcing the cx23885 driver to deal with both PCIe emulation of legacy PCI INTx and PCIe MSI will likely be very involved. (Maybe I'm wrong?) Taking a trip down memory lane to 2 Dec 2010... http://www.spinics.net/lists/linux-media/msg25956.html On Wed, 2010-12-01 at 21:52 -0800, David Liontooth wrote: > On 11/29/2010 04:38 AM, Andy Walls wrote: > > On Sun, 2010-11-28 at 23:49 -0800, David Liontooth wrote: > > For a quick band-aid, use "pci=nomsi" on your kernel command line, and > > reboot to reset the CX23888 hardware. > > > > The problem is MSI. The cx23885 driver isn't ready for it. The patch > > that enabled MSI for cx23885 probably needs to be reverted until some of > > these issues are sorted out. > -- what do we lose by removing the MSI support patch? Problems mostly. The driver was written to work with emulated legacy PCI INTx interrupts, which are to be treated as level triggered, and not PCIe MSI, which are to be treated as edge triggered. That's why I say the cx23885 driver isn't ready for MSI, but I'm not sure how involved an audit and conversion would be. I know an audit will take time and expertise. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 28, 2011, at 6:32 PM, Andy Walls wrote: > On Tue, 2011-06-28 at 17:39 -0400, Jarod Wilson wrote: > >> Up and running on 3.0-rc5 now, and I'm not seeing the panic, but the >> box keeps hard-locking after some number of keypresses. Can't get a >> peep out of it with sysrq, nmi watchdog doesn't seem to fire, etc. >> >> At the suggestion of "Dark Shadow", I've also tried booting the box >> with pci=nomsi. Works a treat then. Since his HVR-1270 and my HVR-1250 >> both behave much better with pci=nomsi, I'm thinking that in the >> short-term, we should probably make sure msi doesn't get enabled in >> the cx23885 driver, and longer-term, we can look at fixing it. > > Sounds fine. But fixcing the cx23885 driver to deal with both PCIe > emulation of legacy PCI INTx and PCIe MSI will likely be very involved. > (Maybe I'm wrong?) I'm not sure either, but I know a few PCI gurus at work who could probably lend some insight. > Taking a trip down memory lane to 2 Dec 2010... > http://www.spinics.net/lists/linux-media/msg25956.html Man. I really gotta learn to search the list archive (and bugzillas assigned to me...) before sending mail to the list, eh? ;) > On Wed, 2010-12-01 at 21:52 -0800, David Liontooth wrote: >> On 11/29/2010 04:38 AM, Andy Walls wrote: >>> On Sun, 2010-11-28 at 23:49 -0800, David Liontooth wrote: > >>> For a quick band-aid, use "pci=nomsi" on your kernel command line, and >>> reboot to reset the CX23888 hardware. >>> >>> The problem is MSI. The cx23885 driver isn't ready for it. The patch >>> that enabled MSI for cx23885 probably needs to be reverted until some of >>> these issues are sorted out. > >> -- what do we lose by removing the MSI support patch? > > Problems mostly. The driver was written to work with emulated legacy PCI > INTx interrupts, which are to be treated as level triggered, and not > PCIe MSI, which are to be treated as edge triggered. That's why I say > the cx23885 driver isn't ready for MSI, but I'm not sure how involved an > audit and conversion would be. I know an audit will take time and > expertise. Dropping msi support looks to be quite trivial, I got the card behaving after only a few lines of change in cx23885-core.c without having to pass in pci=nomsi, but I *only* tried IR, I haven't tried video capture. I'll see if I can give that a spin tomorrow, and if that behaves, I can send along the diff. If we wanted to get really fancy, I could add a modparam to let people turn msi back on. (Never had a single issue with this card recording with msi enabled, only IR seems busted). Just had another thought though... I have an HVR-1800 that *does* have issues recording, and the way the video is corrupted, its possible that its msi-related... Will have to keep that in mind next time I poke at it.
On Tue, 2011-06-28 at 22:17 -0400, Jarod Wilson wrote: > On Jun 28, 2011, at 6:32 PM, Andy Walls wrote: > > > On Tue, 2011-06-28 at 17:39 -0400, Jarod Wilson wrote: > > > >> I'm thinking that in the > >> short-term, we should probably make sure msi doesn't get enabled in > >> the cx23885 driver, and longer-term, we can look at fixing it. > > > > Sounds fine. But fixcing the cx23885 driver to deal with both PCIe > > emulation of legacy PCI INTx and PCIe MSI will likely be very involved. > > (Maybe I'm wrong?) > > I'm not sure either, but I know a few PCI gurus at work who could > probably lend some insight. > > > > Taking a trip down memory lane to 2 Dec 2010... > > http://www.spinics.net/lists/linux-media/msg25956.html > > Man. I really gotta learn to search the list archive (and bugzillas > assigned to me...) before sending mail to the list, eh? ;) You seem to stumble across things that I happened to run across about 6-7 months ago. So use that as your starting search window. ;) > > On Wed, 2010-12-01 at 21:52 -0800, David Liontooth wrote: > >> On 11/29/2010 04:38 AM, Andy Walls wrote: > >>> On Sun, 2010-11-28 at 23:49 -0800, David Liontooth wrote: > > > >>> For a quick band-aid, use "pci=nomsi" on your kernel command line, and > >>> reboot to reset the CX23888 hardware. > >>> > >>> The problem is MSI. The cx23885 driver isn't ready for it. The patch > >>> that enabled MSI for cx23885 probably needs to be reverted until some of > >>> these issues are sorted out. > > > >> -- what do we lose by removing the MSI support patch? > > > > Problems mostly. The driver was written to work with emulated legacy PCI > > INTx interrupts, which are to be treated as level triggered, and not > > PCIe MSI, which are to be treated as edge triggered. That's why I say > > the cx23885 driver isn't ready for MSI, but I'm not sure how involved an > > audit and conversion would be. I know an audit will take time and > > expertise. > > Dropping msi support looks to be quite trivial, It is trivial. > I got the card behaving > after only a few lines of change in cx23885-core.c without having to pass > in pci=nomsi, but I *only* tried IR, I haven't tried video capture. I'll > see if I can give that a spin tomorrow, and if that behaves, I can send > along the diff. It should. MSI was an ill tested addition to cx23885 IMHO. BTW, IR interrupts with the CX23885 IR unit are tricky. The CX23885 (CX25843) A/V core is I2C connected. Getting the A/V core to clear it's INT_N line (wired to the main core of the CX23885) requires clearing all the audio, video, and IR interrupts in the A/V core unit. The video and audio interrupts from the A/V core are currently masked. The IR interrupts have to be cleared by actually servicing the IR unit. To deal with that headache, when an A/V core interrupt comes in, I masked the PCI_AVCORE_INT (or whatever) interuupt on the CX23885 bridge until the IR unit can be serviced. Since the A/V core is I2C connected, the IR unit is serviced in a workqueue work handler thread, where sleeping is allowed. I'm not sure how MSI can get a storming or stuck interrupt from the CX23885 with me masking the PCI_AVCORE_INT. IIRC the cx23885_irq handler calls some functions that might take a long time to execute. Maybe that matters with MSI enabled. With MSI disabled, you might want to set up ftrace on the cx23885 driver functions and look to see if there are any slow paths being encountered by the cx23885_irq handler. I suspect the cx23885_irq handler should be deferring some work other than just IR handling. http://www.spinics.net/lists/linux-media/msg15762.html Or, of course, my IR handling could be just so f---ed up that it fails to clear the IR interrupt. ;) ftrace might let you see that too. (I don't have a CX23885 chip, only CX23888's, so I can't experiment.) > If we wanted to get really fancy, I could add a modparam > to let people turn msi back on. (Never had a single issue with this card > recording with msi enabled, only IR seems busted). Getting the driver to properly support both MSI and INTx emulation might take a few changes. (a bunch of if statements maybe?) I don't think letting users choose without the driver being inspected and/or corrected to handle both is a good idea. > Just had another thought though... I have an HVR-1800 that *does* have > issues recording, and the way the video is corrupted, its possible that > its msi-related... Will have to keep that in mind next time I poke at it. Uh, huh. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/rc/ir-rc5-decoder.c b/drivers/media/rc/ir-rc5-decoder.c index ebdba55..c4052da 100644 --- a/drivers/media/rc/ir-rc5-decoder.c +++ b/drivers/media/rc/ir-rc5-decoder.c @@ -144,10 +144,15 @@ again: system = (data->bits & 0x007C0) >> 6; toggle = (data->bits & 0x00800) ? 1 : 0; command += (data->bits & 0x01000) ? 0 : 0x40; - scancode = system << 8 | command; - - IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n", - scancode, toggle); + /* Notes + * 1. Should filter unknown systems e.g Hauppauge use 0x1e or 0x1f + * 2. Don't include system in the scancode otherwise input_event() + * doesn't recognise the scancode + */ + scancode = command; + + IR_dprintk(1, "RC5 scancode 0x%02x (system: 0x%02x toggle: %u)\n", + scancode, system, toggle); } rc_keydown(dev, scancode, toggle); diff --git a/drivers/media/video/cx88/cx88-input.c b/drivers/media/video/cx88/cx88-input.c index c820e2f..7281db4 100644 --- a/drivers/media/video/cx88/cx88-input.c +++ b/drivers/media/video/cx88/cx88-input.c @@ -524,7 +524,7 @@ void cx88_ir_irq(struct cx88_core *core) for (todo = 32; todo > 0; todo -= bits) { ev.pulse = samples & 0x80000000 ? false : true; bits = min(todo, 32U - fls(ev.pulse ? samples : ~samples)); - ev.duration = (bits * NSEC_PER_SEC) / (1000 * ir_samplerate); + ev.duration = bits * (NSEC_PER_SEC / (1000 * ir_samplerate)); /* NB avoid 32-bit overflow */ ir_raw_event_store_with_filter(ir->dev, &ev); samples <<= bits; }