* [PATCH 0/3] 3.15 fixes
@ 2014-04-04 22:05 David Härdeman
2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: David Härdeman @ 2014-04-04 22:05 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
The following three patches are the quickfixes that we (Mauro, James, me)
seemed to agree to. I've updated the patches to apply cleanly without
the preceeding patches from my previous patchbomb and I've hopefully fixed
the comments that James provided (I'll let him reply with separate Acked-by
lines if he agrees to each patch).
---
David Härdeman (3):
rc-core: do not change 32bit NEC scancode format for now
rc-core: split dev->s_filter
rc-core: remove generic scancode filter
drivers/media/rc/img-ir/img-ir-hw.c | 15 +++++
drivers/media/rc/img-ir/img-ir-nec.c | 27 +++++----
drivers/media/rc/ir-nec-decoder.c | 5 --
drivers/media/rc/keymaps/rc-tivo.c | 86 +++++++++++++++---------------
drivers/media/rc/rc-main.c | 98 ++++++++++++++++++++++------------
include/media/rc-core.h | 8 ++-
6 files changed, 142 insertions(+), 97 deletions(-)
--
David Härdeman
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now 2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman @ 2014-04-04 22:05 ` David Härdeman 2014-04-04 23:34 ` James Hogan 2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman 2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman 2 siblings, 1 reply; 7+ messages in thread From: David Härdeman @ 2014-04-04 22:05 UTC (permalink / raw) To: linux-media; +Cc: james.hogan, m.chehab This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2 The patch ignores the fact that NEC32 scancodes are generated not only in the NEC raw decoder but also directly in some drivers. Whichever approach is chosen it should be consistent across drivers and this patch needs more discussion. Furthermore, I'm convinced that we have to stop playing games trying to decipher the "meaning" of NEC scancodes (what's the customer/vendor/address, which byte is the MSB, etc). This patch is in preparation for the next few patches in this series. v2: make sure img-ir scancodes are bitrev8():ed as well v3: update comments Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/img-ir/img-ir-nec.c | 27 ++++++----- drivers/media/rc/ir-nec-decoder.c | 5 -- drivers/media/rc/keymaps/rc-tivo.c | 86 +++++++++++++++++----------------- 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c index e7a731b..751d9d9 100644 --- a/drivers/media/rc/img-ir/img-ir-nec.c +++ b/drivers/media/rc/img-ir/img-ir-nec.c @@ -5,6 +5,7 @@ */ #include "img-ir-hw.h" +#include <linux/bitrev.h> /* Convert NEC data to a scancode */ static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols) @@ -22,11 +23,11 @@ static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols) data_inv = (raw >> 24) & 0xff; if ((data_inv ^ data) != 0xff) { /* 32-bit NEC (used by Apple and TiVo remotes) */ - /* scan encoding: aaAAddDD */ - *scancode = addr_inv << 24 | - addr << 16 | - data_inv << 8 | - data; + /* scan encoding: as transmitted, MSBit = first received bit */ + *scancode = bitrev8(addr) << 24 | + bitrev8(addr_inv) << 16 | + bitrev8(data) << 8 | + bitrev8(data_inv); } else if ((addr_inv ^ addr) != 0xff) { /* Extended NEC */ /* scan encoding: AAaaDD */ @@ -54,13 +55,15 @@ static int img_ir_nec_filter(const struct rc_scancode_filter *in, if ((in->data | in->mask) & 0xff000000) { /* 32-bit NEC (used by Apple and TiVo remotes) */ - /* scan encoding: aaAAddDD */ - addr_inv = (in->data >> 24) & 0xff; - addr_inv_m = (in->mask >> 24) & 0xff; - addr = (in->data >> 16) & 0xff; - addr_m = (in->mask >> 16) & 0xff; - data_inv = (in->data >> 8) & 0xff; - data_inv_m = (in->mask >> 8) & 0xff; + /* scan encoding: as transmitted, MSBit = first received bit */ + addr = bitrev8(in->data >> 24); + addr_m = bitrev8(in->mask >> 24); + addr_inv = bitrev8(in->data >> 16); + addr_inv_m = bitrev8(in->mask >> 16); + data = bitrev8(in->data >> 8); + data_m = bitrev8(in->mask >> 8); + data_inv = bitrev8(in->data >> 0); + data_inv_m = bitrev8(in->mask >> 0); } else if ((in->data | in->mask) & 0x00ff0000) { /* Extended NEC */ /* scan encoding AAaaDD */ diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c index 9de1791..35c42e5 100644 --- a/drivers/media/rc/ir-nec-decoder.c +++ b/drivers/media/rc/ir-nec-decoder.c @@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev) if (send_32bits) { /* NEC transport, but modified protocol, used by at * least Apple and TiVo remotes */ - scancode = not_address << 24 | - address << 16 | - not_command << 8 | - command; + scancode = data->bits; IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode); } else if ((address ^ not_address) != 0xff) { /* Extended NEC */ diff --git a/drivers/media/rc/keymaps/rc-tivo.c b/drivers/media/rc/keymaps/rc-tivo.c index 5cc1b45..454e062 100644 --- a/drivers/media/rc/keymaps/rc-tivo.c +++ b/drivers/media/rc/keymaps/rc-tivo.c @@ -15,62 +15,62 @@ * Initial mapping is for the TiVo remote included in the Nero LiquidTV bundle, * which also ships with a TiVo-branded IR transceiver, supported by the mceusb * driver. Note that the remote uses an NEC-ish protocol, but instead of having - * a command/not_command pair, it has a vendor ID of 0x3085, but some keys, the + * a command/not_command pair, it has a vendor ID of 0xa10c, but some keys, the * NEC extended checksums do pass, so the table presently has the intended * values and the checksum-passed versions for those keys. */ static struct rc_map_table tivo[] = { - { 0x3085f009, KEY_MEDIA }, /* TiVo Button */ - { 0x3085e010, KEY_POWER2 }, /* TV Power */ - { 0x3085e011, KEY_TV }, /* Live TV/Swap */ - { 0x3085c034, KEY_VIDEO_NEXT }, /* TV Input */ - { 0x3085e013, KEY_INFO }, - { 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */ + { 0xa10c900f, KEY_MEDIA }, /* TiVo Button */ + { 0xa10c0807, KEY_POWER2 }, /* TV Power */ + { 0xa10c8807, KEY_TV }, /* Live TV/Swap */ + { 0xa10c2c03, KEY_VIDEO_NEXT }, /* TV Input */ + { 0xa10cc807, KEY_INFO }, + { 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */ { 0x0085305f, KEY_CYCLEWINDOWS }, - { 0x3085c036, KEY_EPG }, /* Guide */ + { 0xa10c6c03, KEY_EPG }, /* Guide */ - { 0x3085e014, KEY_UP }, - { 0x3085e016, KEY_DOWN }, - { 0x3085e017, KEY_LEFT }, - { 0x3085e015, KEY_RIGHT }, + { 0xa10c2807, KEY_UP }, + { 0xa10c6807, KEY_DOWN }, + { 0xa10ce807, KEY_LEFT }, + { 0xa10ca807, KEY_RIGHT }, - { 0x3085e018, KEY_SCROLLDOWN }, /* Red Thumbs Down */ - { 0x3085e019, KEY_SELECT }, - { 0x3085e01a, KEY_SCROLLUP }, /* Green Thumbs Up */ + { 0xa10c1807, KEY_SCROLLDOWN }, /* Red Thumbs Down */ + { 0xa10c9807, KEY_SELECT }, + { 0xa10c5807, KEY_SCROLLUP }, /* Green Thumbs Up */ - { 0x3085e01c, KEY_VOLUMEUP }, - { 0x3085e01d, KEY_VOLUMEDOWN }, - { 0x3085e01b, KEY_MUTE }, - { 0x3085d020, KEY_RECORD }, - { 0x3085e01e, KEY_CHANNELUP }, - { 0x3085e01f, KEY_CHANNELDOWN }, + { 0xa10c3807, KEY_VOLUMEUP }, + { 0xa10cb807, KEY_VOLUMEDOWN }, + { 0xa10cd807, KEY_MUTE }, + { 0xa10c040b, KEY_RECORD }, + { 0xa10c7807, KEY_CHANNELUP }, + { 0xa10cf807, KEY_CHANNELDOWN }, { 0x0085301f, KEY_CHANNELDOWN }, - { 0x3085d021, KEY_PLAY }, - { 0x3085d023, KEY_PAUSE }, - { 0x3085d025, KEY_SLOW }, - { 0x3085d022, KEY_REWIND }, - { 0x3085d024, KEY_FASTFORWARD }, - { 0x3085d026, KEY_PREVIOUS }, - { 0x3085d027, KEY_NEXT }, /* ->| */ + { 0xa10c840b, KEY_PLAY }, + { 0xa10cc40b, KEY_PAUSE }, + { 0xa10ca40b, KEY_SLOW }, + { 0xa10c440b, KEY_REWIND }, + { 0xa10c240b, KEY_FASTFORWARD }, + { 0xa10c640b, KEY_PREVIOUS }, + { 0xa10ce40b, KEY_NEXT }, /* ->| */ - { 0x3085b044, KEY_ZOOM }, /* Aspect */ - { 0x3085b048, KEY_STOP }, - { 0x3085b04a, KEY_DVD }, /* DVD Menu */ + { 0xa10c220d, KEY_ZOOM }, /* Aspect */ + { 0xa10c120d, KEY_STOP }, + { 0xa10c520d, KEY_DVD }, /* DVD Menu */ - { 0x3085d028, KEY_NUMERIC_1 }, - { 0x3085d029, KEY_NUMERIC_2 }, - { 0x3085d02a, KEY_NUMERIC_3 }, - { 0x3085d02b, KEY_NUMERIC_4 }, - { 0x3085d02c, KEY_NUMERIC_5 }, - { 0x3085d02d, KEY_NUMERIC_6 }, - { 0x3085d02e, KEY_NUMERIC_7 }, - { 0x3085d02f, KEY_NUMERIC_8 }, + { 0xa10c140b, KEY_NUMERIC_1 }, + { 0xa10c940b, KEY_NUMERIC_2 }, + { 0xa10c540b, KEY_NUMERIC_3 }, + { 0xa10cd40b, KEY_NUMERIC_4 }, + { 0xa10c340b, KEY_NUMERIC_5 }, + { 0xa10cb40b, KEY_NUMERIC_6 }, + { 0xa10c740b, KEY_NUMERIC_7 }, + { 0xa10cf40b, KEY_NUMERIC_8 }, { 0x0085302f, KEY_NUMERIC_8 }, - { 0x3085c030, KEY_NUMERIC_9 }, - { 0x3085c031, KEY_NUMERIC_0 }, - { 0x3085c033, KEY_ENTER }, - { 0x3085c032, KEY_CLEAR }, + { 0xa10c0c03, KEY_NUMERIC_9 }, + { 0xa10c8c03, KEY_NUMERIC_0 }, + { 0xa10ccc03, KEY_ENTER }, + { 0xa10c4c03, KEY_CLEAR }, }; static struct rc_map_list tivo_map = { ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now 2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman @ 2014-04-04 23:34 ` James Hogan 0 siblings, 0 replies; 7+ messages in thread From: James Hogan @ 2014-04-04 23:34 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media, m.chehab [-- Attachment #1: Type: text/plain, Size: 8414 bytes --] On Saturday 05 April 2014 00:05:56 David Härdeman wrote: > This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2 > > The patch ignores the fact that NEC32 scancodes are generated not only in > the NEC raw decoder but also directly in some drivers. Whichever approach > is chosen it should be consistent across drivers and this patch needs more > discussion. > > Furthermore, I'm convinced that we have to stop playing games trying to > decipher the "meaning" of NEC scancodes (what's the customer/vendor/address, > which byte is the MSB, etc). > > This patch is in preparation for the next few patches in this series. > > v2: make sure img-ir scancodes are bitrev8():ed as well > > v3: update comments > > Signed-off-by: David Härdeman <david@hardeman.nu> Acked-by: James Hogan <james.hogan@imgtec.com> Thanks James > --- > drivers/media/rc/img-ir/img-ir-nec.c | 27 ++++++----- > drivers/media/rc/ir-nec-decoder.c | 5 -- > drivers/media/rc/keymaps/rc-tivo.c | 86 > +++++++++++++++++----------------- 3 files changed, 59 insertions(+), 59 > deletions(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-nec.c > b/drivers/media/rc/img-ir/img-ir-nec.c index e7a731b..751d9d9 100644 > --- a/drivers/media/rc/img-ir/img-ir-nec.c > +++ b/drivers/media/rc/img-ir/img-ir-nec.c > @@ -5,6 +5,7 @@ > */ > > #include "img-ir-hw.h" > +#include <linux/bitrev.h> > > /* Convert NEC data to a scancode */ > static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 > protocols) @@ -22,11 +23,11 @@ static int img_ir_nec_scancode(int len, u64 > raw, int *scancode, u64 protocols) data_inv = (raw >> 24) & 0xff; > if ((data_inv ^ data) != 0xff) { > /* 32-bit NEC (used by Apple and TiVo remotes) */ > - /* scan encoding: aaAAddDD */ > - *scancode = addr_inv << 24 | > - addr << 16 | > - data_inv << 8 | > - data; > + /* scan encoding: as transmitted, MSBit = first received bit */ > + *scancode = bitrev8(addr) << 24 | > + bitrev8(addr_inv) << 16 | > + bitrev8(data) << 8 | > + bitrev8(data_inv); > } else if ((addr_inv ^ addr) != 0xff) { > /* Extended NEC */ > /* scan encoding: AAaaDD */ > @@ -54,13 +55,15 @@ static int img_ir_nec_filter(const struct > rc_scancode_filter *in, > > if ((in->data | in->mask) & 0xff000000) { > /* 32-bit NEC (used by Apple and TiVo remotes) */ > - /* scan encoding: aaAAddDD */ > - addr_inv = (in->data >> 24) & 0xff; > - addr_inv_m = (in->mask >> 24) & 0xff; > - addr = (in->data >> 16) & 0xff; > - addr_m = (in->mask >> 16) & 0xff; > - data_inv = (in->data >> 8) & 0xff; > - data_inv_m = (in->mask >> 8) & 0xff; > + /* scan encoding: as transmitted, MSBit = first received bit */ > + addr = bitrev8(in->data >> 24); > + addr_m = bitrev8(in->mask >> 24); > + addr_inv = bitrev8(in->data >> 16); > + addr_inv_m = bitrev8(in->mask >> 16); > + data = bitrev8(in->data >> 8); > + data_m = bitrev8(in->mask >> 8); > + data_inv = bitrev8(in->data >> 0); > + data_inv_m = bitrev8(in->mask >> 0); > } else if ((in->data | in->mask) & 0x00ff0000) { > /* Extended NEC */ > /* scan encoding AAaaDD */ > diff --git a/drivers/media/rc/ir-nec-decoder.c > b/drivers/media/rc/ir-nec-decoder.c index 9de1791..35c42e5 100644 > --- a/drivers/media/rc/ir-nec-decoder.c > +++ b/drivers/media/rc/ir-nec-decoder.c > @@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct > ir_raw_event ev) if (send_32bits) { > /* NEC transport, but modified protocol, used by at > * least Apple and TiVo remotes */ > - scancode = not_address << 24 | > - address << 16 | > - not_command << 8 | > - command; > + scancode = data->bits; > IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode); > } else if ((address ^ not_address) != 0xff) { > /* Extended NEC */ > diff --git a/drivers/media/rc/keymaps/rc-tivo.c > b/drivers/media/rc/keymaps/rc-tivo.c index 5cc1b45..454e062 100644 > --- a/drivers/media/rc/keymaps/rc-tivo.c > +++ b/drivers/media/rc/keymaps/rc-tivo.c > @@ -15,62 +15,62 @@ > * Initial mapping is for the TiVo remote included in the Nero LiquidTV > bundle, * which also ships with a TiVo-branded IR transceiver, supported by > the mceusb * driver. Note that the remote uses an NEC-ish protocol, but > instead of having - * a command/not_command pair, it has a vendor ID of > 0x3085, but some keys, the + * a command/not_command pair, it has a vendor > ID of 0xa10c, but some keys, the * NEC extended checksums do pass, so the > table presently has the intended * values and the checksum-passed versions > for those keys. > */ > static struct rc_map_table tivo[] = { > - { 0x3085f009, KEY_MEDIA }, /* TiVo Button */ > - { 0x3085e010, KEY_POWER2 }, /* TV Power */ > - { 0x3085e011, KEY_TV }, /* Live TV/Swap */ > - { 0x3085c034, KEY_VIDEO_NEXT }, /* TV Input */ > - { 0x3085e013, KEY_INFO }, > - { 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */ > + { 0xa10c900f, KEY_MEDIA }, /* TiVo Button */ > + { 0xa10c0807, KEY_POWER2 }, /* TV Power */ > + { 0xa10c8807, KEY_TV }, /* Live TV/Swap */ > + { 0xa10c2c03, KEY_VIDEO_NEXT }, /* TV Input */ > + { 0xa10cc807, KEY_INFO }, > + { 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */ > { 0x0085305f, KEY_CYCLEWINDOWS }, > - { 0x3085c036, KEY_EPG }, /* Guide */ > + { 0xa10c6c03, KEY_EPG }, /* Guide */ > > - { 0x3085e014, KEY_UP }, > - { 0x3085e016, KEY_DOWN }, > - { 0x3085e017, KEY_LEFT }, > - { 0x3085e015, KEY_RIGHT }, > + { 0xa10c2807, KEY_UP }, > + { 0xa10c6807, KEY_DOWN }, > + { 0xa10ce807, KEY_LEFT }, > + { 0xa10ca807, KEY_RIGHT }, > > - { 0x3085e018, KEY_SCROLLDOWN }, /* Red Thumbs Down */ > - { 0x3085e019, KEY_SELECT }, > - { 0x3085e01a, KEY_SCROLLUP }, /* Green Thumbs Up */ > + { 0xa10c1807, KEY_SCROLLDOWN }, /* Red Thumbs Down */ > + { 0xa10c9807, KEY_SELECT }, > + { 0xa10c5807, KEY_SCROLLUP }, /* Green Thumbs Up */ > > - { 0x3085e01c, KEY_VOLUMEUP }, > - { 0x3085e01d, KEY_VOLUMEDOWN }, > - { 0x3085e01b, KEY_MUTE }, > - { 0x3085d020, KEY_RECORD }, > - { 0x3085e01e, KEY_CHANNELUP }, > - { 0x3085e01f, KEY_CHANNELDOWN }, > + { 0xa10c3807, KEY_VOLUMEUP }, > + { 0xa10cb807, KEY_VOLUMEDOWN }, > + { 0xa10cd807, KEY_MUTE }, > + { 0xa10c040b, KEY_RECORD }, > + { 0xa10c7807, KEY_CHANNELUP }, > + { 0xa10cf807, KEY_CHANNELDOWN }, > { 0x0085301f, KEY_CHANNELDOWN }, > > - { 0x3085d021, KEY_PLAY }, > - { 0x3085d023, KEY_PAUSE }, > - { 0x3085d025, KEY_SLOW }, > - { 0x3085d022, KEY_REWIND }, > - { 0x3085d024, KEY_FASTFORWARD }, > - { 0x3085d026, KEY_PREVIOUS }, > - { 0x3085d027, KEY_NEXT }, /* ->| */ > + { 0xa10c840b, KEY_PLAY }, > + { 0xa10cc40b, KEY_PAUSE }, > + { 0xa10ca40b, KEY_SLOW }, > + { 0xa10c440b, KEY_REWIND }, > + { 0xa10c240b, KEY_FASTFORWARD }, > + { 0xa10c640b, KEY_PREVIOUS }, > + { 0xa10ce40b, KEY_NEXT }, /* ->| */ > > - { 0x3085b044, KEY_ZOOM }, /* Aspect */ > - { 0x3085b048, KEY_STOP }, > - { 0x3085b04a, KEY_DVD }, /* DVD Menu */ > + { 0xa10c220d, KEY_ZOOM }, /* Aspect */ > + { 0xa10c120d, KEY_STOP }, > + { 0xa10c520d, KEY_DVD }, /* DVD Menu */ > > - { 0x3085d028, KEY_NUMERIC_1 }, > - { 0x3085d029, KEY_NUMERIC_2 }, > - { 0x3085d02a, KEY_NUMERIC_3 }, > - { 0x3085d02b, KEY_NUMERIC_4 }, > - { 0x3085d02c, KEY_NUMERIC_5 }, > - { 0x3085d02d, KEY_NUMERIC_6 }, > - { 0x3085d02e, KEY_NUMERIC_7 }, > - { 0x3085d02f, KEY_NUMERIC_8 }, > + { 0xa10c140b, KEY_NUMERIC_1 }, > + { 0xa10c940b, KEY_NUMERIC_2 }, > + { 0xa10c540b, KEY_NUMERIC_3 }, > + { 0xa10cd40b, KEY_NUMERIC_4 }, > + { 0xa10c340b, KEY_NUMERIC_5 }, > + { 0xa10cb40b, KEY_NUMERIC_6 }, > + { 0xa10c740b, KEY_NUMERIC_7 }, > + { 0xa10cf40b, KEY_NUMERIC_8 }, > { 0x0085302f, KEY_NUMERIC_8 }, > - { 0x3085c030, KEY_NUMERIC_9 }, > - { 0x3085c031, KEY_NUMERIC_0 }, > - { 0x3085c033, KEY_ENTER }, > - { 0x3085c032, KEY_CLEAR }, > + { 0xa10c0c03, KEY_NUMERIC_9 }, > + { 0xa10c8c03, KEY_NUMERIC_0 }, > + { 0xa10ccc03, KEY_ENTER }, > + { 0xa10c4c03, KEY_CLEAR }, > }; > > static struct rc_map_list tivo_map = { > > -- > 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 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] rc-core: split dev->s_filter 2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman 2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman @ 2014-04-04 22:06 ` David Härdeman 2014-04-04 23:36 ` James Hogan 2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman 2 siblings, 1 reply; 7+ messages in thread From: David Härdeman @ 2014-04-04 22:06 UTC (permalink / raw) To: linux-media; +Cc: james.hogan, m.chehab Overloading dev->s_filter to do two different functions (set wakeup filters and generic hardware filters) makes it impossible to tell what the hardware actually supports, so create a separate dev->s_wakeup_filter and make the distinction explicit. v2: hopefully address James' comments on what should be moved from this to the next patch. Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/img-ir/img-ir-hw.c | 15 ++++++++++++++- drivers/media/rc/rc-main.c | 24 +++++++++++++++++------- include/media/rc-core.h | 6 ++++-- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c index 579a52b..0127dd2 100644 --- a/drivers/media/rc/img-ir/img-ir-hw.c +++ b/drivers/media/rc/img-ir/img-ir-hw.c @@ -504,6 +504,18 @@ unlock: return ret; } +static int img_ir_set_normal_filter(struct rc_dev *dev, + struct rc_scancode_filter *sc_filter) +{ + return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter); +} + +static int img_ir_set_wakeup_filter(struct rc_dev *dev, + struct rc_scancode_filter *sc_filter) +{ + return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter); +} + /** * img_ir_set_decoder() - Set the current decoder. * @priv: IR private data. @@ -986,7 +998,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv) rdev->map_name = RC_MAP_EMPTY; rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv)); rdev->input_name = "IMG Infrared Decoder"; - rdev->s_filter = img_ir_set_filter; + rdev->s_filter = img_ir_set_normal_filter; + rdev->s_wakeup_filter = img_ir_set_wakeup_filter; /* Register hardware decoder */ error = rc_register_device(rdev); diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index 99697aa..ecbc20c 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -923,6 +923,7 @@ static ssize_t store_protocols(struct device *device, int rc, i, count = 0; ssize_t ret; int (*change_protocol)(struct rc_dev *dev, u64 *rc_type); + int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter); struct rc_scancode_filter local_filter, *filter; /* Device is being removed */ @@ -1007,24 +1008,27 @@ static ssize_t store_protocols(struct device *device, * Fall back to clearing the filter. */ filter = &dev->scancode_filters[fattr->type]; + set_filter = (fattr->type == RC_FILTER_NORMAL) + ? dev->s_filter : dev->s_wakeup_filter; + if (old_type != type && filter->mask) { local_filter = *filter; if (!type) { /* no protocol => clear filter */ ret = -1; - } else if (!dev->s_filter) { + } else if (!set_filter) { /* generic filtering => accept any filter */ ret = 0; } else { /* hardware filtering => try setting, otherwise clear */ - ret = dev->s_filter(dev, fattr->type, &local_filter); + ret = set_filter(dev, &local_filter); } if (ret < 0) { /* clear the filter */ local_filter.data = 0; local_filter.mask = 0; - if (dev->s_filter) - dev->s_filter(dev, fattr->type, &local_filter); + if (set_filter) + set_filter(dev, &local_filter); } /* commit the new filter */ @@ -1106,6 +1110,7 @@ static ssize_t store_filter(struct device *device, struct rc_scancode_filter local_filter, *filter; int ret; unsigned long val; + int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter); /* Device is being removed */ if (!dev) @@ -1115,8 +1120,11 @@ static ssize_t store_filter(struct device *device, if (ret < 0) return ret; + set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter : + dev->s_wakeup_filter; + /* Scancode filter not supported (but still accept 0) */ - if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL) + if (!set_filter && fattr->type == RC_FILTER_WAKEUP) return val ? -EINVAL : count; mutex_lock(&dev->lock); @@ -1128,13 +1136,15 @@ static ssize_t store_filter(struct device *device, local_filter.mask = val; else local_filter.data = val; + if (!dev->enabled_protocols[fattr->type] && local_filter.mask) { /* refuse to set a filter unless a protocol is enabled */ ret = -EINVAL; goto unlock; } - if (dev->s_filter) { - ret = dev->s_filter(dev, fattr->type, &local_filter); + + if (set_filter) { + ret = set_filter(dev, &local_filter); if (ret < 0) goto unlock; } diff --git a/include/media/rc-core.h b/include/media/rc-core.h index 0b9f890..6dbc7c1 100644 --- a/include/media/rc-core.h +++ b/include/media/rc-core.h @@ -112,7 +112,8 @@ enum rc_filter_type { * device doesn't interrupt host until it sees IR pulses * @s_learning_mode: enable wide band receiver used for learning * @s_carrier_report: enable carrier reports - * @s_filter: set the scancode filter of a given type + * @s_filter: set the scancode filter + * @s_wakeup_filter: set the wakeup scancode filter */ struct rc_dev { struct device dev; @@ -159,8 +160,9 @@ struct rc_dev { int (*s_learning_mode)(struct rc_dev *dev, int enable); int (*s_carrier_report) (struct rc_dev *dev, int enable); int (*s_filter)(struct rc_dev *dev, - enum rc_filter_type type, struct rc_scancode_filter *filter); + int (*s_wakeup_filter)(struct rc_dev *dev, + struct rc_scancode_filter *filter); }; #define to_rc_dev(d) container_of(d, struct rc_dev, dev) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] rc-core: split dev->s_filter 2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman @ 2014-04-04 23:36 ` James Hogan 0 siblings, 0 replies; 7+ messages in thread From: James Hogan @ 2014-04-04 23:36 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media, m.chehab [-- Attachment #1: Type: text/plain, Size: 6193 bytes --] On Saturday 05 April 2014 00:06:01 David Härdeman wrote: > Overloading dev->s_filter to do two different functions (set wakeup filters > and generic hardware filters) makes it impossible to tell what the > hardware actually supports, so create a separate dev->s_wakeup_filter and > make the distinction explicit. > > v2: hopefully address James' comments on what should be moved from this to > the next patch. > > Signed-off-by: David Härdeman <david@hardeman.nu> Acked-by: James Hogan <james.hogan@imgtec.com> Thanks James > --- > drivers/media/rc/img-ir/img-ir-hw.c | 15 ++++++++++++++- > drivers/media/rc/rc-main.c | 24 +++++++++++++++++------- > include/media/rc-core.h | 6 ++++-- > 3 files changed, 35 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c index 579a52b..0127dd2 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -504,6 +504,18 @@ unlock: > return ret; > } > > +static int img_ir_set_normal_filter(struct rc_dev *dev, > + struct rc_scancode_filter *sc_filter) > +{ > + return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter); > +} > + > +static int img_ir_set_wakeup_filter(struct rc_dev *dev, > + struct rc_scancode_filter *sc_filter) > +{ > + return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter); > +} > + > /** > * img_ir_set_decoder() - Set the current decoder. > * @priv: IR private data. > @@ -986,7 +998,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv) > rdev->map_name = RC_MAP_EMPTY; > rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv)); > rdev->input_name = "IMG Infrared Decoder"; > - rdev->s_filter = img_ir_set_filter; > + rdev->s_filter = img_ir_set_normal_filter; > + rdev->s_wakeup_filter = img_ir_set_wakeup_filter; > > /* Register hardware decoder */ > error = rc_register_device(rdev); > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > index 99697aa..ecbc20c 100644 > --- a/drivers/media/rc/rc-main.c > +++ b/drivers/media/rc/rc-main.c > @@ -923,6 +923,7 @@ static ssize_t store_protocols(struct device *device, > int rc, i, count = 0; > ssize_t ret; > int (*change_protocol)(struct rc_dev *dev, u64 *rc_type); > + int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter); > struct rc_scancode_filter local_filter, *filter; > > /* Device is being removed */ > @@ -1007,24 +1008,27 @@ static ssize_t store_protocols(struct device > *device, * Fall back to clearing the filter. > */ > filter = &dev->scancode_filters[fattr->type]; > + set_filter = (fattr->type == RC_FILTER_NORMAL) > + ? dev->s_filter : dev->s_wakeup_filter; > + > if (old_type != type && filter->mask) { > local_filter = *filter; > if (!type) { > /* no protocol => clear filter */ > ret = -1; > - } else if (!dev->s_filter) { > + } else if (!set_filter) { > /* generic filtering => accept any filter */ > ret = 0; > } else { > /* hardware filtering => try setting, otherwise clear */ > - ret = dev->s_filter(dev, fattr->type, &local_filter); > + ret = set_filter(dev, &local_filter); > } > if (ret < 0) { > /* clear the filter */ > local_filter.data = 0; > local_filter.mask = 0; > - if (dev->s_filter) > - dev->s_filter(dev, fattr->type, &local_filter); > + if (set_filter) > + set_filter(dev, &local_filter); > } > > /* commit the new filter */ > @@ -1106,6 +1110,7 @@ static ssize_t store_filter(struct device *device, > struct rc_scancode_filter local_filter, *filter; > int ret; > unsigned long val; > + int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter); > > /* Device is being removed */ > if (!dev) > @@ -1115,8 +1120,11 @@ static ssize_t store_filter(struct device *device, > if (ret < 0) > return ret; > > + set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter : > + dev->s_wakeup_filter; > + > /* Scancode filter not supported (but still accept 0) */ > - if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL) > + if (!set_filter && fattr->type == RC_FILTER_WAKEUP) > return val ? -EINVAL : count; > > mutex_lock(&dev->lock); > @@ -1128,13 +1136,15 @@ static ssize_t store_filter(struct device *device, > local_filter.mask = val; > else > local_filter.data = val; > + > if (!dev->enabled_protocols[fattr->type] && local_filter.mask) { > /* refuse to set a filter unless a protocol is enabled */ > ret = -EINVAL; > goto unlock; > } > - if (dev->s_filter) { > - ret = dev->s_filter(dev, fattr->type, &local_filter); > + > + if (set_filter) { > + ret = set_filter(dev, &local_filter); > if (ret < 0) > goto unlock; > } > diff --git a/include/media/rc-core.h b/include/media/rc-core.h > index 0b9f890..6dbc7c1 100644 > --- a/include/media/rc-core.h > +++ b/include/media/rc-core.h > @@ -112,7 +112,8 @@ enum rc_filter_type { > * device doesn't interrupt host until it sees IR pulses > * @s_learning_mode: enable wide band receiver used for learning > * @s_carrier_report: enable carrier reports > - * @s_filter: set the scancode filter of a given type > + * @s_filter: set the scancode filter > + * @s_wakeup_filter: set the wakeup scancode filter > */ > struct rc_dev { > struct device dev; > @@ -159,8 +160,9 @@ struct rc_dev { > int (*s_learning_mode)(struct rc_dev *dev, int enable); > int (*s_carrier_report) (struct rc_dev *dev, int enable); > int (*s_filter)(struct rc_dev *dev, > - enum rc_filter_type type, > struct rc_scancode_filter *filter); > + int (*s_wakeup_filter)(struct rc_dev *dev, > + struct rc_scancode_filter *filter); > }; > > #define to_rc_dev(d) container_of(d, struct rc_dev, dev) > > -- > 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 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] rc-core: remove generic scancode filter 2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman 2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman 2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman @ 2014-04-04 22:06 ` David Härdeman 2014-04-04 23:38 ` James Hogan 2 siblings, 1 reply; 7+ messages in thread From: David Härdeman @ 2014-04-04 22:06 UTC (permalink / raw) To: linux-media; +Cc: james.hogan, m.chehab The generic scancode filtering has questionable value and makes it impossible to determine from userspace if there is an actual scancode hw filter present or not. So revert the generic parts. Based on a patch from James Hogan <james.hogan@imgtec.com>, but this version also makes sure that only the valid sysfs files are created in the first place. v2: correct dev->s_filter check v3: move some parts over from the previous patch Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/rc-main.c | 88 +++++++++++++++++++++++++++----------------- include/media/rc-core.h | 2 + 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index ecbc20c..970b93d 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -633,19 +633,13 @@ EXPORT_SYMBOL_GPL(rc_repeat); static void ir_do_keydown(struct rc_dev *dev, int scancode, u32 keycode, u8 toggle) { - struct rc_scancode_filter *filter; - bool new_event = !dev->keypressed || - dev->last_scancode != scancode || - dev->last_toggle != toggle; + bool new_event = (!dev->keypressed || + dev->last_scancode != scancode || + dev->last_toggle != toggle); if (new_event && dev->keypressed) ir_do_keyup(dev, false); - /* Generic scancode filtering */ - filter = &dev->scancode_filters[RC_FILTER_NORMAL]; - if (filter->mask && ((scancode ^ filter->data) & filter->mask)) - return; - input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode); if (new_event && keycode != KEY_RESERVED) { @@ -1011,14 +1005,11 @@ static ssize_t store_protocols(struct device *device, set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter : dev->s_wakeup_filter; - if (old_type != type && filter->mask) { + if (set_filter && old_type != type && filter->mask) { local_filter = *filter; if (!type) { /* no protocol => clear filter */ ret = -1; - } else if (!set_filter) { - /* generic filtering => accept any filter */ - ret = 0; } else { /* hardware filtering => try setting, otherwise clear */ ret = set_filter(dev, &local_filter); @@ -1027,8 +1018,7 @@ static ssize_t store_protocols(struct device *device, /* clear the filter */ local_filter.data = 0; local_filter.mask = 0; - if (set_filter) - set_filter(dev, &local_filter); + set_filter(dev, &local_filter); } /* commit the new filter */ @@ -1072,7 +1062,10 @@ static ssize_t show_filter(struct device *device, return -EINVAL; mutex_lock(&dev->lock); - if (fattr->mask) + if ((fattr->type == RC_FILTER_NORMAL && !dev->s_filter) || + (fattr->type == RC_FILTER_WAKEUP && !dev->s_wakeup_filter)) + val = 0; + else if (fattr->mask) val = dev->scancode_filters[fattr->type].mask; else val = dev->scancode_filters[fattr->type].data; @@ -1120,12 +1113,11 @@ static ssize_t store_filter(struct device *device, if (ret < 0) return ret; + /* Can the scancode filter be set? */ set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter : dev->s_wakeup_filter; - - /* Scancode filter not supported (but still accept 0) */ - if (!set_filter && fattr->type == RC_FILTER_WAKEUP) - return val ? -EINVAL : count; + if (!set_filter) + return -EINVAL; mutex_lock(&dev->lock); @@ -1143,11 +1135,9 @@ static ssize_t store_filter(struct device *device, goto unlock; } - if (set_filter) { - ret = set_filter(dev, &local_filter); - if (ret < 0) - goto unlock; - } + ret = set_filter(dev, &local_filter); + if (ret < 0) + goto unlock; /* Success, commit the new filter */ *filter = local_filter; @@ -1199,27 +1189,45 @@ static RC_FILTER_ATTR(wakeup_filter, S_IRUGO|S_IWUSR, static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR, show_filter, store_filter, RC_FILTER_WAKEUP, true); -static struct attribute *rc_dev_attrs[] = { +static struct attribute *rc_dev_protocol_attrs[] = { &dev_attr_protocols.attr.attr, + NULL, +}; + +static struct attribute_group rc_dev_protocol_attr_grp = { + .attrs = rc_dev_protocol_attrs, +}; + +static struct attribute *rc_dev_wakeup_protocol_attrs[] = { &dev_attr_wakeup_protocols.attr.attr, + NULL, +}; + +static struct attribute_group rc_dev_wakeup_protocol_attr_grp = { + .attrs = rc_dev_wakeup_protocol_attrs, +}; + +static struct attribute *rc_dev_filter_attrs[] = { &dev_attr_filter.attr.attr, &dev_attr_filter_mask.attr.attr, - &dev_attr_wakeup_filter.attr.attr, - &dev_attr_wakeup_filter_mask.attr.attr, NULL, }; -static struct attribute_group rc_dev_attr_grp = { - .attrs = rc_dev_attrs, +static struct attribute_group rc_dev_filter_attr_grp = { + .attrs = rc_dev_filter_attrs, }; -static const struct attribute_group *rc_dev_attr_groups[] = { - &rc_dev_attr_grp, - NULL +static struct attribute *rc_dev_wakeup_filter_attrs[] = { + &dev_attr_wakeup_filter.attr.attr, + &dev_attr_wakeup_filter_mask.attr.attr, + NULL, +}; + +static struct attribute_group rc_dev_wakeup_filter_attr_grp = { + .attrs = rc_dev_wakeup_filter_attrs, }; static struct device_type rc_dev_type = { - .groups = rc_dev_attr_groups, .release = rc_dev_release, .uevent = rc_dev_uevent, }; @@ -1276,7 +1284,7 @@ int rc_register_device(struct rc_dev *dev) static bool raw_init = false; /* raw decoders loaded? */ struct rc_map *rc_map; const char *path; - int rc, devno; + int rc, devno, attr = 0; if (!dev || !dev->map_name) return -EINVAL; @@ -1304,6 +1312,16 @@ int rc_register_device(struct rc_dev *dev) return -ENOMEM; } while (test_and_set_bit(devno, ir_core_dev_number)); + dev->dev.groups = dev->sysfs_groups; + dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp; + if (dev->s_filter) + dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp; + if (dev->s_wakeup_filter) + dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp; + if (dev->change_wakeup_protocol) + dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp; + dev->sysfs_groups[attr++] = NULL; + /* * Take the lock here, as the device sysfs node will appear * when device_add() is called, which may trigger an ir-keytable udev diff --git a/include/media/rc-core.h b/include/media/rc-core.h index 6dbc7c1..fde142e 100644 --- a/include/media/rc-core.h +++ b/include/media/rc-core.h @@ -60,6 +60,7 @@ enum rc_filter_type { /** * struct rc_dev - represents a remote control device * @dev: driver model's view of this device + * @sysfs_groups: sysfs attribute groups * @input_name: name of the input child device * @input_phys: physical path to the input child device * @input_id: id of the input child device (struct input_id) @@ -117,6 +118,7 @@ enum rc_filter_type { */ struct rc_dev { struct device dev; + const struct attribute_group *sysfs_groups[5]; const char *input_name; const char *input_phys; struct input_id input_id; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] rc-core: remove generic scancode filter 2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman @ 2014-04-04 23:38 ` James Hogan 0 siblings, 0 replies; 7+ messages in thread From: James Hogan @ 2014-04-04 23:38 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media, m.chehab [-- Attachment #1: Type: text/plain, Size: 7835 bytes --] On Saturday 05 April 2014 00:06:06 David Härdeman wrote: > The generic scancode filtering has questionable value and makes it > impossible to determine from userspace if there is an actual > scancode hw filter present or not. > > So revert the generic parts. > > Based on a patch from James Hogan <james.hogan@imgtec.com>, but this > version also makes sure that only the valid sysfs files are created > in the first place. > > v2: correct dev->s_filter check > > v3: move some parts over from the previous patch > > Signed-off-by: David Härdeman <david@hardeman.nu> Acked-by: James Hogan <james.hogan@imgtec.com> Thanks James > --- > drivers/media/rc/rc-main.c | 88 > +++++++++++++++++++++++++++----------------- include/media/rc-core.h | > 2 + > 2 files changed, 55 insertions(+), 35 deletions(-) > > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > index ecbc20c..970b93d 100644 > --- a/drivers/media/rc/rc-main.c > +++ b/drivers/media/rc/rc-main.c > @@ -633,19 +633,13 @@ EXPORT_SYMBOL_GPL(rc_repeat); > static void ir_do_keydown(struct rc_dev *dev, int scancode, > u32 keycode, u8 toggle) > { > - struct rc_scancode_filter *filter; > - bool new_event = !dev->keypressed || > - dev->last_scancode != scancode || > - dev->last_toggle != toggle; > + bool new_event = (!dev->keypressed || > + dev->last_scancode != scancode || > + dev->last_toggle != toggle); > > if (new_event && dev->keypressed) > ir_do_keyup(dev, false); > > - /* Generic scancode filtering */ > - filter = &dev->scancode_filters[RC_FILTER_NORMAL]; > - if (filter->mask && ((scancode ^ filter->data) & filter->mask)) > - return; > - > input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode); > > if (new_event && keycode != KEY_RESERVED) { > @@ -1011,14 +1005,11 @@ static ssize_t store_protocols(struct device > *device, set_filter = (fattr->type == RC_FILTER_NORMAL) > ? dev->s_filter : dev->s_wakeup_filter; > > - if (old_type != type && filter->mask) { > + if (set_filter && old_type != type && filter->mask) { > local_filter = *filter; > if (!type) { > /* no protocol => clear filter */ > ret = -1; > - } else if (!set_filter) { > - /* generic filtering => accept any filter */ > - ret = 0; > } else { > /* hardware filtering => try setting, otherwise clear */ > ret = set_filter(dev, &local_filter); > @@ -1027,8 +1018,7 @@ static ssize_t store_protocols(struct device *device, > /* clear the filter */ > local_filter.data = 0; > local_filter.mask = 0; > - if (set_filter) > - set_filter(dev, &local_filter); > + set_filter(dev, &local_filter); > } > > /* commit the new filter */ > @@ -1072,7 +1062,10 @@ static ssize_t show_filter(struct device *device, > return -EINVAL; > > mutex_lock(&dev->lock); > - if (fattr->mask) > + if ((fattr->type == RC_FILTER_NORMAL && !dev->s_filter) || > + (fattr->type == RC_FILTER_WAKEUP && !dev->s_wakeup_filter)) > + val = 0; > + else if (fattr->mask) > val = dev->scancode_filters[fattr->type].mask; > else > val = dev->scancode_filters[fattr->type].data; > @@ -1120,12 +1113,11 @@ static ssize_t store_filter(struct device *device, > if (ret < 0) > return ret; > > + /* Can the scancode filter be set? */ > set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter : > dev->s_wakeup_filter; > - > - /* Scancode filter not supported (but still accept 0) */ > - if (!set_filter && fattr->type == RC_FILTER_WAKEUP) > - return val ? -EINVAL : count; > + if (!set_filter) > + return -EINVAL; > > mutex_lock(&dev->lock); > > @@ -1143,11 +1135,9 @@ static ssize_t store_filter(struct device *device, > goto unlock; > } > > - if (set_filter) { > - ret = set_filter(dev, &local_filter); > - if (ret < 0) > - goto unlock; > - } > + ret = set_filter(dev, &local_filter); > + if (ret < 0) > + goto unlock; > > /* Success, commit the new filter */ > *filter = local_filter; > @@ -1199,27 +1189,45 @@ static RC_FILTER_ATTR(wakeup_filter, > S_IRUGO|S_IWUSR, static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR, > show_filter, store_filter, RC_FILTER_WAKEUP, true); > > -static struct attribute *rc_dev_attrs[] = { > +static struct attribute *rc_dev_protocol_attrs[] = { > &dev_attr_protocols.attr.attr, > + NULL, > +}; > + > +static struct attribute_group rc_dev_protocol_attr_grp = { > + .attrs = rc_dev_protocol_attrs, > +}; > + > +static struct attribute *rc_dev_wakeup_protocol_attrs[] = { > &dev_attr_wakeup_protocols.attr.attr, > + NULL, > +}; > + > +static struct attribute_group rc_dev_wakeup_protocol_attr_grp = { > + .attrs = rc_dev_wakeup_protocol_attrs, > +}; > + > +static struct attribute *rc_dev_filter_attrs[] = { > &dev_attr_filter.attr.attr, > &dev_attr_filter_mask.attr.attr, > - &dev_attr_wakeup_filter.attr.attr, > - &dev_attr_wakeup_filter_mask.attr.attr, > NULL, > }; > > -static struct attribute_group rc_dev_attr_grp = { > - .attrs = rc_dev_attrs, > +static struct attribute_group rc_dev_filter_attr_grp = { > + .attrs = rc_dev_filter_attrs, > }; > > -static const struct attribute_group *rc_dev_attr_groups[] = { > - &rc_dev_attr_grp, > - NULL > +static struct attribute *rc_dev_wakeup_filter_attrs[] = { > + &dev_attr_wakeup_filter.attr.attr, > + &dev_attr_wakeup_filter_mask.attr.attr, > + NULL, > +}; > + > +static struct attribute_group rc_dev_wakeup_filter_attr_grp = { > + .attrs = rc_dev_wakeup_filter_attrs, > }; > > static struct device_type rc_dev_type = { > - .groups = rc_dev_attr_groups, > .release = rc_dev_release, > .uevent = rc_dev_uevent, > }; > @@ -1276,7 +1284,7 @@ int rc_register_device(struct rc_dev *dev) > static bool raw_init = false; /* raw decoders loaded? */ > struct rc_map *rc_map; > const char *path; > - int rc, devno; > + int rc, devno, attr = 0; > > if (!dev || !dev->map_name) > return -EINVAL; > @@ -1304,6 +1312,16 @@ int rc_register_device(struct rc_dev *dev) > return -ENOMEM; > } while (test_and_set_bit(devno, ir_core_dev_number)); > > + dev->dev.groups = dev->sysfs_groups; > + dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp; > + if (dev->s_filter) > + dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp; > + if (dev->s_wakeup_filter) > + dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp; > + if (dev->change_wakeup_protocol) > + dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp; > + dev->sysfs_groups[attr++] = NULL; > + > /* > * Take the lock here, as the device sysfs node will appear > * when device_add() is called, which may trigger an ir-keytable udev > diff --git a/include/media/rc-core.h b/include/media/rc-core.h > index 6dbc7c1..fde142e 100644 > --- a/include/media/rc-core.h > +++ b/include/media/rc-core.h > @@ -60,6 +60,7 @@ enum rc_filter_type { > /** > * struct rc_dev - represents a remote control device > * @dev: driver model's view of this device > + * @sysfs_groups: sysfs attribute groups > * @input_name: name of the input child device > * @input_phys: physical path to the input child device > * @input_id: id of the input child device (struct input_id) > @@ -117,6 +118,7 @@ enum rc_filter_type { > */ > struct rc_dev { > struct device dev; > + const struct attribute_group *sysfs_groups[5]; > const char *input_name; > const char *input_phys; > struct input_id input_id; > > -- > 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 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-04-04 23:38 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman 2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman 2014-04-04 23:34 ` James Hogan 2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman 2014-04-04 23:36 ` James Hogan 2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman 2014-04-04 23:38 ` James Hogan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox