platform-driver-x86: ACPI EC Extra driver for Oaktrail

Hi,

This driver implements an Extra ACPI EC driver for products based on Intel Oaktrail platform.  It is programming the EC space, through existing ACPI EC driver, to provide user space layer the sysfs and rfkill interfaces to enable/disable the Camera, Bluetooth, GPS, WiFi, 3G, and to show the status of Touchscreen.

$ ./scripts/checkpatch.pl
0001-platform-driver-x86-ACPI-EC-Extra-driver-for-Oaktrai.patch
total: 0 errors, 0 warnings, 366 lines checked

0001-platform-driver-x86-ACPI-EC-Extra-driver-for-Oaktrai.patch has no obvious style problems and is ready for submission.

From 5d817d0b120b52ae3685edf1d67a552ab7b6b12a Mon Sep 17 00:00:00 2001
From: Yin Kangkai <[email protected]>
Date: Wed, 22 Dec 2010 10:53:36 +0800
Subject: [PATCH] platform-driver-x86: ACPI EC Extra driver for Oaktrail

This driver implements an Extra ACPI EC driver for products based on Intel Oaktrail platform.  It is programming the EC space, through existing ACPI EC driver, to provide user space layer the sysfs and rfkill interfaces to enable/disable the Camera, Bluetooth, GPS, WiFi, 3G, and to show the status of Touchscreen.

Signed-off-by Yin Kangkai <[email protected]>

---
 drivers/platform/x86/Kconfig          |    9 + drivers/platform/x86/Makefile         |    1 + drivers/platform/x86/intel_oaktrail.c |  349 +++++++++++++++++++++++++++++++++

 3 files changed, 359 insertions(+), 0 deletions(-)

 create mode 100644 drivers/platform/x86/intel_oaktrail.c

drivers/platform/x86/Kconfig +0 -0

index 2b4038a..d1f6981 100644
@@ -655,4 +655,13 @@ config XO1_RFKILL

         Support for enabling/disabling the WLAN interface on the OLPC XO-1 laptop.

drivers/platform/x86/Kconfig +9 -0

+config INTEL_OAKTRAIL
+       tristate "Intel Oaktrail Platform Extras"
+       depends on ACPI
+       depends on RFKILL
+       ---help---
+         Intel Oaktrail platform need this driver to provide interfaces to
+         enable/disable the Camera, WiFi, BT etc. devices. If in doubt, say Y
+         here; it will only load on supported platforms.
+

 endif # X86_PLATFORM_DEVICES

drivers/platform/x86/Makefile +0 -0

index 7ff60e6..add8ab7 100644
@@ -35,3 +35,4 @@ obj-$(CONFIG_INTEL_IPS)               += intel_ips.o

 obj-$(CONFIG_GPIO_INTEL_PMIC)  += intel_pmic_gpio.o obj-$(CONFIG_XO1_RFKILL)       += xo1-rfkill.o obj-$(CONFIG_IBM_RTL)          += ibm_rtl.o

drivers/platform/x86/Makefile +1 -0

+obj-$(CONFIG_INTEL_OAKTRAIL)   += intel_oaktrail.o

drivers/platform/x86/intel_oaktrail.c new file +0 -0

new file mode 100644
index 0000000..d28d14c

drivers/platform/x86/intel_oaktrail.c new file +26 -0

+/*
+  Copyright (C) 2010 Intel Corporation
+  Author: Yin Kangkai ([email protected])
+
+  based on Compal driver
+
+  Copyright (C) 2008 Cezary Jackiewicz <cezary.jackiewicz (at) gmail.com>
+
+  based on MSI driver
+
+  Copyright (C) 2006 Lennart Poettering <mzxreary (at) 0pointer (dot) de>
+
+  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.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+  02110-1301, USA.

drivers/platform/x86/intel_oaktrail.c new file +42 -0

+ * In addition to these platform device attributes the driver registers itself
+ * in the Linux rfkill subsystem and is available to userspace under
+ * /sys/class/rfkill/rfkillX/
+ *
+ * This driver might work on other products based on Oaktrail. If you
+ * want to try it you can pass force=1 as argument to the module which
+ * will force it to load even when the DMI data doesn't identify the
+ * product as compatible.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/dmi.h>
+#include <linux/rfkill.h>
+
+#define DRIVER_NAME    "intel_oaktrail"
+#define DRIVER_VERSION "0.1"
+
+/*
+ * This is the devices status address in EC space, and the control bits
+ * definition:
+ *
+ * (1 << 0):   Camera enable/disable, RW.
+ * (1 << 1):   Bluetooth enable/disable, RW.
+ * (1 << 2):   GPS enable/disable, RW.
+ * (1 << 3):   WiFi enable/disable, RW.
+ * (1 << 4):   WWAN (3G) enable/disalbe, RW.
+ * (1 << 5):   Touchscreen enable/disable, Read Only.
+ */
+#define OT_EC_DEVICE_STATE_ADDRESS     0xD6
+
+#define OT_EC_CAMERA_MASK      (1 << 0)
+#define OT_EC_BT_MASK          (1 << 1)
+#define OT_EC_GPS_MASK         (1 << 2)
+#define OT_EC_WIFI_MASK                (1 << 3)
+#define OT_EC_WWAN_MASK                (1 << 4)
+#define OT_EC_TS_MASK          (1 << 5)

drivers/platform/x86/intel_oaktrail.c new file +62 -0

