2007-06-15 14:54:50

by Richard Hughes

[permalink] [raw]
Subject: Re: [patch] Reporting the lid status using INPUT

On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> On 6/15/07, Richard Hughes <[email protected]> wrote:
> > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > in response to an event, but I'm thinking in a resume hook we should
> > > probably do acpi_evaluate_integer(handle, "_LID", NULL, &state) and then
> > > send an event, just so userspace is aware of what the state of the panel
> > > is.
> >
> > Attached patch fixed the issue for me. Comments?
> >
>
> The patch makes perfect sense. The only issue I have is this:
>
> > + /* on resume we send the state; it might be the same, but userspace
> > + * should handle duplicated events */
>
> If switch state matches to what input layer thinks it is the event
> will not even reach userspace.

Okay, new patch attached, thanks for the speedy review.

Signed-off-by: Richard Hughes <[email protected]>


Attachments:
button-resume.patch (2.16 kB)

2007-06-16 17:13:21

by Richard Hughes

[permalink] [raw]
Subject: Re: [patch] Reporting the lid status using INPUT

On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote:
> On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> > On 6/15/07, Richard Hughes <[email protected]> wrote:
> > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > > in response to an event, but I'm thinking in a resume hook we should
> > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, &state) and then
> > > > send an event, just so userspace is aware of what the state of the panel
> > > > is.
> > >
> > > Attached patch fixed the issue for me. Comments?
> > >
> >
> > The patch makes perfect sense. The only issue I have is this:
> >
> > > + /* on resume we send the state; it might be the same, but userspace
> > > + * should handle duplicated events */
> >
> > If switch state matches to what input layer thinks it is the event
> > will not even reach userspace.
>
> Okay, new patch attached, thanks for the speedy review.

This fix is also confirmed by somebody else, see
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030

It would be great if this could go into .22, although I appreciate it's
quite late in the day.

Richard.


2007-06-16 17:51:36

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [patch] Reporting the lid status using INPUT

On Saturday 16 June 2007 13:11, Richard Hughes wrote:
> On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote:
> > On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> > > On 6/15/07, Richard Hughes <[email protected]> wrote:
> > > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > > > in response to an event, but I'm thinking in a resume hook we should
> > > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, &state) and then
> > > > > send an event, just so userspace is aware of what the state of the panel
> > > > > is.
> > > >
> > > > Attached patch fixed the issue for me. Comments?
> > > >
> > >
> > > The patch makes perfect sense. The only issue I have is this:
> > >
> > > > + /* on resume we send the state; it might be the same, but userspace
> > > > + * should handle duplicated events */
> > >
> > > If switch state matches to what input layer thinks it is the event
> > > will not even reach userspace.
> >
> > Okay, new patch attached, thanks for the speedy review.
>
> This fix is also confirmed by somebody else, see
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030
>
> It would be great if this could go into .22, although I appreciate it's
> quite late in the day.
>

This is of course Len's call but in my book this is a bugfix and as such
is appropriate for -rc4/rc5.

--
Dmitry

2007-06-24 22:06:48

by Richard Hughes

[permalink] [raw]
Subject: Re: [patch] Reporting the lid status using INPUT

On 16/06/07, Dmitry Torokhov <[email protected]> wrote:
> On Saturday 16 June 2007 13:11, Richard Hughes wrote:
> > On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote:
> > > On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> > > > On 6/15/07, Richard Hughes <[email protected]> wrote:
> > > > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > > > > in response to an event, but I'm thinking in a resume hook we should
> > > > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, &state) and then
> > > > > > send an event, just so userspace is aware of what the state of the panel
> > > > > > is.
> > > > >
> > > > > Attached patch fixed the issue for me. Comments?
> > > > >
> > > >
> > > > The patch makes perfect sense. The only issue I have is this:
> > > >
> > > > > + /* on resume we send the state; it might be the same, but userspace
> > > > > + * should handle duplicated events */
> > > >
> > > > If switch state matches to what input layer thinks it is the event
> > > > will not even reach userspace.
> > >
> > > Okay, new patch attached, thanks for the speedy review.
> >
> > This fix is also confirmed by somebody else, see
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030
> >
> > It would be great if this could go into .22, although I appreciate it's
> > quite late in the day.
> >
>
> This is of course Len's call but in my book this is a bugfix and as such
> is appropriate for -rc4/rc5.

