On Jun 05 2017 or thereabouts, Zheng, Lv wrote:
> Hi, Benjamin
> 
> > From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com]
> > Subject: [WIP PATCH 3/4] ACPI: button: Let input filter out the LID events
> > 
> > The input stack already filters out the LID events. So instead of
> > filtering them out at the source, we can hook up after the input
> > processing and propagate the lid switch events when the input stack
> > tells us to.
> > 
> > An other benefit is that if userspace (think libinput) "fixes" the lid
> > switch state by some heuristics, this new state is forwarded to the
> > listeners in the kernel.
> 
> See my comments to PATCH 4.
> IMO, it sounds better that
> 1. ACPI lid works as a driver of SW_LID, and
> 2. i915 registers notification (the only user) via input layer.
> So it looks i915 rather than button driver should call 
> input_register_handler().
> And input layer may help to provide a simplified interface for drivers to 
> register key notifications.

Sounds good.

Dmitry, would a simplified API for other drivers to listen to input
events be something you would agree on?

Cheers,
Benjamin

> 
> Thanks and best regards
> Lv
> 
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoi...@redhat.com>
> > ---
> >  drivers/acpi/button.c | 156 
> > ++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 139 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> > index 9ad7604..03e5981 100644
> > --- a/drivers/acpi/button.c
> > +++ b/drivers/acpi/button.c
> > @@ -109,8 +109,6 @@ struct acpi_button {
> >     struct input_dev *input;
> >     char phys[32];                  /* for input device */
> >     unsigned long pushed;
> > -   int last_state;
> > -   ktime_t last_time;
> >     bool suspended;
> >  };
> > 
> > @@ -184,7 +182,6 @@ static int acpi_lid_evaluate_state(struct acpi_device 
> > *device)
> >  static int acpi_lid_notify_state(struct acpi_device *device, int state)
> >  {
> >     struct acpi_button *button = acpi_driver_data(device);
> > -   int ret;
> > 
> >     /* button_input_lock must be held */
> > 
> > @@ -205,20 +202,129 @@ static int acpi_lid_notify_state(struct acpi_device 
> > *device, int state)
> >     if (state)
> >             pm_wakeup_hard_event(&device->dev);
> > 
> > -   ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device);
> > -   if (ret == NOTIFY_DONE)
> > -           ret = blocking_notifier_call_chain(&acpi_lid_notifier, state,
> > -                                              device);
> > -   if (ret == NOTIFY_DONE || ret == NOTIFY_OK) {
> > -           /*
> > -            * It is also regarded as success if the notifier_chain
> > -            * returns NOTIFY_OK or NOTIFY_DONE.
> > -            */
> > -           ret = 0;
> > +   return 0;
> > +}
> > +
> > +/*
> > + * Pass incoming event to all connected clients.
> > + */
> > +static void acpi_button_lid_events(struct input_handle *handle,
> > +                              const struct input_value *vals,
> > +                              unsigned int count)
> > +{
> > +   const struct input_value *v;
> > +   int state = -1;
> > +   int ret;
> > +
> > +   for (v = vals; v != vals + count; v++) {
> > +           switch (v->type) {
> > +           case EV_SYN:
> > +                   if (v->code == SYN_REPORT && state >= 0) {
> > +                           ret = 
> > blocking_notifier_call_chain(&acpi_lid_notifier,
> > +                                                           state,
> > +                                                           lid_device);
> > +                           if (ret == NOTIFY_DONE)
> > +                                   ret = 
> > blocking_notifier_call_chain(&acpi_lid_notifier,
> > +                                                           state,
> > +                                                           lid_device);
> > +                           if (ret == NOTIFY_DONE || ret == NOTIFY_OK) {
> > +                                   /*
> > +                                    * It is also regarded as success if
> > +                                    * the notifier_chain returns NOTIFY_OK
> > +                                    * or NOTIFY_DONE.
> > +                                    */
> > +                                   ret = 0;
> > +                           }
> > +                   }
> > +                   break;
> > +           case EV_SW:
> > +                   if (v->code == SW_LID)
> > +                           state = !v->value;
> > +                   break;
> > +           }
> >     }
> > -   return ret;
> >  }
> > 
> > +static int acpi_button_lid_connect(struct input_handler *handler,
> > +                              struct input_dev *dev,
> > +                              const struct input_device_id *id)
> > +{
> > +   struct input_handle *handle;
> > +   int error;
> > +
> > +   handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> > +   if (!handle)
> > +           return -ENOMEM;
> > +
> > +   handle->dev = dev;
> > +   handle->handler = handler;
> > +   handle->name = "acpi-button-lid";
> > +
> > +   error = input_register_handle(handle);
> > +   if (error) {
> > +           dev_err(&lid_device->dev, "Error installing input handle\n");
> > +           goto err_free;
> > +   }
> > +
> > +   error = input_open_device(handle);
> > +   if (error) {
> > +           dev_err(&lid_device->dev, "Failed to open input device\n");
> > +           goto err_unregister;
> > +   }
> > +
> > +   return 0;
> > +
> > + err_unregister:
> > +   input_unregister_handle(handle);
> > + err_free:
> > +   kfree(handle);
> > +   return error;
> > +}
> > +
> > +static void acpi_button_lid_disconnect(struct input_handle *handle)
> > +{
> > +   input_close_device(handle);
> > +   input_unregister_handle(handle);
> > +   kfree(handle);
> > +}
> > +
> > +bool acpi_button_lid_match(struct input_handler *handler,
> > +                      struct input_dev *dev)
> > +{
> > +   struct acpi_button *button;
> > +
> > +   if (!lid_device)
> > +           return false;
> > +
> > +   button = acpi_driver_data(lid_device);
> > +
> > +   if (dev != button->input)
> > +           return false;
> > +
> > +   return true;
> > +}
> > +
> > +static const struct input_device_id acpi_button_lid_ids[] = {
> > +   {
> > +           .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > +           .evbit = { BIT_MASK(EV_SW) },
> > +           .swbit = { BIT_MASK(SW_LID) },
> > +   },
> > +   { },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(input, acpi_button_lid_ids);
> > +
> > +static struct input_handler acpi_button_lid_handler = {
> > +   .match          = acpi_button_lid_match,
> > +   .connect        = acpi_button_lid_connect,
> > +   .disconnect     = acpi_button_lid_disconnect,
> > +   .events         = acpi_button_lid_events,
> > +   .name           = "acpi-lid-callchain",
> > +   .id_table       = acpi_button_lid_ids,
> > +};
> > +
> > +
> >  static int acpi_button_state_seq_show(struct seq_file *seq, void *offset)
> >  {
> >     struct acpi_device *device = seq->private;
> > @@ -581,8 +687,6 @@ static int acpi_button_add(struct acpi_device *device)
> >             strcpy(name, ACPI_BUTTON_DEVICE_NAME_LID);
> >             sprintf(class, "%s/%s",
> >                     ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
> > -           button->last_state = !!acpi_lid_evaluate_state(device);
> > -           button->last_time = ktime_get();
> >     } else {
> >             printk(KERN_ERR PREFIX "Unsupported hid [%s]\n", hid);
> >             error = -ENODEV;
> > @@ -674,4 +778,22 @@ module_param_call(lid_init_state,
> >               NULL, 0644);
> >  MODULE_PARM_DESC(lid_init_state, "Behavior for reporting LID initial 
> > state");
> > 
> > -module_acpi_driver(acpi_button_driver);
> > +static int __init acpi_button_init(void)
> > +{
> > +   int error;
> > +
> > +   error = input_register_handler(&acpi_button_lid_handler);
> > +   if (error)
> > +           return error;
> > +
> > +   return acpi_bus_register_driver(&acpi_button_driver);
> > +}
> > +
> > +static void __exit acpi_button_exit(void)
> > +{
> > +   acpi_bus_unregister_driver(&acpi_button_driver);
> > +   input_unregister_handler(&acpi_button_lid_handler);
> > +}
> > +
> > +module_init(acpi_button_init);
> > +module_exit(acpi_button_exit);
> > --
> > 2.9.4
> 
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to