On Fri, 28 Mar 2008 19:39:19 +0000 Ângelo Miguel Arrifano <miknix@xxxxxxxxx> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On Fri, 28 Mar 2008 09:36:55 +0800 > Shaohua Li <shaohua.li@xxxxxxxxx> 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@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. > > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html