+#define SIMPLE_MASKED_STORE_SHOW(NAME, MASK)                           \
+static ssize_t NAME##_show(struct device *dev,                         \
+       struct device_attribute *attr, char *buf)                       \
+{                                                                      \
+       u8 value;                                                       \
+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &value);                    \
+       return sprintf(buf, "%d\n", ((value & MASK) != 0));             \
+}                                                                      \
+static ssize_t NAME##_store(struct device *dev,                                \
+       struct device_attribute *attr, const char *buf, size_t count)   \
+{                                                                      \
+       int state;                                                      \
+       u8 old_val;                                                     \
+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &old_val);                  \
+       if (sscanf(buf, "%d", &state) != 1 || (state < 0 || state > 1)) \
+               return -EINVAL;                                         \
+       ec_write(OT_EC_DEVICE_STATE_ADDRESS, state ?                    \
+                (old_val | MASK) : (old_val & ~MASK));                 \
+       return count;                                                   \
+}
+
+SIMPLE_MASKED_STORE_SHOW(camera, OT_EC_CAMERA_MASK)
+SIMPLE_MASKED_STORE_SHOW(bluetooth, OT_EC_BT_MASK)
+SIMPLE_MASKED_STORE_SHOW(gps, OT_EC_GPS_MASK)
+SIMPLE_MASKED_STORE_SHOW(wifi, OT_EC_WIFI_MASK)
+SIMPLE_MASKED_STORE_SHOW(wwan, OT_EC_WWAN_MASK)
+SIMPLE_MASKED_STORE_SHOW(touchscreen, OT_EC_TS_MASK)
+
+static DEVICE_ATTR(camera, 0644, camera_show, camera_store);
+static DEVICE_ATTR(bluetooth, 0644, bluetooth_show, bluetooth_store);
+static DEVICE_ATTR(gps, 0644, gps_show, gps_store);
+static DEVICE_ATTR(wifi, 0644, wifi_show, wifi_store);
+static DEVICE_ATTR(wwan, 0644, wwan_show, wwan_store);
+static DEVICE_ATTR(touchscreen, 0444, touchscreen_show, NULL);
+
+static struct attribute *oaktrail_attributes[] = {
+       &dev_attr_camera.attr,
+       &dev_attr_bluetooth.attr,
+       &dev_attr_gps.attr,
+       &dev_attr_wifi.attr,
+       &dev_attr_wwan.attr,
+       &dev_attr_touchscreen.attr,
+       NULL
+};
+
+static struct attribute_group oaktrail_attribute_group = {
+       .attrs = oaktrail_attributes
+};
+
+static int oaktrail_rfkill_set(void *data, bool blocked)
+{
+       u8 value;
+       u8 result;
+       unsigned long radio = (unsigned long) data;
+
+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &result);
+
+       if (!blocked)
+               value = (u8) (result | radio);
+       else
+               value = (u8) (result & ~radio);
+

drivers/platform/x86/intel_oaktrail.c new file +55 -0

+{
+       int ret;
+
+       wifi_rfkill = rfkill_alloc("oaktrail-wifi", &oaktrail_device->dev,
+                               RFKILL_TYPE_WLAN, &oaktrail_rfkill_ops,
+                               (void *) OT_EC_WIFI_MASK);
+       if (!wifi_rfkill)
+               return -ENOMEM;
+
+       ret = rfkill_register(wifi_rfkill);
+       if (ret)
+               goto err_register_wifi;
+
+       bt_rfkill = rfkill_alloc("oaktrail-bluetooth", &oaktrail_device->dev,
+                               RFKILL_TYPE_BLUETOOTH, &oaktrail_rfkill_ops,
+                               (void *) OT_EC_BT_MASK);
+       if (!bt_rfkill) {
+               ret = -ENOMEM;
+               goto err_allocate_bt;
+       }
+       ret = rfkill_register(bt_rfkill);
+       if (ret)
+               goto err_register_bt;
+
+       gps_rfkill = rfkill_alloc("oaktrail-gps", &oaktrail_device->dev,
+                               RFKILL_TYPE_GPS, &oaktrail_rfkill_ops,
+                               (void *) OT_EC_GPS_MASK);
+       if (!gps_rfkill) {
+               ret = -ENOMEM;
+               goto err_allocate_gps;
+       }
+       ret = rfkill_register(gps_rfkill);
+       if (ret)
+               goto err_register_gps;
+
+       wwan_rfkill = rfkill_alloc("oaktrail-wwan", &oaktrail_device->dev,
+                               RFKILL_TYPE_WWAN, &oaktrail_rfkill_ops,
+                               (void *) OT_EC_WWAN_MASK);
+       if (!wwan_rfkill) {
+               ret = -ENOMEM;
+               goto err_allocate_wwan;
+       }
+       ret = rfkill_register(wwan_rfkill);
+       if (ret)
+               goto err_register_wwan;
+
+       return 0;
+
+err_register_wwan:
+       rfkill_destroy(wwan_rfkill);
+err_allocate_wwan:
+       rfkill_unregister(gps_rfkill);
+err_register_gps:
+       rfkill_destroy(gps_rfkill);
+err_allocate_gps:

drivers/platform/x86/intel_oaktrail.c new file +98 -0

+static int __devexit oaktrail_remove(struct platform_device *pdev)
+{
+       sysfs_remove_group(&pdev->dev.kobj, &oaktrail_attribute_group);
+
+       return 0;
+}
+
+static struct platform_driver oaktrail_driver = {
+       .driver = {
+               .name = DRIVER_NAME,
+               .owner = THIS_MODULE,
+       },
+       .probe  = oaktrail_probe,
+       .remove = __devexit_p(oaktrail_remove)
+};
+
+static int dmi_check_cb(const struct dmi_system_id *id)
+{
+       pr_info("Identified model '%s'\n", id->ident);
+       return 0;
+}
+
+static struct dmi_system_id __initdata oaktrail_dmi_table[] = {
+       {
+               .ident = "OakTrail platform",
+               .matches = {
+                       DMI_MATCH(DMI_PRODUCT_NAME, "OakTrail platform"),
+               },
+               .callback = dmi_check_cb
+       },
+       { }
+};
+
+static int __init oaktrail_init(void)
+{
+       int ret;
+
+       if (acpi_disabled) {
+               pr_err("ACPI needs to be enabled for this driver to work!\n");
+               return -ENODEV;
+       }
+
+       if (!force && !dmi_check_system(oaktrail_dmi_table)) {
+               pr_err("Platform not recognized (You could try the module's force-parameter)");
+               return -ENODEV;
+       }
+
+       ret = platform_driver_register(&oaktrail_driver);
+       if (ret) {
+               pr_warning("Unable to register platform driver\n");
+               goto err_driver_reg;
+       }
+
+       oaktrail_device = platform_device_alloc(DRIVER_NAME, -1);
+       if (!oaktrail_device) {
+               pr_warning("Unable to allocate platform device\n");
+               ret = -ENOMEM;
+               goto err_device_alloc;
+       }
+
+       ret = platform_device_add(oaktrail_device);
+       if (ret) {
+               pr_warning("Unable to add platform device\n");
+               goto err_device_add;
+       }
+
+       ret = setup_rfkill();
+       if (ret) {
+               pr_warning("Setup rfkill failed\n");
+               goto err_rfkill;
+       }
+
+       pr_info("Driver "DRIVER_VERSION" successfully loaded\n");
+       return 0;
+
+err_rfkill:
+       platform_device_del(oaktrail_device);
+err_device_add:
+       platform_device_put(oaktrail_device);
+err_device_alloc:
+       platform_driver_unregister(&oaktrail_driver);
+err_driver_reg:
+       return ret;
+}
+
+static void __exit oaktrail_cleanup(void)
+{
+       platform_device_unregister(oaktrail_device);
+       platform_driver_unregister(&oaktrail_driver);
+       rfkill_unregister(wifi_rfkill);
+       rfkill_unregister(bt_rfkill);
+       rfkill_unregister(gps_rfkill);
+       rfkill_unregister(wwan_rfkill);
+       rfkill_destroy(wifi_rfkill);
+       rfkill_destroy(bt_rfkill);
+       rfkill_destroy(gps_rfkill);
+       rfkill_destroy(wwan_rfkill);
+

Hi,

drivers/platform/x86/intel_oaktrail.c line 1

Quoted 2 lines

drivers/platform/x86/intel_oaktrail.c new file +1 -0

@@ -0,0 +1,349 @@
+/*-*-linux-c-*-*/

I don't know what's our general policy about that, but I don't think each text editor should be allowed to add its own header on each files. Most of the time you can configure your editor to set the right indent style based on the path of the file or something like that.

drivers/platform/x86/intel_oaktrail.c line 38

Quoted 3 lines

drivers/platform/x86/intel_oaktrail.c new file +3 -0

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

drivers/platform/x86/intel_oaktrail.c line 41

Quoted 3 lines

drivers/platform/x86/intel_oaktrail.c new file +3 -0

+ * camera - Camera subsystem enabled: contains either 0 or 1. (rw)
+ * bluetooth - Bluetooth subsystem enabled: contains either 0 or 1. (rw)
+ * touchscreen - Touchscreen subsystem enabled: contains either 0 or 1. (ro)

This should be in Documentation/ABI/testing/

drivers/platform/x86/intel_oaktrail.c line 94

Quoted 6 lines

drivers/platform/x86/intel_oaktrail.c new file +6 -0

+
+static struct platform_device *oaktrail_device;
+static struct rfkill *bt_rfkill;
+static struct rfkill *gps_rfkill;
+static struct rfkill *wifi_rfkill;
+static struct rfkill *wwan_rfkill;

Here you could create two (four ?) helpers that contains the logic, and craft dummy functions which only call the helpers with the right parameters in your macros.

These helpers could also be used by later functions.

drivers/platform/x86/intel_oaktrail.c line 167

Quoted 1 line

drivers/platform/x86/intel_oaktrail.c new file +1 -0

+static int setup_rfkill(void)

oaktrail_rfkill_init() ?

Quoted 6 lines

drivers/platform/x86/intel_oaktrail.c new file +6 -0

+       rfkill_destroy(wwan_rfkill);
+err_allocate_wwan:
+       rfkill_unregister(gps_rfkill);
+err_register_gps:
+       rfkill_destroy(gps_rfkill);
+err_allocate_gps:

drivers/platform/x86/intel_oaktrail.c line 232

Quoted 10 lines

drivers/platform/x86/intel_oaktrail.c new file +10 -0

+       rfkill_unregister(bt_rfkill);
+err_register_bt:
+       rfkill_destroy(bt_rfkill);
+err_allocate_bt:
+       rfkill_unregister(wifi_rfkill);
+err_register_wifi:
+       rfkill_destroy(wifi_rfkill);
+
+       return ret;
+}

Here I'd write an helper function to call rfkill_alloc, rfkill_register and handle
rfkill_register failure. And then, if any of the helper calls fail, just call oaktrail_rfkill_exit (which which rfkill if the rfkill pointer is NULL or not).

oaktrail_rfkill_exit could also be used in the module exit function.

drivers/platform/x86/intel_oaktrail.c line 240

Quoted 7 lines

drivers/platform/x86/intel_oaktrail.c new file +7 -0

+static int __devinit oaktrail_probe(struct platform_device *pdev)
+{
+       int err;
+
+       err = sysfs_create_group(&pdev->dev.kobj, &oaktrail_attribute_group);
+       return err;
+}

return sysfs_create_group(&pdev->dev.kobj, &oaktrail_attribute_group);

we don't really need err right now, do we ?

drivers/platform/x86/intel_oaktrail.c line 349

Quoted 4 lines

drivers/platform/x86/intel_oaktrail.c new file +4 -0

+MODULE_AUTHOR("Yin Kangkai ([email protected])");
+MODULE_DESCRIPTION("Intel Oaktrail Platform ACPI Extras");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL");

Maybe you could add some MODULE_ALIAS("dmi:xxxxx") lines to enable module autoloading ?

Quoted 1 line · Corentin Chary

Hi,

Thanks for the review and comments.

Quoted 7 lines

drivers/platform/x86/intel_oaktrail.c line 1

Quoted 2 lines

drivers/platform/x86/intel_oaktrail.c new file +1 -0

@@ -0,0 +1,349 @@
+/*-*-linux-c-*-*/

I don't know what's our general policy about that, but I don't think each text editor should be allowed to add its own header on each files. Most of the time you can configure your editor to set the right indent style based on the path of the file or something like that.

Yes, I agree with you, will remove that.

Quoted 4 lines

Quoted 3 lines

drivers/platform/x86/intel_oaktrail.c new file +3 -0

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Quoted 2 lines

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

Provide a alternative way to enable/disable these devices, and also for debugging. Does that make any sense?

Quoted 6 lines

drivers/platform/x86/intel_oaktrail.c line 41

Quoted 4 lines

drivers/platform/x86/intel_oaktrail.c new file +3 -0

+ * camera - Camera subsystem enabled: contains either 0 or 1. (rw)
+ * bluetooth - Bluetooth subsystem enabled: contains either 0 or 1. (rw)
+ * touchscreen - Touchscreen subsystem enabled: contains either 0 or 1. (ro)

This should be in Documentation/ABI/testing/

Should I prepare the document now and submit also?

Quoted 11 lines

drivers/platform/x86/intel_oaktrail.c line 94

Quoted 6 lines

drivers/platform/x86/intel_oaktrail.c new file +6 -0

+
+static struct platform_device *oaktrail_device;
+static struct rfkill *bt_rfkill;
+static struct rfkill *gps_rfkill;
+static struct rfkill *wifi_rfkill;
+static struct rfkill *wwan_rfkill;

Here you could create two (four ?) helpers that contains the logic, and craft dummy functions which only call the helpers with the right parameters in your macros.

Quoted 2 lines

These helpers could also be used by later functions.

I will try to define some micros..

Quoted 3 lines

drivers/platform/x86/intel_oaktrail.c line 167

Quoted 1 line

drivers/platform/x86/intel_oaktrail.c new file +1 -0

+static int setup_rfkill(void)

oaktrail_rfkill_init() ?

Sure.

Quoted 22 lines

Quoted 6 lines

drivers/platform/x86/intel_oaktrail.c new file +6 -0

+       rfkill_destroy(wwan_rfkill);
+err_allocate_wwan:
+       rfkill_unregister(gps_rfkill);
+err_register_gps:
+       rfkill_destroy(gps_rfkill);
+err_allocate_gps:

drivers/platform/x86/intel_oaktrail.c line 232

Quoted 10 lines

drivers/platform/x86/intel_oaktrail.c new file +10 -0

+       rfkill_unregister(bt_rfkill);
+err_register_bt:
+       rfkill_destroy(bt_rfkill);
+err_allocate_bt:
+       rfkill_unregister(wifi_rfkill);
+err_register_wifi:
+       rfkill_destroy(wifi_rfkill);
+
+       return ret;
+}

