ACPI: Always report a sync event after a lid state change

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 <[email protected]>

---
 drivers/acpi/button.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

drivers/acpi/button.c +2 -1

index 1dfec41..59352d9 100644
@@ -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);

Signature · 6 lines

1.6.0.2

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

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

Quoted 40 lines · 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 <[email protected]>

---
 drivers/acpi/button.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

drivers/acpi/button.c +2 -1

index 1dfec41..59352d9 100644
@@ -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);

Signature · 6 lines

1.6.0.2

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

Hi Len,

Quoted 6 lines · Len Brown

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.

Quoted 3 lines

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.

Quoted 6 lines

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?

Quoted 1 line

Ack?

Yep.

Quoted 34 lines · Dmitry Torokhov

Hi Len,

Quoted 6 lines · Len Brown

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.

Quoted 3 lines

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.

Quoted 6 lines

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?

Quoted 3 lines

Quoted 1 line

Ack?

Yep.

-stable material?