* [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code
@ 2010-10-20 17:18 Mauro Carvalho Chehab
2010-10-21 13:46 ` Jarod Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-20 17:18 UTC (permalink / raw)
To: Linux Media Mailing List
The RC5 decoding is complete at a BIT_END state. there's no reason
to wait for the next space to produce a code.
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
index df4770d..4139678 100644
--- a/drivers/media/IR/ir-rc5-decoder.c
+++ b/drivers/media/IR/ir-rc5-decoder.c
@@ -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;
}
diff --git a/drivers/media/IR/mceusb.c b/drivers/media/IR/mceusb.c
index 61ccb8f..d779f20 100644
--- a/drivers/media/IR/mceusb.c
+++ b/drivers/media/IR/mceusb.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code
2010-10-20 17:18 [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code Mauro Carvalho Chehab
@ 2010-10-21 13:46 ` Jarod Wilson
2010-10-21 13:56 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 6+ messages in thread
From: Jarod Wilson @ 2010-10-21 13:46 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List
On Oct 20, 2010, at 1:18 PM, Mauro Carvalho Chehab wrote:
> The RC5 decoding is complete at a BIT_END state. 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).
--
Jarod Wilson
jarod@wilsonet.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code
2010-10-21 13:46 ` Jarod Wilson
@ 2010-10-21 13:56 ` Mauro Carvalho Chehab
2010-10-23 1:25 ` Maxim Levitsky
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-21 13:56 UTC (permalink / raw)
To: Jarod Wilson; +Cc: Linux Media Mailing List
Em 21-10-2010 11:46, Jarod Wilson escreveu:
> On Oct 20, 2010, at 1:18 PM, Mauro Carvalho Chehab wrote:
>
>> The RC5 decoding is complete at a BIT_END state. 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.
> 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).
>
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code
2010-10-21 13:56 ` Mauro Carvalho Chehab
@ 2010-10-23 1:25 ` Maxim Levitsky
2010-10-23 1:39 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 6+ messages in thread
From: Maxim Levitsky @ 2010-10-23 1:25 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Jarod Wilson, Linux Media Mailing List
On Thu, 2010-10-21 at 11:56 -0200, Mauro Carvalho Chehab wrote:
> Em 21-10-2010 11:46, Jarod Wilson escreveu:
> > On Oct 20, 2010, at 1:18 PM, Mauro Carvalho Chehab wrote:
> >
> >> The RC5 decoding is complete at a BIT_END state. 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.
>
> > 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).
> >
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code
2010-10-23 1:25 ` Maxim Levitsky
@ 2010-10-23 1:39 ` Mauro Carvalho Chehab
2010-10-23 2:02 ` Maxim Levitsky
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-23 1:39 UTC (permalink / raw)
To: Maxim Levitsky; +Cc: Jarod Wilson, Linux Media Mailing List
Em 22-10-2010 23:25, Maxim Levitsky escreveu:
> On Thu, 2010-10-21 at 11:56 -0200, Mauro Carvalho Chehab wrote:
>> Em 21-10-2010 11:46, Jarod Wilson escreveu:
>>> On Oct 20, 2010, at 1:18 PM, Mauro Carvalho Chehab wrote:
>>>
>>>> The RC5 decoding is complete at a BIT_END state. 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.
>>
>>> 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).
>>>
>>
>> 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code
2010-10-23 1:39 ` Mauro Carvalho Chehab
@ 2010-10-23 2:02 ` Maxim Levitsky
0 siblings, 0 replies; 6+ messages in thread
From: Maxim Levitsky @ 2010-10-23 2:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Jarod Wilson, Linux Media Mailing List
On Fri, 2010-10-22 at 23:39 -0200, Mauro Carvalho Chehab wrote:
> Em 22-10-2010 23:25, Maxim Levitsky escreveu:
> > On Thu, 2010-10-21 at 11:56 -0200, Mauro Carvalho Chehab wrote:
> >> Em 21-10-2010 11:46, Jarod Wilson escreveu:
> >>> On Oct 20, 2010, at 1:18 PM, Mauro Carvalho Chehab wrote:
> >>>
> >>>> The RC5 decoding is complete at a BIT_END state. 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.
> >>
> >>> 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).
> >>>
> >>
> >> 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.
>
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-23 2:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-20 17:18 [PATCH RFC] ir-rc5-decoder: don't wait for the end space to produce a code Mauro Carvalho Chehab
2010-10-21 13:46 ` Jarod Wilson
2010-10-21 13:56 ` Mauro Carvalho Chehab
2010-10-23 1:25 ` Maxim Levitsky
2010-10-23 1:39 ` Mauro Carvalho Chehab
2010-10-23 2:02 ` Maxim Levitsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox