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

[Dmitry Torokhov]

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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>
> + */
> +
> +#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@xxxxxxxxxx>");
> +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