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

Ângelo Miguel Arrifano
2008-08-24T16:04:27Z

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?

Best regards,
--
Angelo Arrifano AKA MiKNiX
CSE Student at UBI, Portugal
Gentoo Linux AMD64 Arch Tester
Linwizard Developer
http://miknix.homelinux.com
PGP Pubkey 0x3D92BB0B