On Sunday 16 March 2014 10:22:02 Antti Seppälä wrote: > On 15 March 2014 01:04, James Hogan <james@xxxxxxxxxxxxx> wrote: > > A recent discussion about proposed interfaces for setting up the > > hardware wakeup filter lead to the conclusion that it could help to have > > the generic capability to encode and modulate scancodes into raw IR > > events so that drivers for hardware with a low level wake filter (on the > > level of pulse/space durations) can still easily implement the higher > > level scancode interface that is proposed. > > > > I posted an RFC patchset showing how this could work, and Antti Seppälä > > posted additional patches to support rc5-sz and nuvoton-cir. This > > patchset improves the original RFC patches and combines & updates > > Antti's patches. > > > > I'm happy these patches are a good start at tackling the problem, as > > long as Antti is happy with them and they work for him of course. > > > > Future work could include: > > - Encoders for more protocols. > > - Carrier signal events (no use unless a driver makes use of it). > > > > Patch 1 adds the new encode API. > > Patches 2-3 adds some modulation helpers. > > Patches 4-6 adds some raw encode implementations. > > Patch 7 adds some rc-core support for encode based wakeup filtering. > > Patch 8 adds debug loopback of encoded scancode when filter set. > > Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir. > > Hi James. > > This is looking very good. I've reviewed the series and have only > minor comments to some of the patches which I'll post individually > shortly. > > I've also tested the nuvoton with actual hardware with rc-5-sz and nec > encoders and both generate wakeup samples correctly and can wake the > system. Thanks for reviewing and testing! > While doing my tests I also noticed that there is a small bug in the > wakeup_protocols handling where one can enable multiple protocols with > the + -notation. E.g. echo "+nec +rc-5" > > /sys/class/rc/rc0/wakeup_protocols shouldn't probably succeed. Yeh I'm in two minds about this now. It's actually a little awkward since some of the protocols have multiple variants (i.e. "rc-5" = RC5+RC5X), but an encoded message is only ever a single variant, so technically if you're going to draw the line for wakeup protocols it should probably be at one enabled variant, which isn't always convenient or necessary. Note, ATM even disallowing "+proto" and "-proto" we would already have to guess which variant is desired from the scancode data, which in the case of NEC scancodes is a bit horrid since NEC scancodes are ambiguous. This actually means it's driver specific whether a filter mask of 0x0000ffff filters out NEC32/NEC-X messages (scancode/encode driver probably will since it needs to pick a variant, but software fallback won't). Ideally there'd now be a way to specify the protocol variants exactly as well as whole protocols groups through this sysfs interface (and probably NEC should have protocol bits for each variant too, which is tricky ATM since keymaps can use scancodes of multiple variants and it's hard to guarantee which variant was intended for each key mapping by reading it). Adding proto_names entries for each variant is easy enough, though I'm not sure what you'd call the one for RC_BIT_RC5 (since "rc-5" is taken to mean both RC5 and RC5X). We could then check that the enabled wakeup protocols fit entirely within one of the proto_names entry's proto mask if we think it's helpful (which would allow you to specify e.g. "sony12", or "sony" (sony 12,15,20), or "sony -sony12" (sony 15,20), but not "+sony +nec"). Cheers James
Attachment:
signature.asc
Description: This is a digitally signed message part.