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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx",
> > > +
> > > 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.
> > > + }
> > 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.
Thanks,
Shaohua
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
