ir-rc5-decoder: don't wait for the end space to produce a code
RFC PATCH 1/1 ir-rc5-decoder: don't wait for the end space to produce a code
The RC5 decoding is complete at a BIT_END state. there's no reason to wait for the next space to produce a code.
drivers/media/IR/ir-rc5-decoder.c +29 -29
@@ -98,9 +98,36 @@ again:if (!is_transition (&ev, &ir_dev->raw->prev_ev))break ; -if (data->count == data->wanted_bits) +if (data->count == data->wanted_bits) { data->state =STATE_FINISHED ; -else if (data->count ==CHECK_RC5X_NBITS ) + +if (data->wanted_bits ==RC5X_NBITS ) { +/* RC5X */ +u8 xdata, command, system; + xdata = (data->bits &0x0003F ) >>0 ; + command = (data->bits &0x00FC0 ) >>6 ; + system = (data->bits &0x1F000 ) >>12 ; + toggle = (data->bits &0x20000 ) ?1 :0 ; + command += (data->bits &0x01000 ) ?0 :0x40 ; + scancode = system <<16 | command <<8 | xdata; + +IR_dprintk (1 ,"RC5X scancode 0x%06x (toggle: %u)\n" , + scancode, toggle); + }else { +/* RC5 */ +u8 command, system; + command = (data->bits &0x0003F ) >>0 ; + system = (data->bits &0x007C0 ) >>6 ; + toggle = (data->bits &0x00800 ) ?1 :0 ; + command += (data->bits &0x01000 ) ?0 :0x40 ; + scancode = system <<8 | command; + +IR_dprintk (1 ,"RC5 scancode 0x%04x (toggle: %u)\n" , + scancode, toggle); + } + +ir_keydown (input_dev, scancode, toggle); + }else if (data->count ==CHECK_RC5X_NBITS ) data->state =STATE_CHECK_RC5X ;else data->state =STATE_BIT_START ;
@@ -124,33 +151,6 @@ again:if (ev.pulse)break ; -if (data->wanted_bits ==RC5X_NBITS ) { -/* RC5X */ -u8 xdata, command, system; - xdata = (data->bits &0x0003F ) >>0 ; - command = (data->bits &0x00FC0 ) >>6 ; - system = (data->bits &0x1F000 ) >>12 ; - toggle = (data->bits &0x20000 ) ?1 :0 ; - command += (data->bits &0x01000 ) ?0 :0x40 ; - scancode = system <<16 | command <<8 | xdata; - -IR_dprintk (1 ,"RC5X scancode 0x%06x (toggle: %u)\n" , - scancode, toggle); - - }else { -/* RC5 */ -u8 command, system; - command = (data->bits &0x0003F ) >>0 ; - system = (data->bits &0x007C0 ) >>6 ; - toggle = (data->bits &0x00800 ) ?1 :0 ; - command += (data->bits &0x01000 ) ?0 :0x40 ; - scancode = system <<8 | command; - -IR_dprintk (1 ,"RC5 scancode 0x%04x (toggle: %u)\n" , - scancode, toggle); - } - -ir_keydown (input_dev, scancode, toggle); data->state =STATE_INACTIVE ;return 0 ; }
drivers/media/IR/mceusb.c +9 -2
@@ -674,6 +674,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len) {struct ir_raw_event rawir = { .pulse = false, .duration =0 };int i, start_index =0 ; +bool has_events;u8 hdr =MCE_CONTROL_HEADER ;/* skip meaningless 0xb1 0x60 header bytes on orig receiver */
@@ -698,9 +699,12 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)return ; } + has_events = false;for (; (ir->rem >0 ) && (i < buf_len); i++) { ir->rem--; + has_events = true; + rawir.pulse = ((ir->buf_in[i] &MCE_PULSE_BIT ) !=0 ); rawir.duration = (ir->buf_in[i] &MCE_PULSE_MASK ) *MCE_TIME_UNIT *1000 ;
@@ -728,8 +732,10 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)if (ir->buf_in[i] ==0x80 || ir->buf_in[i] ==0x9f ) ir->rem =0 ; -dev_dbg (ir->dev,"calling ir_raw_event_handle\n" ); -ir_raw_event_handle (ir->idev); +if (has_events) { +dev_dbg (ir->dev,"calling ir_raw_event_handle\n" ); +ir_raw_event_handle (ir->idev); + } } }
@@ -772,6 +778,7 @@ static void mceusb_dev_recv(struct urb *urb, struct pt_regs *regs)case -EPIPE :default : +dev_dbg (ir->dev,"Error: urb status = %d\n" );break ; }
Quoted 2 lines · Mauro Carvalho Chehab
The RC5 decoding is complete at a
BIT_ENDstate. there's no reason to wait for the next space to produce a code.
Well, if I'm reading things correctly here, I think the only true functional difference made to the decoder here was to skip the if (ev.pulse) break; check in STATE_FINISHED, no? In other words, this looks like it was purely an issue with the receiver data parsing, which was ending on a pulse instead of a space. I can make this guess in greater confidence having seen another patch somewhere that implements a different buffer parsing routine for the polaris devices though... ;)
The mceusb portion of the patch is probably a worthwhile micro-optimization of its ir processing routine though -- don't call ir_raw_event_handle if there's no event to handle. Lemme just go ahead and merge that part via my staging tree, if you don't mind. (I've got a dozen or so IR patches that have been queueing up, planning on another pull req relatively soon).
Signature · 2 lines
Jarod Wilson [email protected]
Em 21-10-2010 11:46, Jarod Wilson escreveu:
Quoted 9 lines · Mauro Carvalho Chehab
Quoted 3 lines
The RC5 decoding is complete at a
BIT_ENDstate. there's no reason to wait for the next space to produce a code.Well, if I'm reading things correctly here, I think the only true functional difference made to the decoder here was to skip the if (ev.pulse) break; check in
STATE_FINISHED, no? In other words, this looks like it was purely an issue with the receiver data parsing, which was ending on a pulse instead of a space. I can make this guess in greater confidence having seen another patch somewhere that implements a different buffer parsing routine for the polaris devices though... ;)
This patch doesn't solve the Polaris issue ;)
While I made it in the hope that it would fix Polaris (it ended by not solving), I still think it can be kept, as it speeds up a little bit the RC-5 output, by not waiting for the last space.
I'll be forwarding soon the polaris decoder fixes patch, and another mceusb patch I did, improving data decode on debug mode.
Quoted 4 lines
The mceusb portion of the patch is probably a worthwhile micro-optimization of its ir processing routine though -- don't call
ir_raw_event_handleif there's no event to handle. Lemme just go ahead and merge that part via my staging tree, if you don't mind. (I've got a dozen or so IR patches that have been queueing up, planning on another pull req relatively soon).
Oh! I didn't notice that this went into the patch... for sure it doesn't belong here. Yes, it is just a cleanup for mceusb. Feel free to split it, adding a proper description for it and preserving my SOB.
Thanks,
Mauro
Quoted 19 lines · Mauro Carvalho Chehab
Em 21-10-2010 11:46, Jarod Wilson escreveu:
Quoted 9 lines · Mauro Carvalho Chehab
Quoted 3 lines
The RC5 decoding is complete at a
BIT_ENDstate. there's no reason to wait for the next space to produce a code.Well, if I'm reading things correctly here, I think the only true functional difference made to the decoder here was to skip the if (ev.pulse) break; check in
STATE_FINISHED, no? In other words, this looks like it was purely an issue with the receiver data parsing, which was ending on a pulse instead of a space. I can make this guess in greater confidence having seen another patch somewhere that implements a different buffer parsing routine for the polaris devices though... ;)This patch doesn't solve the Polaris issue ;)
While I made it in the hope that it would fix Polaris (it ended by not solving), I still think it can be kept, as it speeds up a little bit the RC-5 output, by not waiting for the last space.
I'll be forwarding soon the polaris decoder fixes patch, and another mceusb patch I did, improving data decode on debug mode.
Quoted 8 lines
Quoted 4 lines
The mceusb portion of the patch is probably a worthwhile micro-optimization of its ir processing routine though -- don't call
ir_raw_event_handleif there's no event to handle. Lemme just go ahead and merge that part via my staging tree, if you don't mind. (I've got a dozen or so IR patches that have been queueing up, planning on another pull req relatively soon).Oh! I didn't notice that this went into the patch... for sure it doesn't belong here. Yes, it is just a cleanup for mceusb. Feel free to split it, adding a proper description for it and preserving my SOB.
No need in this patch.
My patch resolves the issue genericly by making the driver send the timeout message at the end of the data among the current length of the space (which will continue to grow).
Just make mceusb send the timeout sample.
Best regards,
Maxim Levitsky
Em 22-10-2010 23:25, Maxim Levitsky escreveu:
Quoted 29 lines · Mauro Carvalho Chehab
Quoted 19 lines
Em 21-10-2010 11:46, Jarod Wilson escreveu:
Quoted 9 lines · Mauro Carvalho Chehab
Quoted 3 lines
The RC5 decoding is complete at a
BIT_ENDstate. there's no reason to wait for the next space to produce a code.Well, if I'm reading things correctly here, I think the only true functional difference made to the decoder here was to skip the if (ev.pulse) break; check in
STATE_FINISHED, no? In other words, this looks like it was purely an issue with the receiver data parsing, which was ending on a pulse instead of a space. I can make this guess in greater confidence having seen another patch somewhere that implements a different buffer parsing routine for the polaris devices though... ;)This patch doesn't solve the Polaris issue ;)
While I made it in the hope that it would fix Polaris (it ended by not solving), I still think it can be kept, as it speeds up a little bit the RC-5 output, by not waiting for the last space.
I'll be forwarding soon the polaris decoder fixes patch, and another mceusb patch I did, improving data decode on debug mode.
Quoted 8 lines
Quoted 4 lines
The mceusb portion of the patch is probably a worthwhile micro-optimization of its ir processing routine though -- don't call
ir_raw_event_handleif there's no event to handle. Lemme just go ahead and merge that part via my staging tree, if you don't mind. (I've got a dozen or so IR patches that have been queueing up, planning on another pull req relatively soon).Oh! I didn't notice that this went into the patch... for sure it doesn't belong here. Yes, it is just a cleanup for mceusb. Feel free to split it, adding a proper description for it and preserving my SOB.
Quoted 6 lines
No need in this patch.
My patch resolves the issue genericly by making the driver send the timeout message at the end of the data among the current length of the space (which will continue to grow).Just make mceusb send the timeout sample.
Hmm... now, a RC6(0) remote is also being decoded as a RC5 remote. Could this be due to your patches?
Not handling the entire state machine could cause troubles with some decoders.
Cheers,
Mauro
Quoted 35 lines · Mauro Carvalho Chehab
Em 22-10-2010 23:25, Maxim Levitsky escreveu:
Quoted 34 lines · Mauro Carvalho Chehab
Quoted 19 lines
Em 21-10-2010 11:46, Jarod Wilson escreveu:
Quoted 9 lines · Mauro Carvalho Chehab
Quoted 3 lines
The RC5 decoding is complete at a
BIT_ENDstate. there's no reason to wait for the next space to produce a code.Well, if I'm reading things correctly here, I think the only true functional difference made to the decoder here was to skip the if (ev.pulse) break; check in
STATE_FINISHED, no? In other words, this looks like it was purely an issue with the receiver data parsing, which was ending on a pulse instead of a space. I can make this guess in greater confidence having seen another patch somewhere that implements a different buffer parsing routine for the polaris devices though... ;)This patch doesn't solve the Polaris issue ;)
While I made it in the hope that it would fix Polaris (it ended by not solving), I still think it can be kept, as it speeds up a little bit the RC-5 output, by not waiting for the last space.
I'll be forwarding soon the polaris decoder fixes patch, and another mceusb patch I did, improving data decode on debug mode.
Quoted 8 lines
Quoted 4 lines
The mceusb portion of the patch is probably a worthwhile micro-optimization of its ir processing routine though -- don't call
ir_raw_event_handleif there's no event to handle. Lemme just go ahead and merge that part via my staging tree, if you don't mind. (I've got a dozen or so IR patches that have been queueing up, planning on another pull req relatively soon).Oh! I didn't notice that this went into the patch... for sure it doesn't belong here. Yes, it is just a cleanup for mceusb. Feel free to split it, adding a proper description for it and preserving my SOB.
No need in this patch.
My patch resolves the issue genericly by making the driver send the timeout message at the end of the data among the current length of the space (which will continue to grow).
Quoted 5 lines
Quoted 1 line
Just make mceusb send the timeout sample.
Hmm... now, a RC6(0) remote is also being decoded as a RC5 remote. Could this be due to your patches?
Very unlikely.
Probably the remote is really RC5
RC5 and RC6 just differ too much.
And besides mceusb doesn't even send the timeouts yet. Still you could revert and see if it helps.
The only patch that has any potential of breaking anything is the '[PATCH 1/3] IR: extend ir_raw_event and do refactoring'
Note that I tried going the way this patch does.
And it really doesn't work.
Best regards,
Maxim Levitsky