Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
On Sunday 16 February 2014 19:04:01 Antti Seppälä wrote: > On 12 February 2014 01:39, James Hogan wrote: > > On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote: > >> Are you working on the wakeup protocol selector sysfs interface? > > > > I gave it a try yesterday, but it's a bit of a work in progress at the > > moment. It's also a bit more effort for img-ir to work properly with it, > > so I'd probably just limit the allowed wakeup protocols to the enabled > > normal protocol at first in img-ir. > > > > Here's what I have (hopefully kmail won't corrupt it), feel free to take > > and improve/fix it. I'm not keen on the invasiveness of the > > allowed_protos/enabled_protocols change (which isn't complete), but it > > should probably be abstracted at some point anyway. > > In general the approach here looks good. At least I couldn't figure > any easy way to be less intrusive towards drivers/decoders and still > support wakeup filters. Thanks for taking a look (and sorry for the delay getting back to this, holiday and sickness got in the way). FYI I've cleaned up my wakeup_protocols patches a lot and will probably post tomorrow (after rebasing my patches to allow me to test img-ir properly with it). Cheers James signature.asc Description: This is a digitally signed message part.
Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
On 12 February 2014 01:39, James Hogan wrote: > On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote: >> Are you working on the wakeup protocol selector sysfs interface? > > I gave it a try yesterday, but it's a bit of a work in progress at the moment. > It's also a bit more effort for img-ir to work properly with it, so I'd > probably just limit the allowed wakeup protocols to the enabled normal > protocol at first in img-ir. > > Here's what I have (hopefully kmail won't corrupt it), feel free to take and > improve/fix it. I'm not keen on the invasiveness of the > allowed_protos/enabled_protocols change (which isn't complete), but it > should probably be abstracted at some point anyway. > Hi James. In general the approach here looks good. At least I couldn't figure any easy way to be less intrusive towards drivers/decoders and still support wakeup filters. I've just sent a new version of my rc-5-sz encoder patches for review. I was thinking that once you are finished with the wakeup protocol sysfs interface maybe you could combine my patchset and your changes into a single "review patch" series and send it to the list? That would keep the entire wakeup filter series easier to track. Br, -Antti -- 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: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote:
> On 10 February 2014 22:50, James Hogan 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?
I gave it a try yesterday, but it's a bit of a work in progress at the moment.
It's also a bit more effort for img-ir to work properly with it, so I'd
probably just limit the allowed wakeup protocols to the enabled normal
protocol at first in img-ir.
Here's what I have (hopefully kmail won't corrupt it), feel free to take and
improve/fix it. I'm not keen on the invasiveness of the
allowed_protos/enabled_protocols change (which isn't complete), but it
should probably be abstracted at some point anyway.
Cheers
James
diff --git a/Documentation/ABI/testing/sysfs-class-rc
b/Documentation/ABI/testing/sysfs-class-rc
index c0e1d14..1e4ecc8 100644
--- a/Documentation/ABI/testing/sysfs-class-rc
+++ b/Documentation/ABI/testing/sysfs-class-rc
@@ -61,6 +61,25 @@ Description:
an error.
This value may be reset to 0 if the current protocol is altered.
+What: /sys/class/rc/rcN/wakeup_protocol
+Date: Feb 2014
+KernelVersion: 3.15
+Contact: Mauro Carvalho Chehab
+Description:
+ Reading this file returns a list of available protocols to use
+ for the wakeup filter, something like:
+ "rc5 rc6 nec jvc [sony]"
+ The enabled wakeup protocol is shown in [] brackets.
+ Writing "+proto" will add a protocol to the list of enabled
+ wakeup protocols.
+ Writing "-proto" will remove a protocol from the list of enabled
+ wakeup protocols.
+ Writing "proto" will use "proto" for wakeup events.
+ Writing "none" will disable wakeup.
+ Write fails with EINVAL if more than one protocol would be
+ enabled, an unknown protocol name is used, or if wakeup is not
+ supported by the hardware.
+
What: /sys/class/rc/rcN/wakeup_filter
Date: Jan 2014
KernelVersion: 3.15
@@ -74,7 +93,7 @@ Description:
scancodes which match the filter will wake the system from e.g.
suspend to RAM or power off.
Otherwise the write will fail with an error.
- This value may be reset to 0 if the current protocol is altered.
+ This value may be reset to 0 if the wakeup protocol is altered.
What: /sys/class/rc/rcN/wakeup_filter_mask
Date: Jan 2014
@@ -89,4 +108,4 @@ Description:
scancodes which match the filter will wake the system from e.g.
suspend to RAM or power off.
Otherwise the write will fail with an error.
- This value may be reset to 0 if the current protocol is altered.
+ This value may be reset to 0 if the wakeup protocol is altered.
diff --git a/drivers/media/rc/ir-jvc-decoder.c
b/drivers/media/rc/ir-jvc-decoder.c
index 3948138..7fb9467 100644
--- a/drivers/media/rc/ir-jvc-decoder.c
+++ b/drivers/media/rc/ir-jvc-decoder.c
@@ -47,7 +47,7 @@ static int ir_jvc_decode(struct rc_dev *dev, struct
ir_raw_event ev)
{
struct jvc_dec *data = &dev->raw->jvc;
- if (!(dev->enabled_protocols & RC_BIT_JVC))
+ if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_JVC))
return 0;
if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-mce_kbd-decoder.c
b/drivers/media/rc/ir-mce_kbd-decoder.c
index 9f3c9b5..bc93e11 100644
--- a/drivers/media/rc/ir-mce_kbd-decoder.c
+++ b/drivers/media/rc/ir-mce_kbd-decoder.c
@@ -216,7 +21
Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
On 10 February 2014 22:50, James Hogan 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
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
signature.asc
Description: This is a digitally signed message part.
Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
Hi James.
On 10 February 2014 12:30, James Hogan 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ä
>> ---
>> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
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ä
> ---
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support
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ä
---
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;
+
+ /* 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)
--
1.8.3.2
--
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