Here  I'd write an helper function to call rfkill_alloc, rfkill_register and handle
rfkill_register failure. And then, if any of the helper calls fail, just call oaktrail_rfkill_exit (which which rfkill if the rfkill pointer is NULL or not).

Quoted 1 line

oaktrail_rfkill_exit could also be used in the module exit function.

Yes, will try to do that.

Quoted 11 lines

drivers/platform/x86/intel_oaktrail.c line 240

Quoted 7 lines

drivers/platform/x86/intel_oaktrail.c new file +7 -0

+static int __devinit oaktrail_probe(struct platform_device *pdev)
+{
+       int err;
+
+       err = sysfs_create_group(&pdev->dev.kobj, &oaktrail_attribute_group);
+       return err;
+}

return sysfs_create_group(&pdev->dev.kobj, &oaktrail_attribute_group);

we don't really need err right now, do we ?

Will change this.

Quoted 7 lines

drivers/platform/x86/intel_oaktrail.c line 349

Quoted 4 lines

drivers/platform/x86/intel_oaktrail.c new file +4 -0

+MODULE_AUTHOR("Yin Kangkai ([email protected])");
+MODULE_DESCRIPTION("Intel Oaktrail Platform ACPI Extras");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL");

Maybe you could add some MODULE_ALIAS("dmi:xxxxx") lines to enable module autoloading ?

Will try to.

Thanks for the review.

Regards,
Kangkai

Quoted 3 lines

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

Quoted 3 lines

+ * camera - Camera subsystem enabled: contains either 0 or 1. (rw)
+ * bluetooth - Bluetooth subsystem enabled: contains either 0 or 1. (rw)
+ * touchscreen - Touchscreen subsystem enabled: contains either 0 or 1. (ro)

Quoted 6 lines · Corentin Chary

Quoted 3 lines

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

This is not the same functionality.

enable/disable using files in /sys/devices/platform will completely cut power of those components. For example, if disabled, you can not list out those components using lsusb. Can we achieve this using rfkill?

Quoted 8 lines · Yin Kangkai

Quoted 6 lines · Corentin Chary

Quoted 3 lines

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

Quoted 7 lines

