mbox series

[v5,0/4] Input: joystick: xpad: Add X-Box Adaptive Controller support

Message ID 20220825222420.6833-1-nate@yocom.org
Headers show
Series Input: joystick: xpad: Add X-Box Adaptive Controller support | expand

Message

Nate Yocom Aug. 25, 2022, 10:24 p.m. UTC
Adds support for the X-Box Adaptive Controller, which is protocol
compatible with the XTYPE_XBOXONE support in the driver with two deltas:

 - The X-Box button sets 0x02 as its activation ID, where others set
   0x01
 - The controller has an additional Profile button with 4 active states,
   which this change maps to an Axis control with 4 possible values

Patch series adds device to the supported table, adds support for the
Profile button, and adds support for the X-Box button as distinct
changes.

Signed-off-by: Nate Yocom <nate@yocom.org>

Nate Yocom (4):
  Input: joystick: xpad: Add X-Box Adaptive Controller support
  Input: joystick: xpad: Add ABS_PROFILE axis value to uapi
  Input: joystick: xpad: Add X-Box Adaptive Controller Profile button
  Input: joystick: xpad: Add X-Box Adaptive Controller XBox button

 v2: Fix warning Reported-by: kernel test robot <lkp@intel.com>
 v3: Break into multi-part and remove VID/PID check for XBox button
 v4: Rename Layer -> Profile as suggested by Bastien Nocera
 v5: Add new ABS_PROFILE axis to uapi and use it for the profile button
<hadess@hadess.net>

 drivers/input/joystick/xpad.c          | 15 ++++++++++++++-
 include/uapi/linux/input-event-codes.h |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)


base-commit: 15205c2829ca2cbb5ece5ceaafe1171a8470e62b

Comments

Bastien Nocera Aug. 27, 2022, 10:40 a.m. UTC | #1
On Thu, 2022-08-25 at 15:24 -0700, Nate Yocom wrote:
> Add an ABS_PROFILE axis for input devices which need it, e.g. X-Box
> Adaptive Controller and X-Box Elite 2.
> ---
>  include/uapi/linux/input-event-codes.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/uapi/linux/input-event-codes.h
> b/include/uapi/linux/input-event-codes.h
> index dff8e7f17074..7ad931a32970 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -862,6 +862,7 @@
>  #define ABS_TOOL_WIDTH         0x1c
>  
>  #define ABS_VOLUME             0x20
> +#define ABS_PROFILE            0x21
>  
>  #define ABS_MISC               0x28
>  


You probably also want to add it to the absolutes array in
drivers/hid/hid-debug.c.

Again, you might want to wait for confirmation from Dmitry that this is
the right way to do this for the profiles.
Nate Yocom Aug. 29, 2022, 7:07 p.m. UTC | #2
On Sat, Aug 27, 2022 at 12:40:46PM +0200, Bastien Nocera wrote:
> On Thu, 2022-08-25 at 15:24 -0700, Nate Yocom wrote:
> > Add an ABS_PROFILE axis for input devices which need it, e.g. X-Box
> > Adaptive Controller and X-Box Elite 2.
> > ---
> >  include/uapi/linux/input-event-codes.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/uapi/linux/input-event-codes.h
> > b/include/uapi/linux/input-event-codes.h
> > index dff8e7f17074..7ad931a32970 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -862,6 +862,7 @@
> >  #define ABS_TOOL_WIDTH         0x1c
> >  
> >  #define ABS_VOLUME             0x20
> > +#define ABS_PROFILE            0x21
> >  
> >  #define ABS_MISC               0x28
> >  
> 
> 
> You probably also want to add it to the absolutes array in
> drivers/hid/hid-debug.c.

doh, roger.

> Again, you might want to wait for confirmation from Dmitry that this is
> the right way to do this for the profiles.

