On Thu, Nov 16, 2017 at 3:44 PM, Chris Chiu <chiu@xxxxxxxxxxxx> wrote:
> New Acer laptops in 2018 will have a separate ACPI device for
> notifications from the airplane mode hotkey. The device name in
> the DSDT is SMKB and its ACPI _HID is 10251229.
>
> For these models, when the airplane mode hotkey (Fn+F3) pressed,
> a query 0x02 is started in the Embedded Controller, and all this
> query does is a notify SMKB with the value 0x80.
>
> Scope (_SB.PCI0.LPCB.EC0)
> {
> (...)
> Method (_Q02, 0, NotSerialized) // _Qxx: EC Query
> {
> HKEV (0x2, One)
> Notify (SMKB, 0x80) // Status Change
> }
> }
>
> Based on code from asus-wireless
Thanks for the patch! Overall looks good, comment below.
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
Either from that, not both.
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +#include <linux/input.h>
> +#include <linux/pci_ids.h>
Why do you need this one?
Okay, you are using Vendor ID from there.
Besides above, please keep them in alphabetical order.
> +
> +struct acer_wireless_data {
> + struct input_dev *idev;
> + struct acpi_device *adev;
Do you need this one?
I suppose whenever you need struct device out of it you may find it
via input->parent. Right?
> +};
> +
> +static const struct acpi_device_id device_ids[] = {
acer_wireless_acpi_ids
> + {"10251229", 0},
> + {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, device_ids);
> +
> +static void acer_wireless_notify(struct acpi_device *adev, u32 event)
> +{
> + struct acer_wireless_data *data = acpi_driver_data(adev);
> +
> + dev_dbg(&adev->dev, "event=%#x\n", event);
This makes sense after the check, otherwise you will get two messages in a row.
> + if (event != 0x80) {
> + dev_notice(&adev->dev, "Unknown SMKB event: %#x\n", event);
> + return;
> + }
> + input_report_key(data->idev, KEY_RFKILL, 1);
> + input_report_key(data->idev, KEY_RFKILL, 0);
> + data->idev = devm_input_allocate_device(&adev->dev);
> + if (!data->idev)
> + return -ENOMEM;
> + data->idev->name = "Acer Wireless Radio Control";
> + data->idev->phys = "acer-wireless/input0";
> + data->idev->id.bustype = BUS_HOST;
> + data->idev->id.vendor = PCI_VENDOR_ID_AI;
Where is product ID?
I suppose you can use ACPI ID (which is basically PCI one in ACPI
table) and split it.
u32 id = simple_strtoul(...ACPI ID..., 16);
vendor = id >> 16;
product = id & 0xffff;
Or just hard code product ID for now.
> +static int acer_wireless_remove(struct acpi_device *adev)
> +{
> + return 0;
> +}
> +
No need UI suppose.
> +MODULE_LICENSE("GPL");
GPL v2 ?
--
With Best Regards,
Andy Shevchenko
