* [PATCH] platform/x86: Add Acer Wireless Radio Control driver
@ 2017-11-16 13:44 Chris Chiu
2017-11-17 14:25 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Chris Chiu @ 2017-11-16 13:44 UTC (permalink / raw)
To: dvhart, andy, linux-kernel, platform-driver-x86; +Cc: linux
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
Signed-off-by: Chris Chiu <chiu@endlessm.com>
Reviewed-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
drivers/platform/x86/Kconfig | 14 ++++++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/acer-wireless.c | 86 ++++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+)
create mode 100644 drivers/platform/x86/acer-wireless.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b87954f6dd..6c8b3eaf35df 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -36,6 +36,20 @@ config ACER_WMI
If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M
here.
+config ACER_WIRELESS
+ tristate "Acer Wireless Radio Control Driver"
+ depends on ACPI
+ depends on INPUT
+ ---help---
+ The Acer Wireless Radio Control handles the airplane mode hotkey
+ present on new Acer laptops.
+
+ Say Y or M here if you have an Acer notebook with an airplane mode
+ hotkey.
+
+ If you choose to compile this driver as a module the module will be
+ called acer-wireless.
+
config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 91cec1751461..4fd15503d842 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_DELL_WMI_LED) += dell-wmi-led.o
obj-$(CONFIG_DELL_SMO8800) += dell-smo8800.o
obj-$(CONFIG_DELL_RBTN) += dell-rbtn.o
obj-$(CONFIG_ACER_WMI) += acer-wmi.o
+obj-$(CONFIG_ACER_WIRELESS) += acer-wireless.o
obj-$(CONFIG_ACERHDF) += acerhdf.o
obj-$(CONFIG_HP_ACCEL) += hp_accel.o
obj-$(CONFIG_HP_WIRELESS) += hp-wireless.o
diff --git a/drivers/platform/x86/acer-wireless.c b/drivers/platform/x86/acer-wireless.c
new file mode 100644
index 000000000000..9c320943d3f5
--- /dev/null
+++ b/drivers/platform/x86/acer-wireless.c
@@ -0,0 +1,86 @@
+/*
+ * Acer Wireless Radio Control Driver
+ *
+ * Copyright (C) 2017 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 acer_wireless_data {
+ struct input_dev *idev;
+ struct acpi_device *adev;
+};
+
+static const struct acpi_device_id device_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);
+ 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);
+ input_sync(data->idev);
+}
+
+static int acer_wireless_add(struct acpi_device *adev)
+{
+ struct acer_wireless_data *data;
+
+ data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ adev->driver_data = data;
+ data->adev = adev;
+
+ 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;
+ set_bit(EV_KEY, data->idev->evbit);
+ set_bit(KEY_RFKILL, data->idev->keybit);
+
+ return input_register_device(data->idev);
+}
+
+static int acer_wireless_remove(struct acpi_device *adev)
+{
+ return 0;
+}
+
+static struct acpi_driver acer_wireless_driver = {
+ .name = "Acer Wireless Radio Control Driver",
+ .class = "hotkey",
+ .ids = device_ids,
+ .ops = {
+ .add = acer_wireless_add,
+ .remove = acer_wireless_remove,
+ .notify = acer_wireless_notify,
+ },
+};
+module_acpi_driver(acer_wireless_driver);
+
+MODULE_DESCRIPTION("Acer Wireless Radio Control Driver");
+MODULE_AUTHOR("Chris Chiu <chiu@gmail.com>");
+MODULE_LICENSE("GPL");
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: Add Acer Wireless Radio Control driver
2017-11-16 13:44 [PATCH] platform/x86: Add Acer Wireless Radio Control driver Chris Chiu
@ 2017-11-17 14:25 ` Andy Shevchenko
2017-11-20 6:31 ` Chris Chiu
0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2017-11-17 14:25 UTC (permalink / raw)
To: Chris Chiu
Cc: dvhart, Andy Shevchenko, linux-kernel, Platform Driver,
Linux Upstreaming Team
On Thu, Nov 16, 2017 at 3:44 PM, Chris Chiu <chiu@endlessm.com> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: Add Acer Wireless Radio Control driver
2017-11-17 14:25 ` Andy Shevchenko
@ 2017-11-20 6:31 ` Chris Chiu
2017-11-20 12:24 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Chris Chiu @ 2017-11-20 6:31 UTC (permalink / raw)
To: Andy Shevchenko
Cc: dvhart, Andy Shevchenko, linux-kernel, Platform Driver,
Linux Upstreaming Team
On Fri, Nov 17, 2017 at 10:25 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Nov 16, 2017 at 3:44 PM, Chris Chiu <chiu@endlessm.com> 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?
>
But I think it would make life easier to have the following line in
.notify callback
struct acer_wireless_data * data = acpi_driver_data(adev);
I can get its parent acer_wireless_data and point to input_dev easier.
>> +};
>> +
>> +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.
>
Thanks. I will hard code the product ID for now with
data->idev->id.vendor = 0x1229;
>> +static int acer_wireless_remove(struct acpi_device *adev)
>> +{
>> + return 0;
>> +}
>> +
>
> No need UI suppose.
>
You mean remove this acer_wireless_remove which does nothing? Will do.
>> +MODULE_LICENSE("GPL");
>
> GPL v2 ?
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: Add Acer Wireless Radio Control driver
2017-11-20 6:31 ` Chris Chiu
@ 2017-11-20 12:24 ` Andy Shevchenko
2017-11-21 3:34 ` Chris Chiu
0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2017-11-20 12:24 UTC (permalink / raw)
To: Chris Chiu
Cc: dvhart, Andy Shevchenko, linux-kernel, Platform Driver,
Linux Upstreaming Team
On Mon, Nov 20, 2017 at 8:31 AM, Chris Chiu <chiu@endlessm.com> wrote:
> On Fri, Nov 17, 2017 at 10:25 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Thu, Nov 16, 2017 at 3:44 PM, Chris Chiu <chiu@endlessm.com> wrote:
>>> +
>>> +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?
>>
>
> But I think it would make life easier to have the following line in
> .notify callback
> struct acer_wireless_data * data = acpi_driver_data(adev);
>
> I can get its parent acer_wireless_data and point to input_dev easier.
Yes, though if I'm not mistaken, you may get rid completely of your
custom container and use struct input_dev for accessing that via
to_acpi_device(idev->dev.parent).
It would be less code, less data structures, cleaner view.
Right?
>>> +static int acer_wireless_remove(struct acpi_device *adev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>
>> No need UI suppose.
>>
>
> You mean remove this acer_wireless_remove which does nothing? Will do.
Correct.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: Add Acer Wireless Radio Control driver
2017-11-20 12:24 ` Andy Shevchenko
@ 2017-11-21 3:34 ` Chris Chiu
0 siblings, 0 replies; 5+ messages in thread
From: Chris Chiu @ 2017-11-21 3:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: dvhart, Andy Shevchenko, linux-kernel, Platform Driver,
Linux Upstreaming Team
On Mon, Nov 20, 2017 at 8:24 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Nov 20, 2017 at 8:31 AM, Chris Chiu <chiu@endlessm.com> wrote:
>> On Fri, Nov 17, 2017 at 10:25 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Thu, Nov 16, 2017 at 3:44 PM, Chris Chiu <chiu@endlessm.com> wrote:
>
>>>> +
>>>> +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?
>>>
>>
>> But I think it would make life easier to have the following line in
>> .notify callback
>> struct acer_wireless_data * data = acpi_driver_data(adev);
>>
>> I can get its parent acer_wireless_data and point to input_dev easier.
>
> Yes, though if I'm not mistaken, you may get rid completely of your
> custom container and use struct input_dev for accessing that via
> to_acpi_device(idev->dev.parent).
>
> It would be less code, less data structures, cleaner view.
> Right?
>
>>>> +static int acer_wireless_remove(struct acpi_device *adev)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +
>>>
>>> No need UI suppose.
>>>
>>
>> You mean remove this acer_wireless_remove which does nothing? Will do.
>
> Correct.
>
> --
> With Best Regards,
> Andy Shevchenko
I think I get your point. Will soon have a v2 patch later.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-21 3:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-16 13:44 [PATCH] platform/x86: Add Acer Wireless Radio Control driver Chris Chiu
2017-11-17 14:25 ` Andy Shevchenko
2017-11-20 6:31 ` Chris Chiu
2017-11-20 12:24 ` Andy Shevchenko
2017-11-21 3:34 ` Chris Chiu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®