[4/5] Input: xpad - restore LED state after device resume
diff mbox

Message ID 20170205003002.28160-5-aicommander@gmail.com
State Under Review
Headers show

Commit Message

Cameron Gutman Feb. 5, 2017, 12:30 a.m. UTC
The state of pad LEDs can be inconsistent when the system is
woken up after sleep. Rather than leaving the controllers blinking,
let's resend the last LED command to Xbox 360 pads on resume.

Since Xbox One pads stop flashing only when reinitialized, we'll
send them the initialization packet so they calm down too.

Signed-off-by: Cameron Gutman <aicommander@gmail.com>
---
 drivers/input/joystick/xpad.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

kbuild test robot Feb. 5, 2017, 1:17 a.m. UTC | #1
Hi Cameron,

[auto build test ERROR on input/next]
[also build test ERROR on v4.10-rc6 next-20170203]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Cameron-Gutman/Correctly-support-some-quirky-Xbox-One-pads/20170205-083659
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: x86_64-randconfig-x016-201706 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/input/joystick/xpad.c: In function 'xpad_resume':
>> drivers/input/joystick/xpad.c:1700:10: error: 'struct usb_xpad' has no member named 'led'
     if (xpad->led)
             ^~
>> drivers/input/joystick/xpad.c:1701:3: error: implicit declaration of function 'xpad_send_led_command' [-Werror=implicit-function-declaration]
      xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
      ^~~~~~~~~~~~~~~~~~~~~
   drivers/input/joystick/xpad.c:1701:35: error: 'struct usb_xpad' has no member named 'led'
      xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
                                      ^~
   cc1: some warnings being treated as errors

vim +1700 drivers/input/joystick/xpad.c

  1694				retval = xpad_start_xbox_one(xpad);
  1695			}
  1696			mutex_unlock(&input->mutex);
  1697		}
  1698	
  1699		/* LED state is lost across resume, so resend the last command */
> 1700		if (xpad->led)
> 1701			xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
  1702	
  1703		return retval;
  1704	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Cameron Gutman Feb. 5, 2017, 1:47 a.m. UTC | #2
On 02/04/2017 05:17 PM, kbuild test robot wrote:
> Hi Cameron,
> 
> [auto build test ERROR on input/next]
> [also build test ERROR on v4.10-rc6 next-20170203]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Cameron-Gutman/Correctly-support-some-quirky-Xbox-One-pads/20170205-083659
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
> config: x86_64-randconfig-x016-201706 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/input/joystick/xpad.c: In function 'xpad_resume':
>>> drivers/input/joystick/xpad.c:1700:10: error: 'struct usb_xpad' has no member named 'led'
>      if (xpad->led)
>              ^~
>>> drivers/input/joystick/xpad.c:1701:3: error: implicit declaration of function 'xpad_send_led_command' [-Werror=implicit-function-declaration]
>       xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
>       ^~~~~~~~~~~~~~~~~~~~~
>    drivers/input/joystick/xpad.c:1701:35: error: 'struct usb_xpad' has no member named 'led'
>       xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
>                                       ^~
>    cc1: some warnings being treated as errors

Whoops, missed a CONFIG_JOYSTICK_XPAD_LEDS guard around that.

I'll send a v2 in the next day or two, incorporating this fix plus any review
comments made in the meantime.

> 
> vim +1700 drivers/input/joystick/xpad.c
> 
>   1694				retval = xpad_start_xbox_one(xpad);
>   1695			}
>   1696			mutex_unlock(&input->mutex);
>   1697		}
>   1698	
>   1699		/* LED state is lost across resume, so resend the last command */
>> 1700		if (xpad->led)
>> 1701			xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
>   1702	
>   1703		return retval;
>   1704	}
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