This is not the same functionality.

enable/disable using files in /sys/devices/platform will completely cut power of those components. For example, if disabled, you can not list out those components using lsusb. Can we achieve this using rfkill?

oh, sorry then, I didn't understood that.

So, you're lucky, you can control both the device and the radio. I'm not sure what's
the best thing to do here, you can probably keep these files here.

CCing Matthew, he may have something to say about that.

Hi Yin Kangkai,

於 四，2011-01-06 於 11:54 +0100，Corentin Chary 提到：

Quoted 18 lines

Quoted 8 lines · Yin Kangkai

Quoted 6 lines · Corentin Chary

Quoted 3 lines

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

Quoted 7 lines

This is not the same functionality.

enable/disable using files in /sys/devices/platform will completely cut power of those components. For example, if disabled, you can not list out those components using lsusb. Can we achieve this using rfkill?

Quoted 9 lines

oh, sorry then, I didn't understood that.

So, you're lucky, you can control both the device and the radio. I'm not sure what's
the best thing to do here, you can probably keep these files here.

CCing Matthew, he may have something to say about that.

We should used rfkill to provide interface to userland for enable/disable wlan/bluetooth/wwan. You can set the killswitch state to soft block when your driver call EC function to disable devices.

Udev/HAL will forward the killswitch change event to userland, then userland application can do some response, e.g. show up OSD. And, userland app also can control killswitch by /dev/rfkill.

There also have a urfkill daemon to provide DBus to userland app for control killswitch:
http://freedesktop.org/wiki/Software/urfkill

Use rfkill interface is more standard then create special sysfs file in /sys/devices/platform/intel_oaktrail.

Hi Yin Kangkai,

於 四，2011-01-06 於 11:54 +0100，Corentin Chary 提到：

Quoted 16 lines

Quoted 14 lines · Yin Kangkai

Quoted 6 lines · Corentin Chary

Quoted 3 lines

+ * gps - GPS subsystem enabled: contains either 0 or 1. (rw)
+ * wifi - WiFi subsystem enabled: contains either 0 or 1. (rw)
+ * wwan - WWAN (3G) subsystem enabled: contains either 0 or 1. (rw)

Is there a reason do add these files in /sys/devices/platform while the functionality is already provided by rfkill ?

This is not the same functionality.

enable/disable using files in /sys/devices/platform will completely cut power of those components. For example, if disabled, you can not list out those components using lsusb. Can we achieve this using rfkill?

Quoted 7 lines

oh, sorry then, I didn't understood that.

So, you're lucky, you can control both the device and the radio. I'm not sure what's
the best thing to do here, you can probably keep these files here.

CCing Matthew, he may have something to say about that.

We should used rfkill to provide interface to userland for enable/disable wlan/bluetooth/wwan. You can set the killswitch state to soft block when your driver call EC function to disable devices.

Udev/HAL will forward the killswitch change event to userland, then userland application can do some response, e.g. show up OSD. And, userland app also can control killswitch by /dev/rfkill.

There also have a urfkill daemon to provide DBus to userland app for control killswitch:
http://freedesktop.org/wiki/Software/urfkill

Use rfkill interface is more standard then create special sysfs file in /sys/devices/platform/intel_oaktrail.

Patch V2
Changes from V1:
 - remove wifi, bt, gps, wwan from /sys/devices/platform.
 - add some helper function for rfkill alloc and rfkill cleanup.
 - add MODULE_ALIAS.
 - add Documentation/ABI/testing/sysfs-platform-intel-oaktrail
 - various other minor changes.
 - bump driver version from 0.1 to 0.2

Thanks Corentin Chary and Joey Lee for the review.

From ab28d41e89b40e0ab1d73489b7d1a9f6e745c14a Mon Sep 17 00:00:00 2001
From: Yin Kangkai <[email protected]>
Date: Wed, 22 Dec 2010 10:53:36 +0800
Subject: [PATCH] platform-driver-x86: ACPI EC Extra driver for Oaktrail

This driver implements an Extra ACPI EC driver for products based on Intel Oaktrail platform.  It is programming the EC space, through existing ACPI EC driver, to provide user space layer the sysfs and rfkill interfaces to enable/disable the Camera, Bluetooth, GPS, WiFi, 3G, and to show the status of Touchscreen.

Signed-off-by Yin Kangkai <[email protected]>

---
 .../ABI/testing/sysfs-platform-intel-oaktrail      |   13 + drivers/platform/x86/Kconfig                       |    9 + drivers/platform/x86/Makefile                      |    1 + drivers/platform/x86/intel_oaktrail.c              |  347 ++++++++++++++++++++

 4 files changed, 370 insertions(+), 0 deletions(-)

 create mode 100644 Documentation/ABI/testing/sysfs-platform-intel-oaktrail create mode 100644 drivers/platform/x86/intel_oaktrail.c

Documentation/ABI/testing/sysfs-platform-intel-oaktrail new file +13 -0

new file mode 100644
index 0000000..0512235
@@ -0,0 +1,13 @@
+What:          /sys/devices/platform/intel_oaktrail/camera
+Date:          Jan 2011
+KernelVersion: 2.6.37
+Contact:       "Yin Kangkai" <[email protected]>
+Description:
+               Control the camera. 1 means on, 0 means off.
+
+What:          /sys/devices/platform/intel_oaktrail/touchscreen
+Date:          Jan 2011
+KernelVersion: 2.6.37
+Contact:       "Yin Kangkai" <[email protected]>
+Description:
+               Show the status of the touch screen. 1 means on, 0 means off.

drivers/platform/x86/Kconfig +0 -0

