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

["Ângelo Miguel Arrifano" <miknix@gmail.com>]

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
--
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