[PATCH] ACPI: Always report a sync event after a lid state change

[PATCH] ACPI: Always report a sync event after a lid state change

[Guillem Jover]

Currently not always an EV_SYN event is reported to userland
after the EV_SW SW_LID event has been sent. This is easy to verify
by using “input-events” from input-utils and just closing and opening
the lid.

Signed-off-by: Guillem Jover <guillem.jover@nokia.com>
---
 drivers/acpi/button.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 1dfec41..59352d9 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -262,6 +262,7 @@ static int acpi_lid_send_state(struct acpi_button *button)
 		return -ENODEV;
 	/* input layer checks if event is redundant */
 	input_report_switch(button->input, SW_LID, !state);
+	input_sync(button->input);
 	return 0;
 }
 
@@ -285,8 +286,8 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 			input_report_key(input, keycode, 1);
 			input_sync(input);
 			input_report_key(input, keycode, 0);
+			input_sync(input);
 		}
-		input_sync(input);
 
 		acpi_bus_generate_proc_event(button->device, event,
 					++button->pushed);
-- 
1.6.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Always report a sync event after a lid state change

[Len Brown]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1838 bytes --]

Dmitry,

Looking around the tree, the other calls to
input_report_switch() and input_report_key()
are all followed by input_sync().

I guess we didn't understand the API when we added
these calls to button.c?

Looks like the other users of input_report_* in
drivers/acpi and drivers/misc are okay,
with the exception of toshiba_acpi.c --
the most neglected driver we have,
so I"ll do the same to that one?

Ack?

thanks,
-Len


On Fri, 24 Oct 2008, Guillem Jover wrote:

> Currently not always an EV_SYN event is reported to userland
> after the EV_SW SW_LID event has been sent. This is easy to verify
> by using “input-events” from input-utils and just closing and opening
> the lid.
> 
> Signed-off-by: Guillem Jover <guillem.jover@nokia.com>
> ---
>  drivers/acpi/button.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 1dfec41..59352d9 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -262,6 +262,7 @@ static int acpi_lid_send_state(struct acpi_button *button)
>  		return -ENODEV;
>  	/* input layer checks if event is redundant */
>  	input_report_switch(button->input, SW_LID, !state);
> +	input_sync(button->input);
>  	return 0;
>  }
>  
> @@ -285,8 +286,8 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
>  			input_report_key(input, keycode, 1);
>  			input_sync(input);
>  			input_report_key(input, keycode, 0);
> +			input_sync(input);
>  		}
> -		input_sync(input);
>  
>  		acpi_bus_generate_proc_event(button->device, event,
>  					++button->pushed);
> -- 
> 1.6.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] ACPI: Always report a sync event after a lid state change

[Dmitry Torokhov]

Hi Len,

On Friday 24 October 2008, Len Brown wrote:
> Dmitry,
> 
> Looking around the tree, the other calls to
> input_report_switch() and input_report_key()
> are all followed by input_sync().
> 

The idea is that userspace can accumulate input events and not act on
them till it gets the "whole state" of the device which is indicated by
sending EV_SYN/SYN_REPORT event. This really does not matter for most
of the simple devices (such as most button devices) but is required
when you need to report state of a touchpad or a tablet (ABS_X, ABS_Y,
ABS_PRESSURE, host of buttons and so on). And that is the reason to have
2 input_sync when you you reporting button down and up back to back -
if there was only one input_sync userspace may (although I don't think
anyone does) wait to reacting till the sync and by that time the up event
will "cancel out" the down event.

> I guess we didn't understand the API when we added
> these calls to button.c?
>

The code was fine until acpi_lid_send_state was used un resume path,
then it needed its own input_sync.
 
> Looks like the other users of input_report_* in
> drivers/acpi and drivers/misc are okay,
> with the exception of toshiba_acpi.c --
> the most neglected driver we have,
> so I"ll do the same to that one?
> 
> Ack?

Yep.

> 
> thanks,
> -Len
> 
> 
> On Fri, 24 Oct 2008, Guillem Jover wrote:
> 
> > Currently not always an EV_SYN event is reported to userland
> > after the EV_SW SW_LID event has been sent. This is easy to verify
> > by using “input-events” from input-utils and just closing and opening
> > the lid.
> > 
> > Signed-off-by: Guillem Jover <guillem.jover@nokia.com>
> > ---
> >  drivers/acpi/button.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> > index 1dfec41..59352d9 100644
> > --- a/drivers/acpi/button.c
> > +++ b/drivers/acpi/button.c
> > @@ -262,6 +262,7 @@ static int acpi_lid_send_state(struct acpi_button *button)
> >  		return -ENODEV;
> >  	/* input layer checks if event is redundant */
> >  	input_report_switch(button->input, SW_LID, !state);
> > +	input_sync(button->input);
> >  	return 0;
> >  }
> >  
> > @@ -285,8 +286,8 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
> >  			input_report_key(input, keycode, 1);
> >  			input_sync(input);
> >  			input_report_key(input, keycode, 0);
> > +			input_sync(input);
> >  		}
> > -		input_sync(input);
> >  
> >  		acpi_bus_generate_proc_event(button->device, event,
> >  					++button->pushed);
> > -- 
> > 1.6.0.2
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 



-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Always report a sync event after a lid state change

[Rafael J. Wysocki]

On Sunday, 26 of October 2008, Dmitry Torokhov wrote:
> Hi Len,
> 
> On Friday 24 October 2008, Len Brown wrote:
> > Dmitry,
> > 
> > Looking around the tree, the other calls to
> > input_report_switch() and input_report_key()
> > are all followed by input_sync().
> > 
> 
> The idea is that userspace can accumulate input events and not act on
> them till it gets the "whole state" of the device which is indicated by
> sending EV_SYN/SYN_REPORT event. This really does not matter for most
> of the simple devices (such as most button devices) but is required
> when you need to report state of a touchpad or a tablet (ABS_X, ABS_Y,
> ABS_PRESSURE, host of buttons and so on). And that is the reason to have
> 2 input_sync when you you reporting button down and up back to back -
> if there was only one input_sync userspace may (although I don't think
> anyone does) wait to reacting till the sync and by that time the up event
> will "cancel out" the down event.
> 
> > I guess we didn't understand the API when we added
> > these calls to button.c?
> >
> 
> The code was fine until acpi_lid_send_state was used un resume path,
> then it needed its own input_sync.
>  
> > Looks like the other users of input_report_* in
> > drivers/acpi and drivers/misc are okay,
> > with the exception of toshiba_acpi.c --
> > the most neglected driver we have,
> > so I"ll do the same to that one?
> > 
> > Ack?
> 
> Yep.

-stable material?

Rafael

Re: [PATCH] ACPI: Always report a sync event after a lid state change

[Dmitry Torokhov]

On Sunday 26 October 2008, Rafael J. Wysocki wrote:
> On Sunday, 26 of October 2008, Dmitry Torokhov wrote:
> > Hi Len,
> > 
> > On Friday 24 October 2008, Len Brown wrote:
> > > Dmitry,
> > > 
> > > Looking around the tree, the other calls to
> > > input_report_switch() and input_report_key()
> > > are all followed by input_sync().
> > > 
> > 
> > The idea is that userspace can accumulate input events and not act on
> > them till it gets the "whole state" of the device which is indicated by
> > sending EV_SYN/SYN_REPORT event. This really does not matter for most
> > of the simple devices (such as most button devices) but is required
> > when you need to report state of a touchpad or a tablet (ABS_X, ABS_Y,
> > ABS_PRESSURE, host of buttons and so on). And that is the reason to have
> > 2 input_sync when you you reporting button down and up back to back -
> > if there was only one input_sync userspace may (although I don't think
> > anyone does) wait to reacting till the sync and by that time the up event
> > will "cancel out" the down event.
> > 
> > > I guess we didn't understand the API when we added
> > > these calls to button.c?
> > >
> > 
> > The code was fine until acpi_lid_send_state was used un resume path,
> > then it needed its own input_sync.
> >  
> > > Looks like the other users of input_report_* in
> > > drivers/acpi and drivers/misc are okay,
> > > with the exception of toshiba_acpi.c --
> > > the most neglected driver we have,
> > > so I"ll do the same to that one?
> > > 
> > > Ack?
> > 
> > Yep.
> 
> -stable material?

Nah, I don't think userspace using these events actually cares so
stable can stay as is.

-- 
Dmitry