Re: [PATCH] ACPI: Platform driver to support App Hot Startup (PNP0C32)

Zhang Rui
2008-08-25T01:29:42Z

On Mon, 2008-08-25 at 00:39 +0800, Ângelo Miguel Arrifano wrote:
> On Fri, 28 Mar 2008 19:39:19 +0000
> Ângelo Miguel Arrifano <miknix@gmail.com> wrote:
>
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > On Fri, 28 Mar 2008 09:36:55 +0800
> > Shaohua Li <shaohua.li@intel.com> wrote:
> >
> > >
> > > On Thu, 2008-03-27 at 16:03 +0000, Ângelo Miguel Arrifano wrote:
> > > > On Thu, 27 Mar 2008 10:30:13 +0800
> > > > Shaohua Li <shaohua.li@intel.com> wrote:
> > > >
> > > > >
> > > > > On Thu, 2008-03-27 at 01:51 +0800, Ângelo Miguel Arrifano
> wrote:
> > > > > > PATCH
> > > > > > -
> -----------------------------------------------------------------
> > > > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > > > > index f688c21..fb096ee 100644
> > > > > > - --- a/drivers/acpi/Kconfig
> > > > > > +++ b/drivers/acpi/Kconfig
> > > > > > @@ -196,6 +196,14 @@ config ACPI_THERMAL
> > > > > > recommended that this option be enabled, as your
> > > > > > processor(s)
> > > > > > may be damaged without it.
> > > > > >
> > > > > > +config ACPI_QUICKSTART
> > > > > > + tristate "Quickstart"
> > > > > > + default y
> > > > > default m?
> > > > >
> > > > >
> > > > > > +
> > > > > > +static struct quickstart_driver_data {
> > > > > > + struct quickstart_btn *btn_lst;
> > > > > > + struct quickstart_btn *pressed;
> > > > > > +} quickstart_data = {
> > > > > > + .btn_lst = NULL,
> > > > > > + .pressed = NULL,
> > > > > > +};
> > > > > They are NULL, you don't need initialize them.
> > > > >
> > > > >
> > > > > > + * Platform driver structs
> > > > > > + */
> > > > > > +static ssize_t buttons_show(struct device *dev,
> > > > > > + struct
> device_attribute *attr,
> > > > > > + char *buf);
> > > > > > +static ssize_t pressed_button_show(struct device *dev,
> > > > > > + struct
> device_attribute *attr,
> > > > > > + char *buf);
> > > > > > +static ssize_t pressed_button_store(struct device *dev,
> > > > > > + struct
> device_attribute *attr,
> > > > > > + const char *buf,
> > > > > > + size_t count);
> > > > > > +static DEVICE_ATTR(pressed_button, 0666,
> pressed_button_show,
> > > > > > +
> pressed_button_store);
> > > > > this file can be changed by non-root?
> > > >
> > > > The only thing that writing to this file does is clearing the
> > > > pressed button.
> > > > Is it a bad idea that can be changed by non-root?
> > > Just abnormal, eg. non-root can clear it but root might not read
> it
> > > (though it doesn't happen in current case but possible if runtime
> event
> > > support).
> > >
> > > > > > +
> > > > > > +/* ACPI Driver functions */
> > > > > > +static void quickstart_acpi_notify(acpi_handle handle, u32
> event,
> > > > > > void *data)
> > > > > > +{
> > > > > > + struct quickstart_acpi *quickstart = data;
> > > > > > +
> > > > > > + if (!quickstart)
> > > > > > + return;
> > > > > > +
> > > > > > + if (event == QUICKSTART_EVENT_WAKE) {
> > > > > > + quickstart_data.pressed = quickstart->btn;
> > > > > > + printk(KERN_ERR "quickstart: Quickbutton %s
> > > > > > pressed.\n",
> > > > > > +
> > > > > > acpi_device_bid(quickstart->device));
> > > > > > + } else if (event == QUICKSTART_EVENT_RUNTIME) {
> > > > > > + printk(KERN_ERR "quickstart: Runtime button
> %s
> > > > > > pressed.\n\t"
> > > > > > + "please report on
> linux-acpi@vger.kernel.org",
> > > > > > +
> > > > > > acpi_device_bid(quickstart->device));
> > > > > > + }
> > > > > Please remove above printks. they are misleading, which isn't
> an error.
> > > > > please send an ACPI event to userspace in the notify, we need
> it for
> > > > > runtime.
> > > >
> > > > According to the Microsoft paper, runtime hot button reporting
> is optional.
> > > > My laptop doesn't report them and on the SF.net project page
> there was no
> > > > reports of people with hardware supporting this.
> > > > Sorry, I don't have a way to test this..
> > > Did you tried suspend/resume. From the doc, resume can send a
> > > notification too. Just calling acpi event eject routines, an event
> eject
> > > model can be well integrated into existing acpi daemons.
> >
> > Yes, suspend to mem works.
> > When the laptop is in suspend state, if DVD button is pressed the
> laptop resumes
> > and the platform driver receives a notify. The driver then exports
> the
> > button name to the sysfs pressed_button file.
> >
> > >
> > > > > > + }
> > > > > I'd suggest the GHID info should be exported to sysfs too.
> It's hard to
> > > > > judge what a button is just per its name. You know it but an
> > > > > inexperienced user doesn't.
> > > >
> > > > I also would like to have a more descriptive information about
> the button but
> > > > the returned buffer is a BYTE/WORD/DWORD with a decimal number
> telling the button
> > > > usage ID.
> > > >
> > > > Looking at my DSDT piece of code:
> > > >
> > > > Method (GHID, 0, NotSerialized)
> > > > {
> > > > If (LEqual (HOTB, 0x05))
> > > > {
> > > > Notify (DBTN, 0x02)
> > > > Store (0x00, HOTB)
> > > > }
> > > >
> > > > Return (Buffer (0x01)
> > > > {
> > > > /* 0000 */ 0x04
> > > > })
> > > > }
> > > >
> > > > IMHO, a value like 0x04 for button usageID is totally useless..
> > > Ok, I got it. Maybe we need tool to help user identify what
> purpose a
> > > button is, but sounds impossible till the button can trigger
> runtime
> > > event.
> >
> > I've noticed that HP laptops have the same names for button
> > devices (at least dv6k, dv8k and tx series). Maybe other vendors do
> this as well?
> > If this holds true, maybe building a vendor table for getting
> > button name -> description correspondence? .. just an idea ..
> >
> > >
> > > Thanks,
> > > Shaohua
> > >
> >
> > Thanks all,
> > - --
> > Angelo Arrifano AKA MiKNiX
> > CSE Student at UBI, Portugal
> > Gentoo Linux AMD64 Arch Tester
> > Linwizard Developer
> > http://miknix.homelinux.com
> > PGP Pubkey 0x3D92BB0B
> >
> > - - -
> > Computers are like air conditioners. Both stop working, if you open
> windows.
> > -- Adam Heath
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v2.0.7 (GNU/Linux)
> >
> > iD8DBQFH7UlnNahyoD2SuwsRApPtAJ9GcFwvwuAgCyCnS87XaGI+nHAbYgCeM65K
> > lvOIGf01TPFzAgqFbSoFJYY=
> > =vrGR
> > -----END PGP SIGNATURE-----
>
> Hello,
>
> I have been testing the driver against latest trees. The driver still
> works without changes.
> I would like to ask if, is this driver eligible for inclusion on the
> acpi-test branch?
>
oops, sorry that we lost track of this patch, but it should be somewhere
in Len's tree.
cc len. :)

thanks,
rui