| Message ID | b211ba5a-0a48-783b-530d-b61fd16273a1@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 27, 2016 at 06:23:19PM -0700, Cameron Gutman wrote: > On 07/27/2016 05:29 PM, Dmitry Torokhov wrote: > > On Wed, Jul 27, 2016 at 05:10:41PM -0700, Cameron Gutman wrote: > >> On 07/27/2016 05:04 PM, Dmitry Torokhov wrote: > >>> On Wed, Jul 27, 2016 at 04:24:06PM -0700, Cameron Gutman wrote: > >>>> On 07/27/2016 02:39 PM, Dmitry Torokhov wrote: > >>>>> On Wed, Jul 27, 2016 at 02:32:22PM -0700, Dmitry Torokhov wrote: > >>>>>> On Mon, Jul 25, 2016 at 10:35:08PM -0700, Cameron Gutman wrote: > >>>>>>> When the USB wireless adapter is suspended, the controllers > >>>>>>> lose their connection. This causes them to start flashing > >>>>>>> their LED rings and searching for the wireless adapter > >>>>>>> again, wasting the controller's battery power. > >>>>>>> > >>>>>>> Instead, we will tell the controllers to power down when > >>>>>>> we suspend. This mirrors the behavior of the controllers > >>>>>>> when connected to the console itself and how the official > >>>>>>> Xbox One wireless adapter behaves on Windows. > >>>>>>> > >>>>>>> Signed-off-by: Cameron Gutman <aicommander@gmail.com> > >>>>>>> --- > >>>>>>> This patch is independent of the other xpad patch [0] that I > >>>>>>> submitted (and decided to wait on). It applies against > >>>>>>> unmodified xpad.c in master. > >>>>>>> > >>>>>>> [0] http://www.spinics.net/lists/linux-input/msg46062.html > >>>>>>> --- > >>>>>>> drivers/input/joystick/xpad.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > >>>>>>> 1 file changed, 43 insertions(+) > >>>>>>> > >>>>>>> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c > >>>>>>> index a529a45..3408019 100644 > >>>>>>> --- a/drivers/input/joystick/xpad.c > >>>>>>> +++ b/drivers/input/joystick/xpad.c > >>>>>>> @@ -115,6 +115,10 @@ static bool sticks_to_null; > >>>>>>> module_param(sticks_to_null, bool, S_IRUGO); > >>>>>>> MODULE_PARM_DESC(sticks_to_null, "Do not map sticks at all for unknown pads"); > >>>>>>> > >>>>>>> +static bool disable_auto_poweroff; > >>>>>>> +module_param(disable_auto_poweroff, bool, S_IRUGO); > >>>>>>> +MODULE_PARM_DESC(disable_auto_poweroff, "Do not power off wireless controllers on suspend"); > >>>>>> > >>>>>> Why negating? Why not do > >>>>>> > >>>>>> static bool xpad_auto_poweroff = true; > >>>>>> > >>>>>> ? > >>>>>> > >>>>>> (No need to resubmit if agree/disagree, I can fix up on my side). > >>>> > >>>> Yeah, sounds fine to make it default Y and get rid of the negation. > >>>> > >>>> I'm fine with it if you want to make that change and apply it. See my > >>>> comments below about the other issues. > >>>> > >>>>> > >>>>> By the way, I think we can allow root writing to it, there is nothing > >>>>> that stops us from changing behavior at runtime. > >>>> > >>>> Yep, I was following the convention of the existing module params. They > >>>> can probably all be changed to allow root writing. > >>> > >>> My rule of thumb is we allow writing if behavior changes immediately. > >>> The other parameters only affect devices that will be bound after the > >>> parameter has been changed, which woudl be confusing. So they are RO and > >>> you have to specify them at module load time. > >>> > >>> Thanks. > >>> > >> > >> Sounds reasonable. Would you like to make the permission change also or > >> do you prefer I resubmit the patch with those 2 modifications? > > > > No, I made them. If you have a moment please take a look at my "next" > > branch on kernel.org. > > > > I reviewed and tested your changes. I think you forgot to undo one of the > negations. With the patch below applied on the commit in next, it works > correctly. Oops, you are right. I'll fold it into the original patch. > > --- > drivers/input/joystick/xpad.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c > index 0c8d5c3..83af17a 100644 > --- a/drivers/input/joystick/xpad.c > +++ b/drivers/input/joystick/xpad.c > @@ -1631,7 +1631,7 @@ static int xpad_suspend(struct usb_interface *intf, pm_message_t message) > * Unless explicitly disabled, power them down > * so they don't just sit there flashing. > */ > - if (!auto_poweroff && xpad->pad_present) > + if (auto_poweroff && xpad->pad_present) > xpad360w_poweroff_controller(xpad); > } else { > mutex_lock(&input->mutex); > -- > 2.7.4 >
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c index 0c8d5c3..83af17a 100644 --- a/drivers/input/joystick/xpad.c +++ b/drivers/input/joystick/xpad.c @@ -1631,7 +1631,7 @@ static int xpad_suspend(struct usb_interface *intf, pm_message_t message) * Unless explicitly disabled, power them down * so they don't just sit there flashing. */ - if (!auto_poweroff && xpad->pad_present) + if (auto_poweroff && xpad->pad_present) xpad360w_poweroff_controller(xpad); } else { mutex_lock(&input->mutex);