* Re: [PATCH v3 01/10] Input: xbox_gip - Add new driver for Xbox GIP
From: Vicki Pfau @ 2026-03-11 0:41 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
In-Reply-To: <20260310052017.1289494-2-vi@endrift.com>
Hello,
In my haste, it seems I forgot to run checkpatch before submitting this. There were a bunch of stylistic errors, which I have fixed locally and will have fixed in v4. That said, I would still appreciate a review on things that aren't directly found by checkpatch as I would like v4 to be the last version of the patch submitted before being merged.
On 3/9/26 10:19 PM, Vicki Pfau wrote:
> This introduces a new driver for the Xbox One/Series controller protocol,
> officially known as the Gaming Input Protocol, or GIP for short.
>
> Microsoft released documentation on (some of) GIP in late 2024, upon which
> this driver is based. Though the documentation was incomplete, it still
> provided enough information to warrant a clean start over the previous,
> incomplete implementation.
>
> This driver is already at feature parity with the GIP support in xpad,
> along with several more enhancements:
>
> - Proper support for parsing message length and fragmented messages
> - Metadata parsing, allowing for auto-detection on various parameters,
> including the presence and location in the message of the share button,
> as well as detection of specific device types
>
> The framework set out in this driver also allows future expansion for
> specialized device types and additional features more cleanly than xpad.
>
> Future plans include:
>
> - Adding support for more device types, such as arcade sticks, racing
> wheels and flight sticks.
> - Support for the security handshake, which is required for devices that
> use wireless dongles.
> - Exposing a raw character device to enable sending vendor-specific
> commands from userspace.
> - Event logging to either sysfs or dmesg.
> - Support for the headphone jack.
>
> Signed-off-by: Vicki Pfau <vi@endrift.com>
> ---
> MAINTAINERS | 6 +
> drivers/input/joystick/Kconfig | 2 +
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/gip/Kconfig | 30 +
> drivers/input/joystick/gip/Makefile | 3 +
> drivers/input/joystick/gip/gip-core.c | 2536 ++++++++++++++++++++++
> drivers/input/joystick/gip/gip-drivers.c | 210 ++
> drivers/input/joystick/gip/gip.h | 309 +++
> 8 files changed, 3097 insertions(+)
> create mode 100644 drivers/input/joystick/gip/Kconfig
> create mode 100644 drivers/input/joystick/gip/Makefile
> create mode 100644 drivers/input/joystick/gip/gip-core.c
> create mode 100644 drivers/input/joystick/gip/gip-drivers.c
> create mode 100644 drivers/input/joystick/gip/gip.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9ed6d11a77466..6c744d0af359d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27927,6 +27927,12 @@ S: Maintained
> F: drivers/media/rc/keymaps/rc-xbox-dvd.c
> F: drivers/media/rc/xbox_remote.c
>
> +XBOX GIP
> +M: Vicki Pfau <vi@endrift.com>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: drivers/input/joystick/gip/
> +
> XC2028/3028 TUNER DRIVER
> M: Mauro Carvalho Chehab <mchehab@kernel.org>
> L: linux-media@vger.kernel.org
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 7755e5b454d2c..d4665c80a3713 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -291,6 +291,8 @@ config JOYSTICK_JOYDUMP
> To compile this driver as a module, choose M here: the
> module will be called joydump.
>
> +source "drivers/input/joystick/gip/Kconfig"
> +
> config JOYSTICK_XPAD
> tristate "Xbox gamepad support"
> depends on USB_ARCH_HAS_HCD
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 9976f596a9208..323392921b7dc 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -39,5 +39,6 @@ obj-$(CONFIG_JOYSTICK_TURBOGRAFX) += turbografx.o
> obj-$(CONFIG_JOYSTICK_TWIDJOY) += twidjoy.o
> obj-$(CONFIG_JOYSTICK_WARRIOR) += warrior.o
> obj-$(CONFIG_JOYSTICK_WALKERA0701) += walkera0701.o
> +obj-$(CONFIG_JOYSTICK_XBOX_GIP) += gip/
> obj-$(CONFIG_JOYSTICK_XPAD) += xpad.o
> obj-$(CONFIG_JOYSTICK_ZHENHUA) += zhenhua.o
> diff --git a/drivers/input/joystick/gip/Kconfig b/drivers/input/joystick/gip/Kconfig
> new file mode 100644
> index 0000000000000..83293df3b0410
> --- /dev/null
> +++ b/drivers/input/joystick/gip/Kconfig
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# Gaming Input Protocol driver configuration
> +#
> +config JOYSTICK_XBOX_GIP
> + tristate "Xbox One/Series controller support"
> + depends on USB_ARCH_HAS_HCD
> + select USB
> + help
> + Say Y here if you want to use Xbox One and Series controllers with your
> + computer. Make sure to say Y to "Joystick support" (CONFIG_INPUT_JOYDEV)
> + and/or "Event interface support" (CONFIG_INPUT_EVDEV) as well.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called xbox_gip.
> +
> +config JOYSTICK_XBOX_GIP_FF
> + bool "Xbox One/Series controller rumble support"
> + depends on JOYSTICK_XBOX_GIP && INPUT
> + select INPUT_FF_MEMLESS
> + help
> + Say Y here if you want to take advantage of Xbox One/Series rumble.
> +
> +config JOYSTICK_XBOX_GIP_LEDS
> + bool "LED Support for the Xbox One/Series controller Guide button"
> + depends on JOYSTICK_XBOX_GIP && LEDS_CLASS_MULTICOLOR
> + help
> + This option enables support for the LED which surrounds the Big X on
> + Xbox One/Series controllers.
This config entry isn't used until a later patch and I have already locally moved it to that patch.
> +
> diff --git a/drivers/input/joystick/gip/Makefile b/drivers/input/joystick/gip/Makefile
> new file mode 100644
> index 0000000000000..a75e0cace0f92
> --- /dev/null
> +++ b/drivers/input/joystick/gip/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +obj-$(CONFIG_JOYSTICK_XBOX_GIP) += xbox-gip.o
> +xbox-gip-y := gip-core.o gip-drivers.o
> diff --git a/drivers/input/joystick/gip/gip-core.c b/drivers/input/joystick/gip/gip-core.c
> new file mode 100644
> index 0000000000000..0881797592fea
> --- /dev/null
> +++ b/drivers/input/joystick/gip/gip-core.c
> @@ -0,0 +1,2536 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Gaming Input Protocol driver for Xbox One/Series controllers
> + *
> + * Copyright (c) 2025 Valve Software
> + *
> + * TODO:
> + * - Audio device support
> + * - Security packet handshake
> + * - Event logging
> + * - Sending fragmented messages
> + * - Raw character device
> + * - Wheel support
> + * - Flight stick support
> + * - Arcade stick support
> + * - Split into driver-per-attachment GIP-as-a-bus approach drivers
> + *
> + * This driver is based on the Microsoft GIP spec at:
> + * https://aka.ms/gipdocs
> + * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gipusb/e7c90904-5e21-426e-b9ad-d82adeee0dbc
> + */
> +
> +#include <linux/module.h>
> +#include <linux/uuid.h>
> +#include "gip.h"
> +
> +#define GIP_WIRED_INTF_DATA 0
> +#define GIP_WIRED_INTF_AUDIO 1
> +
> +#define MAX_MESSAGE_LENGTH 0x4000
> +
> +#define MAX_AUDIO_MESSAGES 9
> +
> +#define GIP_DATA_CLASS_COMMAND (0u << 5)
> +#define GIP_DATA_CLASS_LOW_LATENCY (1u << 5)
> +#define GIP_DATA_CLASS_STANDARD_LATENCY (2u << 5)
> +#define GIP_DATA_CLASS_AUDIO (3u << 5)
> +
> +#define GIP_DATA_CLASS_SHIFT 5
> +#define GIP_DATA_CLASS_MASK (7u << 5)
> +
> +/* Undocumented Elite 2 vendor messages */
> +#define GIP_CMD_RAW_REPORT 0x0c
> +#define GIP_CMD_GUIDE_COLOR 0x0e
> +#define GIP_SL_ELITE_CONFIG 0x4d
> +
> +#define GIP_BTN_OFFSET_XBE1 28
> +#define GIP_BTN_OFFSET_XBE2 14
> +
> +#define GIP_FLAG_FRAGMENT BIT(7)
> +#define GIP_FLAG_INIT_FRAG BIT(6)
> +#define GIP_FLAG_SYSTEM BIT(5)
> +#define GIP_FLAG_ACME BIT(4)
> +#define GIP_FLAG_ATTACHMENT_MASK 0x7
> +
> +#define GIP_AUDIO_FORMAT_NULL 0
> +#define GIP_AUDIO_FORMAT_8000HZ_1CH 1
> +#define GIP_AUDIO_FORMAT_8000HZ_2CH 2
> +#define GIP_AUDIO_FORMAT_12000HZ_1CH 3
> +#define GIP_AUDIO_FORMAT_12000HZ_2CH 4
> +#define GIP_AUDIO_FORMAT_16000HZ_1CH 5
> +#define GIP_AUDIO_FORMAT_16000HZ_2CH 6
> +#define GIP_AUDIO_FORMAT_20000HZ_1CH 7
> +#define GIP_AUDIO_FORMAT_20000HZ_2CH 8
> +#define GIP_AUDIO_FORMAT_24000HZ_1CH 9
> +#define GIP_AUDIO_FORMAT_24000HZ_2CH 10
> +#define GIP_AUDIO_FORMAT_32000HZ_1CH 11
> +#define GIP_AUDIO_FORMAT_32000HZ_2CH 12
> +#define GIP_AUDIO_FORMAT_40000HZ_1CH 13
> +#define GIP_AUDIO_FORMAT_40000HZ_2CH 14
> +#define GIP_AUDIO_FORMAT_48000HZ_1CH 15
> +#define GIP_AUDIO_FORMAT_48000HZ_2CH 16
> +#define GIP_AUDIO_FORMAT_48000HZ_6CH 32
> +#define GIP_AUDIO_FORMAT_48000HZ_8CH 33
> +#define MAX_GIP_AUDIO_FORMAT GIP_AUDIO_FORMAT_48000HZ_8CH
> +
> +/* Protocol Control constants */
> +#define GIP_CONTROL_CODE_ACK 0
> +#define GIP_CONTROL_CODE_NACK 1 /* obsolete */
> +#define GIP_CONTROL_CODE_UNK 2 /* obsolete */
> +#define GIP_CONTROL_CODE_AB 3 /* obsolete */
> +#define GIP_CONTROL_CODE_MPER 4 /* obsolete */
> +#define GIP_CONTROL_CODE_STOP 5 /* obsolete */
> +#define GIP_CONTROL_CODE_START 6 /* obsolete */
> +#define GIP_CONTROL_CODE_ERR 7 /* obsolete */
> +
> +/* Status Device constants */
> +#define GIP_POWER_LEVEL_OFF 0
> +#define GIP_POWER_LEVEL_STANDBY 1 /* obsolete */
> +#define GIP_POWER_LEVEL_FULL 2
> +
> +#define GIP_NOT_CHARGING 0
> +#define GIP_CHARGING 1
> +#define GIP_CHARGE_ERROR 2
> +
> +#define GIP_BATTERY_ABSENT 0
> +#define GIP_BATTERY_STANDARD 1
> +#define GIP_BATTERY_RECHARGEABLE 2
> +
> +#define GIP_BATTERY_CRITICAL 0
> +#define GIP_BATTERY_LOW 1
> +#define GIP_BATTERY_MEDIUM 2
> +#define GIP_BATTERY_FULL 3
> +
> +#define GIP_EVENT_FAULT 0x0002
> +
> +#define GIP_FAULT_UNKNOWN 0
> +#define GIP_FAULT_HARD 1
> +#define GIP_FAULT_NMI 2
> +#define GIP_FAULT_SVC 3
> +#define GIP_FAULT_PEND_SV 4
> +#define GIP_FAULT_SMART_PTR 5
> +#define GIP_FAULT_MCU 6
> +#define GIP_FAULT_BUS 7
> +#define GIP_FAULT_USAGE 8
> +#define GIP_FAULT_RADIO_HANG 9
> +#define GIP_FAULT_WATCHDOG 10
> +#define GIP_FAULT_LINK_STALL 11
> +#define GIP_FAULT_ASSERTION 12
> +
> +/* Metadata constants */
> +#define GIP_MESSAGE_FLAG_BIG_ENDIAN BIT(0)
> +#define GIP_MESSAGE_FLAG_RELIABLE BIT(1)
> +#define GIP_MESSAGE_FLAG_SEQUENCED BIT(2)
> +#define GIP_MESSAGE_FLAG_DOWNSTREAM BIT(3)
> +#define GIP_MESSAGE_FLAG_UPSTREAM BIT(4)
> +#define GIP_MESSAGE_FLAG_DS_REQUEST_RESPONSE BIT(5)
> +
> +#define GIP_DATA_TYPE_CUSTOM 1
> +#define GIP_DATA_TYPE_AUDIO 2
> +#define GIP_DATA_TYPE_SECURITY 3
> +#define GIP_DATA_TYPE_GIP 4
> +
> +/* Set Device State constants */
> +#define GIP_STATE_START 0
> +#define GIP_STATE_STOP 1
> +#define GIP_STATE_STANDBY 2 /* obsolete */
> +#define GIP_STATE_FULL_POWER 3
> +#define GIP_STATE_OFF 4
> +#define GIP_STATE_QUIESCE 5
> +#define GIP_STATE_UNK6 6
> +#define GIP_STATE_RESET 7
> +
> +/* Guide Button Status constants */
> +#define GIP_LED_GUIDE 0
> +#define GIP_LED_IR 1 /* deprecated, for Kinect */
> +
> +#define GIP_LED_GUIDE_OFF 0
> +#define GIP_LED_GUIDE_ON 1
> +#define GIP_LED_GUIDE_FAST_BLINK 2
> +#define GIP_LED_GUIDE_SLOW_BLINK 3
> +#define GIP_LED_GUIDE_CHARGING_BLINK 4
> +#define GIP_LED_GUIDE_RAMP_TO_LEVEL 0xd
> +
> +#define GIP_LED_IR_OFF 0
> +#define GIP_LED_IR_ON_100MS 1
> +#define GIP_LED_IR_PATTERN 4
> +
> +/* Direct Motor Command constants */
> +#define GIP_MOTOR_RIGHT_VIBRATION BIT(0)
> +#define GIP_MOTOR_LEFT_VIBRATION BIT(1)
> +#define GIP_MOTOR_RIGHT_IMPULSE BIT(2)
> +#define GIP_MOTOR_LEFT_IMPULSE BIT(3)
> +#define GIP_MOTOR_ALL 0xf
> +
> +/* Extended Command constants */
> +#define GIP_EXTCMD_GET_CAPABILITIES 0x00
> +#define GIP_EXTCMD_GET_TELEMETRY_DATA 0x01
> +#define GIP_EXTCMD_GET_SERIAL_NUMBER 0x04
> +
> +#define GIP_EXTENDED_STATUS_OK 0
> +#define GIP_EXTENDED_STATUS_NOT_SUPPORTED 1
> +#define GIP_EXTENDED_STATUS_NOT_READY 2
> +#define GIP_EXTENDED_STATUS_ACCESS_DENIED 3
> +#define GIP_EXTENDED_STATUS_FAILED 4
> +
> +/* Internal constants, not part of protocol */
> +#define GIP_DEFAULT_IN_SYSTEM_MESSAGES 0x5e
> +#define GIP_DEFAULT_OUT_SYSTEM_MESSAGES 0x472
> +
> +#define GIP_FEATURE_CONTROLLER BIT(0)
> +#define GIP_FEATURE_CONSOLE_FUNCTION_MAP BIT(1)
> +#define GIP_FEATURE_CONSOLE_FUNCTION_MAP_OVERFLOW BIT(2)
> +#define GIP_FEATURE_ELITE_BUTTONS BIT(3)
> +#define GIP_FEATURE_DYNAMIC_LATENCY_INPUT BIT(4)
> +#define GIP_FEATURE_SECURITY_OPT_OUT BIT(5)
> +#define GIP_FEATURE_MOTOR_CONTROL BIT(6)
> +#define GIP_FEATURE_GUIDE_COLOR BIT(7)
> +#define GIP_FEATURE_EXTENDED_SET_DEVICE_STATE BIT(8)
> +
> +#define GIP_LED_GUIDE_MAX_BRIGHTNESS 100 /* Spec says 47, but larger values work */
> +#define GIP_LED_GUIDE_INIT_BRIGHTNESS 20
> +
> +#ifndef VK_LWIN
> +#define VK_LWIN 0x5b
> +#endif
> +
> +static const guid_t guid_console_function_map =
> + GUID_INIT(0xecddd2fe, 0xd387, 0x4294, 0xbd, 0x96, 0x1a, 0x71, 0x2e, 0x3d, 0xc7, 0x7d);
> +static const guid_t guid_console_function_map_overflow =
> + GUID_INIT(0x137d4bd0, 0x9347, 0x4472, 0xaa, 0x26, 0x8c, 0x34, 0xa0, 0x8f, 0xf9, 0xbd);
> +static const guid_t guid_controller =
> + GUID_INIT(0x9776ff56, 0x9bfd, 0x4581, 0xad, 0x45, 0xb6, 0x45, 0xbb, 0xa5, 0x26, 0xd6);
> +static const guid_t guid_dev_auth_pc_opt_out =
> + GUID_INIT(0x7a34ce77, 0x7de2, 0x45c6, 0x8c, 0xa4, 0x00, 0x42, 0xc0, 0x8b, 0xd9, 0x4a);
> +static const guid_t guid_dynamic_latency_input =
> + GUID_INIT(0x87f2e56b, 0xc3bb, 0x49b1, 0x82, 0x65, 0xff, 0xff, 0xf3, 0x77, 0x99, 0xee);
> +static const guid_t guid_elite_buttons =
> + GUID_INIT(0x37d19ff7, 0xb5c6, 0x49d1, 0xa7, 0x5e, 0x03, 0xb2, 0x4b, 0xef, 0x8c, 0x89);
> +static const guid_t guid_headset =
> + GUID_INIT(0xbc25d1a3, 0xc24e, 0x4992, 0x9d, 0xda, 0xef, 0x4f, 0x12, 0x3e, 0xf5, 0xdc);
> +
> +/*
> + * The following GUIDs are observed, but the exact meanings aren't known, so
> + * for now we document them but don't use them anywhere.
> + *
> + * GamepadEmu: GUID_INIT(0xe2e5f1bc, 0xa6e6, 0x41a2, 0x8f, 0x43, 0x33, 0xcf, 0xa2, 0x51, 0x09, 0x81)
> + * IAudioOnly: GUID_INIT(0x92844cd1, 0xf7c8, 0x49ef, 0x97, 0x77, 0x46, 0x7d, 0xa7, 0x08, 0xad, 0x10)
> + * IControllerProfileModeState: GUID_INIT(0xf758dc66, 0x022c, 0x48b8, 0xa4, 0xf6, 0x45, 0x7b, 0xa8, 0x0e, 0x2a, 0x5b)
> + * ICustomAudio: GUID_INIT(0x63fd9cc9, 0x94ee, 0x4b5d, 0x9c, 0x4d, 0x8b, 0x86, 0x4c, 0x14, 0x9c, 0xac)
> + * IExtendedDeviceFlags: GUID_INIT(0x34ad9b1e, 0x36ad, 0x4fb5, 0x8a, 0xc7, 0x17, 0x23, 0x4c, 0x9f, 0x54, 0x6f)
> + * IProgrammableGamepad: GUID_INIT(0x31c1034d, 0xb5b7, 0x4551, 0x98, 0x13, 0x87, 0x69, 0xd4, 0xa0, 0xe4, 0xf9)
> + * IVirtualDevice: GUID_INIT(0xdfd26825, 0x110a, 0x4e94, 0xb9, 0x37, 0xb2, 0x7c, 0xe4, 0x7b, 0x25, 0x40)
> + * OnlineDevAuth: GUID_INIT(0x632b1fd1, 0xa3e9, 0x44f9, 0x84, 0x20, 0x5c, 0xe3, 0x44, 0xa0, 0x64, 0x04)
> + *
> + * Seen on Elite Controller, Adaptive Controller: 9ebd00a3-b5e6-4c08-a33b-673126459ec4
> + * Seen on Adaptive Controller: ce1e58c5-221c-4bdb-9c24-bf3941601320
> + * Seen on Adaptive Joystick: db02f681-5038-4219-8668-c3459c5c3293
> + * Seen on Elite 2 Controller: f758dc66-022c-48b8-a4f6-457ba80e2a5b (IControllerProfileModeState)
> + * Seen on Elite 2 Controller: 31c1034d-b5b7-4551-9813-8769d4a0e4f9 (IProgrammableGamepad)
> + * Seen on Elite 2 Controller: 34ad9b1e-36ad-4fb5-8ac7-17234c9f546f (IExtendedDeviceFlags)
> + * Seen on Elite 2 Controller: 88e0b694-6bd9-4416-a560-e7fafdfa528f
> + * Seen on Elite 2 Controller: ea96c8c0-b216-448b-be80-7e5deb0698e2
> + */
> +
> +static const int gip_data_class_mtu[8] = { 64, 64, 64, 2048, 0, 0, 0, 0 };
> +
> +struct gip_audio_format {
> + uint16_t rate;
> + uint8_t channels;
> +};
> +
> +static const struct gip_audio_format gip_audio_format_table[MAX_GIP_AUDIO_FORMAT + 1] = {
> + [GIP_AUDIO_FORMAT_8000HZ_1CH] = { .rate = 8000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_8000HZ_2CH] = { .rate = 8000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_12000HZ_1CH] = { .rate = 12000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_12000HZ_2CH] = { .rate = 12000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_16000HZ_1CH] = { .rate = 16000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_16000HZ_2CH] = { .rate = 16000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_20000HZ_1CH] = { .rate = 20000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_20000HZ_2CH] = { .rate = 20000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_24000HZ_1CH] = { .rate = 24000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_24000HZ_2CH] = { .rate = 24000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_32000HZ_1CH] = { .rate = 32000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_32000HZ_2CH] = { .rate = 32000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_40000HZ_1CH] = { .rate = 40000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_40000HZ_2CH] = { .rate = 40000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_48000HZ_1CH] = { .rate = 48000, .channels = 1 },
> + [GIP_AUDIO_FORMAT_48000HZ_2CH] = { .rate = 48000, .channels = 2 },
> + [GIP_AUDIO_FORMAT_48000HZ_6CH] = { .rate = 48000, .channels = 6 },
> + [GIP_AUDIO_FORMAT_48000HZ_8CH] = { .rate = 48000, .channels = 8 },
> +};
> +
> +
> +static const struct gip_quirks base_quirks[] = {
> + /* PDP Rock Candy */
> + { 0x0e6f, 0x0246, 0, .quirks = GIP_QUIRK_NO_HELLO },
> +
> + {0},
> +};
> +
> +struct gip_audio_format_pair {
> + uint8_t inbound;
> + uint8_t outbound;
> +};
> +static_assert(sizeof(struct gip_audio_format_pair) == 2);
> +
> +struct gip_hello_device {
> + uint64_t device_id;
> + uint16_t vendor_id;
> + uint16_t product_id;
> + uint16_t firmware_major_version;
> + uint16_t firmware_minor_version;
> + uint16_t firmware_build_version;
> + uint16_t firmware_revision;
> + uint8_t hardware_major_version;
> + uint8_t hardware_minor_version;
> + uint8_t rf_proto_major_version;
> + uint8_t rf_proto_minor_version;
> + uint8_t security_major_version;
> + uint8_t security_minor_version;
> + uint8_t gip_major_version;
> + uint8_t gip_minor_version;
> +};
> +
> +struct gip_direct_motor {
> + uint8_t command;
> + uint8_t motor_bitmap;
> + uint8_t left_impulse_level;
> + uint8_t right_impulse_level;
> + uint8_t left_vibration_level;
> + uint8_t right_vibration_level;
> + uint8_t duration;
> + uint8_t delay;
> + uint8_t repeat;
> +};
> +
> +static const struct gip_driver* base_drivers[] = {
> + &gip_driver_navigation,
> + &gip_driver_gamepad,
> + NULL /* Sentinel */
> +};
> +
> +static int gip_decode_length(uint64_t *length, const uint8_t *bytes, int num_bytes)
> +{
> + *length = 0;
> + int offset;
> +
> + for (offset = 0; offset < num_bytes; offset++) {
> + uint8_t byte = bytes[offset];
> +
> + *length |= (byte & 0x7full) << (offset * 7);
> + if (!(byte & 0x80)) {
> + offset++;
> + break;
> + }
> + }
> + return offset;
> +}
> +
> +static int gip_encode_length(uint64_t length, uint8_t *bytes, int num_bytes)
> +{
> + int offset;
> +
> + for (offset = 0; offset < num_bytes; offset++) {
> + uint8_t byte = length & 0x7f;
> +
> + length >>= 7;
> + if (length)
> + byte |= 0x80;
> + bytes[offset] = byte;
> + if (!length) {
> + offset++;
> + break;
> + }
> + }
> + return offset;
> +}
> +
> +static bool gip_supports_system_message(struct gip_attachment *attachment,
> + uint8_t command, bool upstream)
> +{
> + if (upstream)
> + return attachment->metadata.device
> + .in_system_messages[command >> 5] & (1u << command);
> + else
> + return attachment->metadata.device
> + .out_system_messages[command >> 5] & (1u << command);
> +}
> +
> +bool gip_supports_vendor_message(struct gip_attachment *attachment,
> + uint8_t command, bool upstream)
> +{
> + size_t i;
> +
> + for (i = 0; i < attachment->metadata.num_messages; i++) {
> + struct gip_message_metadata *metadata =
> + &attachment->metadata.message_metadata[i];
> +
> + if (metadata->type != command)
> + continue;
> + if (metadata->flags & GIP_MESSAGE_FLAG_DS_REQUEST_RESPONSE)
> + return true;
> +
> + if (upstream)
> + return metadata->flags & GIP_MESSAGE_FLAG_UPSTREAM;
> + else
> + return metadata->flags & GIP_MESSAGE_FLAG_DOWNSTREAM;
> + }
> + return false;
> +}
> +
> +static uint8_t gip_sequence_next(struct gip_attachment *attachment,
> + uint8_t command, bool system)
> +{
> + uint8_t seq;
> +
> + if (system) {
> + switch (command) {
> + case GIP_CMD_SECURITY:
> + seq = attachment->seq_security++;
> + if (!seq)
> + seq = attachment->seq_security++;
> + break;
> + case GIP_CMD_EXTENDED:
> + seq = attachment->seq_extended++;
> + if (!seq)
> + seq = attachment->seq_extended++;
> + break;
> + case GIP_AUDIO_DATA:
> + seq = attachment->seq_audio++;
> + if (!seq)
> + seq = attachment->seq_audio++;
> + break;
> + default:
> + seq = attachment->seq_system++;
> + if (!seq)
> + seq = attachment->seq_system++;
> + break;
> + }
> + } else {
> + seq = attachment->seq_vendor++;
> + if (!seq)
> + seq = attachment->seq_vendor++;
> + }
> + return seq;
> +}
> +
> +static void gip_handle_quirks_array(struct gip_attachment *attachment,
> + const struct gip_quirks *quirks)
> +{
> + size_t i, j;
> +
> + for (i = 0; quirks[i].vendor_id; i++) {
> + if (quirks[i].vendor_id != attachment->vendor_id)
> + continue;
> + if (quirks[i].product_id != attachment->product_id)
> + continue;
> + if (quirks[i].attachment_index != attachment->attachment_index)
> + continue;
> +
> + attachment->features |= quirks[i].added_features;
> + attachment->features &= ~quirks[i].filtered_features;
> + attachment->quirks |= quirks[i].quirks;
> +
> + if (quirks[i].override_name)
> + attachment->name = quirks[i].override_name;
> +
> + for (j = 0; j < 8; ++j) {
> + struct gip_device_metadata *metadata = &attachment->metadata.device;
> +
> + metadata->in_system_messages[j] |= quirks[i].extra_in_system[j];
> + metadata->out_system_messages[j] |= quirks[i].extra_out_system[j];
> + }
> +
> + attachment->extra_buttons = quirks[i].extra_buttons;
> + attachment->extra_axes = quirks[i].extra_axes;
> + break;
> + }
> +
> +}
> +
> +static void gip_handle_quirks(struct gip_attachment *attachment)
> +{
> + gip_handle_quirks_array(attachment, base_quirks);
> +
> + if (attachment->driver && attachment->driver->quirks)
> + gip_handle_quirks_array(attachment, attachment->driver->quirks);
> +}
> +
> +static int gip_send_raw_message(struct gip_device *device,
> + uint8_t message_type, uint8_t flags, uint8_t seq, const uint8_t *bytes,
> + int num_bytes)
> +{
> + struct gip_interface *intf;
> + int offset = 3;
> + struct gip_urb *urb = NULL;
> + int i;
> + int rc = 0;
> +
> + if (num_bytes < 0) {
> + dev_warn(GIP_DEV(device), "Invalid message length %d\n", num_bytes);
> + return -EINVAL;
> + }
> +
> + if (num_bytes > gip_data_class_mtu[message_type >> GIP_DATA_CLASS_SHIFT]) {
> + dev_err(GIP_DEV(device),
> + "Attempted to send a message that requires fragmenting, which is not yet supported.\n");
> + return -ENOTSUPP;
> + }
> +
> + if ((message_type & GIP_DATA_CLASS_MASK) == GIP_DATA_CLASS_AUDIO)
> + intf = &device->audio;
> + else
> + intf = &device->data;
> +
> + if (intf->isoc_messages) {
> + /* TODO: Needed for audio support */
> + dev_warn(GIP_DEV(intf), "Unimplemented isochronous message output\n");
> + return -ENOTSUPP;
> + }
> +
> + guard(spinlock_irqsave)(&device->message_lock);
> + for (i = 0; i < MAX_OUT_MESSAGES && !urb; i++) {
> + if (!intf->out_queue[i].urb)
> + continue;
> + if (!intf->out_queue[i].urb->anchor)
> + urb = &intf->out_queue[i];
> + }
> + if (!urb) {
> + dev_err(GIP_DEV(device), "Output queue is full; dropping message\n");
> + return -ENOSPC;
> + }
> + urb->data[0] = message_type;
> + urb->data[1] = flags;
> + urb->data[2] = seq;
> + offset += gip_encode_length(num_bytes, &urb->data[offset],
> + sizeof(urb->data) - offset);
> +
> + if (num_bytes > 0)
> + memcpy(&urb->data[offset], bytes, num_bytes);
> +
> + num_bytes += offset;
> + urb->urb->transfer_buffer_length = num_bytes;
> +
> + print_hex_dump_debug(KBUILD_MODNAME ": Sending message: ",
> + DUMP_PREFIX_OFFSET, 16, 1, urb->data, num_bytes,
> + false);
> +
> + usb_anchor_urb(urb->urb, &intf->out_anchor);
> + rc = usb_submit_urb(urb->urb, GFP_ATOMIC);
> + if (rc) {
> + dev_err(&intf->intf->dev,
> + "%s - usb_submit_urb failed with result %d\n",
> + __func__, rc);
> + usb_unanchor_urb(urb->urb);
> + rc = -EIO;
> + }
> +
> + return rc;
> +}
> +
> +int gip_send_system_message(struct gip_attachment *attachment,
> + uint8_t message_type, uint8_t flags, const void *bytes, int num_bytes)
> +{
> + return gip_send_raw_message(attachment->device, message_type,
> + GIP_FLAG_SYSTEM | attachment->attachment_index | flags,
> + gip_sequence_next(attachment, message_type, true),
> + bytes, num_bytes);
> +}
> +
> +int gip_send_vendor_message(struct gip_attachment *attachment,
> + uint8_t message_type, uint8_t flags, const void *bytes, int num_bytes)
> +{
> + return gip_send_raw_message(attachment->device, message_type, flags,
> + gip_sequence_next(attachment, message_type, false),
> + bytes, num_bytes);
> +}
> +
> +static void gip_metadata_free(struct device *dev, struct gip_metadata *metadata)
> +{
> + devm_kfree(dev, metadata->device.audio_formats);
> +
> + if (metadata->device.preferred_types) {
> + int i;
> +
> + for (i = 0; i < metadata->device.num_preferred_types; i++)
> + devm_kfree(dev, metadata->device.preferred_types[i]);
> + devm_kfree(dev, metadata->device.preferred_types);
> + }
> + devm_kfree(dev, metadata->device.supported_interfaces);
> + devm_kfree(dev, metadata->device.hid_descriptor);
> + devm_kfree(dev, metadata->message_metadata);
> +
> + memset(metadata, 0, sizeof(*metadata));
> +}
> +
> +static int gip_parse_audio_format_metadata(struct device *dev,
> + struct gip_device_metadata *dev_metadata, const uint8_t *bytes,
> + int length, int buffer_offset)
> +{
> + unsigned int i;
> +
> + dev_metadata->num_audio_formats = bytes[buffer_offset];
> + if (buffer_offset + dev_metadata->num_audio_formats * 2 + 1 > length)
> + return -EINVAL;
> + dev_metadata->audio_formats = devm_kmalloc_array(dev,
> + dev_metadata->num_audio_formats, 2, GFP_KERNEL);
> + if (!dev_metadata->audio_formats)
> + return -ENOMEM;
> + memcpy(dev_metadata->audio_formats, &bytes[buffer_offset + 1],
> + dev_metadata->num_audio_formats * 2);
> +
> + for (i = 0; i < dev_metadata->num_audio_formats; i++) {
> + const struct gip_audio_format_pair *pair = &dev_metadata->audio_formats[i];
> + const struct gip_audio_format *inbound = NULL;
> + const struct gip_audio_format *outbound = NULL;
> +
> + if (pair->inbound <= MAX_GIP_AUDIO_FORMAT) {
> + inbound = &gip_audio_format_table[pair->inbound];
> + if (pair->inbound != GIP_AUDIO_FORMAT_NULL && inbound->rate == 0)
> + inbound = NULL;
> + }
> + if (!inbound)
> + dev_warn(dev, "Unknown audio format %u\n", pair->inbound);
> +
> + if (pair->outbound <= MAX_GIP_AUDIO_FORMAT) {
> + outbound = &gip_audio_format_table[pair->outbound];
> + if (pair->outbound != GIP_AUDIO_FORMAT_NULL && outbound->rate == 0)
> + outbound = NULL;
> + }
> + if (!outbound)
> + dev_warn(dev, "Unknown audio format %u\n", pair->outbound);
> +
> + if (inbound && outbound)
> + dev_dbg(dev,
> + "Supported audio format: %uHz %uch inbound, %uHz %uch outbound\n",
> + inbound->rate,
> + inbound->channels,
> + outbound->rate,
> + outbound->channels);
> + }
> + return 0;
> +}
> +
> +static int gip_parse_preferred_types_metadata(struct device *dev,
> + struct gip_device_metadata *dev_metadata, const uint8_t *bytes,
> + int length, int buffer_offset)
> +{
> + int i;
> + int count;
> +
> + dev_metadata->num_preferred_types = bytes[buffer_offset];
> + dev_metadata->preferred_types = devm_kcalloc(dev,
> + dev_metadata->num_preferred_types, sizeof(char *), GFP_KERNEL);
> + if (!dev_metadata->preferred_types)
> + return -ENOMEM;
> +
> + buffer_offset++;
> + for (i = 0; i < dev_metadata->num_preferred_types; i++) {
> + if (buffer_offset + 2 >= length)
> + return -EINVAL;
> +
> + count = bytes[buffer_offset];
> + count |= bytes[buffer_offset];
> + buffer_offset += 2;
> + if (buffer_offset + count > length)
> + return -EINVAL;
> +
> + dev_metadata->preferred_types[i] = devm_kcalloc(dev, count + 1,
> + sizeof(char), GFP_KERNEL);
> + if (!dev_metadata->preferred_types[i])
> + return -ENOMEM;
> + memcpy(dev_metadata->preferred_types[i], &bytes[buffer_offset], count);
> + buffer_offset += count;
> + }
> +
> + return 0;
> +}
> +
> +static int gip_parse_supported_interfaces_metadata(struct device *dev,
> + struct gip_device_metadata *dev_metadata, const uint8_t *bytes,
> + int length, int buffer_offset)
> +{
> + dev_metadata->num_supported_interfaces = bytes[buffer_offset];
> + if (buffer_offset + 1 +
> + (int32_t) (dev_metadata->num_supported_interfaces * sizeof(guid_t)) > length)
> + return -EINVAL;
> +
> + dev_metadata->supported_interfaces = devm_kmalloc_array(dev,
> + dev_metadata->num_supported_interfaces, sizeof(guid_t), GFP_KERNEL);
> + if (!dev_metadata->supported_interfaces)
> + return -ENOMEM;
> +
> + memcpy(dev_metadata->supported_interfaces, &bytes[buffer_offset + 1],
> + sizeof(guid_t) * dev_metadata->num_supported_interfaces);
> +
> + return 0;
> +}
> +
> +static int gip_parse_hid_descriptor_metadata(struct device *dev,
> + struct gip_device_metadata *dev_metadata, const uint8_t *bytes,
> + int length, int buffer_offset)
> +{
> + dev_metadata->hid_descriptor_size = bytes[buffer_offset];
> + if (buffer_offset + 1 + dev_metadata->hid_descriptor_size > length)
> + return -EINVAL;
> +
> + dev_metadata->hid_descriptor = devm_kmalloc(dev,
> + dev_metadata->hid_descriptor_size, GFP_KERNEL);
> + if (!dev_metadata->hid_descriptor)
> + return -ENOMEM;
> +
> + memcpy(dev_metadata->hid_descriptor, &bytes[buffer_offset + 1],
> + dev_metadata->hid_descriptor_size);
> + print_hex_dump_debug(KBUILD_MODNAME ": Received HID descriptor: ",
> + DUMP_PREFIX_OFFSET, 16, 1, dev_metadata->hid_descriptor,
> + dev_metadata->hid_descriptor_size, false);
> +
> + return 0;
> +}
> +
> +static int gip_parse_device_metadata(struct device *dev,
> + struct gip_metadata *metadata, const uint8_t *bytes, int num_bytes,
> + int *offset)
> +{
> + struct gip_device_metadata *dev_metadata = &metadata->device;
> + int buffer_offset;
> + int count;
> + int length;
> + int i;
> + int rc;
> +
> + bytes = &bytes[*offset];
> + num_bytes -= *offset;
> + if (num_bytes < 16)
> + return -EINVAL;
> +
> + length = bytes[0];
> + length |= bytes[1] << 8;
> + if (num_bytes < length)
> + return -EINVAL;
> +
> + /* Skip supported firmware versions for now */
> +
> + buffer_offset = bytes[4];
> + buffer_offset |= bytes[5] << 8;
> + if (buffer_offset >= length)
> + return -EINVAL;
> +
> + if (buffer_offset > 0) {
> + rc = gip_parse_audio_format_metadata(dev, dev_metadata,
> + bytes, length, buffer_offset);
> + if (rc)
> + return rc;
> + }
> +
> + buffer_offset = bytes[6];
> + buffer_offset |= bytes[7] << 8;
> + if (buffer_offset >= length)
> + return -EINVAL;
> +
> + if (buffer_offset > 0) {
> + count = bytes[buffer_offset];
> + if (buffer_offset + count + 1 > length)
> + return -EINVAL;
> +
> + for (i = 0; i < count; i++) {
> + uint8_t message = bytes[buffer_offset + 1 + i];
> +
> + dev_dbg(dev,
> + "Supported upstream system message %02x\n",
> + message);
> + dev_metadata->in_system_messages[message >> 5] |=
> + BIT(message & 0x1F);
> + }
> + }
> +
> + buffer_offset = bytes[8];
> + buffer_offset |= bytes[9] << 8;
> + if (buffer_offset >= length)
> + return -EINVAL;
> +
> + if (buffer_offset > 0) {
> + count = bytes[buffer_offset];
> + if (buffer_offset + count + 1 > length)
> + return -EINVAL;
> +
> + for (i = 0; i < count; i++) {
> + uint8_t message = bytes[buffer_offset + 1 + i];
> +
> + dev_dbg(dev,
> + "Supported downstream system message %02x\n",
> + message);
> + dev_metadata->out_system_messages[message >> 5] |=
> + BIT(message & 0x1F);
> + }
> + }
> +
> + buffer_offset = bytes[10];
> + buffer_offset |= bytes[11] << 8;
> + if (buffer_offset >= length)
> + return -EINVAL;
> +
> + if (buffer_offset > 0) {
> + rc = gip_parse_preferred_types_metadata(dev, dev_metadata,
> + bytes, length, buffer_offset);
> + if (rc)
> + return rc;
> + }
> +
> + buffer_offset = bytes[12];
> + buffer_offset |= bytes[13] << 8;
> + if (buffer_offset >= length)
> + return -EINVAL;
> +
> + if (buffer_offset > 0) {
> + rc = gip_parse_supported_interfaces_metadata(dev,
> + dev_metadata, bytes, length, buffer_offset);
> + if (rc)
> + return rc;
> + }
> +
> + if (metadata->version_major > 1 || metadata->version_minor >= 1) {
> + /* HID descriptor support added in metadata version 1.1 */
> + buffer_offset = bytes[14];
> + buffer_offset |= bytes[15] << 8;
> + if (buffer_offset >= length)
> + return -EINVAL;
> +
> + if (buffer_offset > 0) {
> + rc = gip_parse_hid_descriptor_metadata(dev,
> + dev_metadata, bytes, length, buffer_offset);
> + if (rc)
> + return rc;
> + }
> + }
> +
> + *offset += length;
> + return 0;
> +}
> +
> +static int gip_parse_message_metadata(struct device *dev,
> + struct gip_message_metadata *metadata, const uint8_t *bytes,
> + int num_bytes, int *offset)
> +{
> + uint16_t length;
> +
> + bytes = &bytes[*offset];
> + num_bytes -= *offset;
> +
> + if (num_bytes < 2)
> + return -EINVAL;
> +
> + length = bytes[0];
> + length |= bytes[1] << 8;
> + if (num_bytes < length)
> + return -EINVAL;
> +
> + if (length < 15)
> + return -EINVAL;
> +
> + metadata->type = bytes[2];
> + metadata->length = bytes[3];
> + metadata->length |= bytes[4] << 8;
> + metadata->data_type = bytes[5];
> + metadata->data_type |= bytes[6] << 8;
> + metadata->flags = bytes[7];
> + metadata->flags |= bytes[8] << 8;
> + metadata->flags |= bytes[9] << 16;
> + metadata->flags |= bytes[10] << 24;
> + metadata->period = bytes[11];
> + metadata->period |= bytes[12] << 8;
> + metadata->persistence_timeout = bytes[13];
> + metadata->persistence_timeout |= bytes[14] << 8;
> +
> + dev_dbg(dev,
> + "Supported vendor message type %02x of length %d, %s, %s, %s\n",
> + metadata->type, metadata->length,
> + metadata->flags & GIP_MESSAGE_FLAG_UPSTREAM ?
> + (metadata->flags & GIP_MESSAGE_FLAG_DOWNSTREAM ? "bidirectional" : "upstream") :
> + metadata->flags & GIP_MESSAGE_FLAG_DOWNSTREAM ? "downstream" :
> + metadata->flags & GIP_MESSAGE_FLAG_DS_REQUEST_RESPONSE ? "downstream request response" :
> + "unknown direction",
> + metadata->flags & GIP_MESSAGE_FLAG_SEQUENCED ? "sequenced" : "not sequenced",
> + metadata->flags & GIP_MESSAGE_FLAG_RELIABLE ? "reliable" : "unreliable");
> +
> + *offset += length;
> + return 0;
> +}
> +
> +static bool gip_parse_metadata(struct device *dev,
> + struct gip_metadata *metadata, const uint8_t *bytes, int num_bytes)
> +{
> + int header_size;
> + int metadata_size;
> + int offset = 0;
> + int i;
> + int rc;
> +
> + if (num_bytes < 16)
> + return -EINVAL;
> +
> + print_hex_dump_debug(KBUILD_MODNAME ": Received metadata: ", DUMP_PREFIX_OFFSET,
> + 16, 1, bytes, num_bytes, false);
> +
> + header_size = bytes[0];
> + header_size |= bytes[1] << 8;
> + if (num_bytes < header_size || header_size < 16)
> + return -EINVAL;
> +
> + metadata->version_major = bytes[2];
> + metadata->version_major |= bytes[3] << 8;
> + metadata->version_minor = bytes[4];
> + metadata->version_minor |= bytes[5] << 8;
> + /* Middle bytes are reserved */
> + metadata_size = bytes[14];
> + metadata_size |= bytes[15] << 8;
> +
> + if (num_bytes < metadata_size || metadata_size < header_size)
> + return -EINVAL;
> +
> + offset = header_size;
> +
> + rc = gip_parse_device_metadata(dev, metadata, bytes, num_bytes, &offset);
> + if (rc)
> + goto parse_err;
> +
> + if (offset >= num_bytes)
> + goto parse_err;
> +
> + metadata->num_messages = bytes[offset];
> + offset++;
> + if (metadata->num_messages > 0) {
> + metadata->message_metadata = devm_kcalloc(dev,
> + metadata->num_messages,
> + sizeof(*metadata->message_metadata), GFP_KERNEL);
> + if (!metadata->message_metadata)
> + return -ENOMEM;
> +
> + for (i = 0; i < metadata->num_messages; i++) {
> + rc = gip_parse_message_metadata(dev,
> + &metadata->message_metadata[i], bytes,
> + num_bytes, &offset);
> + if (rc)
> + goto parse_err;
> + }
> + }
> +
> + return 0;
> +
> +parse_err:
> + gip_metadata_free(dev, metadata);
> + return rc;
> +}
> +
> +static int gip_acknowledge(struct gip_device *device,
> + const struct gip_header *header, uint32_t fragment_offset,
> + uint16_t bytes_remaining)
> +{
> + uint8_t buffer[] = {
> + GIP_CONTROL_CODE_ACK,
> + header->message_type,
> + header->flags & GIP_FLAG_SYSTEM,
> + fragment_offset,
> + fragment_offset >> 8,
> + fragment_offset >> 16,
> + fragment_offset >> 24,
> + bytes_remaining,
> + bytes_remaining >> 8,
> + };
> +
> + return gip_send_raw_message(device, GIP_CMD_PROTO_CONTROL,
> + GIP_FLAG_SYSTEM | (header->flags & GIP_FLAG_ATTACHMENT_MASK),
> + header->sequence_id, buffer, sizeof(buffer));
> +}
> +
> +static int gip_fragment_failed(struct gip_attachment *attachment,
> + const struct gip_header *header)
> +{
> + attachment->fragment_retries++;
> + if (attachment->fragment_retries > 8) {
> + devm_kfree(GIP_DEV(attachment), attachment->fragment_data);
> + attachment->fragment_data = NULL;
> + attachment->fragment_message = 0;
> + }
> + return gip_acknowledge(attachment->device, header,
> + attachment->fragment_offset,
> + attachment->total_length - attachment->fragment_offset);
> +}
> +
> +static int gip_bind_driver(struct gip_attachment *attachment, const struct gip_driver *driver)
> +{
> + if (driver->probe) {
> + int rc = driver->probe(attachment);
> +
> + if (rc)
> + return rc;
> + }
> +
> + attachment->driver = driver;
> + memcpy(attachment->vendor_handlers, driver->vendor_handlers,
> + sizeof(attachment->vendor_handlers));
> + return 0;
> +}
> +
> +static int gip_enable_elite_buttons(struct gip_attachment *attachment)
> +{
> + if (attachment->vendor_id == 0x045e) {
> + if (attachment->product_id == 0x02e3) {
> + attachment->xbe_format = GIP_BTN_FMT_XBE1;
> + } else if (attachment->product_id == 0x0b00) {
> + if (attachment->firmware_major_version == 4) {
> + attachment->xbe_format = GIP_BTN_FMT_XBE2_4;
> + } else if (attachment->firmware_major_version == 5) {
> + /*
> + * The exact range for this being necessary is
> + * unknown, but it starts at 5.11 and at either
> + * 5.16 or 5.17. This approach still works on
> + * 5.21, even if it's not necessary, so having
> + * a loose upper limit is fine.
> + */
> + if (attachment->firmware_minor_version >= 11 &&
> + attachment->firmware_minor_version < 17)
> + attachment->xbe_format = GIP_BTN_FMT_XBE2_RAW;
> + else
> + attachment->xbe_format = GIP_BTN_FMT_XBE2_5;
> + }
> + }
> + }
> +
> + if (attachment->xbe_format == GIP_BTN_FMT_XBE2_RAW) {
> + /*
> + * The meaning of this packet is unknown and not documented, but
> + * it's needed for the Elite 2 controller to send raw reports
> + */
> + static const uint8_t enable_raw_report[] = { 7, 0 };
> +
> + return gip_send_vendor_message(attachment, GIP_SL_ELITE_CONFIG,
> + 0, enable_raw_report, sizeof(enable_raw_report));
> + }
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_JOYSTICK_XBOX_GIP_FF
> +static int gip_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
> +{
> + struct gip_attachment *attachment = input_get_drvdata(dev);
> + struct gip_direct_motor control = {
> + .motor_bitmap = GIP_MOTOR_LEFT_VIBRATION | GIP_MOTOR_RIGHT_VIBRATION
> + };
> +
> + if (effect->type != FF_RUMBLE)
> + return 0;
> +
> + control.left_vibration_level = effect->u.rumble.strong_magnitude * 100 / 0xFFFF;
> + control.right_vibration_level = effect->u.rumble.weak_magnitude * 100 / 0xFFFF;
> + control.duration = 255;
> +
> + return gip_send_vendor_message(attachment, GIP_CMD_DIRECT_MOTOR,
> + 0, &control, sizeof(control));
> +}
> +#endif
> +
> +static int gip_send_guide_button_led(struct gip_attachment *attachment,
> + uint8_t pattern, uint8_t intensity)
> +{
> + uint8_t buffer[] = {
> + GIP_LED_GUIDE,
> + pattern,
> + intensity,
> + };
> +
> + if (!gip_supports_system_message(attachment, GIP_CMD_LED, false))
> + return 0;
> +
> + return gip_send_system_message(attachment, GIP_CMD_LED, 0, buffer, sizeof(buffer));
> +}
> +
> +static bool gip_send_set_device_state(struct gip_attachment *attachment, uint8_t state)
> +{
> + uint8_t buffer[] = { state };
> +
> + return gip_send_system_message(attachment, GIP_CMD_SET_DEVICE_STATE,
> + attachment->attachment_index, buffer, sizeof(buffer));
> +}
> +
> +static int gip_handle_command_raw_report(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + struct input_dev *input;
> +
> + if (num_bytes < 17) {
> + dev_dbg(GIP_DEV(attachment), "Discarding too-short raw report\n");
> + return -EINVAL;
> + }
> + guard(rcu)();
> + input = rcu_dereference(attachment->input);
> + if (!input)
> + return -ENODEV;
> +
> + if ((attachment->features & GIP_FEATURE_ELITE_BUTTONS)
> + && attachment->xbe_format == GIP_BTN_FMT_XBE2_RAW) {
> + input_report_abs(input, ABS_PROFILE, bytes[15] & 3);
> + if (bytes[15] & 3) {
> + input_report_key(input, BTN_GRIPL, 0);
> + input_report_key(input, BTN_GRIPR, 0);
> + input_report_key(input, BTN_GRIPL2, 0);
> + input_report_key(input, BTN_GRIPR2, 0);
> + } else {
> + input_report_key(input, BTN_GRIPL,
> + bytes[GIP_BTN_OFFSET_XBE2] & BIT(2));
> + input_report_key(input, BTN_GRIPR,
> + bytes[GIP_BTN_OFFSET_XBE2] & BIT(0));
> + input_report_key(input, BTN_GRIPL2,
> + bytes[GIP_BTN_OFFSET_XBE2] & BIT(3));
> + input_report_key(input, BTN_GRIPR2,
> + bytes[GIP_BTN_OFFSET_XBE2] & BIT(1));
> + }
> +
> + input_sync(input);
> + }
> + return 0;
> +}
> +
> +static int gip_setup_input_device(struct gip_attachment *attachment)
> +{
> + struct input_dev *input;
> + int rc;
> +
> + if (!attachment->driver || !attachment->driver->setup_input)
> + return -ENODEV;
> +
> + rcu_read_lock();
> + input = rcu_dereference(attachment->input);
> + rcu_read_unlock();
> + if (input)
> + return 0;
> +
> + input = input_allocate_device();
> + if (!input)
> + return -ENOMEM;
> + input->id.bustype = BUS_USB;
> + input->id.vendor = attachment->vendor_id;
> + input->id.product = attachment->product_id;
> + input->uniq = attachment->uniq;
> + if (attachment->name)
> + input->name = attachment->name;
> + else if (attachment->attachment_index == 0)
> + input->name = attachment->device->udev->product;
> + input->phys = attachment->phys;
> +
> + rc = attachment->driver->setup_input(attachment, input);
> + if (rc < 0)
> + goto err_free_device;
> +
> + if (attachment->features & GIP_FEATURE_CONSOLE_FUNCTION_MAP)
> + input_set_capability(input, EV_KEY, KEY_RECORD);
> +
> + if (attachment->features & GIP_FEATURE_ELITE_BUTTONS) {
> + input_set_capability(input, EV_KEY, BTN_GRIPL);
> + input_set_capability(input, EV_KEY, BTN_GRIPR);
> + input_set_capability(input, EV_KEY, BTN_GRIPL2);
> + input_set_capability(input, EV_KEY, BTN_GRIPR2);
> + if (attachment->xbe_format == GIP_BTN_FMT_XBE1)
> + input_set_abs_params(input, ABS_PROFILE, 0, 1, 0, 0);
> + else
> + input_set_abs_params(input, ABS_PROFILE, 0, 3, 0, 0);
> +
> + attachment->vendor_handlers[GIP_CMD_RAW_REPORT] = gip_handle_command_raw_report;
> + }
> +
> +#ifdef CONFIG_JOYSTICK_XBOX_GIP_FF
> + if (attachment->features & GIP_FEATURE_MOTOR_CONTROL) {
> + input_set_capability(input, EV_FF, FF_RUMBLE);
> + input_ff_create_memless(input, NULL, gip_play_effect);
> + }
> +#endif
> +
> + input_set_drvdata(input, attachment);
> + rcu_assign_pointer(attachment->input, input);
> + rc = input_register_device(input);
> + if (rc)
> + goto err_free_device;
> +
> + return 0;
> +
> +err_free_device:
> + input_free_device(input);
> + return rc;
> +}
> +
> +static int gip_send_init_sequence(struct gip_attachment *attachment)
> +{
> + int rc = 0;
> + size_t len;
> +
> + if (attachment->features & GIP_FEATURE_EXTENDED_SET_DEVICE_STATE) {
> + /*
> + * The meaning of this packet is unknown and not documented, but it's
> + * needed for the Elite 2 controller to start up on older firmwares
> + */
> + static const uint8_t set_device_state[] = {
> + GIP_STATE_UNK6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> + 0x55, 0x53, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0
> + };
> +
> + rc = gip_send_system_message(attachment,
> + GIP_CMD_SET_DEVICE_STATE, 0, set_device_state,
> + sizeof(set_device_state));
> + if (rc)
> + return rc;
> + }
> + rc = gip_enable_elite_buttons(attachment);
> + if (rc)
> + return rc;
> + if (!gip_supports_system_message(attachment, GIP_CMD_AUDIO_CONTROL, false)) {
> + rc = gip_send_set_device_state(attachment, GIP_STATE_START);
> + if (rc)
> + return rc;
> + attachment->device_state = GIP_STATE_START;
> + } else {
> + rc = gip_send_set_device_state(attachment, GIP_STATE_STOP);
> + if (rc)
> + return rc;
> + attachment->device_state = GIP_STATE_STOP;
> + }
> +
> + rc = gip_send_guide_button_led(attachment,
> + GIP_LED_GUIDE_ON,
> + GIP_LED_GUIDE_INIT_BRIGHTNESS);
> + if (rc)
> + return rc;
> +
> + if (gip_supports_system_message(attachment, GIP_CMD_SECURITY, false)
> + && !(attachment->features & GIP_FEATURE_SECURITY_OPT_OUT)) {
> + /* TODO: Implement Security command property */
> + uint8_t buffer[] = { 0x1, 0x0 };
> +
> + rc = gip_send_system_message(attachment, GIP_CMD_SECURITY, 0,
> + buffer, sizeof(buffer));
> + if (rc)
> + return rc;
> + }
> +
> + usb_make_path(attachment->device->udev, attachment->phys,
> + sizeof(attachment->phys));
> + len = strlen(attachment->phys);
> + if (len < sizeof(attachment->phys) - 1)
> + snprintf(attachment->phys + len,
> + sizeof(attachment->phys) - len, "/input%d",
> + attachment->attachment_index);
> +
> + if (attachment->driver && attachment->driver->init) {
> + rc = attachment->driver->init(attachment);
> + if (rc < 0)
> + return rc;
> + }
> +
> + if (rc != GIP_INIT_NO_INPUT && (attachment->features & GIP_FEATURE_CONTROLLER)) {
> + rc = gip_setup_input_device(attachment);
> + if (rc == -ENODEV)
> + return 0;
> + if (rc)
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static void gip_fragment_timeout(struct work_struct *work)
> +{
> + struct gip_attachment *attachment = container_of(to_delayed_work(work),
> + struct gip_attachment, fragment_timeout);
> +
> + guard(mutex)(&attachment->lock);
> + devm_kfree(GIP_DEV(attachment), attachment->fragment_data);
> + attachment->fragment_data = NULL;
> + attachment->fragment_message = 0;
> +}
> +
> +static void gip_retry_metadata(struct work_struct *work)
> +{
> + struct gip_attachment *attachment = container_of(to_delayed_work(work),
> + struct gip_attachment, metadata_next);
> +
> + guard(mutex)(&attachment->lock);
> + if (attachment->metadata_retries < 4) {
> + attachment->metadata_retries++;
> + schedule_delayed_work(&attachment->metadata_next, HZ / 2);
> + gip_send_system_message(attachment, GIP_CMD_METADATA, 0, NULL, 0);
> + } else {
> + dev_info(GIP_DEV(attachment),
> + "Unable to obtain metadata, attempting to reset device\n");
> + gip_send_set_device_state(attachment, GIP_STATE_RESET);
> + }
> +}
> +
> +static int gip_ensure_metadata(struct gip_attachment *attachment)
> +{
> + switch (attachment->got_metadata) {
> + case GIP_METADATA_GOT:
> + case GIP_METADATA_FAKED:
> + return 0;
> + case GIP_METADATA_NONE:
> + attachment->got_metadata = GIP_METADATA_PENDING;
> + cancel_delayed_work_sync(&attachment->metadata_next);
> + schedule_delayed_work(&attachment->metadata_next, HZ / 2);
> + attachment->metadata_retries = 0;
> + return gip_send_system_message(attachment, GIP_CMD_METADATA, 0, NULL, 0);
> + default:
> + return 0;
> + }
> +}
> +
> +static void gip_set_metadata_defaults(struct gip_attachment *attachment)
> +{
> + if (attachment->got_metadata != GIP_METADATA_NONE)
> + return;
> +
> + attachment->metadata.device.in_system_messages[0] =
> + GIP_DEFAULT_IN_SYSTEM_MESSAGES;
> + attachment->metadata.device.out_system_messages[0] =
> + GIP_DEFAULT_OUT_SYSTEM_MESSAGES;
> + if (attachment->attachment_index == 0) {
> + /* Some decent default settings */
> + attachment->features |= GIP_FEATURE_CONTROLLER;
> + attachment->metadata.device.in_system_messages[0] |= (1u << GIP_CMD_GUIDE_BUTTON);
> + }
> +
> + gip_handle_quirks(attachment);
> + if (attachment->quirks & GIP_QUIRK_NO_HELLO)
> + gip_ensure_metadata(attachment);
> +
> + attachment->got_metadata = GIP_METADATA_FAKED;
> +}
> +
> +static bool gip_handle_command_protocol_control(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + /* TODO */
> + dev_warn(GIP_DEV(attachment), "Unimplemented Protocol Control message\n");
> + return -ENOTSUPP;
> +}
> +
> +static bool gip_handle_command_hello_device(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + struct gip_hello_device message = {0};
> +
> + if (num_bytes != 28)
> + return -EINVAL;
> +
> + message.device_id = (uint64_t) bytes[0];
> + message.device_id |= (uint64_t) bytes[1] << 8;
> + message.device_id |= (uint64_t) bytes[2] << 16;
> + message.device_id |= (uint64_t) bytes[3] << 24;
> + message.device_id |= (uint64_t) bytes[4] << 32;
> + message.device_id |= (uint64_t) bytes[5] << 40;
> + message.device_id |= (uint64_t) bytes[6] << 48;
> + message.device_id |= (uint64_t) bytes[7] << 56;
> +
> + message.vendor_id = bytes[8];
> + message.vendor_id |= bytes[9] << 8;
> +
> + message.product_id = bytes[10];
> + message.product_id |= bytes[11] << 8;
> +
> + message.firmware_major_version = bytes[12];
> + message.firmware_major_version |= bytes[13] << 8;
> +
> + message.firmware_minor_version = bytes[14];
> + message.firmware_minor_version |= bytes[15] << 8;
> +
> + message.firmware_build_version = bytes[16];
> + message.firmware_build_version |= bytes[17] << 8;
> +
> + message.firmware_revision = bytes[18];
> + message.firmware_revision |= bytes[19] << 8;
> +
> + message.hardware_major_version = bytes[20];
> + message.hardware_minor_version = bytes[21];
> +
> + message.rf_proto_major_version = bytes[22];
> + message.rf_proto_minor_version = bytes[23];
> +
> + message.security_major_version = bytes[24];
> + message.security_minor_version = bytes[25];
> +
> + message.gip_major_version = bytes[26];
> + message.gip_minor_version = bytes[27];
> +
> + dev_dbg(GIP_DEV(attachment), "Device hello from %llx (%04x:%04x)\n",
> + message.device_id, message.vendor_id, message.product_id);
> + dev_dbg(GIP_DEV(attachment), "Firmware version %d.%d.%d rev %d\n",
> + message.firmware_major_version, message.firmware_minor_version,
> + message.firmware_build_version, message.firmware_revision);
> +
> + /*
> + * The GIP spec specifies that the host should reject the device if any of these are wrong.
> + * I don't know if Windows or an Xbox do, however, so let's just log warnings instead.
> + */
> + if (message.rf_proto_major_version != 1 && message.rf_proto_minor_version != 0)
> + dev_warn(GIP_DEV(attachment),
> + "Invalid RF protocol version %d.%d, expected 1.0\n",
> + message.rf_proto_major_version, message.rf_proto_minor_version);
> +
> + if (message.security_major_version != 1 && message.security_minor_version != 0)
> + dev_warn(GIP_DEV(attachment),
> + "Invalid security protocol version %d.%d, expected 1.0\n",
> + message.security_major_version, message.security_minor_version);
> +
> + if (message.gip_major_version != 1 && message.gip_minor_version != 0)
> + dev_warn(GIP_DEV(attachment),
> + "Invalid GIP version %d.%d, expected 1.0\n",
> + message.gip_major_version, message.gip_minor_version);
> +
> + attachment->firmware_major_version = message.firmware_major_version;
> + attachment->firmware_minor_version = message.firmware_minor_version;
> + attachment->vendor_id = message.vendor_id;
> + attachment->product_id = message.product_id;
> + attachment->uniq = devm_kasprintf(GIP_DEV(attachment),
> + GFP_KERNEL, "%llx", message.device_id);
> +
> + if (header->flags & GIP_FLAG_ATTACHMENT_MASK)
> + return gip_send_system_message(attachment, GIP_CMD_METADATA, 0, NULL, 0);
> + if (attachment->got_metadata == GIP_METADATA_FAKED)
> + attachment->got_metadata = GIP_METADATA_NONE;
> + return gip_ensure_metadata(attachment);
> +}
> +
> +static int gip_handle_command_status_device(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + int i;
> +
> + if (num_bytes < 1)
> + return -EINVAL;
> +
> + attachment->status.base.battery_level = bytes[0] & 3;
> + attachment->status.base.battery_type = (bytes[0] >> 2) & 3;
> + attachment->status.base.charge = (bytes[0] >> 4) & 3;
> + attachment->status.base.power_level = (bytes[0] >> 6) & 3;
> +
> + if (num_bytes >= 4) {
> + attachment->status.device_active = bytes[1] & 1;
> + if (bytes[1] & 2) {
> + /* Events present */
> + if (num_bytes < 5)
> + return -EINVAL;
> +
> + attachment->status.num_events = bytes[4];
> + if (attachment->status.num_events > 5) {
> + dev_info(GIP_DEV(attachment),
> + "Device reported too many events, %d > 5\n",
> + attachment->status.num_events);
> + return -EINVAL;
> + }
> + if (5 + attachment->status.num_events * 10 > num_bytes)
> + return -EINVAL;
> +
> + for (i = 0; i < attachment->status.num_events; i++) {
> + struct gip_status_event *event = &attachment->status.events[i];
> +
> + event->event_type = bytes[i * 10 + 5];
> + event->event_type |= bytes[i * 10 + 6] << 8;
> + event->fault_tag = bytes[i * 10 + 7];
> + event->fault_tag |= bytes[i * 10 + 8] << 8;
> + event->fault_tag |= bytes[i * 10 + 9] << 16;
> + event->fault_tag |= bytes[i * 10 + 10] << 24;
> + event->fault_address = bytes[i * 10 + 11];
> + event->fault_address |= bytes[i * 10 + 12] << 8;
> + event->fault_address |= bytes[i * 10 + 13] << 16;
> + event->fault_address |= bytes[i * 10 + 14] << 24;
> +
> + dev_info(GIP_DEV(attachment),
> + "Attachment %i event type %i, tag %i address %x\n",
> + attachment->attachment_index,
> + event->event_type,
> + event->fault_tag,
> + event->fault_address);
> + }
> + }
> + }
> +
> + return gip_ensure_metadata(attachment);
> +}
> +
> +static int gip_handle_command_metadata_respose(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + struct gip_metadata metadata = {0};
> + const guid_t *expected_guid = NULL;
> + bool found_expected_guid;
> + bool found_controller_guid = false;
> + int i, j, k;
> + int rc;
> +
> + rc = gip_parse_metadata(GIP_DEV(attachment), &metadata, bytes, num_bytes);
> + if (rc)
> + return rc;
> +
> + if (attachment->got_metadata == GIP_METADATA_GOT) {
> + struct input_dev *input;
> +
> + gip_metadata_free(GIP_DEV(attachment), &attachment->metadata);
> + rcu_read_lock();
> + input = rcu_dereference(attachment->input);
> + rcu_read_unlock();
> + if (input) {
> + rcu_assign_pointer(attachment->input, NULL);
> + synchronize_rcu();
> + input_unregister_device(input);
> + }
> + }
> +
> + attachment->metadata = metadata;
> + attachment->got_metadata = GIP_METADATA_GOT;
> + attachment->features = 0;
> + cancel_delayed_work_sync(&attachment->metadata_next);
> +
> + for (i = 0; i < metadata.device.num_preferred_types; i++) {
> + const char *type = metadata.device.preferred_types[i];
> +
> + dev_dbg(GIP_DEV(attachment), "Device preferred type: %s\n",
> + type);
> + }
> + for (i = 0; i < metadata.device.num_preferred_types; i++) {
> + const char *type = metadata.device.preferred_types[i];
> +
> + for (j = 0; base_drivers[j] && !expected_guid; j++) {
> + for (k = 0; base_drivers[j]->types[k] && !expected_guid; k++) {
> + if (strcmp(type, base_drivers[j]->types[k]) == 0) {
> + rc = gip_bind_driver(attachment, base_drivers[j]);
> + if (rc == 0)
> + expected_guid = &base_drivers[j]->guid;
> + else if (rc != -ENODEV)
> + return rc;
> + }
> + }
> + }
> + if (expected_guid)
> + break;
> +
> + if (strcmp(type, "Windows.Xbox.Input.Chatpad") == 0) {
> + break;
> + }
This is one of the aforementioned stylistic errors. I have fixed it locally.
> + if (strcmp(type, "Windows.Xbox.Input.Headset") == 0) {
> + expected_guid = &guid_headset;
> + break;
> + }
> + }
> +
> + found_expected_guid = !expected_guid;
> + for (i = 0; i < metadata.device.num_supported_interfaces; i++) {
> + const guid_t *guid = &metadata.device.supported_interfaces[i];
> +
> + dev_dbg(GIP_DEV(attachment), "Supported interface: %pUl\n", guid);
> + if (expected_guid && guid_equal(expected_guid, guid))
> + found_expected_guid = true;
> +
> + if (guid_equal(&guid_controller, guid)) {
> + found_controller_guid = true;
> + continue;
> + }
> + if (guid_equal(&gip_driver_navigation.guid, guid)) {
> + attachment->features |= GIP_FEATURE_CONTROLLER;
> + continue;
> + }
> + if (guid_equal(&guid_dev_auth_pc_opt_out, guid)) {
> + attachment->features |= GIP_FEATURE_SECURITY_OPT_OUT;
> + continue;
> + }
> + if (guid_equal(&guid_console_function_map, guid)) {
> + attachment->features |= GIP_FEATURE_CONSOLE_FUNCTION_MAP;
> + continue;
> + }
> + if (guid_equal(&guid_console_function_map_overflow, guid)) {
> + attachment->features |= GIP_FEATURE_CONSOLE_FUNCTION_MAP_OVERFLOW;
> + continue;
> + }
> + if (guid_equal(&guid_elite_buttons, guid)) {
> + attachment->features |= GIP_FEATURE_ELITE_BUTTONS;
> + continue;
> + }
> + if (guid_equal(&guid_dynamic_latency_input, guid)) {
> + attachment->features |= GIP_FEATURE_DYNAMIC_LATENCY_INPUT;
> + continue;
> + }
> + }
> +
> + for (i = 0; i < metadata.num_messages; i++) {
> + struct gip_message_metadata *message = &metadata.message_metadata[i];
> +
> + if (message->type == GIP_CMD_DIRECT_MOTOR && message->length >= 9
> + && (message->flags & GIP_MESSAGE_FLAG_DOWNSTREAM))
> + attachment->features |= GIP_FEATURE_MOTOR_CONTROL;
> + }
> +
> + if (!found_expected_guid || !found_controller_guid)
> + dev_dbg(GIP_DEV(attachment),
> + "Controller was missing expected GUID. "
> + "This controller probably won't work on an actual Xbox.\n");
> +
> + gip_handle_quirks(attachment);
> +
> + if ((attachment->features & GIP_FEATURE_GUIDE_COLOR)
> + && !gip_supports_vendor_message(attachment,
> + GIP_CMD_GUIDE_COLOR, false))
> + attachment->features &= ~GIP_FEATURE_GUIDE_COLOR;
> +
> + dev_dbg(GIP_DEV(attachment),
> + "Attachment %i has features: %02x\n",
> + attachment->attachment_index, attachment->features);
> +
> + return gip_send_init_sequence(attachment);
> +}
> +
> +static int gip_handle_command_security(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + /* TODO: Needed for controllers that connect via dongles */
> + dev_warn(GIP_DEV(attachment), "Unimplemented Security message\n");
> + return -ENOTSUPP;
> +}
> +
> +static int gip_handle_command_guide_button_status(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + struct input_dev *input;
> +
> + if (num_bytes < 2)
> + return -EINVAL;
> +
> + guard(rcu)();
> + input = rcu_dereference(attachment->input);
> + if (!input)
> + return -ENODEV;
> +
> + if (bytes[1] == VK_LWIN) {
> + input_report_key(input, BTN_MODE, bytes[0] & 3);
> + input_sync(input);
> + }
> +
> + return 0;
> +}
> +
> +static int gip_handle_command_audio_control(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + /* TODO: Needed for audio */
> + dev_warn(GIP_DEV(attachment), "Unimplemented Audio Control message\n");
> + return -ENOTSUPP;
> +}
> +
> +static int gip_handle_command_firmware(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + if (num_bytes < 1)
> + return -EINVAL;
> +
> + if (bytes[0] == 1) {
> + uint16_t major, minor, build, rev;
> +
> + if (num_bytes < 14) {
> + dev_dbg(GIP_DEV(attachment),
> + "Discarding too-short firmware message\n");
> +
> + return -EINVAL;
> + }
> + major = bytes[6];
> + major |= bytes[7] << 8;
> + minor = bytes[8];
> + minor |= bytes[9] << 8;
> + build = bytes[10];
> + build |= bytes[11] << 8;
> + rev = bytes[12];
> + rev |= bytes[13] << 8;
> +
> + dev_dbg(GIP_DEV(attachment),
> + "Firmware version: %d.%d.%d rev %d\n", major, minor, build, rev);
> +
> + attachment->firmware_major_version = major;
> + attachment->firmware_minor_version = minor;
> +
> + if (attachment->vendor_id == 0x045e
> + && attachment->product_id == 0x0b00)
> + return gip_enable_elite_buttons(attachment);
> +
> + return 0;
> + }
> +
> + dev_warn(GIP_DEV(attachment), "Unimplemented Firmware message\n");
> +
> + return -ENOTSUPP;
> +}
> +
> +static int gip_handle_command_hid_report(struct gip_attachment *attachment,
> + const struct gip_header *header, uint8_t *bytes, int num_bytes)
> +{
> + dev_warn(GIP_DEV(attachment), "Unimplemented HID report message\n");
> +
> + return -ENOTSUPP;
> +}
> +
> +static int gip_handle_command_extended(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + if (num_bytes < 2)
> + return -EINVAL;
> +
> + if (bytes[1] != GIP_EXTENDED_STATUS_OK) {
> + dev_dbg(GIP_DEV(attachment),
> + "Extended message type %02x failed with status %i\n",
> + bytes[0], bytes[1]);
> + return -EPROTO;
> + }
> +
> + switch (bytes[0]) {
> + case GIP_EXTCMD_GET_SERIAL_NUMBER:
> + memcpy(attachment->serial, &bytes[2],
> + min(sizeof(attachment->serial), (size_t)(num_bytes - 2)));
> + break;
> + default:
> + /* TODO */
> + dev_dbg(GIP_DEV(attachment), "Unimplemented extended message type %02x\n",
> + bytes[0]);
> + return -ENOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int gip_handle_elite_buttons(struct gip_attachment *attachment,
> + struct input_dev *input, const uint8_t *bytes, int num_bytes)
> +{
> + bool grip[4] = { 0, 0, 0, 0 };
> + int profile = -1;
> +
> + if (attachment->xbe_format == GIP_BTN_FMT_XBE1
> + && num_bytes > GIP_BTN_OFFSET_XBE1) {
> + profile = bytes[GIP_BTN_OFFSET_XBE1] >> 4;
> + if (profile) {
> + grip[0] = bytes[GIP_BTN_OFFSET_XBE1] & BIT(0);
> + grip[1] = bytes[GIP_BTN_OFFSET_XBE1] & BIT(1);
> + grip[2] = bytes[GIP_BTN_OFFSET_XBE1] & BIT(2);
> + grip[3] = bytes[GIP_BTN_OFFSET_XBE1] & BIT(3);
> + }
> + } else if ((attachment->xbe_format == GIP_BTN_FMT_XBE2_4
> + || attachment->xbe_format == GIP_BTN_FMT_XBE2_5)
> + && num_bytes > GIP_BTN_OFFSET_XBE2) {
> + int profile_offset;
> +
> + if (attachment->xbe_format == GIP_BTN_FMT_XBE2_4)
> + profile_offset = 15;
> + else
> + profile_offset = 20;
> + profile = bytes[profile_offset] & 3;
> +
> + if (!profile) {
> + grip[0] = bytes[GIP_BTN_OFFSET_XBE2] & BIT(2);
> + grip[1] = bytes[GIP_BTN_OFFSET_XBE2] & BIT(0);
> + grip[2] = bytes[GIP_BTN_OFFSET_XBE2] & BIT(3);
> + grip[3] = bytes[GIP_BTN_OFFSET_XBE2] & BIT(1);
> + }
> + }
> + if (profile >= 0) {
> + input_report_key(input, BTN_GRIPL, grip[0]);
> + input_report_key(input, BTN_GRIPR, grip[1]);
> + input_report_key(input, BTN_GRIPL2, grip[2]);
> + input_report_key(input, BTN_GRIPR2, grip[3]);
> + input_report_abs(input, ABS_PROFILE, profile);
> + }
> + return 0;
> +}
> +
> +static int gip_handle_console_map(struct gip_attachment *attachment,
> + struct input_dev *input, const uint8_t *bytes, int num_bytes)
> +{
> + int function_map_offset = -1;
> + if (num_bytes < 32)
> + return 0;
> +
> + if (attachment->features & GIP_FEATURE_DYNAMIC_LATENCY_INPUT) {
> + /* The dynamic latency input bytes are after the console function map */
> + if (num_bytes >= 40)
> + function_map_offset = num_bytes - 26;
> + } else {
> + function_map_offset = num_bytes - 18;
> + }
> + if (function_map_offset >= 14) {
> + input_report_key(input, KEY_RECORD,
> + bytes[function_map_offset] & BIT(0));
> + }
> + return 0;
> +}
> +
> +static int gip_handle_ll_input_report(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + struct input_dev *input;
> + int rc = 0;
> +
> + guard(rcu)();
> + input = rcu_dereference(attachment->input);
> + if (!input)
> + return -ENODEV;
> +
> + if (attachment->device_state != GIP_STATE_START) {
> + dev_dbg(GIP_DEV(attachment), "Discarding early input report\n");
> + attachment->device_state = GIP_STATE_START;
> + return 0;
> + }
> +
> + if (attachment->driver && attachment->driver->handle_input_report) {
> + rc = attachment->driver->handle_input_report(attachment, input, bytes, num_bytes);
> + if (rc < 0)
> + return rc;
> + }
> +
> + if (attachment->features & GIP_FEATURE_ELITE_BUTTONS) {
> + rc = gip_handle_elite_buttons(attachment, input, bytes, num_bytes);
> + if (rc < 0)
> + goto exit;
> + }
> +
> + rc = gip_handle_console_map(attachment, input, bytes, num_bytes);
> +
> +exit:
> + input_sync(input);
> +
> + return rc;
> +}
> +
> +static int gip_handle_ll_overflow_input_report(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + /* TODO: Unknown if any devices actually use this */
> + dev_dbg(GIP_DEV(attachment), "Unimplemented Overflow Input Report message\n");
> + return -ENOTSUPP;
> +}
> +
> +static int gip_handle_audio_data(struct gip_attachment *attachment,
> + const struct gip_header *header, const uint8_t *bytes, int num_bytes)
> +{
> + /* TODO: Needed for audio support */
> + dev_dbg(GIP_DEV(attachment), "Unimplemented Audio Data message\n");
> + return -ENOTSUPP;
> +}
> +
> +static int gip_handle_system_message(struct gip_attachment *attachment,
> + const struct gip_header *header, uint8_t *bytes, int num_bytes)
> +{
> + if (!gip_supports_system_message(attachment, header->message_type, true)) {
> + dev_warn(GIP_DEV(attachment),
> + "Received claimed-unsupported system message type %02x\n",
> + header->message_type);
> + return -EINVAL;
> + }
> + switch (header->message_type) {
> + case GIP_CMD_PROTO_CONTROL:
> + return gip_handle_command_protocol_control(attachment, header,
> + bytes, num_bytes);
> + case GIP_CMD_HELLO_DEVICE:
> + return gip_handle_command_hello_device(attachment, header,
> + bytes, num_bytes);
> + case GIP_CMD_STATUS_DEVICE:
> + return gip_handle_command_status_device(attachment, header,
> + bytes, num_bytes);
> + case GIP_CMD_METADATA:
> + return gip_handle_command_metadata_respose(attachment, header,
> + bytes, num_bytes);
> + case GIP_CMD_SECURITY:
> + return gip_handle_command_security(attachment, header, bytes,
> + num_bytes);
> + case GIP_CMD_GUIDE_BUTTON:
> + return gip_handle_command_guide_button_status(attachment,
> + header, bytes, num_bytes);
> + case GIP_CMD_AUDIO_CONTROL:
> + return gip_handle_command_audio_control(attachment, header,
> + bytes, num_bytes);
> + case GIP_CMD_FIRMWARE:
> + return gip_handle_command_firmware(attachment, header, bytes,
> + num_bytes);
> + case GIP_CMD_HID_REPORT:
> + return gip_handle_command_hid_report(attachment, header,
> + bytes, num_bytes);
> + case GIP_CMD_EXTENDED:
> + return gip_handle_command_extended(attachment, header, bytes,
> + num_bytes);
> + case GIP_AUDIO_DATA:
> + return gip_handle_audio_data(attachment, header, bytes,
> + num_bytes);
> + default:
> + dev_warn(GIP_DEV(attachment),
> + "Received unknown system message type %02x\n",
> + header->message_type);
> + return -EINVAL;
> + }
> +}
> +
> +static struct gip_attachment *gip_ensure_attachment(struct gip_device *device,
> + uint8_t attachment_index)
> +{
> + struct gip_attachment *attachment = device->attachments[attachment_index];
> +
> + if (!attachment) {
> + attachment = devm_kzalloc(GIP_DEV(device),
> + sizeof(*attachment), GFP_KERNEL);
> + if (!attachment)
> + return ERR_PTR(-ENOMEM);
> +
> + attachment->attachment_index = attachment_index;
> + attachment->device = device;
> +
> + if (attachment_index == 0) {
> + attachment->vendor_id = device->udev->descriptor.idVendor;
> + attachment->product_id = device->udev->descriptor.idProduct;
> + }
> +
> + device->attachments[attachment_index] = attachment;
> +
> + mutex_init(&attachment->lock);
> + INIT_DELAYED_WORK(&attachment->fragment_timeout, gip_fragment_timeout);
> + INIT_DELAYED_WORK(&attachment->metadata_next, gip_retry_metadata);
> +
> + gip_set_metadata_defaults(attachment);
> + }
> + return attachment;
> +}
> +
> +static int gip_handle_message(struct gip_attachment *attachment,
> + const struct gip_header *header, uint8_t *bytes, int num_bytes)
> +{
> + if (header->flags & GIP_FLAG_SYSTEM)
> + return gip_handle_system_message(attachment, header, bytes,
> + num_bytes);
> +
> + if (header->message_type < MAX_GIP_CMD && attachment->vendor_handlers[header->message_type])
> + return attachment->vendor_handlers[header->message_type](attachment,
> + header, bytes, num_bytes);
> +
> + switch (header->message_type) {
> + case GIP_LL_INPUT_REPORT:
> + return gip_handle_ll_input_report(attachment, header, bytes,
> + num_bytes);
> + case GIP_LL_OVERFLOW_INPUT_REPORT:
> + return gip_handle_ll_overflow_input_report(attachment, header,
> + bytes, num_bytes);
> + }
> + dev_warn(GIP_DEV(attachment),
> + "Received unknown vendor message type %02x\n",
> + header->message_type);
> + return -ENOTSUPP;
> +}
> +
> +static int gip_receive_fragment(struct gip_attachment *attachment,
> + const struct gip_header *header, int offset,
> + uint64_t *fragment_offset, uint16_t *bytes_remaining, uint8_t *bytes,
> + int num_bytes)
> +{
> + int rc = 0;
> +
> + if (header->flags & GIP_FLAG_INIT_FRAG) {
> + uint64_t total_length;
> +
> + if (attachment->fragment_message) {
> + /*
> + * Reset fragment buffer if we get a new initial
> + * fragment before finishing the last message.
> + * TODO: Is this the correct behavior?
> + */
> + devm_kfree(GIP_DEV(attachment), attachment->fragment_data);
> + attachment->fragment_data = NULL;
> + }
> + offset += gip_decode_length(&total_length, &bytes[offset],
> + num_bytes - offset);
> + if (total_length > MAX_MESSAGE_LENGTH)
> + return -EINVAL;
> +
> + attachment->total_length = total_length;
> + attachment->fragment_message = header->message_type;
> + if (header->length > num_bytes - offset) {
> + dev_warn(GIP_DEV(attachment),
> + "Received fragment that claims to be %llu bytes, expected %i\n",
> + header->length, num_bytes - offset);
> + return -EINVAL;
> + }
> + if (header->length > total_length) {
> + dev_warn(GIP_DEV(attachment),
> + "Received too long fragment, %llu bytes, exceeds %d\n",
> + header->length, attachment->total_length);
> + return -EINVAL;
> + }
> + attachment->fragment_data = devm_kmalloc(GIP_DEV(attachment),
> + attachment->total_length, GFP_KERNEL);
> + if (!attachment->fragment_data)
> + return -ENOMEM;
> + memcpy(attachment->fragment_data, &bytes[offset],
> + header->length);
> + *fragment_offset = header->length;
> + attachment->fragment_offset = header->length;
> + *bytes_remaining = attachment->total_length - header->length;
> + } else {
> + if (header->message_type != attachment->fragment_message) {
> + dev_warn(GIP_DEV(attachment),
> + "Received out of sequence message type %02x, expected %02x\n",
> + header->message_type, attachment->fragment_message);
> + gip_fragment_failed(attachment, header);
> + return -EINVAL;
> + }
> +
> + offset += gip_decode_length(fragment_offset, &bytes[offset],
> + num_bytes - offset);
> + if (*fragment_offset != attachment->fragment_offset) {
> + dev_warn(GIP_DEV(attachment),
> + "Received out of sequence fragment, (claimed %llu, expected %d)\n",
> + *fragment_offset, attachment->fragment_offset);
> + gip_acknowledge(attachment->device, header,
> + attachment->fragment_offset,
> + attachment->total_length - attachment->fragment_offset);
> + return -EINVAL;
> + } else if (*fragment_offset + header->length > attachment->total_length) {
> + dev_warn(GIP_DEV(attachment),
> + "Received too long fragment, %llu exceeds %d\n",
> + *fragment_offset + header->length, attachment->total_length);
> + gip_fragment_failed(attachment, header);
> + return -EINVAL;
> + }
> +
> + *bytes_remaining = attachment->total_length -
> + (*fragment_offset + header->length);
> + if (header->length != 0) {
> + memcpy(&attachment->fragment_data[*fragment_offset],
> + &bytes[offset], header->length);
> + } else {
> + rc = gip_handle_message(attachment, header,
> + attachment->fragment_data,
> + attachment->total_length);
> + devm_kfree(GIP_DEV(attachment), attachment->fragment_data);
> + attachment->fragment_data = NULL;
> + attachment->fragment_message = 0;
> + }
> + *fragment_offset += header->length;
> + attachment->fragment_offset = *fragment_offset;
> + }
> + cancel_delayed_work_sync(&attachment->fragment_timeout);
> + schedule_delayed_work(&attachment->fragment_timeout, HZ);
> +
> + return rc;
> +}
> +
> +static int gip_receive_message(struct gip_device *device, uint8_t *bytes,
> + int num_bytes)
> +{
> + struct gip_header header;
> + int offset = 3;
> + int rc = 0;
> + uint64_t fragment_offset = 0;
> + uint16_t bytes_remaining = 0;
> + bool is_fragment;
> + uint8_t attachment_index;
> + struct gip_attachment *attachment;
> +
> + if (num_bytes < 5)
> + return -EINVAL;
> +
> + header.message_type = bytes[0];
> + header.flags = bytes[1];
> + header.sequence_id = bytes[2];
> + offset += gip_decode_length(&header.length, &bytes[offset], num_bytes - offset);
> +
> + is_fragment = header.flags & GIP_FLAG_FRAGMENT;
> + attachment_index = header.flags & GIP_FLAG_ATTACHMENT_MASK;
> + attachment = gip_ensure_attachment(device, attachment_index);
> +
> + print_hex_dump_debug(KBUILD_MODNAME ": Received message: ", DUMP_PREFIX_OFFSET,
> + 16, 1, bytes, num_bytes, false);
> +
> + guard(mutex)(&attachment->lock);
> + /* Handle coalescing fragmented messages */
> + if (is_fragment) {
> + rc = gip_receive_fragment(attachment, &header, offset,
> + &fragment_offset, &bytes_remaining, bytes, num_bytes);
> + } else if (header.length + offset > num_bytes) {
> + dev_warn(GIP_DEV(device),
> + "Received message with erroneous length (claimed %llu, actual %d), discarding\n",
> + header.length + offset, num_bytes);
> + rc = -EINVAL;
> + } else {
> + num_bytes -= offset;
> + bytes += offset;
> + fragment_offset = header.length;
> + rc = gip_handle_message(attachment, &header, bytes, num_bytes);
> + }
> +
> + if (!rc && (header.flags & GIP_FLAG_ACME))
> + gip_acknowledge(device, &header, fragment_offset, bytes_remaining);
> +
> + return rc;
> +}
> +
> +static void gip_receive_work(struct work_struct *work)
> +{
> + struct gip_device *device = container_of(work, struct gip_device,
> + receive_message);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&device->message_lock, flags);
> + while (device->pending_in_messages) {
> + struct gip_raw_message *message = &device->in_queue[device->next_in_message];
> +
> + spin_unlock_irqrestore(&device->message_lock, flags);
> +
> + gip_receive_message(device, message->bytes, message->num_bytes);
> +
> + spin_lock_irqsave(&device->message_lock, flags);
> + device->next_in_message = (device->next_in_message + 1) % MAX_IN_MESSAGES;
> + device->pending_in_messages--;
> + }
> + spin_unlock_irqrestore(&device->message_lock, flags);
> +}
> +
> +static void gip_urb_in(struct urb *urb)
> +{
> + struct gip_interface *intf = urb->context;
> + struct gip_device *gip = intf->device;
> + struct device *dev = &intf->intf->dev;
> + int status = urb->status;
> + int message_id;
> + struct gip_raw_message *message;
> + unsigned long flags;
> +
> + switch (status) {
> + case 0:
> + /* success */
> + break;
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + /* this urb is terminated, clean up */
> + dev_dbg(dev, "%s - urb shutting down with status: %d\n",
> + __func__, status);
> + return;
> + default:
> + dev_dbg(dev, "%s - urb has status of: %d\n",
> + __func__, status);
> + goto exit;
> + }
> + if (intf->isoc_messages) {
> + /* TODO: Needed for audio support */
> + dev_warn(GIP_DEV(gip), "Unimplemented isochronous message input\n");
> + goto exit;
> + }
> +
> + spin_lock_irqsave(&gip->message_lock, flags);
> + if (gip->pending_in_messages >= MAX_IN_MESSAGES) {
> + dev_err(GIP_DEV(gip), "Input queue is full; dropping message\n");
> + } else {
> + message_id = (gip->next_in_message + gip->pending_in_messages) % MAX_IN_MESSAGES;
> + message = &gip->in_queue[message_id];
> + gip->pending_in_messages++;
> + memcpy(message->bytes, intf->in_data, urb->actual_length);
> + message->num_bytes = urb->actual_length;
> + }
> + spin_unlock_irqrestore(&gip->message_lock, flags);
> + schedule_work(&gip->receive_message);
> +
> +exit:
> + status = usb_submit_urb(urb, GFP_ATOMIC);
> + if (status)
> + dev_err(dev, "%s - usb_submit_urb failed with result %d\n",
> + __func__, status);
> +}
> +
> +static void gip_urb_out(struct urb *urb)
> +{
> + struct gip_interface *intf = urb->context;
> + struct device *dev = &intf->intf->dev;
> + int status = urb->status;
> +
> + guard(spinlock_irqsave)(&intf->device->message_lock);
> +
> + switch (status) {
> + case 0:
> + /* success */
> + break;
> +
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + /* this urb is terminated, clean up */
> + dev_dbg(dev, "%s - urb shutting down with status: %d\n",
> + __func__, status);
> + break;
> +
> + default:
> + dev_dbg(dev, "%s - nonzero urb status received: %d\n",
> + __func__, status);
> + break;
> + }
> +}
> +
> +static int gip_init_input(struct gip_interface *intf,
> + struct usb_endpoint_descriptor *ep_in)
> +{
> + int error;
> + struct usb_device *udev = interface_to_usbdev(intf->intf);
> +
> + intf->urb_in = usb_alloc_urb(intf->isoc_messages, GFP_KERNEL);
> + if (!intf->urb_in)
> + return -ENOMEM;
> +
> + intf->in_data = usb_alloc_coherent(udev, intf->mtu, GFP_KERNEL,
> + &intf->urb_in->transfer_dma);
> +
> + if (!intf->in_data) {
> + return -ENOMEM;
> + goto err_free_urb;
> + }
> +
> + usb_fill_int_urb(intf->urb_in, udev,
> + usb_rcvintpipe(udev, ep_in->bEndpointAddress),
> + intf->in_data, intf->mtu, gip_urb_in, intf,
> + ep_in->bInterval);
> + intf->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> + if (intf->isoc_messages)
> + intf->urb_in->transfer_flags |= URB_ISO_ASAP;
> +
> + return 0;
> +
> +err_free_urb:
> + usb_free_urb(intf->urb_in);
> + intf->urb_in = NULL;
> +
> + return error;
> +}
> +
> +static int gip_init_output(struct gip_interface *intf,
> + struct usb_endpoint_descriptor *ep_out)
> +{
> + int error;
> + struct usb_device *udev = interface_to_usbdev(intf->intf);
> + int i;
> +
> + if (usb_ifnum_to_if(udev, GIP_WIRED_INTF_AUDIO)) {
> + /*
> + * 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.
> + */
> + error = usb_set_interface(udev, GIP_WIRED_INTF_AUDIO, 0);
> + if (error)
> + dev_warn(GIP_DEV(intf),
> + "unable to disable audio interface: %d\n",
> + error);
> + }
> +
> + init_usb_anchor(&intf->out_anchor);
> +
> + for (i = 0; i < MAX_OUT_MESSAGES; i++) {
> + intf->out_queue[i].urb = usb_alloc_urb(intf->isoc_messages, GFP_KERNEL);
> + if (!intf->out_queue[i].urb) {
> + error = -ENOMEM;
> + goto err_free_urbs;
> + }
> +
> + intf->out_queue[i].data = usb_alloc_coherent(udev, intf->mtu, GFP_KERNEL,
> + &intf->out_queue[i].urb->transfer_dma);
> +
> + if (!intf->out_queue[i].data) {
> + return -ENOMEM;
> + goto err_free_urbs;
> + }
> +
> + usb_fill_int_urb(intf->out_queue[i].urb, udev,
> + usb_sndintpipe(udev, ep_out->bEndpointAddress),
> + intf->out_queue[i].data, intf->mtu, gip_urb_out, intf,
> + ep_out->bInterval);
> + intf->out_queue[i].urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> + if (intf->isoc_messages)
> + intf->out_queue[i].urb->transfer_flags |= URB_ISO_ASAP;
> + }
> +
> + return 0;
> +
> +err_free_urbs:
> + for (i = 0; i < MAX_OUT_MESSAGES; i++) {
> + if (intf->out_queue[i].data)
> + usb_free_coherent(udev, intf->mtu, intf->out_queue[i].data,
> + intf->out_queue[i].urb->transfer_dma);
> + if (intf->out_queue[i].urb) {
> + usb_free_urb(intf->out_queue[i].urb);
> + intf->out_queue[i].urb = NULL;
> + }
> + }
> + return error;
> +}
> +
> +static void gip_deinit_output(struct gip_interface *intf)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_OUT_MESSAGES; i++) {
> + if (!intf->out_queue[i].urb)
> + continue;
> + usb_free_coherent(interface_to_usbdev(intf->intf), intf->mtu,
> + intf->out_queue[i].data, intf->out_queue[i].urb->transfer_dma);
> + usb_free_urb(intf->out_queue[i].urb);
> + intf->out_queue[i].data = NULL;
> + intf->out_queue[i].urb = NULL;
> + }
> +}
> +
> +static void gip_deinit_input(struct gip_interface *intf)
> +{
> + usb_free_coherent(interface_to_usbdev(intf->intf), intf->mtu,
> + intf->in_data, intf->urb_in->transfer_dma);
> + usb_free_urb(intf->urb_in);
> + intf->urb_in = NULL;
> +}
> +
> +static int gip_interface_init(struct gip_interface *intf)
> +{
> + struct usb_endpoint_descriptor *ep_in = NULL;
> + struct usb_endpoint_descriptor *ep_out = NULL;
> + int error = usb_find_common_endpoints(intf->intf->cur_altsetting,
> + NULL, NULL, &ep_in, &ep_out);
> +
> + if (error)
> + return error;
> +
> + if (!ep_in || !ep_out)
> + return -ENODEV;
> +
> + error = gip_init_input(intf, ep_in);
> + if (error)
> + return error;
> +
> + error = gip_init_output(intf, ep_out);
> + if (error)
> + goto err_free_input;
> +
> + if (usb_submit_urb(intf->urb_in, GFP_KERNEL)) {
> + error = -EIO;
> + goto err_free_output;
> + }
> +
> + return 0;
> +
> +err_free_output:
> + gip_deinit_output(intf);
> +err_free_input:
> + gip_deinit_input(intf);
> + return error;
> +}
> +
> +static int gip_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +{
> + struct usb_device *udev = interface_to_usbdev(intf);
> + struct gip_device *gip = NULL;
> + struct gip_attachment *attachment;
> + int rc;
> +
> + if (intf->cur_altsetting->desc.bInterfaceNumber != GIP_WIRED_INTF_DATA) {
> + /*
> + * The Xbox One controller lists three interfaces all with the
> + * same interface class, subclass and protocol. Differentiate by
> + * interface number.
> + */
> + return 0;
> + }
> +
> + gip = devm_kzalloc(&udev->dev, sizeof(*gip), GFP_KERNEL);
> + if (!gip)
> + return -ENOMEM;
> +
> + gip->udev = udev;
> + gip->data.device = gip;
> + gip->data.intf = intf;
> + gip->data.mtu = BASE_GIP_MTU;
> + gip->audio.device = gip;
> + gip->audio.mtu = MAX_GIP_MTU;
> + gip->audio.isoc_messages = MAX_AUDIO_MESSAGES;
> +
> + INIT_WORK(&gip->receive_message, gip_receive_work);
> + spin_lock_init(&gip->message_lock);
> +
> + rc = gip_interface_init(&gip->data);
> + if (rc) {
> + devm_kfree(GIP_DEV(gip), gip);
> + return rc;
> + }
> + /* Don't init audio interface -- we aren't using it yet */
> +
> + usb_set_intfdata(intf, gip);
> +
> + /* Pre-create the first attachment, as it should always exist */
> + attachment = gip_ensure_attachment(gip, 0);
> + if (IS_ERR(attachment))
> + return PTR_ERR(attachment);
> +
> + return 0;
> +}
> +
> +static int gip_shutdown(struct gip_device *device)
> +{
> + int i;
> +
> + cancel_work_sync(&device->receive_message);
> +
> + for (i = 0; i < MAX_ATTACHMENTS; i++) {
> + struct gip_attachment *attachment = device->attachments[i];
> + struct input_dev *input;
> +
> + if (!attachment)
> + continue;
> +
> + scoped_guard (mutex, &attachment->lock) {
> + cancel_delayed_work_sync(&attachment->metadata_next);
> + cancel_delayed_work_sync(&attachment->fragment_timeout);
> +
> + rcu_read_lock();
> + input = rcu_dereference(attachment->input);
> + rcu_read_unlock();
> +
> + rcu_assign_pointer(attachment->input, NULL);
> + synchronize_rcu();
> + }
> +
> + if (input)
> + input_unregister_device(input);
> + }
> +
> + return 0;
> +}
> +
> +static void gip_disconnect(struct usb_interface *intf)
> +{
> + struct gip_device *gip = usb_get_intfdata(intf);
> + unsigned long flags;
> + int i;
> +
> + if (!gip)
> + return;
> +
> + usb_kill_urb(gip->data.urb_in);
> + if (gip->audio.intf)
> + usb_kill_urb(gip->audio.urb_in);
> +
> + gip_shutdown(gip);
> +
> + spin_lock_irqsave(&gip->message_lock, flags);
> + gip_deinit_input(&gip->data);
> + gip_deinit_output(&gip->data);
> + if (gip->audio.intf) {
> + gip_deinit_input(&gip->audio);
> + gip_deinit_output(&gip->audio);
> + }
> + spin_unlock_irqrestore(&gip->message_lock, flags);
> +
> + usb_set_intfdata(intf, NULL);
> +
> + for (i = 0; i < MAX_ATTACHMENTS; i++) {
> + struct gip_attachment *attachment = gip->attachments[i];
> +
> + if (!attachment)
> + continue;
> + devm_kfree(GIP_DEV(attachment), attachment->uniq);
> + devm_kfree(GIP_DEV(attachment), attachment);
> + }
> +
> + devm_kfree(GIP_DEV(gip), gip);
> +}
> +
> +static int gip_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> + struct gip_device *gip = usb_get_intfdata(intf);
> +
> + if (!gip)
> + return 0;
> +
> + usb_kill_urb(gip->data.urb_in);
> + if (gip->audio.intf)
> + usb_kill_urb(gip->audio.urb_in);
> +
> + if (gip->attachments[0]) {
> + struct gip_attachment *attachment = gip->attachments[0];
> +
> + guard(mutex)(&attachment->lock);
> + gip_send_set_device_state(attachment, GIP_STATE_OFF);
> + attachment->device_state = GIP_STATE_OFF;
> + }
> +
> + return gip_shutdown(gip);
> +}
> +
> +static int gip_resume(struct usb_interface *intf)
> +{
> + struct gip_device *gip = usb_get_intfdata(intf);
> +
> + if (!gip)
> + return 0;
> +
> + if (usb_submit_urb(gip->data.urb_in, GFP_KERNEL))
> + return -EIO;
> +
> + return 0;
> +}
> +
> +/* The Xbox One controller uses subclass 71 and protocol 208. */
> +#define GIP_VENDOR(vend) \
> + { \
> + .match_flags = USB_DEVICE_ID_MATCH_VENDOR | USB_DEVICE_ID_MATCH_INT_INFO, \
> + .idVendor = (vend), \
> + .bInterfaceClass = USB_CLASS_VENDOR_SPEC, \
> + .bInterfaceSubClass = 71, \
> + .bInterfaceProtocol = 208 \
> + }
> +
> +static const struct usb_device_id gip_table[] = {
> + /*
> + * Please keep this list sorted by vendor ID.
> + */
> + GIP_VENDOR(0x03f0), /* HP/HyperX */
> + GIP_VENDOR(0x044f), /* ThrustMaster */
> + GIP_VENDOR(0x045e), /* Microsoft */
> + GIP_VENDOR(0x046d), /* Logitech */
> + GIP_VENDOR(0x0738), /* Mad Catz */
> + GIP_VENDOR(0x0b05), /* ASUS */
> + GIP_VENDOR(0x0e6f), /* PDP */
> + GIP_VENDOR(0x0f0d), /* Hori */
> + GIP_VENDOR(0x10f5), /* Turtle Beach */
> + GIP_VENDOR(0x1532), /* Razer */
> + GIP_VENDOR(0x20d6), /* PowerA/BDA */
> + GIP_VENDOR(0x24c6), /* PowerA/BDA/ThrustMaster */
> + GIP_VENDOR(0x294b), /* Snakebyte */
> + GIP_VENDOR(0x2dc8), /* 8BitDo */
> + GIP_VENDOR(0x2e24), /* Hyperkin */
> + GIP_VENDOR(0x2e95), /* SCUF Gaming */
> + GIP_VENDOR(0x3285), /* Nacon */
> + GIP_VENDOR(0x3537), /* GameSir */
> + GIP_VENDOR(0x366c), /* ByoWave */
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(usb, gip_table);
> +
> +static struct usb_driver gip_driver = {
> + .name = "xbox-gip",
> + .probe = gip_probe,
> + .disconnect = gip_disconnect,
> + .suspend = gip_suspend,
> + .resume = gip_resume,
> + .id_table = gip_table,
> +};
> +
> +module_usb_driver(gip_driver);
> +
> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
> +MODULE_DESCRIPTION("Xbox Gaming Input Protocol driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/joystick/gip/gip-drivers.c b/drivers/input/joystick/gip/gip-drivers.c
> new file mode 100644
> index 0000000000000..f5507e6215a94
> --- /dev/null
> +++ b/drivers/input/joystick/gip/gip-drivers.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Base drivers for common GIP devices
> + *
> + * Copyright (c) 2025 Valve Software
> + *
> + * This driver is based on the Microsoft GIP spec at:
> + * https://aka.ms/gipdocs
> + * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gipusb/e7c90904-5e21-426e-b9ad-d82adeee0dbc
> + */
> +#include "gip.h"
> +
> +struct gip_device_capabilities_response {
> + uint8_t extra_button_count;
> + uint8_t extra_axis_count;
> + uint8_t led_count;
> + uint8_t max_global_led_gain;
> +};
> +
> +static bool dpad_as_buttons;
> +
> +static int gip_setup_gamepad_input(struct gip_attachment *attachment, struct input_dev* input)
This is one of the aforementioned stylistic errors. I have fixed all locations of this in the series.
> +{
> + int ret = gip_driver_navigation.setup_input(attachment, input);
> +
> + if (ret < 0)
> + return ret;
> + input_set_capability(input, EV_KEY, BTN_THUMBR);
> + input_set_capability(input, EV_KEY, BTN_THUMBL);
> + input_set_abs_params(input, ABS_X, -32768, 32767, 16, 128);
> + input_set_abs_params(input, ABS_Y, -32768, 32767, 16, 128);
> + input_set_abs_params(input, ABS_RX, -32768, 32767, 16, 128);
> + input_set_abs_params(input, ABS_RY, -32768, 32767, 16, 128);
> + input_set_abs_params(input, ABS_Z, 0, 1023, 0, 0);
> + input_set_abs_params(input, ABS_RZ, 0, 1023, 0, 0);
> +
> + /* Xbox Adaptive Controller */
> + if (attachment->vendor_id == 0x045e && attachment->product_id == 0x0b0a)
> + input_set_abs_params(input, ABS_PROFILE, 0, 3, 0, 0);
> + return 0;
> +}
> +
> +static int gip_handle_gamepad_report(struct gip_attachment *attachment,
> + struct input_dev *input, const uint8_t *bytes, int num_bytes)
> +{
> + int16_t axis;
> + int ret = gip_driver_navigation.handle_input_report(attachment, input, bytes, num_bytes);
> +
> + if (ret < 0)
> + return ret;
> +
> + if (num_bytes < 14) {
> + dev_dbg(GIP_DEV(attachment), "Discarding too-short input report\n");
> + return -EINVAL;
> + }
> +
> + input_report_key(input, BTN_THUMBL, bytes[1] & BIT(6));
> + input_report_key(input, BTN_THUMBR, bytes[1] & BIT(7));
> +
> + axis = bytes[2];
> + axis |= bytes[3] << 8;
> + input_report_abs(input, ABS_Z, axis);
> +
> + axis = bytes[4];
> + axis |= bytes[5] << 8;
> + input_report_abs(input, ABS_RZ, axis);
> +
> + axis = bytes[6];
> + axis |= bytes[7] << 8;
> + input_report_abs(input, ABS_X, axis);
> + axis = bytes[8];
> + axis |= bytes[9] << 8;
> + input_report_abs(input, ABS_Y, ~axis);
> + axis = bytes[10];
> + axis |= bytes[11] << 8;
> + input_report_abs(input, ABS_RX, axis);
> + axis = bytes[12];
> + axis |= bytes[13] << 8;
> + input_report_abs(input, ABS_RY, ~axis);
> +
> + /* Xbox Adaptive Controller */
> + if (attachment->vendor_id == 0x045e && attachment->product_id == 0x0b0a && num_bytes >= 31)
> + input_report_abs(input, ABS_PROFILE, bytes[30] & 3);
> +
> + return 0;
> +}
> +
> +const struct gip_driver gip_driver_gamepad = {
> + .types = (const char* const[]) { "Windows.Xbox.Input.Gamepad", NULL },
> + .guid = GUID_INIT(0x082e402c, 0x07df, 0x45e1, 0xa5, 0xab,
> + 0xa3, 0x12, 0x7a, 0xf1, 0x97, 0xb5),
> +
> + .quirks = (const struct gip_quirks[]) {
> + /* Xbox One Controller (model 1573) */
> + { 0x045e, 0x02d1, 0, .override_name = "Xbox One Controller" },
> +
> + /* Xbox One Controller (model 1697) */
> + { 0x045e, 0x02dd, 0, .override_name = "Xbox One Controller" },
> +
> + /* Xbox Elite */
> + { 0x045e, 0x02e3, 0,
> + .override_name = "Xbox Elite Controller",
> + .added_features = GIP_FEATURE_ELITE_BUTTONS,
> + .filtered_features = GIP_FEATURE_CONSOLE_FUNCTION_MAP },
> +
> + /* Xbox One Controller (model 1708) */
> + { 0x045e, 0x02ea, 0, .override_name = "Xbox One Controller" },
> +
> + /* Xbox Elite 2 */
> + { 0x045e, 0x0b00, 0,
> + .override_name = "Xbox Elite Series 2 Controller",
> + .added_features = GIP_FEATURE_GUIDE_COLOR |
> + GIP_FEATURE_EXTENDED_SET_DEVICE_STATE },
> +
> + /* Xbox Adaptive Controller */
> + { 0x045e, 0x0b0a, 0, .override_name = "Xbox Adaptive Controller" },
> +
> + /* Xbox Wireless Controller */
> + { 0x045e, 0x0b12, 0, .override_name = "Xbox Wireless Controller" },
> +
> + {0},
> + },
> +
> + .probe = NULL,
> + .remove = NULL,
> + .init = NULL,
> + .setup_input = gip_setup_gamepad_input,
> + .handle_input_report = gip_handle_gamepad_report,
> +};
> +
> +static int gip_setup_navigation_input(struct gip_attachment *attachment, struct input_dev *input)
> +{
> + input_set_capability(input, EV_KEY, BTN_Y);
> + input_set_capability(input, EV_KEY, BTN_B);
> + input_set_capability(input, EV_KEY, BTN_X);
> + input_set_capability(input, EV_KEY, BTN_A);
> + input_set_capability(input, EV_KEY, BTN_SELECT);
> + input_set_capability(input, EV_KEY, BTN_MODE);
> + input_set_capability(input, EV_KEY, BTN_START);
> + input_set_capability(input, EV_KEY, BTN_TR);
> + input_set_capability(input, EV_KEY, BTN_TL);
> +
> + attachment->dpad_as_buttons = dpad_as_buttons;
> + if (attachment->dpad_as_buttons) {
> + input_set_capability(input, EV_KEY, BTN_DPAD_UP);
> + input_set_capability(input, EV_KEY, BTN_DPAD_RIGHT);
> + input_set_capability(input, EV_KEY, BTN_DPAD_LEFT);
> + input_set_capability(input, EV_KEY, BTN_DPAD_DOWN);
> + } else {
> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
> + }
> +
> + return 0;
> +}
> +
> +static int gip_handle_navigation_report(struct gip_attachment *attachment,
> + struct input_dev *input, const uint8_t *bytes, int num_bytes)
> +{
> + if (num_bytes < 2) {
> + dev_dbg(GIP_DEV(attachment), "Discarding too-short input report\n");
> + return -EINVAL;
> + }
> +
> + input_report_key(input, BTN_START, bytes[0] & BIT(2));
> + input_report_key(input, BTN_SELECT, bytes[0] & BIT(3));
> + input_report_key(input, BTN_A, bytes[0] & BIT(4));
> + input_report_key(input, BTN_B, bytes[0] & BIT(5));
> + input_report_key(input, BTN_X, bytes[0] & BIT(6));
> + input_report_key(input, BTN_Y, bytes[0] & BIT(7));
> +
> + if (attachment->dpad_as_buttons) {
> + input_report_key(input, BTN_DPAD_UP, bytes[1] & BIT(0));
> + input_report_key(input, BTN_DPAD_DOWN, bytes[1] & BIT(1));
> + input_report_key(input, BTN_DPAD_LEFT, bytes[1] & BIT(2));
> + input_report_key(input, BTN_DPAD_RIGHT, bytes[1] & BIT(3));
> + } else {
> + input_report_abs(input, ABS_HAT0X,
> + !!(bytes[1] & BIT(3)) - !!(bytes[1] & BIT(2)));
> + input_report_abs(input, ABS_HAT0Y,
> + !!(bytes[1] & BIT(1)) - !!(bytes[1] & BIT(0)));
> + }
> +
> + if (attachment->quirks & GIP_QUIRK_SWAP_LB_RB) {
> + /* Previous */
> + input_report_key(input, BTN_TR, bytes[1] & BIT(4));
> + /* Next */
> + input_report_key(input, BTN_TL, bytes[1] & BIT(5));
> + } else {
> + input_report_key(input, BTN_TL, bytes[1] & BIT(4));
> + input_report_key(input, BTN_TR, bytes[1] & BIT(5));
> + }
> +
> + return 0;
> +}
> +
> +const struct gip_driver gip_driver_navigation = {
> + .types = (const char* const[]) { "Windows.Xbox.Input.NavigationController", NULL },
> + .guid = GUID_INIT(0xb8f31fe7, 0x7386, 0x40e9, 0xa9, 0xf8,
> + 0x2f, 0x21, 0x26, 0x3a, 0xcf, 0xb7),
> +
> + .probe = NULL,
> + .remove = NULL,
> + .init = NULL,
> + .setup_input = gip_setup_navigation_input,
> + .handle_input_report = gip_handle_navigation_report,
> +};
> +
> +module_param(dpad_as_buttons, bool, 0444);
> +MODULE_PARM_DESC(dpad_as_buttons, "Map the D-Pad as buttons instead of axes");
> diff --git a/drivers/input/joystick/gip/gip.h b/drivers/input/joystick/gip/gip.h
> new file mode 100644
> index 0000000000000..2c60430c81590
> --- /dev/null
> +++ b/drivers/input/joystick/gip/gip.h
> @@ -0,0 +1,309 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Gaming Input Protocol driver for Xbox One/Series controllers
> + *
> + * Copyright (c) 2025 Valve Software
> + *
> + * This driver is based on the Microsoft GIP spec at:
> + * https://aka.ms/gipdocs
> + * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gipusb/e7c90904-5e21-426e-b9ad-d82adeee0dbc
> + */
> +
> +#ifndef _GIP_H
> +#define _GIP_H
> +
> +#include <linux/rcupdate.h>
> +#include <linux/usb/input.h>
> +
> +#define BASE_GIP_MTU 64
> +#define MAX_GIP_MTU 2048
> +
> +#define MAX_ATTACHMENTS 8
> +
> +#define MAX_IN_MESSAGES 8
> +#define MAX_OUT_MESSAGES 8
> +
> +#define GIP_QUIRK_NO_HELLO BIT(0)
> +#define GIP_QUIRK_NO_IMPULSE_VIBRATION BIT(1)
> +#define GIP_QUIRK_SWAP_LB_RB BIT(2)
> +
> +#define GIP_FEATURE_CONTROLLER BIT(0)
> +#define GIP_FEATURE_CONSOLE_FUNCTION_MAP BIT(1)
> +#define GIP_FEATURE_CONSOLE_FUNCTION_MAP_OVERFLOW BIT(2)
> +#define GIP_FEATURE_ELITE_BUTTONS BIT(3)
> +#define GIP_FEATURE_DYNAMIC_LATENCY_INPUT BIT(4)
> +#define GIP_FEATURE_SECURITY_OPT_OUT BIT(5)
> +#define GIP_FEATURE_MOTOR_CONTROL BIT(6)
> +#define GIP_FEATURE_GUIDE_COLOR BIT(7)
> +#define GIP_FEATURE_EXTENDED_SET_DEVICE_STATE BIT(8)
> +
> +/* System messages */
> +#define GIP_CMD_PROTO_CONTROL 0x01
> +#define GIP_CMD_HELLO_DEVICE 0x02
> +#define GIP_CMD_STATUS_DEVICE 0x03
> +#define GIP_CMD_METADATA 0x04
> +#define GIP_CMD_SET_DEVICE_STATE 0x05
> +#define GIP_CMD_SECURITY 0x06
> +#define GIP_CMD_GUIDE_BUTTON 0x07
> +#define GIP_CMD_AUDIO_CONTROL 0x08
> +#define GIP_CMD_LED 0x0a
> +#define GIP_CMD_HID_REPORT 0x0b
> +#define GIP_CMD_FIRMWARE 0x0c
> +#define GIP_CMD_EXTENDED 0x1e
> +#define GIP_CMD_DEBUG 0x1f
> +#define GIP_AUDIO_DATA 0x60
> +
> +/* Navigation vendor messages */
> +#define GIP_CMD_DIRECT_MOTOR 0x09
> +#define GIP_LL_INPUT_REPORT 0x20
> +#define GIP_LL_OVERFLOW_INPUT_REPORT 0x26
> +
> +/* Wheel and ArcadeStick vendor messages */
> +#define GIP_CMD_INITIAL_REPORTS_REQUEST 0x0a
> +#define GIP_LL_STATIC_CONFIGURATION 0x21
> +#define GIP_LL_BUTTON_INFO_REPORT 0x22
> +
> +#define MAX_GIP_CMD 0x80
> +
> +#define GIP_DEV(p) \
> + _Generic((p), \
> + struct gip_attachment * : gip_attachment_dev, \
> + struct gip_interface * : gip_interface_dev, \
> + struct gip_device * : gip_device_dev)(p)
> +
> +enum gip_init_status {
> + GIP_INIT_OK = 0,
> + GIP_INIT_NO_INPUT = 1,
> +};
> +
> +enum gip_metadata_status {
> + GIP_METADATA_NONE = 0,
> + GIP_METADATA_GOT = 1,
> + GIP_METADATA_FAKED = 2,
> + GIP_METADATA_PENDING = 3,
> +};
> +
> +enum gip_elite_button_format {
> + GIP_BTN_FMT_UNKNOWN,
> + GIP_BTN_FMT_XBE1,
> + GIP_BTN_FMT_XBE2_RAW,
> + GIP_BTN_FMT_XBE2_4,
> + GIP_BTN_FMT_XBE2_5,
> +};
> +
> +struct gip_header {
> + uint8_t message_type;
> + uint8_t flags;
> + uint8_t sequence_id;
> + uint64_t length;
> +};
> +
> +struct gip_raw_message {
> + uint16_t num_bytes;
> + uint8_t bytes[BASE_GIP_MTU];
> +};
> +
> +struct gip_device_metadata {
> + uint8_t num_audio_formats;
> + uint8_t num_preferred_types;
> + uint8_t num_supported_interfaces;
> + uint8_t hid_descriptor_size;
> +
> + uint32_t in_system_messages[8];
> + uint32_t out_system_messages[8];
> +
> + struct gip_audio_format_pair *audio_formats;
> + char **preferred_types;
> + guid_t *supported_interfaces;
> + uint8_t *hid_descriptor;
> +};
> +
> +struct gip_message_metadata {
> + uint8_t type;
> + uint16_t length;
> + uint16_t data_type;
> + uint32_t flags;
> + uint16_t period;
> + uint16_t persistence_timeout;
> +};
> +
> +struct gip_metadata {
> + uint16_t version_major;
> + uint16_t version_minor;
> +
> + struct gip_device_metadata device;
> +
> + uint8_t num_messages;
> + struct gip_message_metadata *message_metadata;
> +};
> +
> +struct gip_status {
> + int power_level;
> + int charge;
> + int battery_type;
> + int battery_level;
> +};
> +
> +struct gip_status_event {
> + uint16_t event_type;
> + uint32_t fault_tag;
> + uint32_t fault_address;
> +};
> +
> +struct gip_extended_status {
> + struct gip_status base;
> + bool device_active;
> +
> + int num_events;
> + struct gip_status_event events[5];
> +};
> +
> +struct gip_attachment;
> +typedef int (*gip_command_handler)(struct gip_attachment *a, const struct gip_header *header,
> + const uint8_t *bytes, int num_bytes);
> +
> +struct gip_device;
> +struct gip_attachment {
> + const struct gip_driver *driver;
> + struct gip_device *device;
> + void *driver_data;
> + gip_command_handler vendor_handlers[MAX_GIP_CMD];
> +
> + uint8_t attachment_index;
> + struct input_dev __rcu *input;
> + uint16_t vendor_id;
> + uint16_t product_id;
> + char *uniq;
> + const char *name;
> + char phys[32];
> + char serial[32];
> + struct mutex lock;
> +
> + uint8_t fragment_message;
> + uint16_t total_length;
> + uint8_t *fragment_data;
> + uint32_t fragment_offset;
> + struct delayed_work fragment_timeout;
> + int fragment_retries;
> +
> + uint16_t firmware_major_version;
> + uint16_t firmware_minor_version;
> +
> + enum gip_metadata_status got_metadata;
> + struct delayed_work metadata_next;
> + int metadata_retries;
> + struct gip_metadata metadata;
> +
> + uint8_t seq_system;
> + uint8_t seq_security;
> + uint8_t seq_extended;
> + uint8_t seq_audio;
> + uint8_t seq_vendor;
> +
> + int device_state;
> +
> + struct gip_extended_status status;
> +
> + enum gip_elite_button_format xbe_format;
> + uint32_t features;
> + uint32_t quirks;
> +
> + int extra_buttons;
> + int extra_axes;
> +
> + bool dpad_as_buttons;
> + struct hid_device __rcu *hdev;
> +};
> +
> +struct gip_urb {
> + struct urb *urb;
> + uint8_t *data;
> + unsigned int offset;
> +};
> +
> +struct gip_interface {
> + struct gip_device *device;
> + struct usb_interface *intf;
> + uint32_t mtu;
> + int isoc_messages;
> +
> + struct urb *urb_in;
> + uint8_t *in_data;
> +
> + struct usb_anchor out_anchor;
> + struct gip_urb out_queue[MAX_OUT_MESSAGES];
> +};
> +
> +struct gip_device {
> + struct usb_device *udev;
> +
> + struct gip_interface data;
> + struct gip_interface audio;
> +
> + struct gip_raw_message in_queue[MAX_IN_MESSAGES];
> + int pending_in_messages;
> + int next_in_message;
> +
> + struct work_struct receive_message;
> + spinlock_t message_lock;
> +
> + struct gip_attachment *attachments[MAX_ATTACHMENTS];
> +};
> +
> +struct gip_quirks {
> + uint16_t vendor_id;
> + uint16_t product_id;
> + uint8_t attachment_index;
> + const char *override_name;
> + uint32_t added_features;
> + uint32_t filtered_features;
> + uint32_t quirks;
> + uint32_t extra_in_system[8];
> + uint32_t extra_out_system[8];
> + uint8_t extra_buttons;
> + uint8_t extra_axes;
> +};
> +
> +struct gip_driver {
> + const char *const *types;
> + guid_t guid;
> +
> + const struct gip_quirks *quirks;
> +
> + int (*probe)(struct gip_attachment *a);
> + void (*remove)(struct gip_attachment *a);
> + int (*init)(struct gip_attachment *a);
> + int (*setup_input)(struct gip_attachment *a, struct input_dev* input);
> + int (*handle_input_report)(struct gip_attachment *a,
> + struct input_dev* input, const uint8_t *bytes, int num_bytes);
These are two of the aforementioned stylistic errors. I have fixed all locations of this in the series.
> + gip_command_handler vendor_handlers[MAX_GIP_CMD];
> +};
> +
> +static inline struct device *gip_attachment_dev(struct gip_attachment *attachment)
> +{
> + return &attachment->device->udev->dev;
> +}
> +
> +static inline struct device *gip_interface_dev(struct gip_interface *intf)
> +{
> + return &intf->device->udev->dev;
> +}
> +
> +static inline struct device *gip_device_dev(struct gip_device *device)
> +{
> + return &device->udev->dev;
> +}
> +
> +bool gip_supports_vendor_message(struct gip_attachment *attachment, uint8_t command, bool upstream);
> +
> +int gip_send_system_message(struct gip_attachment *attachment,
> + uint8_t message_type, uint8_t flags, const void *bytes, int num_bytes);
> +int gip_send_vendor_message(struct gip_attachment *attachment,
> + uint8_t message_type, uint8_t flags, const void *bytes, int num_bytes);
> +
> +extern const struct gip_driver gip_driver_navigation;
> +extern const struct gip_driver gip_driver_gamepad;
> +extern const struct gip_driver gip_driver_arcade_stick;
> +extern const struct gip_driver gip_driver_wheel;
> +extern const struct gip_driver gip_driver_flight_stick;
> +#endif
Thanks,
Vicki
^ permalink raw reply
* Re: [PATCH 2/2] Input: Touchscreen: tsc200x - delegate wakeup IRQ management to I2C core
From: Dmitry Torokhov @ 2026-03-11 0:21 UTC (permalink / raw)
To: phucduc.bui
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ingo Molnar,
Thomas Gleixner, Marek Vasut, Michael Welling, linux-input,
devicetree, linux-kernel
In-Reply-To: <20260309110045.108209-3-phucduc.bui@gmail.com>
On Mon, Mar 09, 2026 at 06:00:44PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> The tsc200x driver supports both I2C (tsc2004) and SPI (tsc2005)
> interfaces.
> Currently, the driver attempts to manually manage the wakeup interrupt by
> calling enable_irq_wake() and disable_irq_wake() during suspend and resume.
>
> However, for I2C devices, the I2C core already automatically handles the
> wakeup source initialization and IRQ management if the "wakeup-source"
> property is present in the device tree. Manually managing it again in the
> driver is redundant and can lead to unbalanced IRQ wake reference counts.
>
> Clean up the wakeup IRQ handling by checking the bus type:
> - For I2C (BUS_I2C): Rely entirely on the I2C core for wakeup management.
> - For SPI (BUS_SPI): Explicitly call device_init_wakeup() in probe and
> manually manage enable/disable_irq_wake() during suspend/resume.
>
> The ts->wake_irq_enabled flag is also updated accordingly to ensure the
> driver accurately tracks the wakeup state across both buses.
>
> Note: This patch is based on code analysis of the I2C subsystem and
> has not been verified on actual hardware yet.
>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
> drivers/input/touchscreen/tsc200x-core.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
> index eba53613b005..d14d967845c8 100644
> --- a/drivers/input/touchscreen/tsc200x-core.c
> +++ b/drivers/input/touchscreen/tsc200x-core.c
> @@ -465,6 +465,7 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
> ts->idev = input_dev;
> ts->regmap = regmap;
> ts->tsc200x_cmd = tsc200x_cmd;
> + ts->bustype = tsc_id->bustype;
>
> error = device_property_read_u32(dev, "ti,x-plate-ohms", &x_plate_ohm);
> ts->x_plate_ohm = error ? TSC200X_DEF_RESISTOR : x_plate_ohm;
> @@ -547,8 +548,9 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
> return error;
> }
>
> - device_init_wakeup(dev,
> - device_property_read_bool(dev, "wakeup-source"));
> + if (ts->bustype == BUS_SPI)
> + device_init_wakeup(dev,
> + device_property_read_bool(dev, "wakeup-source"));
>
> return 0;
> }
> @@ -565,8 +567,13 @@ static int tsc200x_suspend(struct device *dev)
>
> ts->suspended = true;
>
> - if (device_may_wakeup(dev))
> - ts->wake_irq_enabled = enable_irq_wake(ts->irq) == 0;
> + if (device_may_wakeup(dev)) {
> + if (ts->bustype == BUS_SPI)
> + ts->wake_irq_enabled = enable_irq_wake(ts->irq) == 0;
> + else
> + ts->wake_irq_enabled = true;
> + } else
> + ts->wake_irq_enabled = false;
Sorry, but this just makes it all worse. There is no downside from
letting the driver to control wakeup if it wants to, so I'd rather leave
it as it was, at least for now.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 38/61] net: Prefer IS_ERR_OR_NULL over manual NULL check
From: Russell King (Oracle) @ 2026-03-11 0:16 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Igor Russkikh, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit
In-Reply-To: <20260310-b4-is_err_or_null-v1-38-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 12:49:04PM +0100, Philipp Hahn wrote:
> diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c
> index a8f91a4b7fed0927ee14e408000cd3a2bfb9b09a..09b30b563295c6085dc1358ac361301e5cf6b2a8 100644
> --- a/drivers/net/mdio/mdio-xgene.c
> +++ b/drivers/net/mdio/mdio-xgene.c
> @@ -265,7 +265,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr)
> struct phy_device *phy_dev;
>
> phy_dev = get_phy_device(bus, phy_addr, false);
> - if (!phy_dev || IS_ERR(phy_dev))
> + if (IS_ERR_OR_NULL(phy_dev))
As noted in reply to your cover message, the check for NULL here is
incorrect - get_phy_device() returns either a valid pointer or an
error pointer, but never NULL.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Russell King (Oracle) @ 2026-03-11 0:09 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Julia Lawall, Nicolas Palix, Chris Mason,
David Sterba, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Theodore Ts'o, Andreas Dilger, Steve French, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Miklos Szeredi,
Konstantin Komarov, Andreas Gruenbacher, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Jan Kara, Phillip Lougher, Alexander Viro,
Christian Brauner, Jan Kara, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Tejun Heo, David Vernet, Andrea Righi,
Changwoo Min, Ingo Molnar, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Ben Segall, Mel Gorman,
Valentin Schneider, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Sylwester Nawrocki, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Max Filippov,
Paolo Bonzini, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Alasdair Kergon, Mike Snitzer,
Mikulas Patocka, Benjamin Marzinski, David S. Miller, David Ahern,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Jamal Hadi Salim, Jiri Pirko,
Marcelo Ricardo Leitner, Xin Long, Trond Myklebust,
Anna Schumaker, Chuck Lever, Jeff Layton, NeilBrown,
Olga Kornievskaia, Dai Ngo, Jon Maloy, Johannes Berg,
Catalin Marinas, John Crispin, Thomas Bogendoerfer,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Zhenyu Wang,
Zhi Wang, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Alex Deucher, Christian König, Sandy Huang,
Heiko Stübner, Andy Yan, Igor Russkikh, Andrew Lunn,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
Marc Zyngier, Thomas Gleixner, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, Vinod Koul, Linus Walleij, Ulf Hansson,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Martin K. Petersen,
Eduardo Valentin, Keerthy, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Alex Williamson, Mark Greer,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Shuah Khan, Kieran Bingham, Mauro Carvalho Chehab, Joerg Roedel,
Will Deacon, Robin Murphy, Lee Jones, Pavel Machek, Dave Penkler,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Justin Sanders, Jens Axboe, Georgi Djakov, Michael Turquette,
Stephen Boyd, Philipp Zabel, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Pali Rohár, Dmitry Torokhov
In-Reply-To: <20260310-b4-is_err_or_null-v1-0-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 12:48:26PM +0100, Philipp Hahn wrote:
> While doing some static code analysis I stumbled over a common pattern,
> where IS_ERR() is combined with a NULL check. For that there is
> IS_ERR_OR_NULL().
One thing you need to check for each of these cases is whether the tests
are actually correct.
There are certainly cases amongst those that you have identified where
the check for NULL is redundant.
For example, get_phy_device() never returns NULL, yet in your netdev
patch, you have at least one instance where the return value of
get_phy_device() is checked for NULL and IS_ERR() which you then
turn into IS_ERR_OR_NULL(). Instead, the NULL check should be dropped
(as a separate patch.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH] Input: mpr121: Drop redundant wakeup handling
From: Dmitry Torokhov @ 2026-03-11 0:06 UTC (permalink / raw)
To: phucduc.bui; +Cc: linux-input, linux-kernel
In-Reply-To: <20260309071413.92709-1-phucduc.bui@gmail.com>
On Mon, Mar 09, 2026 at 02:14:13PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> The driver currently calls device_init_wakeup() and manually toggles
> IRQ wake in suspend and resume paths. This is unnecessary since the
> I2C core already handles wakeup configuration when the device is
> described in Device Tree with the "wakeup-source" property.
>
> Note: Compile-tested only, not verified on hardware.
>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: touchscreen: sitronix,st1232: Add wakeup-source
From: Dmitry Torokhov @ 2026-03-10 23:35 UTC (permalink / raw)
To: phucduc.bui
Cc: krzk+dt, geert+renesas, krzk, krzysztof.kozlowski, conor+dt,
devicetree, hechtb, javier.carrasco, jeff, linux-input,
linux-kernel, linux-renesas-soc, magnus.damm, robh, wsa+renesas
In-Reply-To: <20260309000319.74880-2-phucduc.bui@gmail.com>
On Mon, Mar 09, 2026 at 07:03:18AM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Document the 'wakeup-source' property for Sitronix ST1232 touchscreen
> controllers to allow the device to wake the system from suspend.
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 0/2] Input: st1232 - add system wakeup support
From: Wolfram Sang @ 2026-03-10 23:09 UTC (permalink / raw)
To: phucduc.bui
Cc: krzk+dt, geert+renesas, krzk, krzysztof.kozlowski, conor+dt,
devicetree, dmitry.torokhov, hechtb, javier.carrasco, jeff,
linux-input, linux-kernel, linux-renesas-soc, magnus.damm, robh
In-Reply-To: <20260309000319.74880-1-phucduc.bui@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 215 bytes --]
Hi,
> Demo video showing wakeup from suspend:
> https://youtu.be/POJhbguiA7A
Nice video! You really put some effort here, kudos.
Really awesome seeing Linux 7 on this old platform :)
Happy hacking,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 2/2] arm: dts: renesas: r8a7740-armadillo800eva: Add wakeup-source to st1232
From: Wolfram Sang @ 2026-03-10 23:08 UTC (permalink / raw)
To: phucduc.bui
Cc: krzk+dt, geert+renesas, krzk, krzysztof.kozlowski, conor+dt,
devicetree, dmitry.torokhov, hechtb, javier.carrasco, jeff,
linux-input, linux-kernel, linux-renesas-soc, magnus.damm, robh
In-Reply-To: <20260309000319.74880-3-phucduc.bui@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On Mon, Mar 09, 2026 at 07:03:19AM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Add the wakeup-source property to the ST1232 touchscreen node
> in the device tree so that the touchscreen interrupt can wake
> the system from suspend when the panel is touched.
>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] HID: input: Add HID_BATTERY_QUIRK_DYNAMIC for Elan touchscreens
From: Sebastian Reichel @ 2026-03-10 21:55 UTC (permalink / raw)
To: Hans de Goede
Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, linux-input,
ggrundik
In-Reply-To: <20260228145258.76937-2-johannes.goede@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 4606 bytes --]
Hi,
On Sat, Feb 28, 2026 at 03:52:58PM +0100, Hans de Goede wrote:
> Elan touchscreens have a HID-battery device for the stylus which is always
> there even if there is no stylus.
>
> This is causing upower to report an empty battery for the stylus and some
> desktop-environments will show a notification about this, which is quite
> annoying.
>
> Because of this the HID-battery is being ignored on all Elan I2c and USB
> touchscreens, but this causes there to be no battery reporting for
> the stylus at all.
>
> This adds a new HID_BATTERY_QUIRK_DYNAMIC and uses these for the Elan
> touchscreens.
>
> This new quirks causes the present value of the battery to start at 0,
> which will make userspace ignore it and only sets present to 1 after
> receiving a battery input report which only happens when the stylus
> gets in range.
>
> Reported-by: ggrundik@gmail.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221118
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> drivers/hid/hid-input.c | 14 +++++++++++---
> include/linux/hid.h | 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 67ca1e88ce13..8fc20df99b97 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -354,6 +354,7 @@ static enum power_supply_property hidinput_battery_props[] = {
> #define HID_BATTERY_QUIRK_FEATURE (1 << 1) /* ask for feature report */
> #define HID_BATTERY_QUIRK_IGNORE (1 << 2) /* completely ignore the battery */
> #define HID_BATTERY_QUIRK_AVOID_QUERY (1 << 3) /* do not query the battery */
> +#define HID_BATTERY_QUIRK_DYNAMIC (1 << 4) /* report present only after life signs */
>
> static const struct hid_device_id hid_battery_quirks[] = {
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
> @@ -398,8 +399,8 @@ static const struct hid_device_id hid_battery_quirks[] = {
> * Elan HID touchscreens seem to all report a non present battery,
> * set HID_BATTERY_QUIRK_IGNORE for all Elan I2C and USB HID devices.
> */
> - { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_IGNORE },
> - { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_IGNORE },
> + { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_DYNAMIC },
> + { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_DYNAMIC },
> {}
> };
>
> @@ -456,11 +457,14 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> int ret = 0;
>
> switch (prop) {
> - case POWER_SUPPLY_PROP_PRESENT:
> case POWER_SUPPLY_PROP_ONLINE:
> val->intval = 1;
> break;
>
> + case POWER_SUPPLY_PROP_PRESENT:
> + val->intval = dev->battery_present;
> + break;
> +
> case POWER_SUPPLY_PROP_CAPACITY:
> if (dev->battery_status != HID_BATTERY_REPORTED &&
> !dev->battery_avoid_query) {
> @@ -573,6 +577,8 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY)
> dev->battery_avoid_query = true;
>
> + dev->battery_present = (quirks & HID_BATTERY_QUIRK_DYNAMIC) ? false : true;
> +
> dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
> if (IS_ERR(dev->battery)) {
> error = PTR_ERR(dev->battery);
> @@ -628,6 +634,7 @@ static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> return;
>
> if (hidinput_update_battery_charge_status(dev, usage, value)) {
> + dev->battery_present = true;
> power_supply_changed(dev->battery);
> return;
> }
> @@ -643,6 +650,7 @@ static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> if (dev->battery_status != HID_BATTERY_REPORTED ||
> capacity != dev->battery_capacity ||
> ktime_after(ktime_get_coarse(), dev->battery_ratelimit_time)) {
> + dev->battery_present = true;
> dev->battery_capacity = capacity;
> dev->battery_status = HID_BATTERY_REPORTED;
> dev->battery_ratelimit_time =
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index dce862cafbbd..d9b54f0e8671 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -682,6 +682,7 @@ struct hid_device {
> __s32 battery_charge_status;
> enum hid_battery_status battery_status;
> bool battery_avoid_query;
> + bool battery_present;
> ktime_t battery_ratelimit_time;
> #endif
>
> --
> 2.52.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad
From: Andy Shevchenko @ 2026-03-10 19:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hugo Villeneuve, robin, andy, geert, robh, krzk+dt, conor+dt,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <abBkk4Ng-_MDHd6t@google.com>
On Tue, Mar 10, 2026 at 06:37:34PM +0000, Dmitry Torokhov wrote:
> On Fri, Mar 06, 2026 at 04:21:09PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 05, 2026 at 02:20:50PM -0500, Hugo Villeneuve wrote:
...
> > > + int oline;
> >
> > Why signed?
> >
> > > + int code;
> > > +
> > > + for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
> >
> > Can be like
> >
> > code = 0;
> > for (unsigned int oline = 0; oline < keypad->nlines; oline++) {
> >
> > as iterator is not used outside the loop.
> >
> > > + DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> >
> > > + int iline;
> >
> > Why signed?
>
> Does it make any difference given practical limits on nlines?
Maybe not, but might lead to interesting bugs in the future in case if used in
some arithmetics.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 60/61] Input alps: Drop unlikely() around IS_ERR_OR_NULL()
From: Pali Rohár @ 2026-03-10 18:55 UTC (permalink / raw)
To: Philipp Hahn; +Cc: linux-input, linux-kernel, Dmitry Torokhov
In-Reply-To: <20260310-b4-is_err_or_null-v1-60-bd63b656022d@avm.de>
On Tuesday 10 March 2026 12:49:26 Philipp Hahn wrote:
> IS_ERR_OR_NULL() already uses likely(!ptr) internally. checkpatch does
> not like nesting it:
> > WARNING: nested (un)?likely() calls, IS_ERR_OR_NULL already uses
> > unlikely() internally
> Remove the explicit use of unlikely().
>
> Change generated with coccinelle.
Hello, exactly same patch was sent to the list more times and was rejected.
The last attempt was here:
https://lore.kernel.org/linux-input/1570869407-41262-1-git-send-email-zhengbin13@huawei.com/t/#u
And the previous one with discussion and explanation is there:
https://lore.kernel.org/linux-input/1566298572-12409-2-git-send-email-info@metux.net/t/#u
> To: "Pali Rohár" <pali@kernel.org>
> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> drivers/input/mouse/alps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index f3d3b6b4e02d798e75a90333ace72a367befdbac..82e11efad7f7f02b4aaefde340f9b71fa792cf6b 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1482,7 +1482,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> /* On V2 devices the DualPoint Stick reports bare packets */
> dev = priv->dev2;
> dev2 = psmouse->dev;
> - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
> + } else if (IS_ERR_OR_NULL(priv->dev3)) {
> /* Register dev3 mouse if we received PS/2 packet first time */
> if (!IS_ERR(priv->dev3))
> psmouse_queue_work(psmouse, &priv->dev3_register_work,
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Kuan-Wei Chiu @ 2026-03-10 18:40 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs
In-Reply-To: <20260310-b4-is_err_or_null-v1-0-bd63b656022d@avm.de>
Hi Philipp,
On Tue, Mar 10, 2026 at 12:48:26PM +0100, Philipp Hahn wrote:
> While doing some static code analysis I stumbled over a common pattern,
> where IS_ERR() is combined with a NULL check. For that there is
> IS_ERR_OR_NULL().
>
> I've written a Coccinelle patch to find and patch those instances.
> The patches follow grouped by subsystem.
>
> Patches 55-58 may be dropped as they have a (minor?) semantic change:
> They use WARN_ON() or WARN_ON_ONCE(), but only in the IS_ERR() path, not
> for the NULL check. Iff it is okay to print the warning also for NULL,
> then the patches can be applied.
>
> While generating the patch set `checkpatch` complained about mixing
> [un]likely() with IS_ERR_OR_NULL(), which already uses likely()
> internally. I found and fixed several locations, where that combination
> has been used.
Thanks for the patchset. However, I think we need a explanation for why
switching to IS_ERR_OR_NULL() is an improvement over the existing code.
IMHO, the necessity of IS_ERR_OR_NULL() often highlights a confusing or
flawed API design. It usually implies that the caller is unsure whether
a failure results in an error pointer or a NULL pointer. Rather than
doing a treewide conversion of this pattern, I believe it would be much
more meaningful to review these instances case-by-case and fix the
underlying APIs or caller logic instead.
Additionally, a treewide refactoring like this has the practical
drawback of creating unnecessary merge conflicts when backporting to
stable trees.
Regards,
Kuan-Wei
^ permalink raw reply
* Re: [PATCH v4 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad
From: Dmitry Torokhov @ 2026-03-10 18:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hugo Villeneuve, robin, andy, geert, robh, krzk+dt, conor+dt,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <aari1Y1CPZSYEVj3@ashevche-desk.local>
On Fri, Mar 06, 2026 at 04:21:09PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 05, 2026 at 02:20:50PM -0500, Hugo Villeneuve wrote:
> > +static void charlieplex_keypad_poll(struct input_dev *input)
> > +{
> > + struct charlieplex_keypad *keypad = input_get_drvdata(input);
>
> > + int oline;
>
> Why signed?
>
> > + int code;
> > +
> > + for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
>
> Can be like
>
> code = 0;
> for (unsigned int oline = 0; oline < keypad->nlines; oline++) {
>
> as iterator is not used outside the loop.
>
> > + DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
>
> > + int iline;
>
> Why signed?
Does it make any difference given practical limits on nlines?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 03/61] ceph: Prefer IS_ERR_OR_NULL over manual NULL check
From: Viacheslav Dubeyko @ 2026-03-10 18:13 UTC (permalink / raw)
To: dm-devel@lists.linux.dev, phahn-oss@avm.de,
intel-wired-lan@lists.osuosl.org, linux-erofs@lists.ozlabs.org,
linux-security-module@vger.kernel.org, kvm@vger.kernel.org,
linux-sctp@vger.kernel.org, linux-pm@vger.kernel.org,
apparmor@lists.ubuntu.com, linux-ext4@vger.kernel.org,
amd-gfx@lists.freedesktop.org, linux-clk@vger.kernel.org,
linux-mips@vger.kernel.org, linux-media@vger.kernel.org,
netdev@vger.kernel.org, iommu@lists.linux.dev,
linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-usb@vger.kernel.org,
sched-ext@lists.linux.dev, linux-btrfs@vger.kernel.org,
linux-bluetooth@vger.kernel.org, linux-s390@vger.kernel.org,
samba-technical@lists.samba.org, intel-gfx@lists.freedesktop.org,
linux-trace-kernel@vger.kernel.org, ntfs3@lists.linux.dev,
linux-phy@lists.infradead.org, v9fs@lists.linux.dev,
ceph-devel@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
linux-mtd@lists.infradead.org, linux-scsi@vger.kernel.org,
target-devel@vger.kernel.org, linux-gpio@vger.kernel.org,
cocci@inria.fr, linux-sh@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-cifs@vger.kernel.org, linux-modules@vger.kernel.org,
linux-sound@vger.kernel.org, bpf@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org,
linux-leds@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org,
linux-nfs@vger.kernel.org, gfs2@lists.linux.dev,
linux-wireless@vger.kernel.org, linux-omap@vger.kernel.org
Cc: idryomov@gmail.com, Alex Markuze, slava@dubeyko.com
In-Reply-To: <20260310-b4-is_err_or_null-v1-3-bd63b656022d@avm.de>
On Tue, 2026-03-10 at 12:48 +0100, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Ilya Dryomov <idryomov@gmail.com>
> To: Alex Markuze <amarkuze@redhat.com>
> To: Viacheslav Dubeyko <slava@dubeyko.com>
> Cc: ceph-devel@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> fs/ceph/dir.c | 2 +-
> fs/ceph/snap.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 86d7aa594ea99335af3e91a95c0a418fdc1b8a8a..934250748ae4fd4c148fd27bdf91175047c2877d 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -889,7 +889,7 @@ int ceph_handle_notrace_create(struct inode *dir, struct dentry *dentry)
> {
> struct dentry *result = ceph_lookup(dir, dentry, 0);
>
> - if (result && !IS_ERR(result)) {
> + if (!IS_ERR_OR_NULL(result)) {
> /*
> * We created the item, then did a lookup, and found
> * it was already linked to another inode we already
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index 52b4c2684f922bfed39550311e793bfe3622cd26..528ad581be160713f91416115659e2dc6f259576 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -902,7 +902,7 @@ int ceph_update_snap_trace(struct ceph_mds_client *mdsc,
> bad:
> err = -EIO;
> fail:
> - if (realm && !IS_ERR(realm))
> + if (!IS_ERR_OR_NULL(realm))
> ceph_put_snap_realm(mdsc, realm);
> if (first_realm)
> ceph_put_snap_realm(mdsc, first_realm);
Looks good.
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Thanks,
Slava.
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Dmitry Torokhov @ 2026-03-10 17:29 UTC (permalink / raw)
To: Hendrik Noack
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ferass El Hafidi,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260307181557.66927-2-hendrik-noack@gmx.de>
Hi Hendrik,
On Sat, Mar 07, 2026 at 07:15:32PM +0100, Hendrik Noack wrote:
> Add bindings for Wacom W9002 and two Wacom W9007 variants which can be
> found in tablets.
>
> Co-developed-by: Ferass El Hafidi <funderscore@postmarketos.org>
> Signed-off-by: Ferass El Hafidi <funderscore@postmarketos.org>
> Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
> ---
> .../input/touchscreen/wacom,w9007a-lt03.yaml | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
> new file mode 100644
> index 000000000000..feb87f5db39d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/wacom,w9007a-lt03.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wacom W9000-series penabled I2C touchscreen
> +
> +maintainers:
> + - Hendrik Noack <hendrik-noack@gmx.de>
> +
> +description: |
> + The W9000-series are penabled touchscreen controllers by Wacom.
> +
> + The firmware of chips between devices can differ and with it also
> + how the chips behaves.
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - wacom,w9002
> + - wacom,w9007a-lt03
> + - wacom,w9007a-v1
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + vdd-supply:
> + description:
> + Optional regulator for the VDD digital voltage.
> +
> + flash-mode-gpios:
> + maxItems: 1
> + description:
> + Optional GPIO specifier for the touchscreen's flash-mode pin.
> +
> + pen-inserted-gpios:
> + maxItems: 1
> + description:
> + Optional GPIO specifier for the touchscreen's pen-insert pin.
Looking at this again this has nothing to do with the W9000 touchscreen
controller. The behavior is applicable to any device with a touchscreen
and a pen.
Rather this is a generic functionality/policy to put the pen
interface in low power mode when it is put away. I think this should be
done by userspace through combination to listening to the
SW_PEN_INSERTED events (via gpio-keys driver) and toggling "inhibit" on
the touchscreen device.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Ferass El Hafidi @ 2026-03-10 17:08 UTC (permalink / raw)
To: Krzysztof Kozlowski, Hendrik Noack
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
In-Reply-To: <0200ee01-b14c-4fec-bbbb-b2a3c0f140f2@kernel.org>
On Tue, 10 Mar 2026 16:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>On 10/03/2026 17:50, Ferass El Hafidi wrote:
>> Hi Krzysztof & Hendrik,
>>
>> On Mon, 09 Mar 2026 13:21, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> On 09/03/2026 14:12, Hendrik Noack wrote:
>>>> Hello Krzysztof,
>>>>
>>>> 09.03.2026 13:56:41 Krzysztof Kozlowski <krzk@kernel.org>:
>>>>
>>>>> On 09/03/2026 13:54, Hendrik Noack wrote:
>>>>>> Hello Krzysztof,
>>>>>>
>>>>>> 08.03.2026 10:15:35 Krzysztof Kozlowski <krzk@kernel.org>:
>>>>>>
>>>>>>> You received review and instruction what to do. Did you read it?
>>>>>>
>>>>>> I read the review of Dmitry and incorporated it into this version.
>>>>>
>>>>> So you ignored my email completely or it did not reach you (it is on
>>>>> lore.kernel.org though)?
>>>>
>>>> I don't know what email you mean. You gave reviews on my first verison,
>>>> which I already incorporated in v2 and then gave a review-by on v2,
>>>> which I also added on v3, but now dropped, because I added a property
>>>> to the DT binding.
>>>
>>> The instruction I gave you when giving review.
>>
>> If you gave review on the v3 dt-bindings, I can't find it on lore:
>> https://lore.kernel.org/all/20251205152858.14415-2-hendrik-noack@gmx.de/
>>
>> On v2 you gave a R-b to the dt-bindings along with some instructions:
>> https://lore.kernel.org/all/2bf9dbd4-351e-4a79-9fcf-e41c5273d0be@kernel.org/
>> which were followed from what I can tell; your R-b was retained on v3,
>> but dropped on this revision (v4) because of new properties being added
>> (new `reset-gpio` property, technically also `wacom,w9002` compatible
>> but my understanding is new compatibles don't really matter much), as
>> your email said "Please add Acked-by/Reviewed-by/Tested-by tags when
>> posting new versions [...] unless patch changed (e.g. new properties
>> added to the DT bindings)".
>
>Thanks. I also provided the link further explaining what one has to do
>if one ever decide to drop the review.
Assuming you're talking about that R-b dropping should be mentioned in
the cover letter, that was probably just overlooked.
>
>Dropping it silently is for me rather sign I should ignore this patchset
>to avoid wasting time of lost reviews.
That was most likely not the intent, I think your feedback is valuable.
Best regards,
Ferass
[PS: I see you left some review after your last email, thank you for
your time and sorry for the inconvenience.]
>
>Best regards,
>Krzysztof
^ permalink raw reply
* Re: [6.18.] ThinkPad T14 Gen 2 AMD (LEN2073) - Synaptics touchpad remains PS/2, intertouch=1 ineffective, lost sync events
From: Rácz Máté @ 2026-03-10 17:08 UTC (permalink / raw)
To: Thorsten Leemhuis; +Cc: linux-input, linux-i2c, Linux kernel regressions list
In-Reply-To: <CANkKV91attPiZ4+6DQKcgG10apUbF6k5hs7aJBrfsnfc5jQCew@mail.gmail.com>
Hi Thorsten and all,
I tested the issue on the latest mainline kernel release candidate 7.0-rc3:
1. Built and booted 7.0-rc3 without any additional patches:
- The touchpad (LEN2073) still exhibits the same "lost sync" behaviour.
2. Applied a local patch adding LEN2073 to the Synaptics SMBus
whitelist in drivers/input/mouse/synaptics.c:
- Rebuilt and booted the kernel with this patch.
- Behaviour did not change; the touchpad still does not work properly.
- The patch simply adds the LEN2073 PNP ID to the SMBus whitelist.
Patch content (for reference):
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -191,6 +191,7 @@ static const char * const smbus_pnp_ids[] = {
"LEN2054", /* E480 */
"LEN2055", /* E580 */
"LEN2068", /* T14 Gen 1 */
+ "LEN2073", /* T14 Gen 2 */
"SYN1221", /* TUXEDO InfinityBook Pro 14 v5 */
"SYN3003", /* HP EliteBook 850 G1 */
"SYN3015", /* HP EliteBook 840 G2 */
Additional observations:
- psmouse is compiled as builtin.
- Verified both with and without the `psmouse.synaptics_intertouch=1`
boot parameter.
- I2C devices are visible under /sys/bus/i2c/devices/.
- dmesg output shows:
[ 2.288744] psmouse serio1: synaptics: Touchpad model: 1, fw:
10.32, id: 0x1e2a1, caps: 0xf014a3/0x940300/0x12e800/0x500000, board
id: 3471, fw id: 2909640
[ 2.331172] input: SynPS/2 Synaptics TouchPad as
/devices/platform/i8042/serio1/input/input6
[ 252.979513] psmouse serio1: TouchPad at isa0060/serio1/input0 lost
sync at byte 1
[ 252.980606] psmouse serio1: TouchPad at isa0060/serio1/input0 lost
sync at byte 1
[ 252.981855] psmouse serio1: TouchPad at isa0060/serio1/input0 lost
sync at byte 1
[ 253.008752] psmouse serio1: TouchPad at isa0060/serio1/input0 lost
sync at byte 1
[ 253.009947] psmouse serio1: TouchPad at isa0060/serio1/input0 lost
sync at byte 1
- The trackpoint was previously disabled in BIOS; after enabling it,
the touchpad behaves slightly better, but the "lost sync" issue
persists and PS/2 fallback is still active.
It seems that on this hardware, adding the device to the whitelist
alone does not enable proper I2C/SMBus operation, and the device does
not function as a Synaptics touchpad under Linux.
Best regards,
Mate
^ permalink raw reply
* Re: [PATCH v2 0/7] HID: asus: increase robustness of the driver
From: Jiri Kosina @ 2026-03-10 17:04 UTC (permalink / raw)
To: Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Luke D . Jones,
Mateusz Schyboll, Denis Benato
In-Reply-To: <20260228191010.3830758-1-denis.benato@linux.dev>
On Sat, 28 Feb 2026, Denis Benato wrote:
> Hi all,
>
> Previous asus-wmi maintainer and asus-linux developer has become less
> active in the project and left me in charge of advancing the support
> for ASUS equipement on Linux.
>
> I am preparing to send a patchset of his revised work to support ASUS
> ROG Ally handhelds devices and since that work is also useful in
> expanding support for 2025 models it is important to improve the
> hid-asus driver as future patches will build on top of these changes.
Patches 1, 4, 5, 6 and 7 applied to hid.git#for-7.1/asus.
Patch 2 applied to hid.git#for-7.0/upstream-fixes.
Patch 3 dropped, as it's fixed by 7b2f88cc9dd4c2b9f3d in
hid.git#for-7.1/asus.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Krzysztof Kozlowski @ 2026-03-10 16:58 UTC (permalink / raw)
To: Hendrik Noack, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Ferass El Hafidi, linux-input, devicetree, linux-kernel
In-Reply-To: <20260307181557.66927-2-hendrik-noack@gmx.de>
On 07/03/2026 19:15, Hendrik Noack wrote:
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + vdd-supply:
> + description:
> + Optional regulator for the VDD digital voltage.
Drop "Optional regulator for"
Also, vdd is mandatory, not optional. How device can work without supply?
> +
> + flash-mode-gpios:
> + maxItems: 1
> + description:
> + Optional GPIO specifier for the touchscreen's flash-mode pin.
Drop "Optional GPIO specifier for the". Don't repeat constraints in free
form text. Schema tells what is optional or what is not. This also
cannot be anything else than GPIO specifier, so repeating obvious is not
making it more readable.
> +
> + pen-inserted-gpios:
> + maxItems: 1
> + description:
> + Optional GPIO specifier for the touchscreen's pen-insert pin.
Drop "Optional GPIO specifier for the".
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Optional GPIO specifier for the touchscreen's reset pin.
Drop description completely. Redundant. It cannot be anything else than
reset pin.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - vdd-supply
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 00/19] HID: Add Legion Go and Go S Drivers
From: Jiri Kosina @ 2026-03-10 16:55 UTC (permalink / raw)
To: Derek J. Clark
Cc: Benjamin Tissoires, Richard Hughes, Mario Limonciello,
Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais,
linux-input, linux-doc, linux-kernel
In-Reply-To: <20260310072937.3295875-1-derekjohn.clark@gmail.com>
On Tue, 10 Mar 2026, Derek J. Clark wrote:
> Change Log
> v6:
> - Include multiple bug fixes from Ethan Tidmore.
> - Make all local attributes static.
> - Invert the rgb_speed logic for the go driver. On the Go this
> attribute sets a delay, so flip the logic to match Go S where the
> larger number means faster so userspace can target this consistently.
> - Include 3 new patches that fix formatting issues with v5 authored by
> additional developers.
I've just finished going through the patchdiff and all the reports, and it
seems that you've indeed addressed all of it.
This is now in hid.git#for-7.1/lenovo-v2.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Krzysztof Kozlowski @ 2026-03-10 16:55 UTC (permalink / raw)
To: Ferass El Hafidi, Hendrik Noack
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
In-Reply-To: <tbp04l.1q4xkmdkoohga@postmarketos.org>
On 10/03/2026 17:50, Ferass El Hafidi wrote:
> Hi Krzysztof & Hendrik,
>
> On Mon, 09 Mar 2026 13:21, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> On 09/03/2026 14:12, Hendrik Noack wrote:
>>> Hello Krzysztof,
>>>
>>> 09.03.2026 13:56:41 Krzysztof Kozlowski <krzk@kernel.org>:
>>>
>>>> On 09/03/2026 13:54, Hendrik Noack wrote:
>>>>> Hello Krzysztof,
>>>>>
>>>>> 08.03.2026 10:15:35 Krzysztof Kozlowski <krzk@kernel.org>:
>>>>>
>>>>>> You received review and instruction what to do. Did you read it?
>>>>>
>>>>> I read the review of Dmitry and incorporated it into this version.
>>>>
>>>> So you ignored my email completely or it did not reach you (it is on
>>>> lore.kernel.org though)?
>>>
>>> I don't know what email you mean. You gave reviews on my first verison,
>>> which I already incorporated in v2 and then gave a review-by on v2,
>>> which I also added on v3, but now dropped, because I added a property
>>> to the DT binding.
>>
>> The instruction I gave you when giving review.
>
> If you gave review on the v3 dt-bindings, I can't find it on lore:
> https://lore.kernel.org/all/20251205152858.14415-2-hendrik-noack@gmx.de/
>
> On v2 you gave a R-b to the dt-bindings along with some instructions:
> https://lore.kernel.org/all/2bf9dbd4-351e-4a79-9fcf-e41c5273d0be@kernel.org/
> which were followed from what I can tell; your R-b was retained on v3,
> but dropped on this revision (v4) because of new properties being added
> (new `reset-gpio` property, technically also `wacom,w9002` compatible
> but my understanding is new compatibles don't really matter much), as
> your email said "Please add Acked-by/Reviewed-by/Tested-by tags when
> posting new versions [...] unless patch changed (e.g. new properties
> added to the DT bindings)".
Thanks. I also provided the link further explaining what one has to do
if one ever decide to drop the review.
Dropping it silently is for me rather sign I should ignore this patchset
to avoid wasting time of lost reviews.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Ferass El Hafidi @ 2026-03-10 16:50 UTC (permalink / raw)
To: Krzysztof Kozlowski, Hendrik Noack
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Ferass El Hafidi, linux-input, devicetree, linux-kernel
In-Reply-To: <21aec29b-595d-4889-a71e-abe9e5ce834f@kernel.org>
Hi Krzysztof & Hendrik,
On Mon, 09 Mar 2026 13:21, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>On 09/03/2026 14:12, Hendrik Noack wrote:
>> Hello Krzysztof,
>>
>> 09.03.2026 13:56:41 Krzysztof Kozlowski <krzk@kernel.org>:
>>
>>> On 09/03/2026 13:54, Hendrik Noack wrote:
>>>> Hello Krzysztof,
>>>>
>>>> 08.03.2026 10:15:35 Krzysztof Kozlowski <krzk@kernel.org>:
>>>>
>>>>> You received review and instruction what to do. Did you read it?
>>>>
>>>> I read the review of Dmitry and incorporated it into this version.
>>>
>>> So you ignored my email completely or it did not reach you (it is on
>>> lore.kernel.org though)?
>>
>> I don't know what email you mean. You gave reviews on my first verison,
>> which I already incorporated in v2 and then gave a review-by on v2,
>> which I also added on v3, but now dropped, because I added a property
>> to the DT binding.
>
>The instruction I gave you when giving review.
If you gave review on the v3 dt-bindings, I can't find it on lore:
https://lore.kernel.org/all/20251205152858.14415-2-hendrik-noack@gmx.de/
On v2 you gave a R-b to the dt-bindings along with some instructions:
https://lore.kernel.org/all/2bf9dbd4-351e-4a79-9fcf-e41c5273d0be@kernel.org/
which were followed from what I can tell; your R-b was retained on v3,
but dropped on this revision (v4) because of new properties being added
(new `reset-gpio` property, technically also `wacom,w9002` compatible
but my understanding is new compatibles don't really matter much), as
your email said "Please add Acked-by/Reviewed-by/Tested-by tags when
posting new versions [...] unless patch changed (e.g. new properties
added to the DT bindings)".
You also gave an in-depth review on the actual driver itself on v1:
https://lore.kernel.org/all/20251028-funky-rose-rook-3ccab5@kuoka/
Best regards,
Ferass
>
>Please wrap your emails to standard email style, so reading will be easier.
>
>Best regards,
>Krzysztof
^ permalink raw reply
* Re: [PATCH 01/61] Coccinelle: Prefer IS_ERR_OR_NULL over manual NULL check
From: Markus Elfring @ 2026-03-10 15:41 UTC (permalink / raw)
To: Philipp Hahn, cocci, Julia Lawall
Cc: amd-gfx, apparmor, bpf, ceph-devel, dm-devel, dri-devel, gfs2,
intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, nicolas.palix, ntfs3, samba-technical, sched-ext,
target-devel, tipc-discussion, v9fs
In-Reply-To: <20260310-b4-is_err_or_null-v1-1-bd63b656022d@avm.de>
> Find and convert uses of IS_ERR() plus NULL check to IS_ERR_OR_NULL().
…
Can this information trigger any more consequences on corresponding summary phrases?
…
> +++ b/scripts/coccinelle/api/is_err_or_null.cocci
> @@ -0,0 +1,125 @@
…
> +virtual patch
> +virtual report
> +virtual org
How will interests evolve further for the support of the operation mode “context”?
> +@p1 depends on patch@
> +expression E;
> +@@
> +(
> +- E != NULL && !IS_ERR(E)
> ++ !IS_ERR_OR_NULL(E)
> +|
> +- E == NULL || IS_ERR(E)
> ++ IS_ERR_OR_NULL(E)
> +|
> +- !IS_ERR(E) && E != NULL
> ++ !IS_ERR_OR_NULL(E)
> +|
> +- IS_ERR(E) || E == NULL
> ++ IS_ERR_OR_NULL(E)
> +)
Did you eventually check probabilities for the occurrence of mentioned case distinctions?
> +@p2 depends on patch@
…
I suggest to reconsider “side effects” according to the splitting of these SmPL rules
once more.
…
> +@r2 depends on report || org@
> +identifier I;
> +expression E;
> +position p;
> +@@
> +(
> +* (I = E) != NULL && ... && !IS_ERR@p(I)
> +|
> +* (I = E) == NULL || ... || IS_ERR@p(I)
> +)
I doubt that the usage of SmPL asterisks fits to these two operation modes.
…
> +@p5 depends on patch disable unlikely @
> +expression E;
> +@@
> +-\( likely \| unlikely \)(
> +(
> + IS_ERR_OR_NULL(E)
> +|
> + !IS_ERR_OR_NULL(E)
> +)
> +-)
* Would it be nicer to move such SmPL code to the end of the patch rule listing?
* Can this source code search pattern matter also for further operation modes?
Regards,
Markus
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 38/61] net: Prefer IS_ERR_OR_NULL over manual NULL check
From: Elad Nachman @ 2026-03-10 15:07 UTC (permalink / raw)
To: Philipp Hahn, amd-gfx@lists.freedesktop.org,
apparmor@lists.ubuntu.com, bpf@vger.kernel.org,
ceph-devel@vger.kernel.org, cocci@inria.fr,
dm-devel@lists.linux.dev, dri-devel@lists.freedesktop.org,
gfs2@lists.linux.dev, intel-gfx@lists.freedesktop.org,
intel-wired-lan@lists.osuosl.org, iommu@lists.linux.dev,
kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-block@vger.kernel.org, linux-bluetooth@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-cifs@vger.kernel.org,
linux-clk@vger.kernel.org, linux-erofs@lists.ozlabs.org,
linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-leds@vger.kernel.org, linux-media@vger.kernel.org,
linux-mips@vger.kernel.org, linux-mm@kvack.org,
linux-modules@vger.kernel.org, linux-mtd@lists.infradead.org,
linux-nfs@vger.kernel.org, linux-omap@vger.kernel.org,
linux-phy@lists.infradead.org, linux-pm@vger.kernel.org,
linux-rockchip@lists.infradead.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-sctp@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-sh@vger.kernel.org,
linux-sound@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-trace-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
ntfs3@lists.linux.dev, samba-technical@lists.samba.org,
sched-ext@lists.linux.dev, target-devel@vger.kernel.org,
tipc-discussion@lists.sourceforge.net, v9fs@lists.linux.dev
Cc: Igor Russkikh, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Pavan Chebbi, Michael Chan,
Potnuri Bharat Teja, Tony Nguyen, Przemek Kitszel, Taras Chornyi,
Maxime Coquelin, Alexandre Torgue, Iyappan Subramanian,
Keyur Chudgar, Quan Nguyen, Heiner Kallweit, Russell King
In-Reply-To: <20260310-b4-is_err_or_null-v1-38-bd63b656022d@avm.de>
>
>
> From: Philipp Hahn <phahn-oss@avm.de>
> Sent: Tuesday, March 10, 2026 1:49 PM
> To: amd-gfx@lists.freedesktop.org; apparmor@lists.ubuntu.com; bpf@vger.kernel.org; ceph-devel@vger.kernel.org; cocci@inria.fr; dm-devel@lists.linux.dev; dri-devel@lists.freedesktop.org; gfs2@lists.linux.dev; intel-gfx@lists.freedesktop.org; intel-wired-lan@lists.osuosl.org; iommu@lists.linux.dev; kvm@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-block@vger.kernel.org; linux-bluetooth@vger.kernel.org; linux-btrfs@vger.kernel.org; linux-cifs@vger.kernel.org; linux-clk@vger.kernel.org; linux-erofs@lists.ozlabs.org; linux-ext4@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-gpio@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; linux-leds@vger.kernel.org; linux-media@vger.kernel.org; linux-mips@vger.kernel.org; linux-mm@kvack.org; linux-modules@vger.kernel.org; linux-mtd@lists.infradead.org; linux-nfs@vger.kernel.org; linux-omap@vger.kernel.org; linux-phy@lists.infradead.org; linux-pm@vger.kernel.org; linux-rockchip@lists.infradead.org; linux-s390@vger.kernel.org; linux-scsi@vger.kernel.org; linux-sctp@vger.kernel.org; linux-security-module@vger.kernel.org; linux-sh@vger.kernel.org; linux-sound@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-trace-kernel@vger.kernel.org; linux-usb@vger.kernel.org; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; ntfs3@lists.linux.dev; samba-technical@lists.samba.org; sched-ext@lists.linux.dev; target-devel@vger.kernel.org; tipc-discussion@lists.sourceforge.net; v9fs@lists.linux.dev; Philipp Hahn <phahn-oss@avm.de>
> Cc: Igor Russkikh <irusskikh@marvell.com>; Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Pavan Chebbi <pavan.chebbi@broadcom.com>; Michael Chan <mchan@broadcom.com>; Potnuri Bharat Teja <bharat@chelsio.com>; Tony Nguyen <anthony.l.nguyen@intel.com>; Przemek Kitszel <przemyslaw.kitszel@intel.com>; Taras Chornyi <taras.chornyi@plvision.eu>; Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue <alexandre.torgue@foss.st.com>; Iyappan Subramanian <iyappan@os.amperecomputing.com>; Keyur Chudgar <keyur@os.amperecomputing.com>; Quan Nguyen <quan@os.amperecomputing.com>; Heiner Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>
> Subject: [EXTERNAL] [PATCH 38/61] net: Prefer IS_ERR_OR_NULL over manual NULL check
> ZjQcmQRYFpfptBannerEnd
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Igor Russkikh <mailto:irusskikh@marvell.com>
> To: Andrew Lunn <mailto:andrew+netdev@lunn.ch>
> To: "David S. Miller" <mailto:davem@davemloft.net>
> To: Eric Dumazet <mailto:edumazet@google.com>
> To: Jakub Kicinski <mailto:kuba@kernel.org>
> To: Paolo Abeni <mailto:pabeni@redhat.com>
> To: Pavan Chebbi <mailto:pavan.chebbi@broadcom.com>
> To: Michael Chan <mailto:mchan@broadcom.com>
> To: Potnuri Bharat Teja <mailto:bharat@chelsio.com>
> To: Tony Nguyen <mailto:anthony.l.nguyen@intel.com>
> To: Przemek Kitszel <mailto:przemyslaw.kitszel@intel.com>
> To: Taras Chornyi <mailto:taras.chornyi@plvision.eu>
> To: Maxime Coquelin <mailto:mcoquelin.stm32@gmail.com>
> To: Alexandre Torgue <mailto:alexandre.torgue@foss.st.com>
> To: Iyappan Subramanian <mailto:iyappan@os.amperecomputing.com>
> To: Keyur Chudgar <mailto:keyur@os.amperecomputing.com>
> To: Quan Nguyen <mailto:quan@os.amperecomputing.com>
> To: Heiner Kallweit <mailto:hkallweit1@gmail.com>
> To: Russell King <mailto:linux@armlinux.org.uk>
> Cc: mailto:netdev@vger.kernel.org
> Cc: mailto:linux-kernel@vger.kernel.org
> Cc: mailto:intel-wired-lan@lists.osuosl.org
> Cc: mailto:linux-stm32@st-md-mailman.stormreply.com
> Cc: mailto:linux-arm-kernel@lists.infradead.org
> Cc: mailto:linux-usb@vger.kernel.org
> Signed-off-by: Philipp Hahn <mailto:phahn-oss@avm.de>
> ---
> drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
> drivers/net/ethernet/broadcom/tg3.c | 2 +-
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 3 +--
> drivers/net/ethernet/intel/ice/devlink/devlink.c | 2 +-
> drivers/net/ethernet/marvell/prestera/prestera_router.c | 2 +-
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> drivers/net/mdio/mdio-xgene.c | 2 +-
> drivers/net/usb/r8152.c | 2 +-
> 8 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index e270327e47fd804cc8ee5cfd53ed1b993c955c41..43edef35c4b1ff606b2f1519a07fad4c9a990ad4 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> @@ -810,7 +810,7 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
> }
>
> skb = aq_xdp_run_prog(aq_nic, &xdp, rx_ring, buff);
> - if (IS_ERR(skb) || !skb)
> + if (IS_ERR_OR_NULL(skb))
> continue;
>
> if (ptp_hwtstamp_len > 0)
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 2328fce336447eb4a796f9300ccc0ab536ff0a35..8ed79f34f03d81184dcc12e6eaff009cb8f7756e 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -7943,7 +7943,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
>
> segs = skb_gso_segment(skb, tp->dev->features &
> ~(NETIF_F_TSO | NETIF_F_TSO6));
> - if (IS_ERR(segs) || !segs) {
> + if (IS_ERR_OR_NULL(segs)) {
> tnapi->tx_dropped++;
> goto tg3_tso_bug_end;
> }
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> index 3307e50426819087ad985178c4a5383f16b8e7b4..1c8a6445d4b2e3535d8f1b7908dd02d8dd2f23fa 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> @@ -1032,8 +1032,7 @@ static void ch_flower_stats_handler(struct work_struct *work)
> do {
> rhashtable_walk_start(&iter);
>
> - while ((flower_entry = rhashtable_walk_next(&iter)) &&
> - !IS_ERR(flower_entry)) {
> + while (!IS_ERR_OR_NULL((flower_entry = rhashtable_walk_next(&iter)))) {
> ret = cxgb4_get_filter_counters(adap->port[0],
> flower_entry->filter_id,
> &packets, &bytes,
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> index 6c72bd15db6d75a1d4fa04ef8fefbd26fb6e84bd..3d08b9187fd76ca3198af28111b6f1c1765ea01e 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> @@ -791,7 +791,7 @@ static void ice_traverse_tx_tree(struct devlink *devlink, struct ice_sched_node
> node->parent->rate_node);
> }
>
> - if (rate_node && !IS_ERR(rate_node))
> + if (!IS_ERR_OR_NULL(rate_node))
> node->rate_node = rate_node;
>
> traverse_children:
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index b036b173a308b5f994ad8538eb010fa27196988c..4492938e8a3da91d32efe8d45ccbe2eb437c0e49 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -1061,7 +1061,7 @@ static void __prestera_k_arb_hw_state_upd(struct prestera_switch *sw,
> n = NULL;
> }
>
> - if (!IS_ERR(n) && n) {
> + if (!IS_ERR_OR_NULL(n)) {
> neigh_event_send(n, NULL);
> neigh_release(n);
> } else {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6827c99bde8c22db42b363d2d36ad6f26075ed50..356a4e9ce04b1fcf8786d7274d31ace404be2cf6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1275,7 +1275,7 @@ static int stmmac_init_phy(struct net_device *dev)
> /* Some DT bindings do not set-up the PHY handle. Let's try to
> * manually parse it
> */
> - if (!phy_fwnode || IS_ERR(phy_fwnode)) {
> + if (IS_ERR_OR_NULL(phy_fwnode)) {
> int addr = priv->plat->phy_addr;
> struct phy_device *phydev;
>
> diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c
> index a8f91a4b7fed0927ee14e408000cd3a2bfb9b09a..09b30b563295c6085dc1358ac361301e5cf6b2a8 100644
> --- a/drivers/net/mdio/mdio-xgene.c
> +++ b/drivers/net/mdio/mdio-xgene.c
> @@ -265,7 +265,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr)
> struct phy_device *phy_dev;
>
> phy_dev = get_phy_device(bus, phy_addr, false);
> - if (!phy_dev || IS_ERR(phy_dev))
> + if (IS_ERR_OR_NULL(phy_dev))
> return NULL;
>
> if (phy_device_register(phy_dev))
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0c83bbbea2e7c322ee6339893e281237663bd3ae..73f17ebd7d40007eec5004f887a46249defd28ab 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -2218,7 +2218,7 @@ static void r8152_csum_workaround(struct r8152 *tp, struct sk_buff *skb,
>
> features &= ~(NETIF_F_SG | NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
> segs = skb_gso_segment(skb, features);
> - if (IS_ERR(segs) || !segs)
> + if (IS_ERR_OR_NULL(segs))
> goto drop;
>
> __skb_queue_head_init(&seg_list);
>
> --
> 2.43.0
>
>
Acked-by: Elad Nachman <enachman@marvell.com>
^ permalink raw reply
* Re: [PATCH 17/61] module: Prefer IS_ERR_OR_NULL over manual NULL check
From: Aaron Tomlin @ 2026-03-10 14:45 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen
In-Reply-To: <20260310-b4-is_err_or_null-v1-17-bd63b656022d@avm.de>
[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]
On Tue, Mar 10, 2026 at 12:48:43PM +0100, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Luis Chamberlain <mcgrof@kernel.org>
> To: Petr Pavlu <petr.pavlu@suse.com>
> To: Daniel Gomez <da.gomez@kernel.org>
> To: Sami Tolvanen <samitolvanen@google.com>
> To: Aaron Tomlin <atomlin@atomlin.com>
> Cc: linux-modules@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> kernel/module/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c3ce106c70af165e2dc1a3c79f5a074a5c3e3d34..7f62f0620dcd75960e431f7af3d1cadf4cc41e4b 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1551,7 +1551,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
> case SHN_UNDEF:
> ksym = resolve_symbol_wait(mod, info, name);
> /* Ok if resolved. */
> - if (ksym && !IS_ERR(ksym)) {
> + if (!IS_ERR_OR_NULL(ksym)) {
> sym[i].st_value = kernel_symbol_value(ksym);
> break;
> }
>
> --
> 2.43.0
>
Hi Philipp,
Thank you.
Have you considered other users of IS_ERR() in kernel/module/main.c too?
Perhaps it might be best to prepare a clean up for each applicable
subsystem in isolation.
Kind regards,
--
Aaron Tomlin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox