| 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
| ||
|
Message-ID: <20141128113328.GS1304@lahna.fi.intel.com>
Date: Fri, 28 Nov 2014 13:33:28 +0200
From: Mika Westerberg <mika.westerberg@...ux.intel.com>
To: Pali Rohár <pali.rohar@...il.com>
Cc: Matthew Garrett <mjg59@...f.ucam.org>,
Darren Hart <dvhart@...radead.org>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
Gabriele Mazzotta <gabriele.mzt@...il.com>,
Alex Hung <alex.hung@...onical.com>
Subject: Re: [PATCH 1/3] platform: x86: dell-rbtn: Dell Airplane Mode Switch
driver
On Sun, Nov 23, 2014 at 04:09:19PM +0100, Pali Rohár wrote:
> This is an ACPI driver for Dell laptops which receive HW switch events.
> It exports rfkill device dell-rbtn which provide correct hard rfkill state.
>
> It does not provide support for setting soft rfkill state yet.
>
> Signed-off-by: Pali Rohár <pali.rohar@...il.com>
> ---
> drivers/platform/x86/Kconfig | 13 +++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/dell-rbtn.c | 179 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 193 insertions(+)
> create mode 100644 drivers/platform/x86/dell-rbtn.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 4dcfb71..5a2ba64 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -137,6 +137,19 @@ config DELL_SMO8800
> To compile this driver as a module, choose M here: the module will
> be called dell-smo8800.
>
> +config DELL_RBTN
> + tristate "Dell Airplane Mode Switch driver"
> + depends on ACPI
> + depends on RFKILL
> + ---help---
> + Say Y here if you want to support Dell Airplane Mode Switch ACPI
> + device on Dell laptops. Sometimes it has names: DELLABCE or DELRBTN.
> + This driver register rfkill device and receives HW rfkill events
> + (when wifi/bluetooth was enabled) and set correct hard rfkill state.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called dell-rbtn.
> +
>
> config FUJITSU_LAPTOP
> tristate "Fujitsu Laptop Extras"
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index f82232b..b3e54ed 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DELL_LAPTOP) += dell-laptop.o
> obj-$(CONFIG_DELL_WMI) += dell-wmi.o
> obj-$(CONFIG_DELL_WMI_AIO) += dell-wmi-aio.o
> obj-$(CONFIG_DELL_SMO8800) += dell-smo8800.o
> +obj-$(CONFIG_DELL_RBTN) += dell-rbtn.o
> obj-$(CONFIG_ACER_WMI) += acer-wmi.o
> obj-$(CONFIG_ACERHDF) += acerhdf.o
> obj-$(CONFIG_HP_ACCEL) += hp_accel.o
> diff --git a/drivers/platform/x86/dell-rbtn.c b/drivers/platform/x86/dell-rbtn.c
> new file mode 100644
> index 0000000..f1f039a
> --- /dev/null
> +++ b/drivers/platform/x86/dell-rbtn.c
> @@ -0,0 +1,179 @@
> +/*
> + Dell Airplane Mode Switch driver
> + Copyright (C) 2014 Pali Rohár <pali.rohar@...il.com>
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 2 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
Please check Documentation/CodingStyle, block comments look like this
/*
* This is block comment.
*/
> +*/
> +
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/rfkill.h>
> +
> +/*** helper functions ***/
> +
> +static int rbtn_check(struct acpi_device *device)
> +{
> + acpi_status status;
> + unsigned long long output;
> +
> + status = acpi_evaluate_integer(device->handle, "CRBT", NULL, &output);
Do you think it is worth documenting what CRBT is supposed to do? Why
return value <= 3 is OK and > 3 is not?
> + if (ACPI_FAILURE(status))
> + return -EINVAL;
> +
> + if (output > 3)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +
Delete the second blank line.
> +/*** rfkill ops ***/
> +
> +static void rbtn_query(struct rfkill *rfkill, void *data)
> +{
> + struct acpi_device *device = data;
> + acpi_status status;
> + unsigned long long output;
> +
> + status = acpi_evaluate_integer(device->handle, "GRBT", NULL, &output);
Same comment here about the documentation.
> + if (ACPI_FAILURE(status))
> + return;
> +
> + rfkill_set_states(rfkill, !output, !output);
You can also write it like:
if (ACPI_SUCCESS(status))
rfkill_set_states(rfkill, !output, !output);
which looks better to me at least.
> +}
> +
> +static int rbtn_set_block(void *data, bool blocked)
> +{
> + /* FIXME: setting soft rfkill cause problems, so disable it for now */
> + return -EINVAL;
> +}
> +
> +struct rfkill_ops rbtn_ops = {
static? const?
> + .query = rbtn_query,
> + .set_block = rbtn_set_block,
> +};
> +
> +
Delete the second blank line.
> +/*** acpi driver ***/
> +
> +static int rbtn_add(struct acpi_device *device);
> +static int rbtn_remove(struct acpi_device *device);
> +static void rbtn_notify(struct acpi_device *device, u32 event);
> +
> +static const struct acpi_device_id rbtn_ids[] = {
> + { "DELRBTN", 0 },
> + { "DELLABCE", 0 },
> + { "", 0 },
> +};
> +
> +static struct acpi_driver rbtn_driver = {
> + .name = "dell-rbtn",
> + .ids = rbtn_ids,
> + .ops = {
> + .add = rbtn_add,
> + .remove = rbtn_remove,
> + .notify = rbtn_notify,
> + },
> + .owner = THIS_MODULE,
> +};
> +
> +
Ditto.
> +/*** rfkill enable/disable ***/
> +
> +static int rbtn_enable(struct acpi_device *device)
> +{
> + struct rfkill *rfkill = device->driver_data;
> + int ret;
> +
> + if (rfkill)
> + return 0;
> +
> + /* NOTE: rbtn controls all radio devices, not only WLAN
> + but rfkill interface does not support "ANY" type
> + so "WLAN" type is used
> + */
Block comment style.
> + rfkill = rfkill_alloc("dell-rbtn", &device->dev, RFKILL_TYPE_WLAN,
> + &rbtn_ops, device);
> + if (!rfkill)
> + return -ENOMEM;
> +
> + ret = rfkill_register(rfkill);
> + if (ret) {
> + rfkill_destroy(rfkill);
> + return ret;
> + }
> +
> + device->driver_data = rfkill;
> + return 0;
> +}
> +
> +static void rbtn_disable(struct acpi_device *device)
> +{
> + struct rfkill *rfkill = device->driver_data;
> +
> + if (!rfkill)
> + return;
> +
> + rfkill_unregister(rfkill);
> + rfkill_destroy(rfkill);
> + device->driver_data = NULL;
> +}
> +
> +
Extra blank line.
> +/*** acpi driver functions ***/
> +
> +static int rbtn_add(struct acpi_device *device)
> +{
> + int ret;
> +
> + ret = rbtn_check(device);
> + if (ret < 0)
> + return ret;
> +
> + return rbtn_enable(device);
> +}
> +
> +static int rbtn_remove(struct acpi_device *device)
> +{
> + rbtn_disable(device);
> + return 0;
> +}
> +
> +static void rbtn_notify(struct acpi_device *device, u32 event)
> +{
> + struct rfkill *rfkill = device->driver_data;
> +
> + if (event == 0x80)
> + rbtn_query(rfkill, device);
> + else
> + dev_info(&device->dev, "Received unknown event (0x%x)\n", event);
> +}
> +
> +
Extra blank line.
> +/*** module functions ***/
> +
> +static int __init rbtn_init(void)
> +{
> + return acpi_bus_register_driver(&rbtn_driver);
> +}
> +
> +static void __exit rbtn_exit(void)
> +{
> + acpi_bus_unregister_driver(&rbtn_driver);
> +}
> +
> +module_init(rbtn_init);
> +module_exit(rbtn_exit);
module_acpi_driver()?
> +
> +MODULE_DEVICE_TABLE(acpi, rbtn_ids);
> +MODULE_DESCRIPTION("Dell Airplane Mode Switch driver");
> +MODULE_AUTHOR("Pali Rohár <pali.rohar@...il.com>");
> +MODULE_LICENSE("GPL");
> --
> 1.7.9.5
>
> --
> 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/
--
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