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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] FAP command macros incorrectly embed a hardcoded Client ID, violating the subsystem's software ID convention.
--

commit 18d7a2d2e6622c1a347a9e072d766882d23cb46c
Author: Baraa Atta (Dev Exalt) <exalt.dev.team@gmail.com>

HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)

Adds support in the Logitech HID++ driver for the HID++ Multi-Platform
feature (0x4531), introducing a per-device sysfs attribute "platform" to
allow selecting a target operating system platform.

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 90b0184df777f..b3b846c8bdaf8 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
[ ... ]
> @@ -4423,6 +4426,398 @@ static bool hidpp_application_equals(struct hid_device *hdev,
>  	return report && report->application == application;
>  }
>  
> +/* -------------------------------------------------------------------------- */
> +/* 0x4531: Multi-Platform Support                                             */
> +/* -------------------------------------------------------------------------- */
[ ... ]
> +#define HIDPP_MULTIPLATFORM_FEAT_ID			0x4531
> +#define HIDPP_MULTIPLATFORM_GET_FEATURE_INFO		0x0F
> +#define HIDPP_MULTIPLATFORM_GET_PLATFORM_DESCRIPTOR	0x1F
> +#define HIDPP_MULTIPLATFORM_SET_CURRENT_PLATFORM	0x3F

[Severity: Medium]
Do these FAP command constants incorrectly have the Client ID 0xF baked into
the lower nibble? By subsystem design, they should specify only the function
index (e.g., 0x00, 0x10, 0x30).

Because these constants hardcode 0xF, when hidpp_send_fap_command_sync() is
called to format the message:

drivers/hid/hid-logitech-hidpp.c:hidpp_send_fap_command_sync() {
    ...
    message->fap.funcindex_clientid = funcindex_clientid | LINUX_KERNEL_SW_ID;
    ...
}

The bitwise OR with LINUX_KERNEL_SW_ID (which is 1) will result in 0xF,
bypassing the driver's intentional Linux Kernel Software ID allocation.

Could this incorrect Client ID cause the device to misattribute state changes
or fail to route events back to the kernel properly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701063022.29547-1-exalt.dev.team@gmail.com?part=1