From c2284261113f09bca4d362f5d51c008b65f55b6a Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Sat, 29 May 2010 14:17:27 -0300 Subject: V4L/DVB: IR: let all protocol decoders have a go at raw data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Fri, May 28, 2010 at 3:59 PM, Jarod Wilson wrote: > The mceusb driver I'm about to submit handles just about any raw IR you > can throw at it. The ir-core loads up all protocol decoders, starting > with NEC, then RC5, then RC6. RUN_DECODER() was trying them in the same > order, and exiting if any of the decoders didn't like the data. The > default mceusb remote talks RC6(6A). Well, the RC6 decoder never gets a > chance to run unless you move the RC6 decoder to the front of the list. > > What I believe to be correct is to have RUN_DECODER keep trying all of > the decoders, even when one triggers an error. I don't think the errors > matter so much as it matters that at least one was successful -- i.e., > that _sumrc is > 0. The following works for me w/my mceusb driver and > the default decoder ordering -- NEC and RC5 still fail, but RC6 still > gets a crack at it, and successfully does its job. > > Signed-off-by: Jarod Wilson > > --- >  drivers/media/IR/ir-raw-event.c |    7 ++++--- > > diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c > index ea68a3f..44162db 100644 > --- a/drivers/media/IR/ir-raw-event.c > +++ b/drivers/media/IR/ir-raw-event.c > @@ -36,14 +36,15 @@ static DEFINE_SPINLOCK(ir_raw_handler_lock); >  */ >  #define RUN_DECODER(ops, ...) ({                                           \ >        struct ir_raw_handler           *_ir_raw_handler;                   \ > -       int _sumrc = 0, _rc;                                                \ > +       int _sumrc = 0, _rc, _fail;                                         \ >        spin_lock(&ir_raw_handler_lock);                                    \ >        list_for_each_entry(_ir_raw_handler, &ir_raw_handler_list, list) {  \ >                if (_ir_raw_handler->ops) {                                 \ >                        _rc = _ir_raw_handler->ops(__VA_ARGS__);            \ >                        if (_rc < 0)                                        \ > -                               break;                                      \ > -                       _sumrc += _rc;                                      \ > +                               _fail++;                                    \ > +                       else                                                \ > +                               _sumrc += _rc;                              \ Self-NAK. The only place we actually *care* about the retval from a RUN_DECODER() call is in __ir_input_register(), and currently, its looking for retval < 0, which is currently never possible. When we're running the decoders, either they fail and return -EINVAL or they succeed and return 0, and in the register case, we get either a negative error (ex: -ENOMEM from rc6) or 0, so with the above, _sumrc will *always* be 0 in the two cases I'm looking at. The third place where RUN_DECODER gets called (decoder unregister) doesn't care about the retval either. New patch below, including updated comments about the macro. Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/IR/ir-raw-event.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/media/IR/ir-raw-event.c') diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index ea68a3f2effa..d3bd3f98e008 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -31,8 +31,9 @@ static DEFINE_SPINLOCK(ir_raw_handler_lock); * * Calls ir_raw_handler::ops for all registered IR handlers. It prevents * new decode addition/removal while running, by locking ir_raw_handler_lock - * mutex. If an error occurs, it stops the ops. Otherwise, it returns a sum - * of the return codes. + * mutex. If an error occurs, we keep going, as in the decode case, each + * decoder must have a crack at decoding the data. We return a sum of the + * return codes, which will be either 0 or negative for current callers. */ #define RUN_DECODER(ops, ...) ({ \ struct ir_raw_handler *_ir_raw_handler; \ @@ -41,8 +42,6 @@ static DEFINE_SPINLOCK(ir_raw_handler_lock); list_for_each_entry(_ir_raw_handler, &ir_raw_handler_list, list) { \ if (_ir_raw_handler->ops) { \ _rc = _ir_raw_handler->ops(__VA_ARGS__); \ - if (_rc < 0) \ - break; \ _sumrc += _rc; \ } \ } \ -- cgit v1.2.1 From 667c9ebe97f7e5f1e48e7eb321644c6fb1668de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20H=C3=83=C2=A4rdeman?= Date: Sun, 13 Jun 2010 17:29:31 -0300 Subject: V4L/DVB: ir-core: centralize sysfs raw decoder enabling/disabling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the current logic, each raw decoder needs to add a copy of the exact same sysfs code. This is both unnecessary and also means that (re)loading an IR driver after raw decoder modules have been loaded won't work as expected. This patch moves that logic into ir-raw-event and adds a single sysfs file per device. Reading that file returns something like: "rc5 [rc6] nec jvc [sony]" (with enabled protocols in [] brackets) Writing either "+protocol" or "-protocol" to that file will enable or disable the according protocol decoder. An additional benefit is that the disabling of a decoder will be remembered across module removal/insertion so a previously disabled decoder won't suddenly be activated again. The default setting is to enable all decoders. This is also necessary for the next patch which moves even more decoder state into the central raw decoding structs. Signed-off-by: David Härdeman Acked-by: Jarod Wilson Tested-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/IR/ir-raw-event.c | 112 +++++++++++++++++++++++----------------- 1 file changed, 65 insertions(+), 47 deletions(-) (limited to 'drivers/media/IR/ir-raw-event.c') diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index d3bd3f98e008..fb3336c37191 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -21,8 +21,9 @@ #define MAX_IR_EVENT_SIZE 512 /* Used to handle IR raw handler extensions */ -static LIST_HEAD(ir_raw_handler_list); static DEFINE_SPINLOCK(ir_raw_handler_lock); +static LIST_HEAD(ir_raw_handler_list); +static u64 available_protocols; /** * RUN_DECODER() - runs an operation on all IR decoders @@ -64,52 +65,6 @@ static void ir_raw_event_work(struct work_struct *work) RUN_DECODER(decode, raw->input_dev, ev); } -int ir_raw_event_register(struct input_dev *input_dev) -{ - struct ir_input_dev *ir = input_get_drvdata(input_dev); - int rc; - - ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL); - if (!ir->raw) - return -ENOMEM; - - ir->raw->input_dev = input_dev; - INIT_WORK(&ir->raw->rx_work, ir_raw_event_work); - - rc = kfifo_alloc(&ir->raw->kfifo, sizeof(s64) * MAX_IR_EVENT_SIZE, - GFP_KERNEL); - if (rc < 0) { - kfree(ir->raw); - ir->raw = NULL; - return rc; - } - - rc = RUN_DECODER(raw_register, input_dev); - if (rc < 0) { - kfifo_free(&ir->raw->kfifo); - kfree(ir->raw); - ir->raw = NULL; - return rc; - } - - return rc; -} - -void ir_raw_event_unregister(struct input_dev *input_dev) -{ - struct ir_input_dev *ir = input_get_drvdata(input_dev); - - if (!ir->raw) - return; - - cancel_work_sync(&ir->raw->rx_work); - RUN_DECODER(raw_unregister, input_dev); - - kfifo_free(&ir->raw->kfifo); - kfree(ir->raw); - ir->raw = NULL; -} - /** * ir_raw_event_store() - pass a pulse/space duration to the raw ir decoders * @input_dev: the struct input_dev device descriptor @@ -203,6 +158,66 @@ void ir_raw_event_handle(struct input_dev *input_dev) } EXPORT_SYMBOL_GPL(ir_raw_event_handle); +/* used internally by the sysfs interface */ +u64 +ir_raw_get_allowed_protocols() +{ + u64 protocols; + spin_lock(&ir_raw_handler_lock); + protocols = available_protocols; + spin_unlock(&ir_raw_handler_lock); + return protocols; +} + +/* + * Used to (un)register raw event clients + */ +int ir_raw_event_register(struct input_dev *input_dev) +{ + struct ir_input_dev *ir = input_get_drvdata(input_dev); + int rc; + + ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL); + if (!ir->raw) + return -ENOMEM; + + ir->raw->input_dev = input_dev; + INIT_WORK(&ir->raw->rx_work, ir_raw_event_work); + ir->raw->enabled_protocols = ~0; + rc = kfifo_alloc(&ir->raw->kfifo, sizeof(s64) * MAX_IR_EVENT_SIZE, + GFP_KERNEL); + if (rc < 0) { + kfree(ir->raw); + ir->raw = NULL; + return rc; + } + + rc = RUN_DECODER(raw_register, input_dev); + if (rc < 0) { + kfifo_free(&ir->raw->kfifo); + kfree(ir->raw); + ir->raw = NULL; + return rc; + } + + return rc; +} + +void ir_raw_event_unregister(struct input_dev *input_dev) +{ + struct ir_input_dev *ir = input_get_drvdata(input_dev); + + if (!ir->raw) + return; + + cancel_work_sync(&ir->raw->rx_work); + RUN_DECODER(raw_unregister, input_dev); + + kfifo_free(&ir->raw->kfifo); + kfree(ir->raw); + ir->raw = NULL; +} + /* * Extension interface - used to register the IR decoders */ @@ -211,7 +226,9 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler) { spin_lock(&ir_raw_handler_lock); list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list); + available_protocols |= ir_raw_handler->protocols; spin_unlock(&ir_raw_handler_lock); + return 0; } EXPORT_SYMBOL(ir_raw_handler_register); @@ -220,6 +237,7 @@ void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler) { spin_lock(&ir_raw_handler_lock); list_del(&ir_raw_handler->list); + available_protocols &= ~ir_raw_handler->protocols; spin_unlock(&ir_raw_handler_lock); } EXPORT_SYMBOL(ir_raw_handler_unregister); -- cgit v1.2.1 From c216369e61fae586cd48c0913cca2a37fbfeb912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20H=C3=83=C2=A4rdeman?= Date: Sun, 13 Jun 2010 17:29:36 -0300 Subject: V4L/DVB: ir-core: move decoding state to ir_raw_event_ctrl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (but still kept around for the benefit of the lirc decoder). 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 Acked-by: Jarod Wilson Tested-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/IR/ir-raw-event.c | 73 +++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 36 deletions(-) (limited to 'drivers/media/IR/ir-raw-event.c') diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index fb3336c37191..5f98ab823057 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -20,36 +20,14 @@ /* Define the max number of pulse/space transitions to buffer */ #define MAX_IR_EVENT_SIZE 512 +/* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */ +static LIST_HEAD(ir_raw_client_list); + /* Used to handle IR raw handler extensions */ static DEFINE_SPINLOCK(ir_raw_handler_lock); static LIST_HEAD(ir_raw_handler_list); static u64 available_protocols; -/** - * RUN_DECODER() - runs an operation on all IR decoders - * @ops: IR raw handler operation to be called - * @arg: arguments to be passed to the callback - * - * Calls ir_raw_handler::ops for all registered IR handlers. It prevents - * new decode addition/removal while running, by locking ir_raw_handler_lock - * mutex. If an error occurs, we keep going, as in the decode case, each - * decoder must have a crack at decoding the data. We return a sum of the - * return codes, which will be either 0 or negative for current callers. - */ -#define RUN_DECODER(ops, ...) ({ \ - struct ir_raw_handler *_ir_raw_handler; \ - int _sumrc = 0, _rc; \ - spin_lock(&ir_raw_handler_lock); \ - list_for_each_entry(_ir_raw_handler, &ir_raw_handler_list, list) { \ - if (_ir_raw_handler->ops) { \ - _rc = _ir_raw_handler->ops(__VA_ARGS__); \ - _sumrc += _rc; \ - } \ - } \ - spin_unlock(&ir_raw_handler_lock); \ - _sumrc; \ -}) - #ifdef MODULE /* Used to load the decoders */ static struct work_struct wq_load; @@ -58,11 +36,17 @@ static struct work_struct wq_load; static void ir_raw_event_work(struct work_struct *work) { struct ir_raw_event ev; + struct ir_raw_handler *handler; struct ir_raw_event_ctrl *raw = container_of(work, struct ir_raw_event_ctrl, rx_work); - while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) - RUN_DECODER(decode, raw->input_dev, ev); + while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) { + spin_lock(&ir_raw_handler_lock); + list_for_each_entry(handler, &ir_raw_handler_list, list) + handler->decode(raw->input_dev, ev); + spin_unlock(&ir_raw_handler_lock); + raw->prev_ev = ev; + } } /** @@ -176,6 +160,7 @@ int ir_raw_event_register(struct input_dev *input_dev) { struct ir_input_dev *ir = input_get_drvdata(input_dev); int rc; + struct ir_raw_handler *handler; ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL); if (!ir->raw) @@ -192,26 +177,32 @@ int ir_raw_event_register(struct input_dev *input_dev) return rc; } - rc = RUN_DECODER(raw_register, input_dev); - if (rc < 0) { - kfifo_free(&ir->raw->kfifo); - kfree(ir->raw); - ir->raw = NULL; - return rc; - } + spin_lock(&ir_raw_handler_lock); + list_add_tail(&ir->raw->list, &ir_raw_client_list); + list_for_each_entry(handler, &ir_raw_handler_list, list) + if (handler->raw_register) + handler->raw_register(ir->raw->input_dev); + spin_unlock(&ir_raw_handler_lock); - return rc; + return 0; } void ir_raw_event_unregister(struct input_dev *input_dev) { struct ir_input_dev *ir = input_get_drvdata(input_dev); + struct ir_raw_handler *handler; if (!ir->raw) return; cancel_work_sync(&ir->raw->rx_work); - RUN_DECODER(raw_unregister, input_dev); + + spin_lock(&ir_raw_handler_lock); + list_del(&ir->raw->list); + list_for_each_entry(handler, &ir_raw_handler_list, list) + if (handler->raw_unregister) + handler->raw_unregister(ir->raw->input_dev); + spin_unlock(&ir_raw_handler_lock); kfifo_free(&ir->raw->kfifo); kfree(ir->raw); @@ -224,8 +215,13 @@ void ir_raw_event_unregister(struct input_dev *input_dev) int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler) { + struct ir_raw_event_ctrl *raw; + spin_lock(&ir_raw_handler_lock); list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list); + if (ir_raw_handler->raw_register) + list_for_each_entry(raw, &ir_raw_client_list, list) + ir_raw_handler->raw_register(raw->input_dev); available_protocols |= ir_raw_handler->protocols; spin_unlock(&ir_raw_handler_lock); @@ -235,8 +231,13 @@ EXPORT_SYMBOL(ir_raw_handler_register); void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler) { + struct ir_raw_event_ctrl *raw; + spin_lock(&ir_raw_handler_lock); list_del(&ir_raw_handler->list); + if (ir_raw_handler->raw_unregister) + list_for_each_entry(raw, &ir_raw_client_list, list) + ir_raw_handler->raw_unregister(raw->input_dev); available_protocols &= ~ir_raw_handler->protocols; spin_unlock(&ir_raw_handler_lock); } -- cgit v1.2.1 From ca4146985db7cbb97816e9b961b8db79e63d9e86 Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Sat, 3 Jul 2010 01:07:53 -0300 Subject: V4L/DVB: IR: add ir-core to lirc userspace decoder bridge driver v2: copy of buffer data from userspace done inside this plugin/driver, keeping the actual drivers minimal, and more flexible in what we can deliver to them later on (they may be fed from within kernelspace later on, by an in-kernel IR encoder). Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/IR/ir-raw-event.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/media/IR/ir-raw-event.c') diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index 5f98ab823057..6f192ef31db1 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -253,6 +253,7 @@ static void init_decoders(struct work_struct *work) load_rc6_decode(); load_jvc_decode(); load_sony_decode(); + load_lirc_codec(); /* If needed, we may later add some init code. In this case, it is needed to change the CONFIG_MODULE test at ir-core.h -- cgit v1.2.1