Regards,
Cameron
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Feb. 6, 2017, 10:05 p.m. UTC | #3
On Sat, Feb 04, 2017 at 04:30:01PM -0800, Cameron Gutman wrote:
> The state of pad LEDs can be inconsistent when the system is
> woken up after sleep. Rather than leaving the controllers blinking,
> let's resend the last LED command to Xbox 360 pads on resume.
> 
> Since Xbox One pads stop flashing only when reinitialized, we'll
> send them the initialization packet so they calm down too.
> 
> Signed-off-by: Cameron Gutman <aicommander@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 13f298d..1179266 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1683,11 +1683,23 @@ static int xpad_resume(struct usb_interface *intf)
>  		retval = xpad360w_start_input(xpad);
>  	} else {
>  		mutex_lock(&input->mutex);
> -		if (input->users)
> +		if (input->users) {
>  			retval = xpad_start_input(xpad);
> +		} else if (xpad->xtype == XTYPE_XBOXONE) {
> +			/*
> +			 * Even if there are no users, we'll send Xbox One pads
> +			 * the startup sequence so they don't sit there and
> +			 * blink until somebody opens the input device again.
> +			 */
> +			retval = xpad_start_xbox_one(xpad);
> +		}
>  		mutex_unlock(&input->mutex);
>  	}
>  
> +	/* LED state is lost across resume, so resend the last command */
> +	if (xpad->led)
> +		xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);

I can see that we might need to send the "start" command, but I am not
sure that explicitly calling xpad_send_led_command() is needed: LED core
should restore LED state upon resume, and this should happen after its
parent (i.e. xbox device) is resumed.

Thanks.
Cameron Gutman Feb. 6, 2017, 10:57 p.m. UTC | #4
On 02/06/2017 02:05 PM, Dmitry Torokhov wrote:
> On Sat, Feb 04, 2017 at 04:30:01PM -0800, Cameron Gutman wrote:
>> The state of pad LEDs can be inconsistent when the system is
>> woken up after sleep. Rather than leaving the controllers blinking,
>> let's resend the last LED command to Xbox 360 pads on resume.
>>
>> Since Xbox One pads stop flashing only when reinitialized, we'll
>> send them the initialization packet so they calm down too.
>>
>> Signed-off-by: Cameron Gutman <aicommander@gmail.com>
>> ---
>>  drivers/input/joystick/xpad.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
>> index 13f298d..1179266 100644
>> --- a/drivers/input/joystick/xpad.c
>> +++ b/drivers/input/joystick/xpad.c
>> @@ -1683,11 +1683,23 @@ static int xpad_resume(struct usb_interface *intf)
>>  		retval = xpad360w_start_input(xpad);
>>  	} else {
>>  		mutex_lock(&input->mutex);
>> -		if (input->users)
>> +		if (input->users) {
>>  			retval = xpad_start_input(xpad);
>> +		} else if (xpad->xtype == XTYPE_XBOXONE) {
>> +			/*
>> +			 * Even if there are no users, we'll send Xbox One pads
>> +			 * the startup sequence so they don't sit there and
>> +			 * blink until somebody opens the input device again.
>> +			 */
>> +			retval = xpad_start_xbox_one(xpad);
>> +		}
>>  		mutex_unlock(&input->mutex);
>>  	}
>>  
>> +	/* LED state is lost across resume, so resend the last command */
>> +	if (xpad->led)
>> +		xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
> 
> I can see that we might need to send the "start" command, but I am not
> sure that explicitly calling xpad_send_led_command() is needed: LED core
> should restore LED state upon resume, and this should happen after its
> parent (i.e. xbox device) is resumed.
> 
> Thanks.
> 

Ah, so it seems we're missing the LED_CORE_SUSPENDRESUME flag on our LED
device required for that to happen automatically. I'll just use that
instead of manually setting brightness on resume.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 13f298d..1179266 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1683,11 +1683,23 @@  static int xpad_resume(struct usb_interface *intf)
 		retval = xpad360w_start_input(xpad);
 	} else {
 		mutex_lock(&input->mutex);
-		if (input->users)
+		if (input->users) {
 			retval = xpad_start_input(xpad);
+		} else if (xpad->xtype == XTYPE_XBOXONE) {
+			/*
+			 * Even if there are no users, we'll send Xbox One pads
+			 * the startup sequence so they don't sit there and
+			 * blink until somebody opens the input device again.
+			 */
+			retval = xpad_start_xbox_one(xpad);
+		}
 		mutex_unlock(&input->mutex);
 	}
 
+	/* LED state is lost across resume, so resend the last command */
+	if (xpad->led)
+		xpad_send_led_command(xpad, xpad->led->led_cdev.brightness);
+
 	return retval;
 }