[PATCH] Fix incorrectly applied patch for MAP_PROFILE_BUTTON

[PATCH] Fix incorrectly applied patch for MAP_PROFILE_BUTTON

[Matthias Benkmann]

Original
 patch can be seen here:
 https://lore.kernel.org/all/20220908173930.28940-6-nate@yocom.org/ The hunk
 in question was supposed to go into xpad**ONE**_process_packet(), but ended
 up in xpad_process_packet(). This fix is based on visual inspection only. I
 do not have the hardware to verify that it works. I CAN confidently say,
 however, that the old code could not possibly have worked, because the
 function xpad_process_packet() is not called for the Microsoft X-Box Adaptive
 Controller since it is tagged as XTYPE_XBOXONE. So at least this fix does not
 break something that worked.

---
 drivers/input/joystick/xpad.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index f642ec8e92dd..29131f1a2f06 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -781,9 +781,6 @@ static void xpad_process_packet(struct usb_xpad
*xpad, u16 cmd, unsigned char *d
  input_report_key(dev, BTN_C, data[8]);
  input_report_key(dev, BTN_Z, data[9]);

- /* Profile button has a value of 0-3, so it is reported as an axis */
- if (xpad->mapping & MAP_PROFILE_BUTTON)
- input_report_abs(dev, ABS_PROFILE, data[34]);

  input_sync(dev);
 }
@@ -1061,6 +1058,10 @@ static void xpadone_process_packet(struct
usb_xpad *xpad, u16 cmd, unsigned char
  (__u16) le16_to_cpup((__le16 *)(data + 8)));
  }

+ /* Profile button has a value of 0-3, so it is reported as an axis */
+ if (xpad->mapping & MAP_PROFILE_BUTTON)
+ input_report_abs(dev, ABS_PROFILE, data[34]);
+
  /* paddle handling */
  /* based on SDL's SDL_hidapi_xboxone.c */
  if (xpad->mapping & MAP_PADDLES) {
-- 
2.25.1

Re: [PATCH] Fix incorrectly applied patch for MAP_PROFILE_BUTTON

[Bastien Nocera]

On Fri, 2023-03-17 at 17:00 +0100, Matthias Benkmann wrote:
> Original
>  patch can be seen here:
>  
> https://lore.kernel.org/all/20220908173930.28940-6-nate@yocom.org/ The
>  hunk
>  in question was supposed to go into xpad**ONE**_process_packet(),
> but ended
>  up in xpad_process_packet(). This fix is based on visual inspection
> only. I
>  do not have the hardware to verify that it works. I CAN confidently
> say,
>  however, that the old code could not possibly have worked, because

In the future, please don't use "old code", we don't know what "old
code" you could be referring to.

However you can remove this whole section, and either Nate or I will
test you v2.

> the
>  function xpad_process_packet() is not called for the Microsoft X-Box
> Adaptive
>  Controller since it is tagged as XTYPE_XBOXONE. So at least this fix
> does not
>  break something that worked.

You need to use a Fixes tag, as well as a Signed-off-by tag, as per:
https://docs.kernel.org/process/submitting-patches.html

Please send a v2 with those and make sure to CC: the folks mentioned in
the original patch (that is, Nate and myself), as well as the
maintainers of the tree in question.

Good catch!

Cheers

> 
> ---
>  drivers/input/joystick/xpad.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c
> b/drivers/input/joystick/xpad.c
> index f642ec8e92dd..29131f1a2f06 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -781,9 +781,6 @@ static void xpad_process_packet(struct usb_xpad
> *xpad, u16 cmd, unsigned char *d
>   input_report_key(dev, BTN_C, data[8]);
>   input_report_key(dev, BTN_Z, data[9]);
> 
> - /* Profile button has a value of 0-3, so it is reported as an axis
> */
> - if (xpad->mapping & MAP_PROFILE_BUTTON)
> - input_report_abs(dev, ABS_PROFILE, data[34]);
> 
>   input_sync(dev);
>  }
> @@ -1061,6 +1058,10 @@ static void xpadone_process_packet(struct
> usb_xpad *xpad, u16 cmd, unsigned char
>   (__u16) le16_to_cpup((__le16 *)(data + 8)));
>   }
> 
> + /* Profile button has a value of 0-3, so it is reported as an axis
> */
> + if (xpad->mapping & MAP_PROFILE_BUTTON)
> + input_report_abs(dev, ABS_PROFILE, data[34]);
> +
>   /* paddle handling */
>   /* based on SDL's SDL_hidapi_xboxone.c */
>   if (xpad->mapping & MAP_PADDLES) {