* [PATCH] rc-core: do not change 32bit NEC scancode format for now @ 2014-03-27 21:00 David Härdeman 2014-03-27 23:21 ` James Hogan 0 siblings, 1 reply; 6+ messages in thread From: David Härdeman @ 2014-03-27 21:00 UTC (permalink / raw) To: james.hogan, linux-media; +Cc: 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). I'll post separate proposals to that effect later. Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/ir-nec-decoder.c | 5 -- drivers/media/rc/keymaps/rc-tivo.c | 86 ++++++++++++++++++------------------ 2 files changed, 44 insertions(+), 47 deletions(-) diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c index 735a509..c4333d5 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] 6+ messages in thread
* Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now 2014-03-27 21:00 [PATCH] rc-core: do not change 32bit NEC scancode format for now David Härdeman @ 2014-03-27 23:21 ` James Hogan 2014-03-28 0:08 ` David Härdeman 0 siblings, 1 reply; 6+ messages in thread From: James Hogan @ 2014-03-27 23:21 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media, m.chehab [-- Attachment #1: Type: text/plain, Size: 8130 bytes --] Hi David, On Thursday 27 March 2014 22:00:37 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. Fair enough. For reference which drivers are you referring to? > 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). Well when all the buttons on a remote have the same address, and the numeric buttons are sequential commands only in a certain bit/byte order, then I think the word "decipher" is probably a bit of a stretch. Nevertheless I don't have any attachment to 32-bit NEC. If it's likely to change again I'd prefer img-ir-nec just not support it for now, so please could you add the following hunks to your patch (or if the original patch is to be dropped this could be squashed into the img-ir-nec patch): diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c index e7a731b..419d087 100644 --- a/drivers/media/rc/img-ir/img-ir-nec.c +++ b/drivers/media/rc/img-ir/img-ir-nec.c @@ -21,12 +21,7 @@ static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols) data = (raw >> 16) & 0xff; 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; + return -EINVAL; } else if ((addr_inv ^ addr) != 0xff) { /* Extended NEC */ /* scan encoding: AAaaDD */ @@ -53,14 +48,7 @@ static int img_ir_nec_filter(const struct rc_scancode_filter *in, data_m = in->mask & 0xff; 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; + return -EINVAL; } else if ((in->data | in->mask) & 0x00ff0000) { /* Extended NEC */ /* scan encoding AAaaDD */ > > I'll post separate proposals to that effect later. Great, please do Cc me (I have a work in progress branch to unify NEC scancodes, but I'm not sure I'd have time to complete it any time soon anyway) Thanks James > > Signed-off-by: David Härdeman <david@hardeman.nu> > --- > drivers/media/rc/ir-nec-decoder.c | 5 -- > drivers/media/rc/keymaps/rc-tivo.c | 86 > ++++++++++++++++++------------------ 2 files changed, 44 insertions(+), 47 > deletions(-) > > diff --git a/drivers/media/rc/ir-nec-decoder.c > b/drivers/media/rc/ir-nec-decoder.c index 735a509..c4333d5 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 related [flat|nested] 6+ messages in thread
* Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now 2014-03-27 23:21 ` James Hogan @ 2014-03-28 0:08 ` David Härdeman 2014-03-28 23:17 ` James Hogan 0 siblings, 1 reply; 6+ messages in thread From: David Härdeman @ 2014-03-28 0:08 UTC (permalink / raw) To: James Hogan; +Cc: linux-media, m.chehab On Thu, Mar 27, 2014 at 11:21:23PM +0000, James Hogan wrote: >Hi David, > >On Thursday 27 March 2014 22:00:37 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. > >Fair enough. For reference which drivers are you referring to? The ones I'm aware of right now are: drivers/media/usb/dvb-usb/dib0700_core.c drivers/media/usb/dvb-usb-v2/az6007.c drivers/media/usb/dvb-usb-v2/af9035.c drivers/media/usb/dvb-usb-v2/rtl28xxu.c drivers/media/usb/dvb-usb-v2/af9015.c drivers/media/usb/em28xx/em28xx-input.c >> 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). > >Well when all the buttons on a remote have the same address, and the numeric >buttons are sequential commands only in a certain bit/byte order, then I think >the word "decipher" is probably a bit of a stretch. I think you misunderstood me. "decipher" is a bit of a stretch when talking of one remote control (I'm guessing you're referring to the Tivo remote). It's not that much of a stretch if we're referring to trying to derive a common meaning from the encoding used for *all* remote controls out there. The discussion about the 24-bit version of NEC and whether the address bytes were in MSB or LSB order was a good example. Andy Walls cited a NEC manual which stated one thing and people also referred to http://www.sbprojects.com/knowledge/ir/nec.php which stated the opposite (while referring to an unnamed VCR service manual). As a third example...I've read a Samsung service manual which happily stated that the remote (which used the NEC protocol) sent IR commands starting with the address x 2 (and looking at the raw NEC command, it did start with something like 0x07 0x07). So don't get me wrong, I wasn't referring to your analysis of the Tivo remote but more the general approach that has been taken until now wrt. the NEC protocol in the kernel drivers. >Nevertheless I don't have any attachment to 32-bit NEC. If it's likely to >change again I'd prefer img-ir-nec just not support it for now, so please >could you add the following hunks to your patch (or if the original patch is >to be dropped this could be squashed into the img-ir-nec patch): I'd rather show you my complete proposal first before doing something radical with your driver. But it was a good reminder that I need to keep the NEC32 parsing in your driver in mind as well. >> I'll post separate proposals to that effect later. > >Great, please do Cc me > >(I have a work in progress branch to unify NEC scancodes, but I'm not sure >I'd have time to complete it any time soon anyway) That is what I'm working on as well at the moment. It's actually to solve two problems...both to unify NEC scancodes (by simply using 32 bit scancodes everywhere and some fallback code...I'm not 100% sure it's doable but I hope so since it's the only sane solution I can think of in the long run)...and to make sure that protocol information actually gets used in keymaps, etc. I hope to post patches soon that'll make it clearer. Regards, David ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now 2014-03-28 0:08 ` David Härdeman @ 2014-03-28 23:17 ` James Hogan 2014-03-29 16:14 ` David Härdeman 2014-03-31 19:43 ` David Härdeman 0 siblings, 2 replies; 6+ messages in thread From: James Hogan @ 2014-03-28 23:17 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media, m.chehab [-- Attachment #1: Type: text/plain, Size: 4605 bytes --] On Friday 28 March 2014 01:08:56 David Härdeman wrote: > On Thu, Mar 27, 2014 at 11:21:23PM +0000, James Hogan wrote: > >Hi David, > > > >On Thursday 27 March 2014 22:00:37 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. > > > >Fair enough. For reference which drivers are you referring to? > > The ones I'm aware of right now are: Thanks, I hadn't looked properly outside of drivers/media/rc/ :( > drivers/media/usb/dvb-usb/dib0700_core.c AFAICT this only seems to support 16bit and 24bit NEC, so NEC-32 doesn't affect it. I may have missed something subtle. > drivers/media/usb/dvb-usb-v2/az6007.c > drivers/media/usb/dvb-usb-v2/af9035.c > drivers/media/usb/dvb-usb-v2/rtl28xxu.c > drivers/media/usb/dvb-usb-v2/af9015.c > drivers/media/usb/em28xx/em28xx-input.c Note, it appears none of these do any bit reversing for the 32bit case compared to 16/24 bit, so they're already different to the NEC32 scancode encoding that the raw nec decoder and tivo keymap were using, which used a different bitorder (!!) between the 32-bit and the 24/16-bit cases. > > >> 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). > > > >Well when all the buttons on a remote have the same address, and the > >numeric buttons are sequential commands only in a certain bit/byte order, > >then I think the word "decipher" is probably a bit of a stretch. > > I think you misunderstood me. "decipher" is a bit of a stretch when > talking of one remote control (I'm guessing you're referring to the Tivo > remote). It's not that much of a stretch if we're referring to trying to > derive a common meaning from the encoding used for *all* remote controls > out there. > > The discussion about the 24-bit version of NEC and whether the address > bytes were in MSB or LSB order was a good example. Andy Walls cited a > NEC manual which stated one thing and people also referred to > http://www.sbprojects.com/knowledge/ir/nec.php which stated the opposite > (while referring to an unnamed VCR service manual). > > As a third example...I've read a Samsung service manual which happily > stated that the remote (which used the NEC protocol) sent IR commands > starting with the address x 2 (and looking at the raw NEC command, it > did start with something like 0x07 0x07). > > So don't get me wrong, I wasn't referring to your analysis of the Tivo > remote but more the general approach that has been taken until now wrt. > the NEC protocol in the kernel drivers. Okay, thanks for the clarification. > > >Nevertheless I don't have any attachment to 32-bit NEC. If it's likely to > >change again I'd prefer img-ir-nec just not support it for now, so please > >could you add the following hunks to your patch (or if the original patch > >is > >to be dropped this could be squashed into the img-ir-nec patch): > I'd rather show you my complete proposal first before doing something > radical with your driver. But it was a good reminder that I need to keep > the NEC32 parsing in your driver in mind as well. Okay no problem. I had assumed you were aiming for a short term fix to prevent the encoding change hitting mainline or an actual release (v3.15). Cheers James > > >> I'll post separate proposals to that effect later. > > > >Great, please do Cc me > > > >(I have a work in progress branch to unify NEC scancodes, but I'm not sure > >I'd have time to complete it any time soon anyway) > > That is what I'm working on as well at the moment. It's actually to > solve two problems...both to unify NEC scancodes (by simply using 32 bit > scancodes everywhere and some fallback code...I'm not 100% sure it's > doable but I hope so since it's the only sane solution I can think of in > the long run)...and to make sure that protocol information actually gets > used in keymaps, etc. > > I hope to post patches soon that'll make it clearer. > > Regards, > David > > -- > 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] 6+ messages in thread
* Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now 2014-03-28 23:17 ` James Hogan @ 2014-03-29 16:14 ` David Härdeman 2014-03-31 19:43 ` David Härdeman 1 sibling, 0 replies; 6+ messages in thread From: David Härdeman @ 2014-03-29 16:14 UTC (permalink / raw) To: James Hogan; +Cc: linux-media, m.chehab On Fri, Mar 28, 2014 at 11:17:09PM +0000, James Hogan wrote: >On Friday 28 March 2014 01:08:56 David Härdeman wrote: >> drivers/media/usb/dvb-usb-v2/az6007.c >> drivers/media/usb/dvb-usb-v2/af9035.c >> drivers/media/usb/dvb-usb-v2/rtl28xxu.c >> drivers/media/usb/dvb-usb-v2/af9015.c >> drivers/media/usb/em28xx/em28xx-input.c > >Note, it appears none of these do any bit reversing for the 32bit case >compared to 16/24 bit, so they're already different to the NEC32 scancode >encoding that the raw nec decoder and tivo keymap were using, which used a >different bitorder (!!) between the 32-bit and the 24/16-bit cases. I know, and none of those drivers have an in-kernel NEC32 keymap, so if anyone is using them in that manner...it's with a homebrew keymap. >> I'd rather show you my complete proposal first before doing something >> radical with your driver. But it was a good reminder that I need to keep >> the NEC32 parsing in your driver in mind as well. > >Okay no problem. I had assumed you were aiming for a short term fix to prevent >the encoding change hitting mainline or an actual release (v3.15). I am aiming for a fix within that time frame...but I hope that it can be more than a short term one :) Patches are on their way right now... Regards, David ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now 2014-03-28 23:17 ` James Hogan 2014-03-29 16:14 ` David Härdeman @ 2014-03-31 19:43 ` David Härdeman 1 sibling, 0 replies; 6+ messages in thread From: David Härdeman @ 2014-03-31 19:43 UTC (permalink / raw) To: James Hogan; +Cc: linux-media, m.chehab On Fri, Mar 28, 2014 at 11:17:09PM +0000, James Hogan wrote: >On Friday 28 March 2014 01:08:56 David Härdeman wrote: >> On Thu, Mar 27, 2014 at 11:21:23PM +0000, James Hogan wrote: >>>On Thursday 27 March 2014 22:00:37 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. >>> >>>Fair enough. For reference which drivers are you referring to? >> >> The ones I'm aware of right now are: > >Thanks, I hadn't looked properly outside of drivers/media/rc/ :( > >> drivers/media/usb/dvb-usb/dib0700_core.c > >AFAICT this only seems to support 16bit and 24bit NEC, so NEC-32 doesn't affect >it. I may have missed something subtle. Nah. You're right, it can be converted to NEC32 by simply removing one check, but it isn't NEC32 capable yet. >> drivers/media/usb/dvb-usb-v2/az6007.c >> drivers/media/usb/dvb-usb-v2/af9035.c >> drivers/media/usb/dvb-usb-v2/rtl28xxu.c >> drivers/media/usb/dvb-usb-v2/af9015.c >> drivers/media/usb/em28xx/em28xx-input.c > >Note, it appears none of these do any bit reversing for the 32bit case >compared to 16/24 bit, so they're already different to the NEC32 scancode >encoding that the raw nec decoder and tivo keymap were using, which used a >different bitorder (!!) between the 32-bit and the 24/16-bit cases. I think that might also be a reason to generate the NEC32 scancode in the order that I've proposed (i.e. it only requires change to the sw NEC decoder). On the other hand I'm still dithering on whether your proposed NEC32 scancode (which is what the sw decoder uses) or my proposal (which is what the other hw decoders use) should be canonical... :) -- David Härdeman ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-31 19:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-27 21:00 [PATCH] rc-core: do not change 32bit NEC scancode format for now David Härdeman 2014-03-27 23:21 ` James Hogan 2014-03-28 0:08 ` David Härdeman 2014-03-28 23:17 ` James Hogan 2014-03-29 16:14 ` David Härdeman 2014-03-31 19:43 ` David Härdeman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.