Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1651629 > unrolled thread

[RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS notification and faked events

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS notification and faked events Lv Zheng <lv.zheng@intel.com> - 2017-05-27 03:30 +0200
    Re: [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS  notification and faked events Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-05-29 18:10 +0200
      RE: [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS  notification and faked events "Zheng, Lv" <lv.zheng@intel.com> - 2017-05-31 11:00 +0200

#1651629 — [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS notification and faked events

This patch adds a parameter to acpi_lid_notify_state() so that it can act
differently against BIOS notification and kernel faked events.

Cc: <systemd-devel@lists.freedesktop.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/button.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 6d5a8c1..874ba60 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -138,7 +138,8 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)
 	return lid_state ? 1 : 0;
 }
 
-static int acpi_lid_notify_state(struct acpi_device *device, int state)
+static int acpi_lid_notify_state(struct acpi_device *device,
+				 int state, bool bios_notify)
 {
 	struct acpi_button *button = acpi_driver_data(device);
 	int ret;
@@ -360,7 +361,8 @@ int acpi_lid_open(void)
 }
 EXPORT_SYMBOL(acpi_lid_open);
 
-static int acpi_lid_update_state(struct acpi_device *device)
+static int acpi_lid_update_state(struct acpi_device *device,
+				 bool bios_notify)
 {
 	int state;
 
@@ -368,17 +370,17 @@ static int acpi_lid_update_state(struct acpi_device *device)
 	if (state < 0)
 		return state;
 
-	return acpi_lid_notify_state(device, state);
+	return acpi_lid_notify_state(device, state, bios_notify);
 }
 
 static void acpi_lid_initialize_state(struct acpi_device *device)
 {
 	switch (lid_init_state) {
 	case ACPI_BUTTON_LID_INIT_OPEN:
-		(void)acpi_lid_notify_state(device, 1);
+		(void)acpi_lid_notify_state(device, 1, false);
 		break;
 	case ACPI_BUTTON_LID_INIT_METHOD:
-		(void)acpi_lid_update_state(device);
+		(void)acpi_lid_update_state(device, false);
 		break;
 	case ACPI_BUTTON_LID_INIT_IGNORE:
 	default:
@@ -398,7 +400,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)
 	case ACPI_BUTTON_NOTIFY_STATUS:
 		input = button->input;
 		if (button->type == ACPI_BUTTON_TYPE_LID) {
-			acpi_lid_update_state(device);
+			acpi_lid_update_state(device, true);
 		} else {
 			int keycode;
 
-- 
2.7.4

[toc] | [next] | [standalone]

#1652628 — Re: [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS notification and faked events

Hi Lv,

On May 27 2017 or thereabouts, Lv Zheng wrote:
> This patch adds a parameter to acpi_lid_notify_state() so that it can act
> differently against BIOS notification and kernel faked events.
> 
> Cc: <systemd-devel@lists.freedesktop.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Peter Hutterer <peter.hutterer@who-t.net>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---

Answering to this one for the entire series:
last week was a mix of public holidays and PTO from me. I was only
able to review this series today, so sorry for the delay.

I still have a feeling this driver is far too engineered for a simple
input node. There are internal states, defers, mangle of events and too
many kernel parameters.

I still need to get my head around it, but the more I think of it, the
more I think the solution provided by Lennart in
https://github.com/systemd/systemd/issues/2807 is the simplest one:
when we are not sure about the state of the LID switch because _LID
might be wrong, we shouldn't export a LID input node.
Which means that all broken cases would be fixed by just a quirk
"unreliable lid switch".

Give me a day or two to get this in a better shape.

Cheers,
Benjamin

[toc] | [prev] | [next] | [standalone]

#1654021 — RE: [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS notification and faked events

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Benjamin
> Tissoires
> Subject: Re: [RFC PATCH v3 1/5] ACPI: button: Add indication of BIOS notification and faked events
> 
> Hi Lv,
> 
> On May 27 2017 or thereabouts, Lv Zheng wrote:
> > This patch adds a parameter to acpi_lid_notify_state() so that it can act
> > differently against BIOS notification and kernel faked events.
> >
> > Cc: <systemd-devel@lists.freedesktop.org>
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Cc: Peter Hutterer <peter.hutterer@who-t.net>
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > ---
> 
> Answering to this one for the entire series:
> last week was a mix of public holidays and PTO from me. I was only
> able to review this series today, so sorry for the delay.

Here we were having "the Dragon Boat Festival".
But we really won't have chances of seeing see dragon boats in major cities.

> I still have a feeling this driver is far too engineered for a simple
> input node. There are internal states, defers, mangle of events and too
> many kernel parameters.

That's the firmware world and windows compliance world. :)

> I still need to get my head around it, but the more I think of it, the
> more I think the solution provided by Lennart in
> https://github.com/systemd/systemd/issues/2807 is the simplest one:
> when we are not sure about the state of the LID switch because _LID
> might be wrong, we shouldn't export a LID input node.
> Which means that all broken cases would be fixed by just a quirk
> "unreliable lid switch".

I checked the post and had no idea about what was going on.
However, my test shows systemd 233 works fine with button.lid_init_state=ignore.
I don't know what has been improved.

But a noticeable thing on old systemd 229 is:
Even with button.lid_init_state=open, systemd still behaves strangely.
It looks systemd 229 really has a problem with its timeout logic.
And in systemd 233, the timeout mechanism seems to work better.

Anyway, the problem disappears when there are only user space changes.

> Give me a day or two to get this in a better shape.

Sure.

Cheers,
Lv

> 
> Cheers,
> Benjamin
> 
> --
> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web