LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/1] media: nec-decoder: remove trailer_space state
@ 2018-04-20 18:51 Vladislav Zhurba
  2018-04-21 10:27 ` Sean Young
  0 siblings, 1 reply; 2+ messages in thread
From: Vladislav Zhurba @ 2018-04-20 18:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: mchehab, linux-media, Daniel Fu, Vladislav Zhurba

From: Daniel Fu <danifu@nvidia.com>

Remove STATE_TRAILER_SPACE from state machine.
Causing 2 issue:
- can not decode the keycode, if it didn't following with
  another keycode/repeat code
- will generate one more code in current logic.
  i.e. key_right + repeat code + key_left + repeat code.
  expect: key_right, key_left.
  Result: key_right, key_right, key_right.
  Reason: when receive repeat code of key_right, state machine will
  stay in STATE_TRAILER_SPACE state, then wait for a new interrupt,
  if an interrupt came after keyup_timer, then will generate another
  fake key.

According to the NEC protocol, it don't need a trailer space. Remove it.

Signed-off-by: Daniel Fu <danifu@nvidia.com>
Signed-off-by: Vladislav Zhurba <vzhurba@nvidia.com>
---
 drivers/media/rc/ir-nec-decoder.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 21647b809e6f..760b66affd1a 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -128,16 +128,6 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
 		if (!eq_margin(ev.duration, NEC_TRAILER_PULSE, NEC_UNIT / 2))
 			break;
 
-		data->state = STATE_TRAILER_SPACE;
-		return 0;
-
-	case STATE_TRAILER_SPACE:
-		if (ev.pulse)
-			break;
-
-		if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2))
-			break;
-
 		if (data->count == NEC_NBITS) {
 			address     = bitrev8((data->bits >> 24) & 0xff);
 			not_address = bitrev8((data->bits >> 16) & 0xff);
-- 
2.16.2

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH 1/1] media: nec-decoder: remove trailer_space state
  2018-04-20 18:51 [PATCH 1/1] media: nec-decoder: remove trailer_space state Vladislav Zhurba
@ 2018-04-21 10:27 ` Sean Young
  0 siblings, 0 replies; 2+ messages in thread
From: Sean Young @ 2018-04-21 10:27 UTC (permalink / raw)
  To: Vladislav Zhurba; +Cc: linux-kernel, mchehab, linux-media, Daniel Fu

On Fri, Apr 20, 2018 at 11:51:39AM -0700, Vladislav Zhurba wrote:
> From: Daniel Fu <danifu@nvidia.com>
> 
> Remove STATE_TRAILER_SPACE from state machine.
> Causing 2 issue:
> - can not decode the keycode, if it didn't following with
>   another keycode/repeat code
> - will generate one more code in current logic.
>   i.e. key_right + repeat code + key_left + repeat code.
>   expect: key_right, key_left.
>   Result: key_right, key_right, key_right.
>   Reason: when receive repeat code of key_right, state machine will
>   stay in STATE_TRAILER_SPACE state, then wait for a new interrupt,
>   if an interrupt came after keyup_timer, then will generate another
>   fake key.

This behaviour is symptomatic of rc driver which does not generate
ir timeouts. If an rc driver does not do this, then it won't be just the
nec protocol which is not parsed correctly. For example, rc-6 in mode 6a
can have 16, 24 or 32 bits and we rely on the ir timeout to demarcate the
end of the IR message -- else we are stuck with the behaviour as you
describe above.

> According to the NEC protocol, it don't need a trailer space. Remove it.

This isn't the right solution, so NAK I'm afraid. 

Please let us know what rc driver you are using, I'm happy to help fix it.


Sean

> 
> Signed-off-by: Daniel Fu <danifu@nvidia.com>
> Signed-off-by: Vladislav Zhurba <vzhurba@nvidia.com>
> ---
>  drivers/media/rc/ir-nec-decoder.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
> index 21647b809e6f..760b66affd1a 100644
> --- a/drivers/media/rc/ir-nec-decoder.c
> +++ b/drivers/media/rc/ir-nec-decoder.c
> @@ -128,16 +128,6 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
>  		if (!eq_margin(ev.duration, NEC_TRAILER_PULSE, NEC_UNIT / 2))
>  			break;
>  
> -		data->state = STATE_TRAILER_SPACE;
> -		return 0;
> -
> -	case STATE_TRAILER_SPACE:
> -		if (ev.pulse)
> -			break;
> -
> -		if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2))
> -			break;
> -
>  		if (data->count == NEC_NBITS) {
>  			address     = bitrev8((data->bits >> 24) & 0xff);
>  			not_address = bitrev8((data->bits >> 16) & 0xff);
> -- 
> 2.16.2

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-04-21 10:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20 18:51 [PATCH 1/1] media: nec-decoder: remove trailer_space state Vladislav Zhurba
2018-04-21 10:27 ` Sean Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).