Makes sense.  Dmitry?
Bastien Nocera Sept. 7, 2022, 3:41 p.m. UTC | #3
On Mon, 2022-08-29 at 12:07 -0700, Nate Yocom wrote:
> On Sat, Aug 27, 2022 at 12:40:46PM +0200, Bastien Nocera wrote:
> > On Thu, 2022-08-25 at 15:24 -0700, Nate Yocom wrote:
> > > Add an ABS_PROFILE axis for input devices which need it, e.g. X-
> > > Box
> > > Adaptive Controller and X-Box Elite 2.
> > > ---
> > >  include/uapi/linux/input-event-codes.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/include/uapi/linux/input-event-codes.h
> > > b/include/uapi/linux/input-event-codes.h
> > > index dff8e7f17074..7ad931a32970 100644
> > > --- a/include/uapi/linux/input-event-codes.h
> > > +++ b/include/uapi/linux/input-event-codes.h
> > > @@ -862,6 +862,7 @@
> > >  #define ABS_TOOL_WIDTH         0x1c
> > >  
> > >  #define ABS_VOLUME             0x20
> > > +#define ABS_PROFILE            0x21
> > >  
> > >  #define ABS_MISC               0x28
> > >  
> > 
> > 
> > You probably also want to add it to the absolutes array in
> > drivers/hid/hid-debug.c.
> 
> doh, roger.
> 
> > Again, you might want to wait for confirmation from Dmitry that
> > this is
> > the right way to do this for the profiles.
> 
> Makes sense.  Dmitry?

Dmitry, could you please confirm whether ABS_PROFILE definition is
correct?

Then Nate can update his patch so we can land the support for that
controller.

Thanks!
Benjamin Tissoires Sept. 8, 2022, 6:51 a.m. UTC | #4
On Thu, Sep 8, 2022 at 7:36 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Wed, Sep 07, 2022 at 05:41:08PM +0200, Bastien Nocera wrote:
> > On Mon, 2022-08-29 at 12:07 -0700, Nate Yocom wrote:
> > > On Sat, Aug 27, 2022 at 12:40:46PM +0200, Bastien Nocera wrote:
> > > > On Thu, 2022-08-25 at 15:24 -0700, Nate Yocom wrote:
> > > > > Add an ABS_PROFILE axis for input devices which need it, e.g. X-
> > > > > Box
> > > > > Adaptive Controller and X-Box Elite 2.
> > > > > ---
> > > > >  include/uapi/linux/input-event-codes.h | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/include/uapi/linux/input-event-codes.h
> > > > > b/include/uapi/linux/input-event-codes.h
> > > > > index dff8e7f17074..7ad931a32970 100644
> > > > > --- a/include/uapi/linux/input-event-codes.h
> > > > > +++ b/include/uapi/linux/input-event-codes.h
> > > > > @@ -862,6 +862,7 @@
> > > > >  #define ABS_TOOL_WIDTH         0x1c
> > > > >
> > > > >  #define ABS_VOLUME             0x20
> > > > > +#define ABS_PROFILE            0x21
> > > > >
> > > > >  #define ABS_MISC               0x28
> > > > >
> > > >
> > > >
> > > > You probably also want to add it to the absolutes array in
> > > > drivers/hid/hid-debug.c.
> > >
> > > doh, roger.
> > >
> > > > Again, you might want to wait for confirmation from Dmitry that
> > > > this is
> > > > the right way to do this for the profiles.
> > >
> > > Makes sense.  Dmitry?
> >
> > Dmitry, could you please confirm whether ABS_PROFILE definition is
> > correct?
>
> Yes, I think this makes sense. Do we have a buy in from userspace folks
> (Peter H, etc) for this?

AFAICT, libinput wouldn't use that new axis, simply because it doesn't
care about game controllers.
So it all comes down to game users to validate this (and probably only
android is now the main consumer of these given that SDL and Steam are
directly using hidraw or libusb to talk to controllers).

Cheers,
Benjamin

