| Message ID | 20230330024752.2405603-3-vi@endrift.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve GIP support | expand |
On Wed, Mar 29, 2023 at 07:47:52PM -0700, Vicki Pfau wrote: > This commit explicitly disables the audio interface the same way the official > driver does. This is needed for some controllers, such as the PowerA Enhanced > Wired Controller for Series X|S (0x20d6:0x200e) to report the guide button. > > Signed-off-by: Vicki Pfau <vi@endrift.com> > --- > drivers/input/joystick/xpad.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c > index 698224e1948f..c31fc4e9b310 100644 > --- a/drivers/input/joystick/xpad.c > +++ b/drivers/input/joystick/xpad.c > @@ -1396,6 +1396,14 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad) > unsigned long flags; > int retval; > > + /* Explicitly disable the audio interface. This is needed for some > + * controllers, such as the PowerA Enhanced Wired Controller > + * for Series X|S (0x20d6:0x200e) to report the guide button */ > + retval = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); > + if (retval) > + dev_warn(&xpad->dev->dev, > + "unable to disable audio interface: %d\n", retval); I would prefer if we first validated that the interface is in fact present. Can we do something like: if (usb_ifnum_to_if(xpad->udev, GIP_WIRED_INTF_AUDIO)) { error = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); if (error) ... } > + > spin_lock_irqsave(&xpad->odata_lock, flags); > > /* > -- > 2.40.0 > Thanks.
On 4/1/23 14:41, Dmitry Torokhov wrote: > On Wed, Mar 29, 2023 at 07:47:52PM -0700, Vicki Pfau wrote: >> This commit explicitly disables the audio interface the same way the official >> driver does. This is needed for some controllers, such as the PowerA Enhanced >> Wired Controller for Series X|S (0x20d6:0x200e) to report the guide button. >> >> Signed-off-by: Vicki Pfau <vi@endrift.com> >> --- >> drivers/input/joystick/xpad.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c >> index 698224e1948f..c31fc4e9b310 100644 >> --- a/drivers/input/joystick/xpad.c >> +++ b/drivers/input/joystick/xpad.c >> @@ -1396,6 +1396,14 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad) >> unsigned long flags; >> int retval; >> >> + /* Explicitly disable the audio interface. This is needed for some >> + * controllers, such as the PowerA Enhanced Wired Controller >> + * for Series X|S (0x20d6:0x200e) to report the guide button */ >> + retval = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); >> + if (retval) >> + dev_warn(&xpad->dev->dev, >> + "unable to disable audio interface: %d\n", retval); > > I would prefer if we first validated that the interface is in fact > present. Can we do something like: > > if (usb_ifnum_to_if(xpad->udev, GIP_WIRED_INTF_AUDIO)) { > error = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); > if (error) > ... > } > Yup, that makes sense. Wasn't sure what the cleanest way to do that was, though I'm unconvinced that the first party driver would work without this interface. It can't hurt to add the check. Should I resubmit both patches in the series, or just this one? >> + >> spin_lock_irqsave(&xpad->odata_lock, flags); >> >> /* >> -- >> 2.40.0 >> > > Thanks. > Thanks, Vicki
On Wed, Apr 05, 2023 at 07:40:32PM -0700, Vicki Pfau wrote: > > > On 4/1/23 14:41, Dmitry Torokhov wrote: > > On Wed, Mar 29, 2023 at 07:47:52PM -0700, Vicki Pfau wrote: > >> This commit explicitly disables the audio interface the same way the official > >> driver does. This is needed for some controllers, such as the PowerA Enhanced > >> Wired Controller for Series X|S (0x20d6:0x200e) to report the guide button. > >> > >> Signed-off-by: Vicki Pfau <vi@endrift.com> > >> --- > >> drivers/input/joystick/xpad.c | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c > >> index 698224e1948f..c31fc4e9b310 100644 > >> --- a/drivers/input/joystick/xpad.c > >> +++ b/drivers/input/joystick/xpad.c > >> @@ -1396,6 +1396,14 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad) > >> unsigned long flags; > >> int retval; > >> > >> + /* Explicitly disable the audio interface. This is needed for some > >> + * controllers, such as the PowerA Enhanced Wired Controller > >> + * for Series X|S (0x20d6:0x200e) to report the guide button */ > >> + retval = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); > >> + if (retval) > >> + dev_warn(&xpad->dev->dev, > >> + "unable to disable audio interface: %d\n", retval); > > > > I would prefer if we first validated that the interface is in fact > > present. Can we do something like: > > > > if (usb_ifnum_to_if(xpad->udev, GIP_WIRED_INTF_AUDIO)) { > > error = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); > > if (error) > > ... > > } > > > > Yup, that makes sense. Wasn't sure what the cleanest way to do that was, though I'm unconvinced that the first party driver would work without this interface. It can't hurt to add the check. > > Should I resubmit both patches in the series, or just this one? Both please. Thanks.
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c index 698224e1948f..c31fc4e9b310 100644 --- a/drivers/input/joystick/xpad.c +++ b/drivers/input/joystick/xpad.c @@ -1396,6 +1396,14 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad) unsigned long flags; int retval; + /* Explicitly disable the audio interface. This is needed for some + * controllers, such as the PowerA Enhanced Wired Controller + * for Series X|S (0x20d6:0x200e) to report the guide button */ + retval = usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0); + if (retval) + dev_warn(&xpad->dev->dev, + "unable to disable audio interface: %d\n", retval); + spin_lock_irqsave(&xpad->odata_lock, flags); /*
This commit explicitly disables the audio interface the same way the official driver does. This is needed for some controllers, such as the PowerA Enhanced Wired Controller for Series X|S (0x20d6:0x200e) to report the guide button. Signed-off-by: Vicki Pfau <vi@endrift.com> --- drivers/input/joystick/xpad.c | 8 ++++++++ 1 file changed, 8 insertions(+)