index 2b4038a..d1f6981 100644
@@ -655,4 +655,13 @@ config XO1_RFKILL

         Support for enabling/disabling the WLAN interface on the OLPC XO-1 laptop.

drivers/platform/x86/Kconfig +9 -0

+config INTEL_OAKTRAIL
+       tristate "Intel Oaktrail Platform Extras"
+       depends on ACPI
+       depends on RFKILL
+       ---help---
+         Intel Oaktrail platform need this driver to provide interfaces to
+         enable/disable the Camera, WiFi, BT etc. devices. If in doubt, say Y
+         here; it will only load on supported platforms.
+

 endif # X86_PLATFORM_DEVICES

drivers/platform/x86/Makefile +0 -0

index 7ff60e6..add8ab7 100644
@@ -35,3 +35,4 @@ obj-$(CONFIG_INTEL_IPS)               += intel_ips.o

 obj-$(CONFIG_GPIO_INTEL_PMIC)  += intel_pmic_gpio.o obj-$(CONFIG_XO1_RFKILL)       += xo1-rfkill.o obj-$(CONFIG_IBM_RTL)          += ibm_rtl.o

drivers/platform/x86/Makefile +1 -0

+obj-$(CONFIG_INTEL_OAKTRAIL)   += intel_oaktrail.o

drivers/platform/x86/intel_oaktrail.c new file +156 -0

new file mode 100644
index 0000000..9ec71c5
@@ -0,0 +1,347 @@
+/*
+  Copyright (C) 2010 Intel Corporation
+  Author: Yin Kangkai ([email protected])
+
+  based on Compal driver
+
+  Copyright (C) 2008 Cezary Jackiewicz <cezary.jackiewicz (at) gmail.com>
+
+  based on MSI driver
+
+  Copyright (C) 2006 Lennart Poettering <mzxreary (at) 0pointer (dot) de>
+
+  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.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+  02110-1301, USA.
+ */
+
+/*
+ * intel_oaktrail.c - Intel OakTrail Platform support.
+ *
+ * This driver exports a few files in /sys/devices/platform/intel_oaktrail/:
+ *
+ * camera - Camera subsystem enabled: contains either 0 or 1. (rw)
+ * touchscreen - Touchscreen subsystem enabled: contains either 0 or 1. (ro)
+ *
+ * In addition to these platform device attributes, the driver registers itself
+ * in the Linux rfkill subsystem for these components:
+ *
+ * wifi
+ * bluetooth
+ * wwan (3g)
+ * gps
+ *
+ * and is available to userspace under /sys/class/rfkill/rfkillX/
+ *
+ * This driver might work on other products based on Oaktrail. If you
+ * want to try it you can pass force=1 as argument to the module which
+ * will force it to load even when the DMI data doesn't identify the
+ * product as compatible.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/dmi.h>
+#include <linux/rfkill.h>
+
+#define DRIVER_NAME    "intel_oaktrail"
+#define DRIVER_VERSION "0.2"
+
+/*
+ * This is the devices status address in EC space, and the control bits
+ * definition:
+ *
+ * (1 << 0):   Camera enable/disable, RW.
+ * (1 << 1):   Bluetooth enable/disable, RW.
+ * (1 << 2):   GPS enable/disable, RW.
+ * (1 << 3):   WiFi enable/disable, RW.
+ * (1 << 4):   WWAN (3G) enable/disalbe, RW.
+ * (1 << 5):   Touchscreen enable/disable, Read Only.
+ */
+#define OT_EC_DEVICE_STATE_ADDRESS     0xD6
+
+#define OT_EC_CAMERA_MASK      (1 << 0)
+#define OT_EC_BT_MASK          (1 << 1)
+#define OT_EC_GPS_MASK         (1 << 2)
+#define OT_EC_WIFI_MASK                (1 << 3)
+#define OT_EC_WWAN_MASK                (1 << 4)
+#define OT_EC_TS_MASK          (1 << 5)
+
+static int force;
+module_param(force, bool, 0);
+MODULE_PARM_DESC(force, "Force driver load, ignore DMI data");
+
+static struct platform_device *oaktrail_device;
+static struct rfkill *bt_rfkill;
+static struct rfkill *gps_rfkill;
+static struct rfkill *wifi_rfkill;
+static struct rfkill *wwan_rfkill;
+
+#define SIMPLE_MASKED_STORE_SHOW(NAME, MASK)                           \
+static ssize_t NAME##_show(struct device *dev,                         \
+       struct device_attribute *attr, char *buf)                       \
+{                                                                      \
+       u8 value;                                                       \
+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &value);                    \
+       return sprintf(buf, "%d\n", ((value & MASK) != 0));             \
+}                                                                      \
+static ssize_t NAME##_store(struct device *dev,                                \
+       struct device_attribute *attr, const char *buf, size_t count)   \
+{                                                                      \
+       int state;                                                      \
+       u8 old_val;                                                     \
+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &old_val);                  \
+       if (sscanf(buf, "%d", &state) != 1 || (state < 0 || state > 1)) \
+               return -EINVAL;                                         \
+       ec_write(OT_EC_DEVICE_STATE_ADDRESS, state ?                    \
+                (old_val | MASK) : (old_val & ~MASK));                 \
+       return count;                                                   \
+}
+
+SIMPLE_MASKED_STORE_SHOW(camera, OT_EC_CAMERA_MASK)
+SIMPLE_MASKED_STORE_SHOW(touchscreen, OT_EC_TS_MASK)
+
+static DEVICE_ATTR(camera, 0644, camera_show, camera_store);
+static DEVICE_ATTR(touchscreen, 0444, touchscreen_show, NULL);
+
+static struct attribute *oaktrail_attributes[] = {
+       &dev_attr_camera.attr,
+       &dev_attr_touchscreen.attr,
+       NULL
+};
+
+static struct attribute_group oaktrail_attribute_group = {
+       .attrs = oaktrail_attributes
+};
+
+static int oaktrail_rfkill_set(void *data, bool blocked)
+{
+       u8 value;
+       u8 result;
+       unsigned long radio = (unsigned long) data;
+
+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &result);
+
+       if (!blocked)
+               value = (u8) (result | radio);
+       else
+               value = (u8) (result & ~radio);
+
+       ec_write(OT_EC_DEVICE_STATE_ADDRESS, value);
+
+       return 0;
+}
+
+static const struct rfkill_ops oaktrail_rfkill_ops = {
+       .set_block = oaktrail_rfkill_set,
+};
+
+static struct rfkill *oaktrail_rfkill_new(char *name, enum rfkill_type type,
+                                         unsigned long mask)

