Re: [PATCH v5] HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)

On Wed, 2026-07-01 at 10:37 +0300, DevExalt wrote:
> From: "Baraa Atta (Dev Exalt)" <[email protected]>
> 
<snip>
> +
> +#define HIDPP_MULTIPLATFORM_FEAT_ID			0x4531
> +#define HIDPP_MULTIPLATFORM_GET_FEATURE_INFO		0x00
> +#define HIDPP_MULTIPLATFORM_GET_PLATFORM_DESCRIPTOR	0x10
> +#define HIDPP_MULTIPLATFORM_SET_CURRENT_PLATFORM	0x30
> +
> +#define
> HIDPP_MULTIPLATFORM_PLATFORM_MASK_TIZEN		BIT(0)
> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINDOWS	BIT(8)
> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINEMB	BIT(9)
> +#define
> HIDPP_MULTIPLATFORM_PLATFORM_MASK_LINUX		BIT(10)
> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_CHROME	BIT(11)
> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_ANDROID	BIT(12)
> +#define
> HIDPP_MULTIPLATFORM_PLATFORM_MASK_MACOS		BIT(13)
> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_IOS		BIT(14)
> +#define
> HIDPP_MULTIPLATFORM_PLATFORM_MASK_WEBOS		BIT(15)
> +
> +struct hidpp_platform_desc {
> +	u8 plat_idx;
> +	u8 desc_idx;
> +	u16 plat_mask;
> +};
> +
> +/*
> + * Platform names exposed through the "platform" sysfs attribute.
> The order of
> + * this array must stay in sync with multiplatform_masks[] below, as
> the index
> + * returned by sysfs_match_string() is used to look up the matching
> mask.
> + */
> +static const char * const multiplatform_names[] = {
> +	"windows", "winemb", "linux", "chrome",
> +	"android", "macos", "ios", "webos", "tizen", NULL
> +};
> +
> +static const u16 multiplatform_masks[] = {
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINDOWS,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINEMB,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_LINUX,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_CHROME,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_ANDROID,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_MACOS,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_IOS,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_WEBOS,
> +	HIDPP_MULTIPLATFORM_PLATFORM_MASK_TIZEN,

Wouldn't something like:
static enum multiplatform_masks = {
	HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINDOWS = 8,
	HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINEMB = 9,
...
};

followed by:
static const char * const multiplatform_names[] = {
  [HIDPP_MULTIPLATFORM_PLATFORM_MASK_WINDOWS] = "windows",
...
};

work? It makes the array a little bit bigger, but not hugely.

That way it completely negates the need to keep 2 arrays, and both of
them in sync.

> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 57d8efdd9b89..577af0075e27 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -537,6 +537,8 @@ static const struct hid_device_id
> hid_have_special_driver[] = {
>  #endif
>  #if IS_ENABLED(CONFIG_HID_LOGITECH_HIDPP)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_G920_WHEEL) },
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_CASA_KEYS_KEYBOARD) },
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_MX_KEYS_S_KEYBOARD) },
>  #endif
>  #if IS_ENABLED(CONFIG_HID_MAGICMOUSE)
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
> USB_DEVICE_ID_APPLE_MAGICMOUSE) },

That hunk shouldn't be necessary (since
e04a0442d33b8cf183bba38646447b891bb02123) unless the device was
absolutely unusable without the HID++ driver.

Rest looks fine, I'll try to test it on my Slim Solar+.

Cheers