Re: [PATCH] media: rc: only turn on LED if keypress generated

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 16, 2014 at 10:56:22PM +0000, James Hogan wrote:
> Since v3.12, specifically 153a60bb0fac ([media] rc: add feedback led
> trigger for rc keypresses), an LED trigger is activated on IR keydown
> whether or not a keypress is generated (i.e. even if there's no matching
> keycode). However the repeat and keyup logic isn't used unless there is
> a keypress, which results in non-keypress keydown events turning on the
> LED and not turning it off again.

Yes, this is a bug. I have a similar patch waiting to be submitted but
you beat me to it. 

Acked-by: Sean Young <sean@xxxxxxxx>

> 
> On the assumption that the intent was for the LED only to light up on
> valid key presses (you probably don't want it lighting up for the wrong
> remote control for example), move the led_trigger_event() call inside
> the keycode check.
> 
> Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
> Cc: Sean Young <sean@xxxxxxxx>
> Cc: Mauro Carvalho Chehab <m.chehab@xxxxxxxxxxx>
> Cc: linux-media@xxxxxxxxxxxxxxx
> ---
> Was that the original intent? If not it could be tweaked to set
> dev->keypressed in either case instead, so that they LED trigger works
> for unmapped scancodes too.

I'd say that the feedback led should only be activated if a valid key is
pressed; doing this for for unmapped scancodes is jus going to cause confusion.

> ---
>  drivers/media/rc/rc-main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 46da365..cff9d53 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -649,9 +649,10 @@ static void ir_do_keydown(struct rc_dev *dev, int scancode,
>  			   "key 0x%04x, scancode 0x%04x\n",
>  			   dev->input_name, keycode, scancode);
>  		input_report_key(dev->input_dev, keycode, 1);
> +
> +		led_trigger_event(led_feedback, LED_FULL);
>  	}
>  
> -	led_trigger_event(led_feedback, LED_FULL);
>  	input_sync(dev->input_dev);
>  }
>  
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux