| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 Jan 2016 15:50:48 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: João Paulo Rechi Vita <jprvita@...il.com>
Cc: Darren Hart <dvhart@...radead.org>,
Corentin Chary <corentin.chary@...il.com>,
platform-driver-x86@...r.kernel.org,
acpi4asus-user@...ts.sourceforge.net,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
João Paulo Rechi Vita <jprvita@...lessm.com>
Subject: Re: [PATCH 1/2] platform/x86: Add Asus Wireless Radio Control driver
On Tue, Jan 5, 2016 at 3:26 PM, João Paulo Rechi Vita <jprvita@...il.com> wrote:
> Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
> separate ACPI device for notifications from the airplane mode hotkey.
> This device is called "Wireless Radio Control" in Asus websites and ASHS
> in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
> above.
>
> For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
> query 0x0B is started in the Embedded Controller, and all this query does
> is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):
>
> Scope (_SB.PCI0.SBRG.EC0)
> {
> (...)
> Method (_Q0B, 0, NotSerialized) // _Qxx: EC Query
> {
> If ((MSOS () >= OSW8))
> {
> Notify (ASHS, 0x88) // Device-Specific
> }
> Else
> {
> (...)
> }
> }
> }
Few nitpicks below. In any case looks fine to me.
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -587,6 +587,21 @@ config EEEPC_WMI
> If you have an ACPI-WMI compatible Eee PC laptop (>= 1000), say Y or M
> here.
>
> +config ASUS_WIRELESS
> + tristate "Asus Wireless Radio Control Driver"
> + depends on ACPI
> + depends on INPUT
> + default m
Hmm… Usually we leave this for OSVs (OS Vendors) to decide. Or, like
you mentioned below, to the user.
> + ---help---
> + The Asus Wireless Radio Control handles the airplane mode hotkey
> + present on some Asus laptops.
> +
> + Say Y or M here if you have an ASUS notebook with an airplane mode
> + hotkey.
…here is inconsistency with default value.
> +
> + If you choose to compile this driver as a module the module will be
> + called asus-wireless.
> +
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -0,0 +1,83 @@
> +/*
> + * Asus Wireless Radio Control Driver
> + *
> + * Copyright (C) 2015-2016 Endless Mobile, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +#include <linux/input.h>
> +#include <linux/pci_ids.h>
> +
> +struct asus_wireless_data {
> + struct input_dev *inputdev;
I would suggest
*inputdev -> *idev
> +};
> +
> +static void asus_wireless_notify(struct acpi_device *device, u32 event)
In ACPI code and drivers we usually use struct acpi_device *adev.
> +{
> + struct asus_wireless_data *data = acpi_driver_data(device);
> +
> + dev_dbg(&device->dev, "event=0x%X\n", event);
> + if (event != 0x88) {
> + dev_notice(&device->dev, "Unknown ASHS event: 0x%X\n", event);
> + return;
I believe %#X will work in both cases instead of 0x%X
> + }
> + input_report_key(data->inputdev, KEY_RFKILL, 1);
> + input_report_key(data->inputdev, KEY_RFKILL, 0);
> + input_sync(data->inputdev);
> +}
> +
> +static int asus_wireless_add(struct acpi_device *device)
> +{
> + struct asus_wireless_data *data;
> +
> + data = devm_kzalloc(&device->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> + device->driver_data = data;
> +
> + data->inputdev = devm_input_allocate_device(&device->dev);
> + if (!data->inputdev)
> + return -ENOMEM;
> + data->inputdev->name = "Asus Wireless Radio Control";
> + data->inputdev->phys = "asus-wireless/input0";
> + data->inputdev->id.bustype = BUS_HOST;
> + data->inputdev->id.vendor = PCI_VENDOR_ID_ASUSTEK;
> + set_bit(EV_KEY, data->inputdev->evbit);
> + set_bit(KEY_RFKILL, data->inputdev->keybit);
> + return input_register_device(data->inputdev);
> +}
> +
> +static int asus_wireless_remove(struct acpi_device *device)
> +{
> + return 0;
> +}
> +
> +static const struct acpi_device_id device_ids[] = {
> + {"ATK4002", 0},
> + {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, device_ids);
> +
> +static struct acpi_driver asus_wireless_driver = {
> + .name = "Asus Wireless Radio Control Driver",
> + .class = "hotkey",
> + .ids = device_ids,
> + .ops = {
> + .add = asus_wireless_add,
> + .remove = asus_wireless_remove,
> + .notify = asus_wireless_notify,
> + },
> +};
> +module_acpi_driver(asus_wireless_driver);
> +
> +MODULE_DESCRIPTION("Asus Wireless Radio Control Driver");
> +MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@...il.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.5.0
>
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists