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

[Zheng, Lv]

Hi,

> From: linux-acpi-ow...@vger.kernel.org 
> [mailto:linux-acpi-ow...@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-de...@lists.freedesktop.org>
> > Cc: Benjamin Tissoires <benjamin.tissoi...@redhat.com>
> > Cc: Peter Hutterer <peter.hutte...@who-t.net>
> > Signed-off-by: Lv Zheng <lv.zh...@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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Zheng, Lv]

Hi,

> From: linux-acpi-ow...@vger.kernel.org 
> [mailto:linux-acpi-ow...@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: 
> > Cc: Benjamin Tissoires 
> > Cc: Peter Hutterer 
> > Signed-off-by: Lv Zheng 
> > ---
> 
> 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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Benjamin Tissoires]

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: 
> Cc: Benjamin Tissoires 
> Cc: Peter Hutterer 
> Signed-off-by: Lv Zheng 
> ---

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

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

[Benjamin Tissoires]

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: 
> Cc: Benjamin Tissoires 
> Cc: Peter Hutterer 
> Signed-off-by: Lv Zheng 
> ---

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