drivers/platform/x86/intel_oaktrail.c new file +156 -1

+}
+
+static int oaktrail_rfkill_init(void)
+{
+       int ret;
+
+       wifi_rfkill = oaktrail_rfkill_new("oaktrail-wifi",
+                                         RFKILL_TYPE_WLAN,
+                                         OT_EC_WIFI_MASK);
+       if (IS_ERR(wifi_rfkill)) {
+               ret = PTR_ERR(wifi_rfkill);
+               wifi_rfkill = NULL;
+               goto cleanup;
+       }
+
+       bt_rfkill = oaktrail_rfkill_new("oaktrail-bluetooth",
+                                       RFKILL_TYPE_BLUETOOTH,
+                                       OT_EC_BT_MASK);
+       if (IS_ERR(bt_rfkill)) {
+               ret = PTR_ERR(bt_rfkill);
+               bt_rfkill = NULL;
+               goto cleanup;
+       }
+
+       gps_rfkill = oaktrail_rfkill_new("oaktrail-gps",
+                                        RFKILL_TYPE_GPS,
+                                        OT_EC_GPS_MASK);
+       if (IS_ERR(gps_rfkill)) {
+               ret = PTR_ERR(gps_rfkill);
+               gps_rfkill = NULL;
+               goto cleanup;
+       }
+
+       wwan_rfkill = oaktrail_rfkill_new("oaktrail-wwan",
+                                         RFKILL_TYPE_WWAN,
+                                         OT_EC_WWAN_MASK);
+       if (IS_ERR(wwan_rfkill)) {
+               ret = PTR_ERR(wwan_rfkill);
+               wwan_rfkill = NULL;
+               goto cleanup;
+       }
+
+       return 0;
+
+cleanup:
+       oaktrail_rfkill_cleanup();
+       return ret;
+}
+
+static int __devinit oaktrail_probe(struct platform_device *pdev)
+{
+       return sysfs_create_group(&pdev->dev.kobj, &oaktrail_attribute_group);
+}
+
+static int __devexit oaktrail_remove(struct platform_device *pdev)
+{
+       sysfs_remove_group(&pdev->dev.kobj, &oaktrail_attribute_group);
+
+       return 0;
+}
+
+static struct platform_driver oaktrail_driver = {
+       .driver = {
+               .name = DRIVER_NAME,
+               .owner = THIS_MODULE,
+       },
+       .probe  = oaktrail_probe,
+       .remove = __devexit_p(oaktrail_remove)
+};
+
+static int dmi_check_cb(const struct dmi_system_id *id)
+{
+       pr_info("Identified model '%s'\n", id->ident);
+       return 0;
+}
+
+static struct dmi_system_id __initdata oaktrail_dmi_table[] = {
+       {
+               .ident = "OakTrail platform",
+               .matches = {
+                       DMI_MATCH(DMI_PRODUCT_NAME, "OakTrail platform"),
+               },
+               .callback = dmi_check_cb
+       },
+       { }
+};
+
+static int __init oaktrail_init(void)
+{
+       int ret;
+
+       if (acpi_disabled) {
+               pr_err("ACPI needs to be enabled for this driver to work!\n");
+               return -ENODEV;
+       }
+
+       if (!force && !dmi_check_system(oaktrail_dmi_table)) {
+               pr_err("Platform not recognized (You could try the module's force-parameter)");
+               return -ENODEV;
+       }
+
+       ret = platform_driver_register(&oaktrail_driver);
+       if (ret) {
+               pr_warning("Unable to register platform driver\n");
+               goto err_driver_reg;
+       }
+
+       oaktrail_device = platform_device_alloc(DRIVER_NAME, -1);
+       if (!oaktrail_device) {
+               pr_warning("Unable to allocate platform device\n");
+               ret = -ENOMEM;
+               goto err_device_alloc;
+       }
+
+       ret = platform_device_add(oaktrail_device);
+       if (ret) {
+               pr_warning("Unable to add platform device\n");
+               goto err_device_add;
+       }
+
+       ret = oaktrail_rfkill_init();
+       if (ret) {
+               pr_warning("Setup rfkill failed\n");
+               goto err_rfkill;
+       }
+
+       pr_info("Driver "DRIVER_VERSION" successfully loaded\n");
+       return 0;
+
+err_rfkill:
+       platform_device_del(oaktrail_device);
+err_device_add:
+       platform_device_put(oaktrail_device);
+err_device_alloc:
+       platform_driver_unregister(&oaktrail_driver);
+err_driver_reg:
+       return ret;
+}
+
+static void __exit oaktrail_cleanup(void)
+{
+       platform_device_unregister(oaktrail_device);
+       platform_driver_unregister(&oaktrail_driver);
+       oaktrail_rfkill_cleanup();
+
+       pr_info("Driver unloaded\n");
+}
+
+module_init(oaktrail_init);
+module_exit(oaktrail_cleanup);
+
+MODULE_AUTHOR("Yin Kangkai ([email protected])");
+MODULE_DESCRIPTION("Intel Oaktrail Platform ACPI Extras");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("dmi:*:svnIntelCorporation:pnOakTrailplatform:*");
--