>
> I'd like some documentation added to Documentation/input/event-codes.rst
> and potentially to Documentation/input/gamepad.rst even though profile
> does not have to be gamepad specific event.
>
> Thanks.
>
> --
> Dmitry
>
Nate Yocom Sept. 8, 2022, 5:36 p.m. UTC | #5
On Wed, Sep 07, 2022 at 10:36:35PM -0700, Dmitry Torokhov wrote:
> On Wed, Sep 07, 2022 at 05:41:08PM +0200, Bastien Nocera wrote:
> > On Mon, 2022-08-29 at 12:07 -0700, Nate Yocom wrote:
> > > On Sat, Aug 27, 2022 at 12:40:46PM +0200, Bastien Nocera wrote:
> > > > On Thu, 2022-08-25 at 15:24 -0700, Nate Yocom wrote:
> > > > > Add an ABS_PROFILE axis for input devices which need it, e.g. X-
> > > > > Box
> > > > > Adaptive Controller and X-Box Elite 2.
> > > > > ---
> > > > >  include/uapi/linux/input-event-codes.h | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/include/uapi/linux/input-event-codes.h
> > > > > b/include/uapi/linux/input-event-codes.h
> > > > > index dff8e7f17074..7ad931a32970 100644
> > > > > --- a/include/uapi/linux/input-event-codes.h
> > > > > +++ b/include/uapi/linux/input-event-codes.h
> > > > > @@ -862,6 +862,7 @@
> > > > >  #define ABS_TOOL_WIDTH         0x1c
> > > > >  
> > > > >  #define ABS_VOLUME             0x20
> > > > > +#define ABS_PROFILE            0x21
> > > > >  
> > > > >  #define ABS_MISC               0x28
> > > > >  
> > > > 
> > > > 
> > > > You probably also want to add it to the absolutes array in
> > > > drivers/hid/hid-debug.c.
> > > 
> > > doh, roger.
> > > 
> > > > Again, you might want to wait for confirmation from Dmitry that
> > > > this is
> > > > the right way to do this for the profiles.
> > > 
> > > Makes sense.  Dmitry?
> > 
> > Dmitry, could you please confirm whether ABS_PROFILE definition is
> > correct?
> 
> Yes, I think this makes sense. Do we have a buy in from userspace folks
> (Peter H, etc) for this?
> 
> I'd like some documentation added to Documentation/input/event-codes.rst
> and potentially to Documentation/input/gamepad.rst even though profile
> does not have to be gamepad specific event.
> 

Will do, v6 incoming shortly.
Peter Hutterer Sept. 9, 2022, 1:25 a.m. UTC | #6
On Thu, Sep 08, 2022 at 08:51:57AM +0200, Benjamin Tissoires wrote:
> On Thu, Sep 8, 2022 at 7:36 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Wed, Sep 07, 2022 at 05:41:08PM +0200, Bastien Nocera wrote:
> > > On Mon, 2022-08-29 at 12:07 -0700, Nate Yocom wrote:
> > > > On Sat, Aug 27, 2022 at 12:40:46PM +0200, Bastien Nocera wrote:
> > > > > On Thu, 2022-08-25 at 15:24 -0700, Nate Yocom wrote:
> > > > > > Add an ABS_PROFILE axis for input devices which need it, e.g. X-
> > > > > > Box
> > > > > > Adaptive Controller and X-Box Elite 2.
> > > > > > ---
> > > > > >  include/uapi/linux/input-event-codes.h | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/include/uapi/linux/input-event-codes.h
> > > > > > b/include/uapi/linux/input-event-codes.h
> > > > > > index dff8e7f17074..7ad931a32970 100644
> > > > > > --- a/include/uapi/linux/input-event-codes.h
> > > > > > +++ b/include/uapi/linux/input-event-codes.h
> > > > > > @@ -862,6 +862,7 @@
> > > > > >  #define ABS_TOOL_WIDTH         0x1c
> > > > > >
> > > > > >  #define ABS_VOLUME             0x20
> > > > > > +#define ABS_PROFILE            0x21
> > > > > >
> > > > > >  #define ABS_MISC               0x28
> > > > > >
> > > > >
> > > > >
> > > > > You probably also want to add it to the absolutes array in
> > > > > drivers/hid/hid-debug.c.
> > > >
> > > > doh, roger.
> > > >
> > > > > Again, you might want to wait for confirmation from Dmitry that
> > > > > this is
> > > > > the right way to do this for the profiles.
> > > >
> > > > Makes sense.  Dmitry?
> > >
> > > Dmitry, could you please confirm whether ABS_PROFILE definition is
> > > correct?
> >
> > Yes, I think this makes sense. Do we have a buy in from userspace folks
> > (Peter H, etc) for this?
> 
> AFAICT, libinput wouldn't use that new axis, simply because it doesn't
> care about game controllers.

yep, that's pretty much the summary from my side so ACK from me.

Cheers,
  Peter

> So it all comes down to game users to validate this (and probably only
> android is now the main consumer of these given that SDL and Steam are
> directly using hidraw or libusb to talk to controllers).
> 
> Cheers,
> Benjamin
> 
> >
> > I'd like some documentation added to Documentation/input/event-codes.rst
> > and potentially to Documentation/input/gamepad.rst even though profile
> > does not have to be gamepad specific event.
> >
> > Thanks.
> >
> > --
> > Dmitry
> >
>