| Message ID | 1391861250-26068-3-git-send-email-a.seppala@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Antti, On 08/02/14 12:07, Antti Seppälä wrote: > The encoding in rc5-sz first inserts a pulse and then simply utilizes the > generic Manchester encoder available in rc-core. > > Signed-off-by: Antti Seppälä <a.seppala@gmail.com> > --- > drivers/media/rc/ir-rc5-sz-decoder.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c > index 984e5b9..0d5e552 100644 > --- a/drivers/media/rc/ir-rc5-sz-decoder.c > +++ b/drivers/media/rc/ir-rc5-sz-decoder.c > @@ -127,9 +127,44 @@ out: > return -EINVAL; > } > > +static struct ir_raw_timings_manchester ir_rc5_sz_timings = { > + .pulse_space_start = 0, > + .clock = RC5_UNIT, > +}; > + > +/* > + * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events > + * > + * @protocols: allowed protocols > + * @scancode: scancode filter describing scancode (helps distinguish between > + * protocol subtypes when scancode is ambiguous) > + * @events: array of raw ir events to write into > + * @max: maximum size of @events > + * > + * This function returns -EINVAL if the scancode filter is invalid or matches > + * multiple scancodes. Otherwise the number of ir_raw_events generated is > + * returned. > + */ > +static int ir_rc5_sz_encode(u64 protocols, > + const struct rc_scancode_filter *scancode, > + struct ir_raw_event *events, unsigned int max) > +{ > + int ret; > + struct ir_raw_event *e = events; Probably worth checking scancode->mask == 0xfff too? > + > + /* RC5-SZ scancode is raw enough for manchester as it is */ > + ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS, > + scancode->data); > + if (ret < 0) > + return ret; I suspect it needs some more space at the end too, to be sure that no more bits afterwards are accepted. > + > + return e - events; > +} > + > static struct ir_raw_handler rc5_sz_handler = { > .protocols = RC_BIT_RC5_SZ, > .decode = ir_rc5_sz_decode, > + .encode = ir_rc5_sz_encode, > }; > > static int __init ir_rc5_sz_decode_init(void) > Cheers James -- 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
Hi James. On 10 February 2014 12:30, James Hogan <james.hogan@imgtec.com> wrote: > Hi Antti, > > On 08/02/14 12:07, Antti Seppälä wrote: >> The encoding in rc5-sz first inserts a pulse and then simply utilizes the >> generic Manchester encoder available in rc-core. >> >> Signed-off-by: Antti Seppälä <a.seppala@gmail.com> >> --- >> drivers/media/rc/ir-rc5-sz-decoder.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c >> index 984e5b9..0d5e552 100644 >> --- a/drivers/media/rc/ir-rc5-sz-decoder.c >> +++ b/drivers/media/rc/ir-rc5-sz-decoder.c >> @@ -127,9 +127,44 @@ out: >> return -EINVAL; >> } >> >> +static struct ir_raw_timings_manchester ir_rc5_sz_timings = { >> + .pulse_space_start = 0, >> + .clock = RC5_UNIT, >> +}; >> + >> +/* >> + * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events >> + * >> + * @protocols: allowed protocols >> + * @scancode: scancode filter describing scancode (helps distinguish between >> + * protocol subtypes when scancode is ambiguous) >> + * @events: array of raw ir events to write into >> + * @max: maximum size of @events >> + * >> + * This function returns -EINVAL if the scancode filter is invalid or matches >> + * multiple scancodes. Otherwise the number of ir_raw_events generated is >> + * returned. >> + */ >> +static int ir_rc5_sz_encode(u64 protocols, >> + const struct rc_scancode_filter *scancode, >> + struct ir_raw_event *events, unsigned int max) >> +{ >> + int ret; >> + struct ir_raw_event *e = events; > > Probably worth checking scancode->mask == 0xfff too? > I guess so. However if I'm not mistaken this makes all wakeup_filter writes fail in user space if wakeup_filter_mask is not set. Is that intended? >> + >> + /* RC5-SZ scancode is raw enough for manchester as it is */ >> + ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS, >> + scancode->data); >> + if (ret < 0) >> + return ret; > > I suspect it needs some more space at the end too, to be sure that no > more bits afterwards are accepted. > I'm sorry but I'm not sure I completely understood what you meant here. For RC-5-SZ the entire scancode gets encoded and nothing more. Do you mean that the encoder should append some ir silence to the end result to make sure the ir sample has ended? -Antti -- 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
Hi Antti, On Monday 10 February 2014 22:09:33 Antti Seppälä wrote: > >> +static int ir_rc5_sz_encode(u64 protocols, > >> + const struct rc_scancode_filter *scancode, > >> + struct ir_raw_event *events, unsigned int max) > >> +{ > >> + int ret; > >> + struct ir_raw_event *e = events; > > > > Probably worth checking scancode->mask == 0xfff too? > > I guess so. However if I'm not mistaken this makes all wakeup_filter > writes fail in user space if wakeup_filter_mask is not set. Is that > intended? Good point, although looking at your patch 3, mask==0 is already permitted silently by the driver, which I think would make it okay. I guess to be safe userland would have to do: wakeup_filter_mask = 0 wakeup_filter = $value wakeup_filter_mask = 0xfff which doesn't sound unreasonable in the absence of a way to update them atomically (sysfs files doing more than one thing is frowned upon I believe). > >> + /* RC5-SZ scancode is raw enough for manchester as it is */ > >> + ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, > >> RC5_SZ_NBITS, + scancode->data); > >> + if (ret < 0) > >> + return ret; > > > > I suspect it needs some more space at the end too, to be sure that no > > more bits afterwards are accepted. > > I'm sorry but I'm not sure I completely understood what you meant > here. For RC-5-SZ the entire scancode gets encoded and nothing more. > Do you mean that the encoder should append some ir silence to the end > result to make sure the ir sample has ended? Yeh something like that. Certainly the raw decoders I've looked at expect a certain amount of space at the end to avoid decoding part of a longer protocol (it's in the pulse distance helper as the trailer space timing). Similarly the IMG hardware decoder has register fields for the free-time to require at the end of the message. In fact it becomes a bit awkward for the raw IR driver for the IMG hardware which uses edge interrupts, as it has to have a timeout to emit a final repeat event after 150ms of inactivity, in order for the raw decoders to accept it (unless you hold the button down in which case the repeat code edges result in the long space). Cheers James
On 10 February 2014 22:50, James Hogan <james.hogan@imgtec.com> wrote: >> > I suspect it needs some more space at the end too, to be sure that no >> > more bits afterwards are accepted. >> >> I'm sorry but I'm not sure I completely understood what you meant >> here. For RC-5-SZ the entire scancode gets encoded and nothing more. >> Do you mean that the encoder should append some ir silence to the end >> result to make sure the ir sample has ended? > > Yeh something like that. Certainly the raw decoders I've looked at expect a > certain amount of space at the end to avoid decoding part of a longer protocol > (it's in the pulse distance helper as the trailer space timing). Similarly the > IMG hardware decoder has register fields for the free-time to require at the > end of the message. > > In fact it becomes a bit awkward for the raw IR driver for the IMG hardware > which uses edge interrupts, as it has to have a timeout to emit a final repeat > event after 150ms of inactivity, in order for the raw decoders to accept it > (unless you hold the button down in which case the repeat code edges result in > the long space). > Ok, I understand now. I suppose I can append some IR silence to the encoded result. The trailer space timing seems like a good way to do it. I'll create new version of my patches sometime later. Are you working on the wakeup protocol selector sysfs interface? -Antti -- 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
This patchset is an improved version of an earlier attempt to add RC5-SZ encoder to ir-core. The set is based on a series posted by James Hogan (rc: ir-raw: Add encode, implement NEC encode). This set extends the series by adding a generic RC-5 encoder and adds support for it to RC-5-SZ protocol. In addition nuvoton-cir driver is modified to read wakeup filters from sysfs and utilize encoding to convert the scancodes to format understood by the underlying hardware. Changes in v2: - (Hopefully) fixed all buffer indexing issues - Write samples to fifo even if if encoded result won't completely fit - Check filter mask in encoder - Append some trailing ir silence to the encoded result - Some cleanups and readability improvements Antti Seppälä (3): rc-core: Add Manchester encoder (phase encoder) support to rc-core ir-rc5-sz: Add ir encoding support nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback drivers/media/rc/ir-raw.c | 56 +++++++++++++++++ drivers/media/rc/ir-rc5-sz-decoder.c | 39 ++++++++++++ drivers/media/rc/nuvoton-cir.c | 119 +++++++++++++++++++++++++++++++++++ drivers/media/rc/nuvoton-cir.h | 1 + drivers/media/rc/rc-core-priv.h | 16 +++++ include/media/rc-core.h | 1 + 6 files changed, 232 insertions(+)
diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c index 984e5b9..0d5e552 100644 --- a/drivers/media/rc/ir-rc5-sz-decoder.c +++ b/drivers/media/rc/ir-rc5-sz-decoder.c @@ -127,9 +127,44 @@ out: return -EINVAL; } +static struct ir_raw_timings_manchester ir_rc5_sz_timings = { + .pulse_space_start = 0, + .clock = RC5_UNIT, +}; + +/* + * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events + * + * @protocols: allowed protocols + * @scancode: scancode filter describing scancode (helps distinguish between + * protocol subtypes when scancode is ambiguous) + * @events: array of raw ir events to write into + * @max: maximum size of @events + * + * This function returns -EINVAL if the scancode filter is invalid or matches + * multiple scancodes. Otherwise the number of ir_raw_events generated is + * returned. + */ +static int ir_rc5_sz_encode(u64 protocols, + const struct rc_scancode_filter *scancode, + struct ir_raw_event *events, unsigned int max) +{ + int ret; + struct ir_raw_event *e = events; + + /* RC5-SZ scancode is raw enough for manchester as it is */ + ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS, + scancode->data); + if (ret < 0) + return ret; + + return e - events; +} + static struct ir_raw_handler rc5_sz_handler = { .protocols = RC_BIT_RC5_SZ, .decode = ir_rc5_sz_decode, + .encode = ir_rc5_sz_encode, }; static int __init ir_rc5_sz_decode_init(void)
The encoding in rc5-sz first inserts a pulse and then simply utilizes the generic Manchester encoder available in rc-core. Signed-off-by: Antti Seppälä <a.seppala@gmail.com> --- drivers/media/rc/ir-rc5-sz-decoder.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)