Re: [PATCH] Input: xpad - power off wireless 360 controllers on suspend

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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.

-- 
Dmitry