Re: [PATCH 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Mon, Jun 21, 2010 at 7:04 AM, Andy Walls wrote: > On Sun, 2010-06-20 at 23:51 -0400, Jarod Wilson wrote: >> On Sun, Jun 20, 2010 at 8:47 PM, Andy Walls wrote: > > >> As for the building your own kernel thing... I've been doing my work >> mainly on a pair of x86_64 systems, one a ThinkPad T61 running Fedora >> 13, and the other an HP xw4400 workstation running RHEL6. In both >> cases, I copied a distro kernel's, config file out of /boot/, and then >> ran make oldconfig over it and build straight from what's in my tree, >> which works well enough on both setups. > > I pulled the config out of the kernel*.src.rpm after 'rpmbuild -bp' (I > only saw the configs in /boot after doing that :P ), and then ran 'make > oldconfig'. > > The first time around I forgot to do a modules_install and the ext[234] > modules weren't in the initramfs. That made it hard for the kernel to > read the /boot and / filesystems. ;) > > After fixing that idiocy, it now hangs in early boot - just a blinking > cursor. I'm speculating it is a problem with support for my old-ish ATI > Radeon Xpress 200 video chipset with a vanilla kernel. I'll work it out > eventually. Seems you've already gotten around this, but it did remind me of someone on one of the fedora mailing lists who said w/their older radeon hardware, they could only boot recent kernels if the passed in nomodeset=1 or something like that. >> > I'll have time on Thursday night to try again. >> >> No rush yet, we've got a while before the merge window still. >> Christoph (Bartelmus) helped me out with a bunch of ioctl >> documentation this weekend, so I've got that to add to the tree, then >> I think I'll be prepared to resubmit the lirc bits. I'll shoot for >> doing that next weekend, and hopefully, that'll give you a chance to >> try 'em out before then and provide any necessary feedback/fixes/etc. >> (Not that we can't also just fix things up as needed post-merge). I'm >> still up in the air as to what I should work on next... So many lirc >> drivers left to port still... Maybe zilog, maybe streamzap... Maybe >> the MCE IR keyboard... > > I've got a PVR-150 and HVR-1600 both with the Zilog Z8's on them. If I > ever get my act together, I'll at least be able to test that and > integrate any changes into the ivtv & cx18 drivers. I've recently seen > users having trouble on IRC, BTW. Yeah, me too. Hoping to dig into porting (and fixing) lirc_zilog RSN... -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Sun, 2010-06-20 at 23:51 -0400, Jarod Wilson wrote: > On Sun, Jun 20, 2010 at 8:47 PM, Andy Walls wrote: > As for the building your own kernel thing... I've been doing my work > mainly on a pair of x86_64 systems, one a ThinkPad T61 running Fedora > 13, and the other an HP xw4400 workstation running RHEL6. In both > cases, I copied a distro kernel's, config file out of /boot/, and then > ran make oldconfig over it and build straight from what's in my tree, > which works well enough on both setups. I pulled the config out of the kernel*.src.rpm after 'rpmbuild -bp' (I only saw the configs in /boot after doing that :P ), and then ran 'make oldconfig'. The first time around I forgot to do a modules_install and the ext[234] modules weren't in the initramfs. That made it hard for the kernel to read the /boot and / filesystems. ;) After fixing that idiocy, it now hangs in early boot - just a blinking cursor. I'm speculating it is a problem with support for my old-ish ATI Radeon Xpress 200 video chipset with a vanilla kernel. I'll work it out eventually. > > I'll have time on Thursday night to try again. > > No rush yet, we've got a while before the merge window still. > Christoph (Bartelmus) helped me out with a bunch of ioctl > documentation this weekend, so I've got that to add to the tree, then > I think I'll be prepared to resubmit the lirc bits. I'll shoot for > doing that next weekend, and hopefully, that'll give you a chance to > try 'em out before then and provide any necessary feedback/fixes/etc. > (Not that we can't also just fix things up as needed post-merge). I'm > still up in the air as to what I should work on next... So many lirc > drivers left to port still... Maybe zilog, maybe streamzap... Maybe > the MCE IR keyboard... I've got a PVR-150 and HVR-1600 both with the Zilog Z8's on them. If I ever get my act together, I'll at least be able to test that and integrate any changes into the ivtv & cx18 drivers. I've recently seen users having trouble on IRC, BTW. Regards, Andy -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Sun, Jun 20, 2010 at 8:47 PM, Andy Walls wrote: > On Thu, 2010-06-17 at 11:11 -0400, Jarod Wilson wrote: >> On Thu, Jun 17, 2010 at 8:14 AM, Andy Walls wrote: > >> >> A fully functional tree carrying both of David's patches and the >> >> entire stack of other patches I've submitted today, based on top of >> >> the linuxtv staging/rc branch, can be found here: >> >> >> >> http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/patches >> >> >> >> Also includes the lirc patches that I believe are ready to be >> >> submitted for actual consideration (note that they're dependent on >> >> David's two patches). >> > >> > >> > I'll try and play with this this weekend along with some cx23885 >> > cleanup. >> >> Excellent. A few things to note... > > Jarrod, > > I was unable to get this task completed in the time I had available this > weekend. A power supply failure, unexpected hard drive replacement, and > my inability to build/install a kernel from a git tree that would > actually boot my Fedora 12 installation didn't help. (My productivity > has tanked since v4l-dvb went to GIT for CM, and the last time I built a > real kernel without rpmbuild was for RedHat 9. I'm still working out > processes for doing basic things, sorry.) Heh, yeah, hardware failure is always fun when you're trying really hard to get something done. :) As for the building your own kernel thing... I've been doing my work mainly on a pair of x86_64 systems, one a ThinkPad T61 running Fedora 13, and the other an HP xw4400 workstation running RHEL6. In both cases, I copied a distro kernel's, config file out of /boot/, and then ran make oldconfig over it and build straight from what's in my tree, which works well enough on both setups. > I'll have time on Thursday night to try again. No rush yet, we've got a while before the merge window still. Christoph (Bartelmus) helped me out with a bunch of ioctl documentation this weekend, so I've got that to add to the tree, then I think I'll be prepared to resubmit the lirc bits. I'll shoot for doing that next weekend, and hopefully, that'll give you a chance to try 'em out before then and provide any necessary feedback/fixes/etc. (Not that we can't also just fix things up as needed post-merge). I'm still up in the air as to what I should work on next... So many lirc drivers left to port still... Maybe zilog, maybe streamzap... Maybe the MCE IR keyboard... -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Thu, 2010-06-17 at 11:11 -0400, Jarod Wilson wrote: > On Thu, Jun 17, 2010 at 8:14 AM, Andy Walls wrote: > >> A fully functional tree carrying both of David's patches and the > >> entire stack of other patches I've submitted today, based on top of > >> the linuxtv staging/rc branch, can be found here: > >> > >> http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/patches > >> > >> Also includes the lirc patches that I believe are ready to be > >> submitted for actual consideration (note that they're dependent on > >> David's two patches). > > > > > > I'll try and play with this this weekend along with some cx23885 > > cleanup. > > Excellent. A few things to note... Jarrod, I was unable to get this task completed in the time I had available this weekend. A power supply failure, unexpected hard drive replacement, and my inability to build/install a kernel from a git tree that would actually boot my Fedora 12 installation didn't help. (My productivity has tanked since v4l-dvb went to GIT for CM, and the last time I built a real kernel without rpmbuild was for RedHat 9. I'm still working out processes for doing basic things, sorry.) I'll have time on Thursday night to try again. > Many of the lirc_dev ioctls are > currently commented out, and haven't in any way been wired up to tx > callbacks, Yes, I saw, that's OK. It should be easy enough to hack something in for testing and prototyping. > I've only enabled the minimum necessary for mceusb. The > ioctls are all using __u32 params, which, if you're on x86_64, will > require a patched lirc userspace build to make the ioctl types match. > I'm using this patch atm: > > http://wilsonet.com/jarod/lirc_misc/lirc-0.8.6-make-ioctls-u32.patch > > (In the future, lirc userspace should obviously just build against > ). I've got all x86_64 bit machines here, so thank you for the tips. Regards, Andy -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Thu, Jun 17, 2010 at 8:14 AM, Andy Walls wrote: > On Wed, 2010-06-16 at 16:41 -0400, Jarod Wilson wrote: >> On Wed, Jun 16, 2010 at 4:04 PM, Jarod Wilson wrote: >> ... >> >> I have another suggestion, let's keep the client register/unregister >> >> callbacks for decoders (but add a comment that they're only used for >> >> lirc). Then teach drivers/media/IR/ir-raw-event.c to keep track of the >> >> raw clients so that it can pass all pre-existing clients to newly added >> >> decoders. >> >> >> >> I'll post two patches (compile tested only) in a few seconds to show >> >> what I mean. >> > >> > Consider them now runtime tested as well. They appear to do the trick, >> > the lirc bridge comes up just fine, even when ir-lirc-codec isn't >> > loaded until after mceusb. *Much* better implementation than my ugly >> > trick. I'll ack your patches and submit a series on top of them for >> > lirc support, hopefully this evening (in addition to a few other fixes >> > that aren't dependent on any of them). >> >> A fully functional tree carrying both of David's patches and the >> entire stack of other patches I've submitted today, based on top of >> the linuxtv staging/rc branch, can be found here: >> >> http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/patches >> >> Also includes the lirc patches that I believe are ready to be >> submitted for actual consideration (note that they're dependent on >> David's two patches). > > > I'll try and play with this this weekend along with some cx23885 > cleanup. Excellent. A few things to note... Many of the lirc_dev ioctls are currently commented out, and haven't in any way been wired up to tx callbacks, I've only enabled the minimum necessary for mceusb. The ioctls are all using __u32 params, which, if you're on x86_64, will require a patched lirc userspace build to make the ioctl types match. I'm using this patch atm: http://wilsonet.com/jarod/lirc_misc/lirc-0.8.6-make-ioctls-u32.patch (In the future, lirc userspace should obviously just build against ). -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Wed, 2010-06-16 at 16:41 -0400, Jarod Wilson wrote: > On Wed, Jun 16, 2010 at 4:04 PM, Jarod Wilson wrote: > ... > >> I have another suggestion, let's keep the client register/unregister > >> callbacks for decoders (but add a comment that they're only used for > >> lirc). Then teach drivers/media/IR/ir-raw-event.c to keep track of the > >> raw clients so that it can pass all pre-existing clients to newly added > >> decoders. > >> > >> I'll post two patches (compile tested only) in a few seconds to show > >> what I mean. > > > > Consider them now runtime tested as well. They appear to do the trick, > > the lirc bridge comes up just fine, even when ir-lirc-codec isn't > > loaded until after mceusb. *Much* better implementation than my ugly > > trick. I'll ack your patches and submit a series on top of them for > > lirc support, hopefully this evening (in addition to a few other fixes > > that aren't dependent on any of them). > > A fully functional tree carrying both of David's patches and the > entire stack of other patches I've submitted today, based on top of > the linuxtv staging/rc branch, can be found here: > > http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/patches > > Also includes the lirc patches that I believe are ready to be > submitted for actual consideration (note that they're dependent on > David's two patches). I'll try and play with this this weekend along with some cx23885 cleanup. Regards, Andy -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Wed, Jun 16, 2010 at 4:04 PM, Jarod Wilson wrote: ... >> I have another suggestion, let's keep the client register/unregister >> callbacks for decoders (but add a comment that they're only used for >> lirc). Then teach drivers/media/IR/ir-raw-event.c to keep track of the >> raw clients so that it can pass all pre-existing clients to newly added >> decoders. >> >> I'll post two patches (compile tested only) in a few seconds to show >> what I mean. > > Consider them now runtime tested as well. They appear to do the trick, > the lirc bridge comes up just fine, even when ir-lirc-codec isn't > loaded until after mceusb. *Much* better implementation than my ugly > trick. I'll ack your patches and submit a series on top of them for > lirc support, hopefully this evening (in addition to a few other fixes > that aren't dependent on any of them). A fully functional tree carrying both of David's patches and the entire stack of other patches I've submitted today, based on top of the linuxtv staging/rc branch, can be found here: http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/patches Also includes the lirc patches that I believe are ready to be submitted for actual consideration (note that they're dependent on David's two patches). -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman wrote: > On Wed, Jun 09, 2010 at 09:25:44PM -0400, Jarod Wilson wrote: >> On Wed, Jun 9, 2010 at 2:15 PM, Jarod Wilson wrote: >> > On Wed, Jun 09, 2010 at 07:56:21PM +0200, David Härdeman wrote: >> >> On Wed, Jun 09, 2010 at 09:29:08AM -0400, Jarod Wilson wrote: >> ... >> >> > So this definitely negatively impacts my ir-core-to-lirc_dev >> >> > (ir-lirc-codec.c) bridge driver, as it was doing the lirc_dev device >> >> > registration work in its register function. However, if (after your >> >> > patchset) we add a new pair of callbacks replacing raw_register and >> >> > raw_unregister, which are optional, that work could be done there >> >> > instead, >> >> > so I don't think this is an insurmountable obstacle for the lirc bits. >> >> >> >> While I'm not sure exactly what callbacks you're suggesting, >> > >> > Essentially: >> > >> > .setup_other_crap >> > .tear_down_other_crap >> > >> > ...which in the ir-lirc-codec case, register ir-lirc-codec for a specific >> > hardware receiver as an lirc_dev client, and conversely, tear it down. >> > >> >> it still >> >> sounds like the callbacks would have the exact same problems that the >> >> current code has (i.e. the decoder will be blissfully unaware of >> >> hardware which exists before the decoder is loaded). Right? >> > >> > In my head, this was going to work out, but you're correct, I still have >> > the exact same problem -- its not in ir_raw_handler_list yet when >> > ir_raw_event_register runs, and thus the callback never fires, so lirc_dev >> > never actually gets wired up to ir-lirc-codec. It now knows about the lirc >> > decoder, but its completely useless. Narf. >> >> And now I have it working atop your patches. Its a bit of a nasty-ish >> hack, at least for the lirc case, but its working, even in the case >> where the decoder drivers aren't actually loaded until after the >> device driver. I've added one extra param to each protocol-specific >> struct in ir-core-priv.h (bool initialized) and hooked into the >> protocol-specific decode functions to both determine whether a >> protocol should be enabled or disabled by default, and to run any >> additionally required initialization (such as in the ir-lirc-codec >> case). >> >> So initially, mceusb comes up with all decoders enabled. Then when ir >> comes in, every protocol-specific decoder fires. Each of them check >> for whether or not they've been fully initialized, and if not, we >> check the loaded keymap, and if it doesn't match, we disable that >> decoder (bringing back the "disable protocol handlers we don't need" >> functionality that disappeared w/this patchset). In the lirc case, we >> actually do all the work needed to wire up the connection over to >> lirc_dev. >> >> This works perfectly fine for all the in-kernel decoders, but has one >> minor shortcoming for ir-lirc-codec, in that /dev/lirc0 won't actually >> exist until the first incoming ir signal is seen. lircd can handle >> this case just fine, it'll wait for /dev/lirc0 to show up, but it >> doesn't come up fast enough to catch and decode the very first >> incoming ir signal. Subsequent ones work perfectly fine though. This >> need to initialize the link via incoming ir is a bit problematic if >> you're using a device for transmit-only (e.g., and mceusb device >> hooked to a mythtv backend in the closet or something), as there would >> be a strong possibility of /dev/lirc0 never getting hooked up. I can >> think of a few workarounds, but none are particularly clean and/or >> automagic. >> >> Not sure how palatable it is, but here it is: > > I think it sounds pretty awful :) So my suspicion wrt palatability was correct. :) > I have another suggestion, let's keep the client register/unregister > callbacks for decoders (but add a comment that they're only used for > lirc). Then teach drivers/media/IR/ir-raw-event.c to keep track of the > raw clients so that it can pass all pre-existing clients to newly added > decoders. > > I'll post two patches (compile tested only) in a few seconds to show > what I mean. Consider them now runtime tested as well. They appear to do the trick, the lirc bridge comes up just fine, even when ir-lirc-codec isn't loaded until after mceusb. *Much* better implementation than my ugly trick. I'll ack your patches and submit a series on top of them for lirc support, hopefully this evening (in addition to a few other fixes that aren't dependent on any of them). -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Wed, Jun 09, 2010 at 09:25:44PM -0400, Jarod Wilson wrote: > On Wed, Jun 9, 2010 at 2:15 PM, Jarod Wilson wrote: > > On Wed, Jun 09, 2010 at 07:56:21PM +0200, David Härdeman wrote: > >> On Wed, Jun 09, 2010 at 09:29:08AM -0400, Jarod Wilson wrote: > ... > >> > So this definitely negatively impacts my ir-core-to-lirc_dev > >> > (ir-lirc-codec.c) bridge driver, as it was doing the lirc_dev device > >> > registration work in its register function. However, if (after your > >> > patchset) we add a new pair of callbacks replacing raw_register and > >> > raw_unregister, which are optional, that work could be done there > >> > instead, > >> > so I don't think this is an insurmountable obstacle for the lirc bits. > >> > >> While I'm not sure exactly what callbacks you're suggesting, > > > > Essentially: > > > > .setup_other_crap > > .tear_down_other_crap > > > > ...which in the ir-lirc-codec case, register ir-lirc-codec for a specific > > hardware receiver as an lirc_dev client, and conversely, tear it down. > > > >> it still > >> sounds like the callbacks would have the exact same problems that the > >> current code has (i.e. the decoder will be blissfully unaware of > >> hardware which exists before the decoder is loaded). Right? > > > > In my head, this was going to work out, but you're correct, I still have > > the exact same problem -- its not in ir_raw_handler_list yet when > > ir_raw_event_register runs, and thus the callback never fires, so lirc_dev > > never actually gets wired up to ir-lirc-codec. It now knows about the lirc > > decoder, but its completely useless. Narf. > > And now I have it working atop your patches. Its a bit of a nasty-ish > hack, at least for the lirc case, but its working, even in the case > where the decoder drivers aren't actually loaded until after the > device driver. I've added one extra param to each protocol-specific > struct in ir-core-priv.h (bool initialized) and hooked into the > protocol-specific decode functions to both determine whether a > protocol should be enabled or disabled by default, and to run any > additionally required initialization (such as in the ir-lirc-codec > case). > > So initially, mceusb comes up with all decoders enabled. Then when ir > comes in, every protocol-specific decoder fires. Each of them check > for whether or not they've been fully initialized, and if not, we > check the loaded keymap, and if it doesn't match, we disable that > decoder (bringing back the "disable protocol handlers we don't need" > functionality that disappeared w/this patchset). In the lirc case, we > actually do all the work needed to wire up the connection over to > lirc_dev. > > This works perfectly fine for all the in-kernel decoders, but has one > minor shortcoming for ir-lirc-codec, in that /dev/lirc0 won't actually > exist until the first incoming ir signal is seen. lircd can handle > this case just fine, it'll wait for /dev/lirc0 to show up, but it > doesn't come up fast enough to catch and decode the very first > incoming ir signal. Subsequent ones work perfectly fine though. This > need to initialize the link via incoming ir is a bit problematic if > you're using a device for transmit-only (e.g., and mceusb device > hooked to a mythtv backend in the closet or something), as there would > be a strong possibility of /dev/lirc0 never getting hooked up. I can > think of a few workarounds, but none are particularly clean and/or > automagic. > > Not sure how palatable it is, but here it is: I think it sounds pretty awful :) I have another suggestion, let's keep the client register/unregister callbacks for decoders (but add a comment that they're only used for lirc). Then teach drivers/media/IR/ir-raw-event.c to keep track of the raw clients so that it can pass all pre-existing clients to newly added decoders. I'll post two patches (compile tested only) in a few seconds to show what I mean. -- David Härdeman -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Wed, Jun 9, 2010 at 2:15 PM, Jarod Wilson wrote: > On Wed, Jun 09, 2010 at 07:56:21PM +0200, David Härdeman wrote: >> On Wed, Jun 09, 2010 at 09:29:08AM -0400, Jarod Wilson wrote: ... >> > So this definitely negatively impacts my ir-core-to-lirc_dev >> > (ir-lirc-codec.c) bridge driver, as it was doing the lirc_dev device >> > registration work in its register function. However, if (after your >> > patchset) we add a new pair of callbacks replacing raw_register and >> > raw_unregister, which are optional, that work could be done there instead, >> > so I don't think this is an insurmountable obstacle for the lirc bits. >> >> While I'm not sure exactly what callbacks you're suggesting, > > Essentially: > > .setup_other_crap > .tear_down_other_crap > > ...which in the ir-lirc-codec case, register ir-lirc-codec for a specific > hardware receiver as an lirc_dev client, and conversely, tear it down. > >> it still >> sounds like the callbacks would have the exact same problems that the >> current code has (i.e. the decoder will be blissfully unaware of >> hardware which exists before the decoder is loaded). Right? > > In my head, this was going to work out, but you're correct, I still have > the exact same problem -- its not in ir_raw_handler_list yet when > ir_raw_event_register runs, and thus the callback never fires, so lirc_dev > never actually gets wired up to ir-lirc-codec. It now knows about the lirc > decoder, but its completely useless. Narf. And now I have it working atop your patches. Its a bit of a nasty-ish hack, at least for the lirc case, but its working, even in the case where the decoder drivers aren't actually loaded until after the device driver. I've added one extra param to each protocol-specific struct in ir-core-priv.h (bool initialized) and hooked into the protocol-specific decode functions to both determine whether a protocol should be enabled or disabled by default, and to run any additionally required initialization (such as in the ir-lirc-codec case). So initially, mceusb comes up with all decoders enabled. Then when ir comes in, every protocol-specific decoder fires. Each of them check for whether or not they've been fully initialized, and if not, we check the loaded keymap, and if it doesn't match, we disable that decoder (bringing back the "disable protocol handlers we don't need" functionality that disappeared w/this patchset). In the lirc case, we actually do all the work needed to wire up the connection over to lirc_dev. This works perfectly fine for all the in-kernel decoders, but has one minor shortcoming for ir-lirc-codec, in that /dev/lirc0 won't actually exist until the first incoming ir signal is seen. lircd can handle this case just fine, it'll wait for /dev/lirc0 to show up, but it doesn't come up fast enough to catch and decode the very first incoming ir signal. Subsequent ones work perfectly fine though. This need to initialize the link via incoming ir is a bit problematic if you're using a device for transmit-only (e.g., and mceusb device hooked to a mythtv backend in the closet or something), as there would be a strong possibility of /dev/lirc0 never getting hooked up. I can think of a few workarounds, but none are particularly clean and/or automagic. Not sure how palatable it is, but here it is: http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=commitdiff;h=8f2be576ecb82448daa0c0d789bf0c978c66b103 -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Wed, Jun 09, 2010 at 07:56:21PM +0200, David Härdeman wrote: > On Wed, Jun 09, 2010 at 09:29:08AM -0400, Jarod Wilson wrote: > > On Tue, Jun 08, 2010 at 11:46:36PM -0400, Jarod Wilson wrote: > > > On Tue, Jun 8, 2010 at 1:50 PM, David Härdeman wrote: > > > > b) Mauro mentioned in <4bdf28c0.4060...@redhat.com> that: > > > > > > > > I liked the idea of your redesign, but I didn't like the removal > > > > of a per-decoder sysfs entry. As already discussed, there are > > > > cases where we'll need a per-decoder sysfs entry (lirc_dev is > > > > probably one of those cases - also Jarod's imon driver is > > > > currently implementing a modprobe parameter that needs to be > > > > moved to the driver). > > > > > > > > could you please confirm if your lirc and/or imon drivers would be > > > > negatively affected by the proposed patches? > > > > > > Will do so once I get them wedged in on top. > > > > Got it all merged and compiling, but not yet runtime tested. Compiling > > alone sheds some light on things though... > > > > So this definitely negatively impacts my ir-core-to-lirc_dev > > (ir-lirc-codec.c) bridge driver, as it was doing the lirc_dev device > > registration work in its register function. However, if (after your > > patchset) we add a new pair of callbacks replacing raw_register and > > raw_unregister, which are optional, that work could be done there instead, > > so I don't think this is an insurmountable obstacle for the lirc bits. > > While I'm not sure exactly what callbacks you're suggesting, Essentially: .setup_other_crap .tear_down_other_crap ...which in the ir-lirc-codec case, register ir-lirc-codec for a specific hardware receiver as an lirc_dev client, and conversely, tear it down. > it still > sounds like the callbacks would have the exact same problems that the > current code has (i.e. the decoder will be blissfully unaware of > hardware which exists before the decoder is loaded). Right? In my head, this was going to work out, but you're correct, I still have the exact same problem -- its not in ir_raw_handler_list yet when ir_raw_event_register runs, and thus the callback never fires, so lirc_dev never actually gets wired up to ir-lirc-codec. It now knows about the lirc decoder, but its completely useless. Narf. > > As for the imon driver, the modprobe parameter in question (iirc) was > > already removed from the driver, as its functionality is replaced by > > implementing a change_protocol callback. However, there's a request to > > add it (or something like it) back to the driver to allow disabling the > > IR part altogether, and there are a few other modparams that might be > > better suited as sysfs entries. However, its actually not relevant to the > > case of registering raw protocol handlers, as the imon devices do their > > decoding in hardware. I can see the possibility for protocol-specific > > knobs in sysfs though. But I think the same optional callbacks I'd use to > > keep the lirc bits working could also be used for this. Can't think of a > > good name for these yet, probably need more coffee first... ;) > > But those sysfs entries wouldn't be > per-decoder-per-hardware-devicethey'd just be > per-hardware-device...right? Most likely. But I think its possible someone would want to want to tweak some parameter that is both protocol and hardware device specific. Just sheer speculation at the moment though, I don't have a concrete example. -- Jarod Wilson ja...@redhat.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Wed, Jun 09, 2010 at 09:29:08AM -0400, Jarod Wilson wrote: > On Tue, Jun 08, 2010 at 11:46:36PM -0400, Jarod Wilson wrote: > > On Tue, Jun 8, 2010 at 1:50 PM, David Härdeman wrote: > > > b) Mauro mentioned in <4bdf28c0.4060...@redhat.com> that: > > > > > > I liked the idea of your redesign, but I didn't like the removal > > > of a per-decoder sysfs entry. As already discussed, there are > > > cases where we'll need a per-decoder sysfs entry (lirc_dev is > > > probably one of those cases - also Jarod's imon driver is > > > currently implementing a modprobe parameter that needs to be > > > moved to the driver). > > > > > > could you please confirm if your lirc and/or imon drivers would be > > > negatively affected by the proposed patches? > > > > Will do so once I get them wedged in on top. > > Got it all merged and compiling, but not yet runtime tested. Compiling > alone sheds some light on things though... > > So this definitely negatively impacts my ir-core-to-lirc_dev > (ir-lirc-codec.c) bridge driver, as it was doing the lirc_dev device > registration work in its register function. However, if (after your > patchset) we add a new pair of callbacks replacing raw_register and > raw_unregister, which are optional, that work could be done there instead, > so I don't think this is an insurmountable obstacle for the lirc bits. While I'm not sure exactly what callbacks you're suggesting, it still sounds like the callbacks would have the exact same problems that the current code has (i.e. the decoder will be blissfully unaware of hardware which exists before the decoder is loaded). Right? > As for the imon driver, the modprobe parameter in question (iirc) was > already removed from the driver, as its functionality is replaced by > implementing a change_protocol callback. However, there's a request to > add it (or something like it) back to the driver to allow disabling the > IR part altogether, and there are a few other modparams that might be > better suited as sysfs entries. However, its actually not relevant to the > case of registering raw protocol handlers, as the imon devices do their > decoding in hardware. I can see the possibility for protocol-specific > knobs in sysfs though. But I think the same optional callbacks I'd use to > keep the lirc bits working could also be used for this. Can't think of a > good name for these yet, probably need more coffee first... ;) But those sysfs entries wouldn't be per-decoder-per-hardware-devicethey'd just be per-hardware-device...right? -- David Härdeman -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Tue, Jun 08, 2010 at 11:46:36PM -0400, Jarod Wilson wrote: > On Tue, Jun 8, 2010 at 1:50 PM, David Härdeman wrote: > > On Mon, Jun 07, 2010 at 04:15:30PM -0400, Jarod Wilson wrote: > >> On Mon, Jun 07, 2010 at 09:00:03PM +0200, David Härdeman wrote: > >> > On Mon, May 03, 2010 at 05:00:05PM -0300, Mauro Carvalho Chehab wrote: > >> > > David Härdeman wrote: > >> > > > This patch moves the state from each raw decoder into the > >> > > > ir_raw_event_ctrl struct. > >> > > > > >> > > > This allows the removal of code like this: > >> > > > > >> > > > spin_lock(&decoder_lock); > >> > > > list_for_each_entry(data, &decoder_list, list) { > >> > > > if (data->ir_dev == ir_dev) > >> > > > break; > >> > > > } > >> > > > spin_unlock(&decoder_lock); > >> > > > return data; > >> > > > > >> > > > which is currently run for each decoder on each event in order > >> > > > to get the client-specific decoding state data. > >> > > > > >> > > > In addition, ir decoding modules and ir driver module load > >> > > > order is now independent. Centralizing the data also allows > >> > > > for a nice code reduction of about 30% per raw decoder as > >> > > > client lists and client registration callbacks are no longer > >> > > > necessary. > >> > > > >> > > The registration callbacks will likely still be needed by lirc, > >> > > as you need to create/delete lirc_dev interfaces, when the module > >> > > is registered, but I might be wrong. It would be interesting to > >> > > add lirc_dev first, in order to be sure about the better interfaces > >> > > for it. > >> > > >> > Or the lirc_dev patch can add whatever interfaces it needs. Anyway, the > >> > current interfaces are not good enough since it'll break if lirc_dev is > >> > loaded after the hardware modules. > >> > >> This is something I've been meaning to mention myself. On system boot, if > >> an mceusb device is connected, it pretty regularly only has the NEC > >> decoder available to use. I have to reload mceusb, or make sure ir-core is > >> explicitly loaded, wait a bit, then load mceusb, if I want to have all of > >> the protocol handlers available -- which includes the needed-by-default > >> rc6 one. I've only briefly tinkered with trying to fix it, sounds like you > >> may already have fixage within this patchset. > > > > The problem is that without the patchset, each decoder is expected to > > carry it's own list of datastructures for each hardware receiver. > > Hardware receiver addition/removal is signalled through a callback to > > the decoder, but the callback will (naturally) not be invoked if the > > hardware driver is already loaded when the decoder is. So loading a > > decoder "late" or reloading a decoder will mean that it doesn't know > > about pre-existing hardware. > > > >> > In addition, random module load order is currently broken (try loading > >> > decoders first and hardware later and you'll see). With this patch, it > >> > works again. > >> > >> Want. > > > > Then please help me with two things: > > > > a) Test the patches I just sent (especially 6/8 and 7/8, they should > > be independent from the rest) > > Working on merging them into a tree here locally. There's been a bit > of churn, so the last few didn't apply cleanly, but I'm almost there. > > > b) Mauro mentioned in <4bdf28c0.4060...@redhat.com> that: > > > > I liked the idea of your redesign, but I didn't like the removal > > of a per-decoder sysfs entry. As already discussed, there are > > cases where we'll need a per-decoder sysfs entry (lirc_dev is > > probably one of those cases - also Jarod's imon driver is > > currently implementing a modprobe parameter that needs to be > > moved to the driver). > > > > could you please confirm if your lirc and/or imon drivers would be > > negatively affected by the proposed patches? > > Will do so once I get them wedged in on top. Got it all merged and compiling, but not yet runtime tested. Compiling alone sheds some light on things though... So this definitely negatively impacts my ir-core-to-lirc_dev (ir-lirc-codec.c) bridge driver, as it was doing the lirc_dev device registration work in its register function. However, if (after your patchset) we add a new pair of callbacks replacing raw_register and raw_unregister, which are optional, that work could be done there instead, so I don't think this is an insurmountable obstacle for the lirc bits. As for the imon driver, the modprobe parameter in question (iirc) was already removed from the driver, as its functionality is replaced by implementing a change_protocol callback. However, there's a request to add it (or something like it) back to the driver to allow disabling the IR part altogether, and there are a few other modparams that might be better suited as sysfs entries. However, its actually not relevant to the case of registering raw protocol handlers, as the imon dev
Re: [PATCH 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Tue, Jun 8, 2010 at 1:50 PM, David Härdeman wrote: > On Mon, Jun 07, 2010 at 04:15:30PM -0400, Jarod Wilson wrote: >> On Mon, Jun 07, 2010 at 09:00:03PM +0200, David Härdeman wrote: >> > On Mon, May 03, 2010 at 05:00:05PM -0300, Mauro Carvalho Chehab wrote: >> > > David Härdeman wrote: >> > > > This patch moves the state from each raw decoder into the >> > > > ir_raw_event_ctrl struct. >> > > > >> > > > This allows the removal of code like this: >> > > > >> > > > spin_lock(&decoder_lock); >> > > > list_for_each_entry(data, &decoder_list, list) { >> > > > if (data->ir_dev == ir_dev) >> > > > break; >> > > > } >> > > > spin_unlock(&decoder_lock); >> > > > return data; >> > > > >> > > > which is currently run for each decoder on each event in order >> > > > to get the client-specific decoding state data. >> > > > >> > > > In addition, ir decoding modules and ir driver module load >> > > > order is now independent. Centralizing the data also allows >> > > > for a nice code reduction of about 30% per raw decoder as >> > > > client lists and client registration callbacks are no longer >> > > > necessary. >> > > >> > > The registration callbacks will likely still be needed by lirc, >> > > as you need to create/delete lirc_dev interfaces, when the module >> > > is registered, but I might be wrong. It would be interesting to >> > > add lirc_dev first, in order to be sure about the better interfaces >> > > for it. >> > >> > Or the lirc_dev patch can add whatever interfaces it needs. Anyway, the >> > current interfaces are not good enough since it'll break if lirc_dev is >> > loaded after the hardware modules. >> >> This is something I've been meaning to mention myself. On system boot, if >> an mceusb device is connected, it pretty regularly only has the NEC >> decoder available to use. I have to reload mceusb, or make sure ir-core is >> explicitly loaded, wait a bit, then load mceusb, if I want to have all of >> the protocol handlers available -- which includes the needed-by-default >> rc6 one. I've only briefly tinkered with trying to fix it, sounds like you >> may already have fixage within this patchset. > > The problem is that without the patchset, each decoder is expected to > carry it's own list of datastructures for each hardware receiver. > Hardware receiver addition/removal is signalled through a callback to > the decoder, but the callback will (naturally) not be invoked if the > hardware driver is already loaded when the decoder is. So loading a > decoder "late" or reloading a decoder will mean that it doesn't know > about pre-existing hardware. > >> > In addition, random module load order is currently broken (try loading >> > decoders first and hardware later and you'll see). With this patch, it >> > works again. >> >> Want. > > Then please help me with two things: > > a) Test the patches I just sent (especially 6/8 and 7/8, they should > be independent from the rest) Working on merging them into a tree here locally. There's been a bit of churn, so the last few didn't apply cleanly, but I'm almost there. > b) Mauro mentioned in <4bdf28c0.4060...@redhat.com> that: > > I liked the idea of your redesign, but I didn't like the removal > of a per-decoder sysfs entry. As already discussed, there are > cases where we'll need a per-decoder sysfs entry (lirc_dev is > probably one of those cases - also Jarod's imon driver is > currently implementing a modprobe parameter that needs to be > moved to the driver). > > could you please confirm if your lirc and/or imon drivers would be > negatively affected by the proposed patches? Will do so once I get them wedged in on top. -- Jarod Wilson ja...@wilsonet.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Mon, Jun 07, 2010 at 04:15:30PM -0400, Jarod Wilson wrote: > On Mon, Jun 07, 2010 at 09:00:03PM +0200, David Härdeman wrote: > > On Mon, May 03, 2010 at 05:00:05PM -0300, Mauro Carvalho Chehab wrote: > > > David Härdeman wrote: > > > > This patch moves the state from each raw decoder into the > > > > ir_raw_event_ctrl struct. > > > > > > > > This allows the removal of code like this: > > > > > > > > spin_lock(&decoder_lock); > > > > list_for_each_entry(data, &decoder_list, list) { > > > > if (data->ir_dev == ir_dev) > > > > break; > > > > } > > > > spin_unlock(&decoder_lock); > > > > return data; > > > > > > > > which is currently run for each decoder on each event in order > > > > to get the client-specific decoding state data. > > > > > > > > In addition, ir decoding modules and ir driver module load > > > > order is now independent. Centralizing the data also allows > > > > for a nice code reduction of about 30% per raw decoder as > > > > client lists and client registration callbacks are no longer > > > > necessary. > > > > > > The registration callbacks will likely still be needed by lirc, > > > as you need to create/delete lirc_dev interfaces, when the module > > > is registered, but I might be wrong. It would be interesting to > > > add lirc_dev first, in order to be sure about the better interfaces > > > for it. > > > > Or the lirc_dev patch can add whatever interfaces it needs. Anyway, the > > current interfaces are not good enough since it'll break if lirc_dev is > > loaded after the hardware modules. > > This is something I've been meaning to mention myself. On system boot, if > an mceusb device is connected, it pretty regularly only has the NEC > decoder available to use. I have to reload mceusb, or make sure ir-core is > explicitly loaded, wait a bit, then load mceusb, if I want to have all of > the protocol handlers available -- which includes the needed-by-default > rc6 one. I've only briefly tinkered with trying to fix it, sounds like you > may already have fixage within this patchset. The problem is that without the patchset, each decoder is expected to carry it's own list of datastructures for each hardware receiver. Hardware receiver addition/removal is signalled through a callback to the decoder, but the callback will (naturally) not be invoked if the hardware driver is already loaded when the decoder is. So loading a decoder "late" or reloading a decoder will mean that it doesn't know about pre-existing hardware. > > In addition, random module load order is currently broken (try loading > > decoders first and hardware later and you'll see). With this patch, it > > works again. > > Want. Then please help me with two things: a) Test the patches I just sent (especially 6/8 and 7/8, they should be independent from the rest) b) Mauro mentioned in <4bdf28c0.4060...@redhat.com> that: I liked the idea of your redesign, but I didn't like the removal of a per-decoder sysfs entry. As already discussed, there are cases where we'll need a per-decoder sysfs entry (lirc_dev is probably one of those cases - also Jarod's imon driver is currently implementing a modprobe parameter that needs to be moved to the driver). could you please confirm if your lirc and/or imon drivers would be negatively affected by the proposed patches? -- David Härdeman -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Mon, Jun 07, 2010 at 09:00:03PM +0200, David Härdeman wrote: > On Mon, May 03, 2010 at 05:00:05PM -0300, Mauro Carvalho Chehab wrote: > > David Härdeman wrote: > > > This patch moves the state from each raw decoder into the > > > ir_raw_event_ctrl struct. > > > > > > This allows the removal of code like this: > > > > > > spin_lock(&decoder_lock); > > > list_for_each_entry(data, &decoder_list, list) { > > > if (data->ir_dev == ir_dev) > > > break; > > > } > > > spin_unlock(&decoder_lock); > > > return data; > > > > > > which is currently run for each decoder on each event in order > > > to get the client-specific decoding state data. > > > > > > In addition, ir decoding modules and ir driver module load > > > order is now independent. Centralizing the data also allows > > > for a nice code reduction of about 30% per raw decoder as > > > client lists and client registration callbacks are no longer > > > necessary. > > > > The registration callbacks will likely still be needed by lirc, > > as you need to create/delete lirc_dev interfaces, when the module > > is registered, but I might be wrong. It would be interesting to > > add lirc_dev first, in order to be sure about the better interfaces > > for it. > > Or the lirc_dev patch can add whatever interfaces it needs. Anyway, the > current interfaces are not good enough since it'll break if lirc_dev is > loaded after the hardware modules. This is something I've been meaning to mention myself. On system boot, if an mceusb device is connected, it pretty regularly only has the NEC decoder available to use. I have to reload mceusb, or make sure ir-core is explicitly loaded, wait a bit, then load mceusb, if I want to have all of the protocol handlers available -- which includes the needed-by-default rc6 one. I've only briefly tinkered with trying to fix it, sounds like you may already have fixage within this patchset. ... > In addition, random module load order is currently broken (try loading > decoders first and hardware later and you'll see). With this patch, it > works again. Want. > Anyway, I'll post a new patch series this evening and then we can go > back to our regular arguing :) Hey, at least we're making progress too! :) -- Jarod Wilson ja...@redhat.com -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
On Mon, May 03, 2010 at 05:00:05PM -0300, Mauro Carvalho Chehab wrote: > David Härdeman wrote: > > This patch moves the state from each raw decoder into the > > ir_raw_event_ctrl struct. > > > > This allows the removal of code like this: > > > > spin_lock(&decoder_lock); > > list_for_each_entry(data, &decoder_list, list) { > > if (data->ir_dev == ir_dev) > > break; > > } > > spin_unlock(&decoder_lock); > > return data; > > > > which is currently run for each decoder on each event in order > > to get the client-specific decoding state data. > > > > In addition, ir decoding modules and ir driver module load > > order is now independent. Centralizing the data also allows > > for a nice code reduction of about 30% per raw decoder as > > client lists and client registration callbacks are no longer > > necessary. > > The registration callbacks will likely still be needed by lirc, > as you need to create/delete lirc_dev interfaces, when the module > is registered, but I might be wrong. It would be interesting to > add lirc_dev first, in order to be sure about the better interfaces > for it. Or the lirc_dev patch can add whatever interfaces it needs. Anyway, the current interfaces are not good enough since it'll break if lirc_dev is loaded after the hardware modules. > Also, from one side, you reduced the code size, but, on the other hand, > you've increased the memory usage, as now the protocol data will be > stored even for protocols that weren't compiled/loaded. In <4bbf3309.6020...@infradead.org>, Mauro Carvalho Chehab wrote: >> Andy Walls wrote: >>> Encoding pulse vs space with a negative sign, even if now hidden >>> with macros, is still just using a sign instead of a boolean. >>> Memory in modern computers (and now microcontrollers) is cheap and >>> only getting cheaper. Don't give up readability, flexibility, or >>> mainatainability, for the sake of saving memory. > > That was my point since the beginning: the amount of saved memory > doesn't justify the lack of readability. Are you worried about memory usage now? > Probably, the code size savings are big enough to justify the runtime > memory footprint, at least with the current decoders. Not sure how big > this will become when we add lirc_dev and other decoders that might be > missing. Right now, the "reasonable default" is a user machine with one hardware decoder and with all of the rc-core decoders loaded (cause that's how his/her distro will set it up). For that machine, the patch will save a lot of memory, not waste it (~380 lines less code...I'll assure you it's a net gain). In addition, random module load order is currently broken (try loading decoders first and hardware later and you'll see). With this patch, it works again. Anyway, I'll post a new patch series this evening and then we can go back to our regular arguing :) -- David Härdeman -- 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 3/4] ir-core: move decoding state to ir_raw_event_ctrl
David Härdeman wrote: > This patch moves the state from each raw decoder into the > ir_raw_event_ctrl struct. > > This allows the removal of code like this: > > spin_lock(&decoder_lock); > list_for_each_entry(data, &decoder_list, list) { > if (data->ir_dev == ir_dev) > break; > } > spin_unlock(&decoder_lock); > return data; > > which is currently run for each decoder on each event in order > to get the client-specific decoding state data. > > In addition, ir decoding modules and ir driver module load > order is now independent. Centralizing the data also allows > for a nice code reduction of about 30% per raw decoder as > client lists and client registration callbacks are no longer > necessary. The registration callbacks will likely still be needed by lirc, as you need to create/delete lirc_dev interfaces, when the module is registered, but I might be wrong. It would be interesting to add lirc_dev first, in order to be sure about the better interfaces for it. Also, from one side, you reduced the code size, but, on the other hand, you've increased the memory usage, as now the protocol data will be stored even for protocols that weren't compiled/loaded. Probably, the code size savings are big enough to justify the runtime memory footprint, at least with the current decoders. Not sure how big this will become when we add lirc_dev and other decoders that might be missing. > > Out-of-tree modules can still use a similar trick to what > the raw decoders did before this patch until they are merged. > > Signed-off-by: David Härdeman > --- > drivers/media/IR/ir-core-priv.h| 37 - > drivers/media/IR/ir-jvc-decoder.c | 90 ++- > drivers/media/IR/ir-nec-decoder.c | 89 +++ > drivers/media/IR/ir-raw-event.c| 48 +++-- > drivers/media/IR/ir-rc5-decoder.c | 103 > +--- > drivers/media/IR/ir-rc6-decoder.c | 92 ++-- > drivers/media/IR/ir-sony-decoder.c | 93 +++-- > 7 files changed, 87 insertions(+), 465 deletions(-) > > diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h > index 821d012..1e9464a 100644 > --- a/drivers/media/IR/ir-core-priv.h > +++ b/drivers/media/IR/ir-core-priv.h > @@ -23,8 +23,6 @@ struct ir_raw_handler { > > u64 protocols; /* which are handled by this handler */ > int (*decode)(struct input_dev *input_dev, struct ir_raw_event event); > - int (*raw_register)(struct input_dev *input_dev); > - int (*raw_unregister)(struct input_dev *input_dev); > }; > > struct ir_raw_event_ctrl { > @@ -34,6 +32,41 @@ struct ir_raw_event_ctrl { > enum raw_event_type last_type; /* last event type */ > struct input_dev*input_dev; /* pointer to the > parent input_dev */ > u64 enabled_protocols; /* enabled raw > protocol decoders */ > + > + /* raw decoder state follows */ > + struct ir_raw_event prev_ev; > + struct nec_dec { > + int state; > + unsigned count; > + u32 bits; > + } nec; > + struct rc5_dec { > + int state; > + u32 bits; > + unsigned count; > + unsigned wanted_bits; > + } rc5; > + struct rc6_dec { > + int state; > + u8 header; > + u32 body; > + bool toggle; > + unsigned count; > + unsigned wanted_bits; > + } rc6; > + struct sony_dec { > + int state; > + u32 bits; > + unsigned count; > + } sony; > + struct jvc_dec { > + int state; > + u16 bits; > + u16 old_bits; > + unsigned count; > + bool first; > + bool toggle; > + } jvc; > }; > > /* macros for IR decoders */ > diff --git a/drivers/media/IR/ir-jvc-decoder.c > b/drivers/media/IR/ir-jvc-decoder.c > index 1055de4..8894d8b 100644 > --- a/drivers/media/IR/ir-jvc-decoder.c > +++ b/drivers/media/IR/ir-jvc-decoder.c > @@ -25,10 +25,6 @@ > #define JVC_TRAILER_PULSE(1 * JVC_UNIT) > #define JVC_TRAILER_SPACE (35 * JVC_UNIT) > > -/* Used to register jvc_decoder clients */ > -static LIST_HEAD(decoder_list); > -DEFINE_SPINLOCK(decoder_lock); > - > enum jvc_state { > STATE_INACTIVE, > STATE_HEADER_SPACE, > @@ -38,39 +34,6 @@ enum jvc_state { > STATE_TRAILER_SPACE, > }; > > -struct decoder_data { > - struct list_headlist; > - struct ir_input_dev *ir_dev; > - > - /* State machine control */ > - enum jvc_state state; > - u16 jvc_bits; > - u16 jvc_old_bits; > - unsignedcount; > - boolfi
[PATCH 3/4] ir-core: move decoding state to ir_raw_event_ctrl
This patch moves the state from each raw decoder into the ir_raw_event_ctrl struct. This allows the removal of code like this: spin_lock(&decoder_lock); list_for_each_entry(data, &decoder_list, list) { if (data->ir_dev == ir_dev) break; } spin_unlock(&decoder_lock); return data; which is currently run for each decoder on each event in order to get the client-specific decoding state data. In addition, ir decoding modules and ir driver module load order is now independent. Centralizing the data also allows for a nice code reduction of about 30% per raw decoder as client lists and client registration callbacks are no longer necessary. Out-of-tree modules can still use a similar trick to what the raw decoders did before this patch until they are merged. Signed-off-by: David Härdeman --- drivers/media/IR/ir-core-priv.h| 37 - drivers/media/IR/ir-jvc-decoder.c | 90 ++- drivers/media/IR/ir-nec-decoder.c | 89 +++ drivers/media/IR/ir-raw-event.c| 48 +++-- drivers/media/IR/ir-rc5-decoder.c | 103 +--- drivers/media/IR/ir-rc6-decoder.c | 92 ++-- drivers/media/IR/ir-sony-decoder.c | 93 +++-- 7 files changed, 87 insertions(+), 465 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 821d012..1e9464a 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -23,8 +23,6 @@ struct ir_raw_handler { u64 protocols; /* which are handled by this handler */ int (*decode)(struct input_dev *input_dev, struct ir_raw_event event); - int (*raw_register)(struct input_dev *input_dev); - int (*raw_unregister)(struct input_dev *input_dev); }; struct ir_raw_event_ctrl { @@ -34,6 +32,41 @@ struct ir_raw_event_ctrl { enum raw_event_type last_type; /* last event type */ struct input_dev*input_dev; /* pointer to the parent input_dev */ u64 enabled_protocols; /* enabled raw protocol decoders */ + + /* raw decoder state follows */ + struct ir_raw_event prev_ev; + struct nec_dec { + int state; + unsigned count; + u32 bits; + } nec; + struct rc5_dec { + int state; + u32 bits; + unsigned count; + unsigned wanted_bits; + } rc5; + struct rc6_dec { + int state; + u8 header; + u32 body; + bool toggle; + unsigned count; + unsigned wanted_bits; + } rc6; + struct sony_dec { + int state; + u32 bits; + unsigned count; + } sony; + struct jvc_dec { + int state; + u16 bits; + u16 old_bits; + unsigned count; + bool first; + bool toggle; + } jvc; }; /* macros for IR decoders */ diff --git a/drivers/media/IR/ir-jvc-decoder.c b/drivers/media/IR/ir-jvc-decoder.c index 1055de4..8894d8b 100644 --- a/drivers/media/IR/ir-jvc-decoder.c +++ b/drivers/media/IR/ir-jvc-decoder.c @@ -25,10 +25,6 @@ #define JVC_TRAILER_PULSE (1 * JVC_UNIT) #defineJVC_TRAILER_SPACE (35 * JVC_UNIT) -/* Used to register jvc_decoder clients */ -static LIST_HEAD(decoder_list); -DEFINE_SPINLOCK(decoder_lock); - enum jvc_state { STATE_INACTIVE, STATE_HEADER_SPACE, @@ -38,39 +34,6 @@ enum jvc_state { STATE_TRAILER_SPACE, }; -struct decoder_data { - struct list_headlist; - struct ir_input_dev *ir_dev; - - /* State machine control */ - enum jvc_state state; - u16 jvc_bits; - u16 jvc_old_bits; - unsignedcount; - boolfirst; - booltoggle; -}; - - -/** - * get_decoder_data() - gets decoder data - * @input_dev: input device - * - * Returns the struct decoder_data that corresponds to a device - */ -static struct decoder_data *get_decoder_data(struct ir_input_dev *ir_dev) -{ - struct decoder_data *data = NULL; - - spin_lock(&decoder_lock); - list_for_each_entry(data, &decoder_list, list) { - if (data->ir_dev == ir_dev) - break; - } - spin_unlock(&decoder_lock); - return data; -} - /** * ir_jvc_decode() - Decode one JVC pulse or space * @input_dev: the struct input_dev descriptor of the device @@ -80,12 +43,8 @@ static struct decoder_data *get_decoder_data(struct ir_input_dev *ir_dev) */ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw