[5/7] rc-core: ir-raw - leave the internals of rc_dev alone
diff mbox

Message ID 149365501711.13489.17027324920634077369.stgit@zeus.hardeman.nu
State New
Headers show

Commit Message

David Härdeman May 1, 2017, 4:10 p.m. UTC
Replace the REP_DELAY value with a static value, which makes more sense.
Automatic repeat handling in the input layer has no relevance for the drivers
idea of "a long time".

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/rc/rc-ir-raw.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Sean Young May 23, 2017, 9:20 a.m. UTC | #1
On Mon, May 01, 2017 at 06:10:17PM +0200, David Härdeman wrote:
> Replace the REP_DELAY value with a static value, which makes more sense.
> Automatic repeat handling in the input layer has no relevance for the drivers
> idea of "a long time".
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
>  drivers/media/rc/rc-ir-raw.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index ae7785c4fbe7..967ab9531e0a 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -102,20 +102,18 @@ int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type)
>  	s64			delta; /* ns */
>  	DEFINE_IR_RAW_EVENT(ev);
>  	int			rc = 0;
> -	int			delay;
>  
>  	if (!dev->raw)
>  		return -EINVAL;
>  
>  	now = ktime_get();
>  	delta = ktime_to_ns(ktime_sub(now, dev->raw->last_event));
> -	delay = MS_TO_NS(dev->input_dev->rep[REP_DELAY]);
>  
>  	/* Check for a long duration since last event or if we're
>  	 * being called for the first time, note that delta can't
>  	 * possibly be negative.
>  	 */
> -	if (delta > delay || !dev->raw->last_type)
> +	if (delta > MS_TO_NS(500) || !dev->raw->last_type)
>  		type |= IR_START_EVENT;

So this is just a fail-safe to ensure that the IR decoders are reset after
a period of IR silence. The decoders should reset themselves anyway if they
receive a long space, so it's just belt and braces.

Why is a static value better? At least REP_DELAY can be changed from
user space.

Maybe we should do away with it.


Sean

>  	else
>  		ev.duration = delta;
David Härdeman May 28, 2017, 8:31 a.m. UTC | #2
On Tue, May 23, 2017 at 10:20:27AM +0100, Sean Young wrote:
>On Mon, May 01, 2017 at 06:10:17PM +0200, David Härdeman wrote:
>> Replace the REP_DELAY value with a static value, which makes more sense.
>> Automatic repeat handling in the input layer has no relevance for the drivers
>> idea of "a long time".
>> 
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>> ---
>>  drivers/media/rc/rc-ir-raw.c |    4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
>> index ae7785c4fbe7..967ab9531e0a 100644
>> --- a/drivers/media/rc/rc-ir-raw.c
>> +++ b/drivers/media/rc/rc-ir-raw.c
>> @@ -102,20 +102,18 @@ int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type)
>>  	s64			delta; /* ns */
>>  	DEFINE_IR_RAW_EVENT(ev);
>>  	int			rc = 0;
>> -	int			delay;
>>  
>>  	if (!dev->raw)
>>  		return -EINVAL;
>>  
>>  	now = ktime_get();
>>  	delta = ktime_to_ns(ktime_sub(now, dev->raw->last_event));
>> -	delay = MS_TO_NS(dev->input_dev->rep[REP_DELAY]);
>>  
>>  	/* Check for a long duration since last event or if we're
>>  	 * being called for the first time, note that delta can't
>>  	 * possibly be negative.
>>  	 */
>> -	if (delta > delay || !dev->raw->last_type)
>> +	if (delta > MS_TO_NS(500) || !dev->raw->last_type)
>>  		type |= IR_START_EVENT;
>
>So this is just a fail-safe to ensure that the IR decoders are reset after
>a period of IR silence. The decoders should reset themselves anyway if they
>receive a long space, so it's just belt and braces.

Not 100% sure but it also checks for the first call to
ir_raw_event_store_edge()...

>Why is a static value better? At least REP_DELAY can be changed from
>user space.

REP_DELAY serves a completely different purpose. It controls how long a
key should be pressed before the input layer should start generating
soft-repeat events.

The timeout here is related to the IR protocol handling...which is a
quite different matter.

>Maybe we should do away with it.

Maybe...but that'd be a different patch...I think this fix should go in
nevertheless.

Patch
diff mbox

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index ae7785c4fbe7..967ab9531e0a 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -102,20 +102,18 @@  int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type)
 	s64			delta; /* ns */
 	DEFINE_IR_RAW_EVENT(ev);
 	int			rc = 0;
-	int			delay;
 
 	if (!dev->raw)
 		return -EINVAL;
 
 	now = ktime_get();
 	delta = ktime_to_ns(ktime_sub(now, dev->raw->last_event));
-	delay = MS_TO_NS(dev->input_dev->rep[REP_DELAY]);
 
 	/* Check for a long duration since last event or if we're
 	 * being called for the first time, note that delta can't
 	 * possibly be negative.
 	 */
-	if (delta > delay || !dev->raw->last_type)
+	if (delta > MS_TO_NS(500) || !dev->raw->last_type)
 		type |= IR_START_EVENT;
 	else
 		ev.duration = delta;