1.6.5

Quoted 7 lines

drivers/platform/x86/intel_oaktrail.c new file +7 -0

+static struct rfkill *oaktrail_rfkill_new(char *name, enum rfkill_type type,
+                                         unsigned long mask)
+{
+       int err;
+       struct rfkill *rfkill_dev;
+
+       rfkill_dev = rfkill_alloc(name, &oaktrail_device->dev, type,

Quoted 5 lines

+                                 &oaktrail_rfkill_ops, (void *)mask);
+       if (!rfkill_dev)
+               return ERR_PTR(-ENOMEM);
+
+       err = rfkill_register(rfkill_dev);

Maybe you should add a rfkill_init_sw_state() call here, to be sure that the rfkill initial state is right.

Quoted 7 lines

+       if (err) {
+               rfkill_destroy(rfkill_dev);
+               return ERR_PTR(err);
+       }
+       return rfkill_dev;

(quoted diff) +2 -0

+}
+

Quoted 7 lines · Corentin Chary

Quoted 7 lines

drivers/platform/x86/intel_oaktrail.c new file +7 -0

+static struct rfkill *oaktrail_rfkill_new(char *name, enum rfkill_type type,
+                                         unsigned long mask)
+{
+       int err;
+       struct rfkill *rfkill_dev;
+
+       rfkill_dev = rfkill_alloc(name, &oaktrail_device->dev, type,

Quoted 8 lines

Quoted 5 lines

+                                 &oaktrail_rfkill_ops, (void *)mask);
+       if (!rfkill_dev)
+               return ERR_PTR(-ENOMEM);
+
+       err = rfkill_register(rfkill_dev);

Maybe you should add a rfkill_init_sw_state() call here, to be sure that the rfkill initial state is right.

Yes, agree with you, and I'm trying to do that, add a rfkill_init_sw_state between rfkill_alloc() and rfkill_register()...

(quoted diff) +2 -0

+       ec_read(OT_EC_DEVICE_STATE_ADDRESS, &value);
+       rfkill_init_sw_state(rfkill_dev, (value & mask) != 1);

However, test shows the EC seems not ready yet when booting runs here (value = 0), and then all the radio set as "blocked". Hence the radio state can't be persistent during reboot.. This is not we want.

Also tried to read out the EC address a little bit later, seems the EC won't be ready until very late.

Am I doing something wrong? or I need to check with HW guy about this.

This driver implements an Extra ACPI EC driver for products based on Intel Oaktrail platform. It is programming the EC space, through existing ACPI EC driver, to provide user space layer the sysfs and rfkill interfaces to enable/disable the Camera, Bluetooth, GPS, WiFi, 3G, and to show the status of Touchscreen.

Quoted 5 lines · Yin Kangkai

This driver implements an Extra ACPI EC driver for products based on Intel Oaktrail platform. It is programming the EC space, through existing ACPI EC driver, to provide user space layer the sysfs and rfkill interfaces to enable/disable the Camera, Bluetooth, GPS, WiFi, 3G, and to show the status of Touchscreen.

As far as I can tell, the only ACPI aspect of this driver is that it makes use of the ACPI EC driver? In that case I think the name's misleading.

Documentation/ABI/testing/sysfs-platform-intel-oaktrail line 6

Quoted 7 lines

Documentation/ABI/testing/sysfs-platform-intel-oaktrail new file +6 -0

@@ -0,0 +1,13 @@
+What:		/sys/devices/platform/intel_oaktrail/camera
+Date:		Jan 2011
+KernelVersion:	2.6.37
+Contact:	"Yin Kangkai" <[email protected]>
+Description:
+		Control the camera. 1 means on, 0 means off.

We really need a better approach for this kind of thing...

Documentation/ABI/testing/sysfs-platform-intel-oaktrail line 13

Quoted 6 lines

Documentation/ABI/testing/sysfs-platform-intel-oaktrail new file +6 -0

+What:		/sys/devices/platform/intel_oaktrail/touchscreen
+Date:		Jan 2011
+KernelVersion:	2.6.37
+Contact:	"Yin Kangkai" <[email protected]>
+Description:
+		Show the status of the touch screen. 1 means on, 0 means off.

And this one, but we don't have one yet so I think this is ok.

drivers/platform/x86/intel_oaktrail.c line 227

Quoted 3 lines

drivers/platform/x86/intel_oaktrail.c new file +3 -0

+	wwan_rfkill = oaktrail_rfkill_new("oaktrail-wwan",
+					  RFKILL_TYPE_WWAN,
+					  OT_EC_WWAN_MASK);

Do you have any way to identify whether the hardware has all these features before registering?

Quoted 5 lines

drivers/platform/x86/intel_oaktrail.c new file +5 -0

+static struct dmi_system_id __initdata oaktrail_dmi_table[] = {
+	{
+		.ident = "OakTrail platform",
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "OakTrail platform"),

drivers/platform/x86/intel_oaktrail.c line 277

Quoted 5 lines

drivers/platform/x86/intel_oaktrail.c new file +5 -0

+		},
+		.callback = dmi_check_cb
+	},
+	{ }
+};

Is any hardware actually going to ship with this identifier, or will vendors change the string?

Quoted 2 lines

Is any hardware actually going to ship with this identifier, or will vendors change the string?

I think vendors and OEMs will change this string. But we can add the DMI_BOARD_NAME, DMI_BOARD_VERSION, etc. back later, can we?