Guys? Any ack-nak?

Richard.

2007-06-25 20:56:26

by Bartók Albert

[permalink] [raw]
Subject: Re: [patch] Reporting the lid status using INPUT





GMail On 16/06/07, Dmitry Torokhov <[email protected]> wrote:
> On Saturday 16 June 2007 13:11, Richard Hughes wrote:
> > On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote:
> > > On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> > > > On 6/15/07, Richard Hughes <[email protected]> wrote:
> > > > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > > > > in response to an event, but I'm thinking in a resume hook we should
> > > > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, &state) and then
> > > > > > send an event, just so userspace is aware of what the state of the panel
> > > > > > is.
> > > > >
> > > > > Attached patch fixed the issue for me. Comments?
> > > > >
> > > >
> > > > The patch makes perfect sense. The only issue I have is this:
> > > >
> > > > > + /* on resume we send the state; it might be the same, but userspace
> > > > > + * should handle duplicated events */
> > > >
> > > > If switch state matches to what input layer thinks it is the event
> > > > will not even reach userspace.
> > >
> > > Okay, new patch attached, thanks for the speedy review.
> >
> > This fix is also confirmed by somebody else, see
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030
> >
> > It would be great if this could go into .22, although I appreciate it's
> > quite late in the day.
> >
>
> This is of course Len's call but in my book this is a bugfix and as such
> is appropriate for -rc4/rc5.

> Guys? Any ack-nak?

works perfectly for me so far.

Albert



15% KEDVEZM?NY minden PLASZTIKAI M?T?TRE az Aesthetica orvosi k?zpontban! Klikk ide!http://www.webdesign.hu/aesthetica/flash_microsite/?id=9;p_code=2079

2007-06-25 21:25:58

by Richard Hughes

[permalink] [raw]
Subject: Re: [patch] Reporting the lid status using INPUT

On Mon, 2007-06-25 at 22:49 +0200, Bartók Albert wrote:
> GMail On 16/06/07, Dmitry Torokhov <[email protected]> wrote:
> > On Saturday 16 June 2007 13:11, Richard Hughes wrote:
> > > On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote:
> > > > On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> > > > > On 6/15/07, Richard Hughes <[email protected]> wrote:
> > > > > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > > > > > in response to an event, but I'm thinking in a resume hook we should
> > > > > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, &state) and then
> > > > > > > send an event, just so userspace is aware of what the state of the panel
> > > > > > > is.
> > > > > >
> > > > > > Attached patch fixed the issue for me. Comments?
> > > > > >
> > > > >
> > > > > The patch makes perfect sense. The only issue I have is this:
> > > > >
> > > > > > + /* on resume we send the state; it might be the same, but userspace
> > > > > > + * should handle duplicated events */
> > > > >
> > > > > If switch state matches to what input layer thinks it is the event
> > > > > will not even reach userspace.
> > > >
> > > > Okay, new patch attached, thanks for the speedy review.
> > >
> > > This fix is also confirmed by somebody else, see
> > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030
> > >
> > > It would be great if this could go into .22, although I appreciate it's
> > > quite late in the day.
> > >
> >
> > This is of course Len's call but in my book this is a bugfix and as such
> > is appropriate for -rc4/rc5.
>
> > Guys? Any ack-nak?
>
> works perfectly for me so far.

Thanks. Len, can you add this to your tree please. Thanks.

Richard.


2007-06-27 19:46:28

by Richard Hughes

[permalink] [raw]
Subject: [patch] Reporting the lid status using INPUT

Len,

* user shuts lid, initiates suspend
{lid is reported down}
* user opens lid, resume completes

But the lid INPUT up event is not sent. This works if you don't initiate
suspend when you shut the lid (blank screen for instance) -- so I know
my hardware is okay.

I see:
input_report_switch(input, SW_LID, !state);

in response to an event, but in a resume hook we should probably do
acpi_evaluate_integer(handle, "_LID", NULL, &state) and then send an
event, just so userspace is aware of what the state of the panel is.

If switch state matches to what input layer thinks it is the event will
not be sent.

Signed-off-by: Richard Hughes <[email protected]>


Attachments:
button-resume.patch (2.16 kB)