* [RFC PATCH] [media] rc: filter out not allowed protocols when decoding @ 2012-09-01 1:57 Du, Changbin 2012-09-03 13:03 ` Sean Young 0 siblings, 1 reply; 5+ messages in thread From: Du, Changbin @ 2012-09-01 1:57 UTC (permalink / raw) To: mchehab Cc: paul.gortmaker, sfr, srinivas.kandagatla, linux-media, linux-kernel, Du, Changbin From: "Du, Changbin" <changbin.du@gmail.com> Each rc-raw device has a property "allowed_protos" stored in structure ir_raw_event_ctrl. But it didn't work because all decoders would be called when decoding. This path makes only allowed protocol decoders been invoked. Signed-off-by: Du, Changbin <changbin.du@gmail.com> --- drivers/media/rc/ir-raw.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c index a820251..198b6d8 100644 --- a/drivers/media/rc/ir-raw.c +++ b/drivers/media/rc/ir-raw.c @@ -63,8 +63,12 @@ static int ir_raw_event_thread(void *data) spin_unlock_irq(&raw->lock); mutex_lock(&ir_raw_handler_lock); - list_for_each_entry(handler, &ir_raw_handler_list, list) - handler->decode(raw->dev, ev); + list_for_each_entry(handler, &ir_raw_handler_list, list) { + /* use all protocol by default */ + if (raw->dev->allowed_protos == RC_TYPE_UNKNOWN || + raw->dev->allowed_protos & handler->protocols) + handler->decode(raw->dev, ev); + } raw->prev_ev = ev; mutex_unlock(&ir_raw_handler_lock); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] [media] rc: filter out not allowed protocols when decoding 2012-09-01 1:57 [RFC PATCH] [media] rc: filter out not allowed protocols when decoding Du, Changbin @ 2012-09-03 13:03 ` Sean Young [not found] ` <CABgQ-ThYGdvhmpf+=GcLpE-qFAhrDUc1j07+XqohDNRa9bStiw@mail.gmail.com> 0 siblings, 1 reply; 5+ messages in thread From: Sean Young @ 2012-09-03 13:03 UTC (permalink / raw) To: Du, Changbin Cc: mchehab, paul.gortmaker, sfr, srinivas.kandagatla, linux-media, linux-kernel On Sat, Sep 01, 2012 at 09:57:09AM +0800, Du, Changbin wrote: > From: "Du, Changbin" <changbin.du@gmail.com> > > Each rc-raw device has a property "allowed_protos" stored in structure > ir_raw_event_ctrl. But it didn't work because all decoders would be > called when decoding. This path makes only allowed protocol decoders > been invoked. > > Signed-off-by: Du, Changbin <changbin.du@gmail.com> > --- > drivers/media/rc/ir-raw.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c > index a820251..198b6d8 100644 > --- a/drivers/media/rc/ir-raw.c > +++ b/drivers/media/rc/ir-raw.c > @@ -63,8 +63,12 @@ static int ir_raw_event_thread(void *data) > spin_unlock_irq(&raw->lock); > > mutex_lock(&ir_raw_handler_lock); > - list_for_each_entry(handler, &ir_raw_handler_list, list) > - handler->decode(raw->dev, ev); > + list_for_each_entry(handler, &ir_raw_handler_list, list) { > + /* use all protocol by default */ > + if (raw->dev->allowed_protos == RC_TYPE_UNKNOWN || > + raw->dev->allowed_protos & handler->protocols) > + handler->decode(raw->dev, ev); > + } Each IR protocol decoder already checks whether it is enabled or not; should it not be so that only allowed protocols can be enabled rather than checking both enabled_protocols and allowed_protocols? Just from reading store_protocols it looks like decoders which aren't in allowed_protocols can be enabled, which makes no sense. Also ir_raw_event_register all protocols are enabled rather than the allowed ones. Lastely I don't know why raw ir drivers should dictate which protocols can be enabled. Would it not be better to remove it entirely? > raw->prev_ev = ev; > mutex_unlock(&ir_raw_handler_lock); > } > -- > 1.7.9.5 > > -- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <CABgQ-ThYGdvhmpf+=GcLpE-qFAhrDUc1j07+XqohDNRa9bStiw@mail.gmail.com>]
* Re: [RFC PATCH] [media] rc: filter out not allowed protocols when decoding [not found] ` <CABgQ-ThYGdvhmpf+=GcLpE-qFAhrDUc1j07+XqohDNRa9bStiw@mail.gmail.com> @ 2012-09-04 3:06 ` Changbin Du 2012-09-04 12:21 ` Sean Young 0 siblings, 1 reply; 5+ messages in thread From: Changbin Du @ 2012-09-04 3:06 UTC (permalink / raw) To: sean Cc: mchehab, paul.gortmaker, sfr, srinivas.kandagatla, linux-media, linux-kernel > > mutex_lock(&ir_raw_handler_lock); > > - list_for_each_entry(handler, &ir_raw_handler_list, list) > > - handler->decode(raw->dev, ev); > > + list_for_each_entry(handler, &ir_raw_handler_list, list) { > > + /* use all protocol by default */ > > + if (raw->dev->allowed_protos == RC_TYPE_UNKNOWN || > > + raw->dev->allowed_protos & handler->protocols) > > + handler->decode(raw->dev, ev); > > + } > > Each IR protocol decoder already checks whether it is enabled or not; > should it not be so that only allowed protocols can be enabled rather > than checking both enabled_protocols and allowed_protocols? > > Just from reading store_protocols it looks like decoders which aren't > in allowed_protocols can be enabled, which makes no sense. Also > ir_raw_event_register all protocols are enabled rather than the > allowed ones. > > > Lastely I don't know why raw ir drivers should dictate which protocols > can be enabled. Would it not be better to remove it entirely? I agree with you. I just thought that the only thing a decoder should care is its decoding logic, but not including decoder management. My idaea is: 1) use enabled_protocols to select decoders in ir_raw.c, but not placed in decoders to do the judgement. 2) remove allowed_protocols or just use it to set the default decoder (also should rename allowed_protocols to default_protocol). I also have a question: Is there a requirement that one more decoders are enabled for a IR device at the same time? And if that will lead to a issue that each decoder may decode a same pulse sequence to different evnets since their protocol is different? [Du, Changbin] > > > > raw->prev_ev = ev; > > mutex_unlock(&ir_raw_handler_lock); > > } > > -- > > 1.7.9.5 > > > > -- > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] [media] rc: filter out not allowed protocols when decoding 2012-09-04 3:06 ` Changbin Du @ 2012-09-04 12:21 ` Sean Young 2012-09-06 9:36 ` Changbin Du 0 siblings, 1 reply; 5+ messages in thread From: Sean Young @ 2012-09-04 12:21 UTC (permalink / raw) To: Changbin Du Cc: mchehab, paul.gortmaker, sfr, srinivas.kandagatla, linux-media, linux-kernel On Tue, Sep 04, 2012 at 11:06:07AM +0800, Changbin Du wrote: > > > mutex_lock(&ir_raw_handler_lock); > > > - list_for_each_entry(handler, &ir_raw_handler_list, list) > > > - handler->decode(raw->dev, ev); > > > + list_for_each_entry(handler, &ir_raw_handler_list, list) { > > > + /* use all protocol by default */ > > > + if (raw->dev->allowed_protos == RC_TYPE_UNKNOWN || > > > + raw->dev->allowed_protos & handler->protocols) > > > + handler->decode(raw->dev, ev); > > > + } > > > > Each IR protocol decoder already checks whether it is enabled or not; > > should it not be so that only allowed protocols can be enabled rather > > than checking both enabled_protocols and allowed_protocols? > > > > Just from reading store_protocols it looks like decoders which aren't > > in allowed_protocols can be enabled, which makes no sense. Also > > ir_raw_event_register all protocols are enabled rather than the > > allowed ones. > > > > > > Lastely I don't know why raw ir drivers should dictate which protocols > > can be enabled. Would it not be better to remove it entirely? > > > I agree with you. I just thought that the only thing a decoder should care > is its decoding logic, but not including decoder management. My idaea is: > 1) use enabled_protocols to select decoders in ir_raw.c, but not > placed in decoders to do the judgement. > 2) remove allowed_protocols or just use it to set the default > decoder (also should rename allowed_protocols to default_protocol). The default decoder should be the one set by the rc keymap. > I also have a question: > Is there a requirement that one more decoders are enabled for a > IR device at the same time? Yes, you want to be able to multiple remotes on the IR device (which you can do as long as the scancodes don't overlap, I think), and the lirc device is implemented as a decoder, so you might want to see the raw IR as well as have it decoded. > And if that will lead to a issue that each decoder may decode a > same pulse sequence to different evnets since their protocol is > different? At the moment, no. David Hardeman has sent a patch for this: http://patchwork.linuxtv.org/patch/11388/ Sean ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] [media] rc: filter out not allowed protocols when decoding 2012-09-04 12:21 ` Sean Young @ 2012-09-06 9:36 ` Changbin Du 0 siblings, 0 replies; 5+ messages in thread From: Changbin Du @ 2012-09-06 9:36 UTC (permalink / raw) To: Sean Young Cc: mchehab, paul.gortmaker, sfr, srinivas.kandagatla, linux-media, linux-kernel Sean , many thanks for your help. I know much more about IR framwork now. I'll try to work out a patch to remove "allowed_protocols". Thanks again! [Du, Changbin] 2012/9/4 Sean Young <sean@mess.org>: > On Tue, Sep 04, 2012 at 11:06:07AM +0800, Changbin Du wrote: >> > > mutex_lock(&ir_raw_handler_lock); >> > > - list_for_each_entry(handler, &ir_raw_handler_list, list) >> > > - handler->decode(raw->dev, ev); >> > > + list_for_each_entry(handler, &ir_raw_handler_list, list) { >> > > + /* use all protocol by default */ >> > > + if (raw->dev->allowed_protos == RC_TYPE_UNKNOWN || >> > > + raw->dev->allowed_protos & handler->protocols) >> > > + handler->decode(raw->dev, ev); >> > > + } >> > >> > Each IR protocol decoder already checks whether it is enabled or not; >> > should it not be so that only allowed protocols can be enabled rather >> > than checking both enabled_protocols and allowed_protocols? >> > >> > Just from reading store_protocols it looks like decoders which aren't >> > in allowed_protocols can be enabled, which makes no sense. Also >> > ir_raw_event_register all protocols are enabled rather than the >> > allowed ones. >> > >> > >> > Lastely I don't know why raw ir drivers should dictate which protocols >> > can be enabled. Would it not be better to remove it entirely? >> >> >> I agree with you. I just thought that the only thing a decoder should care >> is its decoding logic, but not including decoder management. My idaea is: >> 1) use enabled_protocols to select decoders in ir_raw.c, but not >> placed in decoders to do the judgement. >> 2) remove allowed_protocols or just use it to set the default >> decoder (also should rename allowed_protocols to default_protocol). > > The default decoder should be the one set by the rc keymap. > >> I also have a question: >> Is there a requirement that one more decoders are enabled for a >> IR device at the same time? > > Yes, you want to be able to multiple remotes on the IR device (which > you can do as long as the scancodes don't overlap, I think), and the > lirc device is implemented as a decoder, so you might want to see the > raw IR as well as have it decoded. > >> And if that will lead to a issue that each decoder may decode a >> same pulse sequence to different evnets since their protocol is >> different? > > At the moment, no. David Hardeman has sent a patch for this: > > http://patchwork.linuxtv.org/patch/11388/ > > > Sean ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-06 9:36 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-01 1:57 [RFC PATCH] [media] rc: filter out not allowed protocols when decoding Du, Changbin 2012-09-03 13:03 ` Sean Young [not found] ` <CABgQ-ThYGdvhmpf+=GcLpE-qFAhrDUc1j07+XqohDNRa9bStiw@mail.gmail.com> 2012-09-04 3:06 ` Changbin Du 2012-09-04 12:21 ` Sean Young 2012-09-06 9:36 ` Changbin Du
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).