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?
> +static ssize_t buttons_show(struct device *dev,
> + struct device_attribute
> *attr,
> + char *buf)
> +{
> + int count = 0;
> + char tmpbuf[QUICKSTART_MAX_BTN_NAME_LEN + 2];
> + struct quickstart_btn *ptr = quickstart_data.btn_lst;
> +
> + if (!ptr)
> + return snprintf(buf, PAGE_SIZE, "none");
> +
> + while (ptr && (count < PAGE_SIZE)) {
> + if (ptr->name) {
> + strncpy(tmpbuf, ptr->name,
> QUICKSTART_MAX_BTN_NAME_LEN);
> + strcat(tmpbuf, "\n");
> + count += snprintf(buf + count,
> + PAGE_SIZE - count,
> + tmpbuf);
we could simplify:
snprintf(buf + count, PAGE_SIZE -count, "%s\n", ptr->name);
> + }
> + ptr = ptr->next;
> + }
> +
> + return count;
> +}
> +
> +
> +/* 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.
> + return;
> +}
> +
> +static void quickstart_acpi_raise_notify(struct quickstart_acpi
> *quickstart)
> +{
> + acpi_status status;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +
> + if (!quickstart)
> + return;
> +
> + /* This returns a buffer telling the button usage ID
> (useless),
> + * we dont care about it now. The important is to trigger
> + * pending notify events (The ones before booting). */
> + status = acpi_evaluate_object(quickstart->device->handle,
> + "GHID", NULL, &buffer);
> + if (ACPI_FAILURE(status) || !buffer.pointer) {
> + printk(KERN_ERR "quickstart: %s GHID method
> failed.\n",
> + quickstart->btn->name);
> + return;
> + }
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.
> + kfree(buffer.pointer);
> + return;
> +}
> +
> +
> +static int quickstart_acpi_add(struct acpi_device *device)
> +{
> + int ret = 0;
> + acpi_status status = AE_OK;
> + struct quickstart_acpi *quickstart = NULL;
> +
> + if (!device)
> + return -EINVAL;
> +
> + quickstart = kzalloc(sizeof(struct quickstart_acpi),
> GFP_KERNEL);
> + if (!quickstart)
> + return -ENOMEM;
> +
> + quickstart->device = device;
> + strcpy(acpi_device_name(device), QUICKSTART_ACPI_DEVICE_NAME);
> + strcpy(acpi_device_class(device), QUICKSTART_ACPI_CLASS);
> + acpi_driver_data(device) = quickstart;
> +
> + /* Add button to list and initialize some stuff */
> + ret = quickstart_acpi_config(quickstart,
> acpi_device_bid(device));
> + if (ret)
> + goto fail_config;
> +
> + status = acpi_install_notify_handler(device->handle,
> + ACPI_ALL_NOTIFY,
> +
> quickstart_acpi_notify,
> + quickstart);
> + if (ACPI_FAILURE(status)) {
> + printk(KERN_ERR "quickstart: Notify handler install
> error\n");
> + ret = -ENODEV;
> + goto fail_installnotify;
memory is freed but it's still in the list.
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
