Re: [PATCH v2 1/2] Input: Add lid switch notifier

Dmitry Torokhov
2025-11-11T22:34:35Z

Hi Jonathan,

On Tue, Nov 11, 2025 at 09:34:06PM +0000, Jonathan Denose wrote:
> This change creates a new input handler which can be included in the
> build via a new Kconfig option CONFIG_INPUT_LID_NOTIFIER. This input
> handler listens for lid switch events and publishes them through an
> atomic notification chain. Other modules may register for events
> through this notification chain with register_lid_notifier.
>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
> drivers/input/Kconfig | 11 +++++
> drivers/input/Makefile | 1 +
> drivers/input/lid-notifier.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/input.h | 2 +
> 4 files changed, 112 insertions(+)
>
> diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
> index 88ecdf5218ee9ba35e1efec6341f8605b621bd49..16f6d24fd04ac8cb5af9d36cc47155ea9be0e177 100644
> --- a/drivers/input/Kconfig
> +++ b/drivers/input/Kconfig
> @@ -38,6 +38,17 @@ config INPUT_LEDS
> To compile this driver as a module, choose M here: the
> module will be called input-leds.
>
> +config INPUT_LID_NOTIFIER
> + tristate "Include notifier for lid switch events"
> + help
> + Say Y here if you would like to create a notifier to publish lid switch
> + events.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called lid-notifier.

I think this better not surfaced to users but rather interested drivers
'select' it.

> +
> config INPUT_FF_MEMLESS
> tristate "Support for memoryless force-feedback devices"
> help
> diff --git a/drivers/input/Makefile b/drivers/input/Makefile
> index 2cd6e1c9a77844fe09cd3d99533e5d3efb038c7d..1efdba04f79a97e2a122b9198341b18a1855b4b9 100644
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_INPUT_MATRIXKMAP) += matrix-keymap.o
> obj-$(CONFIG_INPUT_VIVALDIFMAP) += vivaldi-fmap.o
>
> obj-$(CONFIG_INPUT_LEDS) += input-leds.o
> +obj-$(CONFIG_INPUT_LID_NOTIFIER) += lid-notifier.o
> obj-$(CONFIG_INPUT_MOUSEDEV) += mousedev.o
> obj-$(CONFIG_INPUT_JOYDEV) += joydev.o
> obj-$(CONFIG_INPUT_EVDEV) += evdev.o
> diff --git a/drivers/input/lid-notifier.c b/drivers/input/lid-notifier.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..954b9855532dbd0514860e309d0b76982e947673
> --- /dev/null
> +++ b/drivers/input/lid-notifier.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Lid event notifier
> + *
> + * Copyright (c) 2025 Jonathan Denose <jdenose@google.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/notifier.h>
> +
> +static struct input_handler lid_handler;
> +static struct atomic_notifier_head input_notifier_head;
> +
> +int register_lid_notifier(struct notifier_block *notifier)
> +{
> + return atomic_notifier_chain_register(&input_notifier_head, notifier);
> +}
> +EXPORT_SYMBOL(register_lid_notifier);

I wonder if we want to expose the "raw" notifier or if we want to
provide a higher-level API that would allocate a notifier blocki, set up
the callback, and return a "cookie" that can be used to free notifier
block later. This way we do not need to worry that some enterprising
driver suppresses notifications for the rest by returning NOTIFY_STOP.

> +
> +static int lid_handler_connect(struct input_handler *handler,
> + struct input_dev *input_dev, const struct input_device_id *id)

Proper alignment of the arguments please.

> +{
> + struct input_handle *handle;
> + int error;
> +
> + handle = devm_kzalloc(&input_dev->dev, sizeof(struct input_handle), GFP_KERNEL);

This is not driver probe path so devm_kzalloc must not be used here.
Also "sizeof(*handle)".

> + if (!handle)
> + return -ENOMEM;
> +
> + handle->dev = input_dev;
> + handle->handler = handler;
> + handle->name = "lid";
> +
> + error = input_register_handle(handle);
> + if (error)
> + goto err_free_handle;
> +
> + error = input_open_device(handle);
> + if (error)
> + goto err_unregister_handle;
> +
> + return 0;
> +
> + err_unregister_handle:
> + input_unregister_handle(handle);
> + err_free_handle:
> + kfree(handle);

Just FYI: One must never use kfree() with devm_kalloc()ed memory.

> + return error;
> +}
> +
> +static void lid_handler_disconnect(struct input_handle *handle)
> +{
> + input_close_device(handle);
> + input_unregister_handle(handle);

kfree(handle);

> +}
> +
> +static void lid_handler_event(struct input_handle *handle, unsigned int type,
> + unsigned int code, int value)
> +{
> + if (type == EV_SW && code == SW_LID)
> + atomic_notifier_call_chain(&input_notifier_head, value, handle->dev);

Why do you need to pass the device from which SW_LID originated?

> +}
> +
> +static const struct input_device_id lid_handler_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT
> + | INPUT_DEVICE_ID_MATCH_BUS,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> + .bustype = 0x19

Why do we need to match in bus type? The LID does not have to always
come from ACPI.

> + },
> + { },
> +};
> +
> +static struct input_handler lid_handler = {
> + .connect = lid_handler_connect,
> + .disconnect = lid_handler_disconnect,
> + .event = lid_handler_event,
> + .name = "lid",
> + .id_table = lid_handler_ids
> +};
> +
> +static int __init lid_notifier_init(void)
> +{
> + return input_register_handler(&lid_handler);
> +}
> +module_init(lid_notifier_init);
> +
> +static void __exit lid_notifier_exit(void)
> +{
> + input_unregister_handler(&lid_handler);
> +}
> +module_exit(lid_notifier_exit);
> +
> +MODULE_AUTHOR("Jonathan Denose <jdenose@google.com>");
> +MODULE_DESCRIPTION("Lid event notifier");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 7d7cb0593a63e93c4906c49cde430188db2d1ab5..023eb92c77d9e8721d482b9787632a671671de08 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -592,3 +592,5 @@ int input_ff_create_memless(struct input_dev *dev, void *data,
> int (*play_effect)(struct input_dev *, void *, struct ff_effect *));
>
> #endif

I think this should go into include/linux/lid-notifier.h.

> +
> +int register_lid_notifier(struct notifier_block *notifier);

Thanks.

--
Dmitry