Re: [PATCH v7 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Registering a statically allocated `struct input_handler` in the per-device probe function violates the driver model and causes list corruption.
- [High] The input handler permanently opens all `EV_KEY` input devices in the system without declaring itself as a passive observer, preventing runtime suspend.
--

commit 3ac5537da45198a85ba82452de2c64484baeac36
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>

input: keyboard: Add driver for ASUS Transformer dock multimedia keys

This adds support for the multimedia top button row of the ASUS Transformer
Mobile Dock keyboard. It defaults to function keys (F1-F12) and uses an input
handler to monitor system-wide key events to switch to multimedia keys when
ScreenLock + AltGr are pressed.

> diff --git a/drivers/input/keyboard/asus-transformer-ec-keys.c b/drivers/input/keyboard/asus-transformer-ec-keys.c
> new file mode 100644
> index 0000000000000..d30d991773744
> --- /dev/null
> +++ b/drivers/input/keyboard/asus-transformer-ec-keys.c
[ ... ]
> +static int asus_ec_input_connect(struct input_handler *handler,
> +				 struct input_dev *dev,
> +				 const struct input_device_id *id)
> +{
> +	struct input_handle *handle;
> +	int error;
[ ... ]
> +	error = input_open_device(handle);
> +	if (error)
> +		goto err_unregister_handle;
> +
> +	return 0;

[ ... ]

> +static struct input_handler asus_ec_input_handler = {
> +	.name =	"asusec-media-handler",
> +	.event = asus_ec_input_event,
> +	.connect = asus_ec_input_connect,
> +	.disconnect = asus_ec_input_disconnect,
> +	.id_table = asus_ec_input_ids,
> +};

Because this handler connects to all EV_KEY devices and calls
input_open_device() without setting .passive_observer = true, will this
increment the dev->users count for every matching input device in the system?

Could this force all EV_KEY input devices to remain open and active
permanently, preventing them from entering runtime suspend?

[ ... ]

> +static int asus_ec_keys_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	asus_ec_input_handler.private = priv;
> +	error = input_register_handler(&asus_ec_input_handler);
> +	if (error)
> +		return error;

Since asus_ec_input_handler is a static global structure, what happens if this
driver is probed multiple times (for instance, via sysfs bind/unbind or
multiple ECs)?

If input_register_handler() is called a second time, it will re-initialize the
list head and add an already linked node. Could this corrupt the
input_handler_list?

Should the input_handler be dynamically allocated per-device or embedded within
struct asus_ec_keys_data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518095907.36158-1-clamor95@gmail.com?part=4