Re: [PATCH v7 09/12] Input: xbox_gip - Add support for PDP guitar controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Payload-dependent format parsing causes inconsistent button mapping and aliasing on the RiffMaster.
--

commit f157752e23d4ee774cfa5a7ee1c93ecfd2cc8d44
Author: Vicki Pfau <vi@endrift.com>

Input: xbox_gip - Add support for PDP guitar controllers

This commit adds support for the PDP guitar controllers (Fender Jaguar and
RiffMaster models) to the xbox_gip driver. It introduces custom input mapping
capabilities and specific report parsing for the two variants of the device.

> diff --git a/drivers/input/joystick/gip/gip-pdp-jaguar.c b/drivers/input/joystick/gip/gip-pdp-jaguar.c
> new file mode 100644
> index 0000000000000..ec0e9d811d125
> --- /dev/null
> +++ b/drivers/input/joystick/gip/gip-pdp-jaguar.c

[ ... ]

> +static int gip_handle_pdp_jaguar_report(struct gip_attachment *attachment,
> +	struct input_dev *input, const uint8_t *bytes, int num_bytes)
> +{

[ ... ]

> +	if (num_bytes >= 7 && (bytes[5] || bytes[6])) {

[Severity: High]
Does this payload-dependent condition break the RiffMaster layout when all
frets are released?

If a RiffMaster user releases all neck frets, bytes[5] and bytes[6] 
become 0. This causes the condition to fail, dropping the driver into the
legacy Jaguar format in the else block below.

> +		/* Newer report version on the RiffMaster */
> +		input_report_key(input, BTN_TRIGGER_HAPPY1, bytes[5] & BIT(0));
> +		input_report_key(input, BTN_TRIGGER_HAPPY2, bytes[5] & BIT(1));

[ ... ]

> +		input_report_key(input, BTN_TRIGGER_HAPPY9, bytes[6] & BIT(3));
> +		input_report_key(input, BTN_TRIGGER_HAPPY10, bytes[6] & BIT(4));
> +	} else {
> +		lower = bytes[1] & BIT(6);

[Severity: High]
When falling back to this legacy path for the RiffMaster, doesn't it cause
unintended button aliasing?

On the RiffMaster, bytes[1] & BIT(6) is mapped to BTN_THUMBR.

If the driver falls back to this path because all frets were temporarily
released, and the user happens to be holding the right stick button (or other
standard buttons located in bytes[0]), won't those inputs falsely register as
fret presses (BTN_TRIGGER_HAPPY1 through 10)?

Could this layout format decision use the static hardware quirk
(attachment->quirks & GIP_QUIRK_PDP_HAS_RIGHT_STICK) instead of relying on
transient payload data to distinguish between the controller variants?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912030426.2997003-1-vi@endrift.com?part=9