[PATCH 0/4] asus-laptop: Pegatron Lucid (WeTab/ExoPC) support

[PATCH 0/4] asus-laptop: Pegatron Lucid (WeTab/ExoPC) support

[Andy Ross]

Accelerometer and ambient light sensor drivers for the Pegatron Lucid
(sold as WeTab and ExoPC) tablet PC.

This was submitted a while back, but got dropped before merge as we
were distracted by other projects.  The last change requested was to
fold the accelerometer driver into asus-laptop, which has been done
here.

Unfortunately there is one new wart: doing so causes initialization to
occur earlier, and we were seeing common panics underneath ACPI
initialization.  So I moved the accelerometer initialization out of
the ACPI startup call tree and into a separate platform driver (as it
was earlier when it was a separate module), which helped some.  But
occasional panics persisted, which I eventually tracked down to the
very first accelerometer reads.

Faking this initial report (which I *think* is being requested out of
the input polldev layer, not userspace) resulted in stable behavior.
This code has been running on a bunch of development systems for
several months now, so I feel as comfortable with the voodoo as I
think I can.

[PATCH 1/4] asus-laptop: Platform detection for Pegatron Lucid

[Andy Ross]

Recognize the Pegatron Lucid tablets by their method signatures.

Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
 drivers/platform/x86/Kconfig       |   13 ++++++++-----
 drivers/platform/x86/asus-laptop.c |   23 ++++++++++++++++++++---
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index faec777..b6f983e 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -62,12 +62,15 @@ config ASUS_LAPTOP
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
 	select INPUT_SPARSEKMAP
+	select INPUT_POLLDEV
 	---help---
-	  This is the new Linux driver for Asus laptops. It may also support some
-	  MEDION, JVC or VICTOR laptops. It makes all the extra buttons generate
-	  standard ACPI events and input events. It also adds
-	  support for video output switching, LCD backlight control, Bluetooth and
-	  Wlan control, and most importantly, allows you to blink those fancy LEDs.
+	  This is a driver for Asus laptops and the Pegatron Lucid
+	  tablet. It may also support some MEDION, JVC or VICTOR
+	  laptops. It makes all the extra buttons generate standard
+	  ACPI events and input events. It also adds support for video
+	  output switching, LCD backlight control, Bluetooth and Wlan
+	  control, and most importantly, allows you to blink those
+	  fancy LEDs.
 
 	  For more information and a userspace daemon for handling the extra
 	  buttons see <http://acpi4asus.sf.net>.
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index d235f44..ec46d71 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2002-2005 Julien Lerouge, 2003-2006 Karol Kozimor
  *  Copyright (C) 2006-2007 Corentin Chary
+ *  Copyright (C) 2011 Wind River Systems
  *
  *  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
@@ -48,6 +49,7 @@
 #include <linux/uaccess.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/input-polldev.h>
 #include <linux/rfkill.h>
 #include <linux/slab.h>
 #include <acpi/acpi_drivers.h>
@@ -210,6 +212,12 @@ static char *display_get_paths[] = {
 #define METHOD_KBD_LIGHT_SET	"SLKB"
 #define METHOD_KBD_LIGHT_GET	"GLKB"
 
+/* For Pegatron Lucid tablet */
+#define DEVICE_NAME_PEGA	"Lucid"
+#define METHOD_PEGA_ENABLE	"ENPR"
+#define METHOD_PEGA_DISABLE	"DAPR"
+#define METHOD_PEGA_READ	"RDLN"
+
 /*
  * Define a specific led structure to keep the main structure clean
  */
@@ -246,6 +254,7 @@ struct asus_laptop {
 
 	int wireless_status;
 	bool have_rsts;
+	bool have_pega_lucid;
 	int lcd_state;
 
 	struct rfkill *gps_rfkill;
@@ -361,6 +370,14 @@ static int acpi_check_handle(acpi_handle handle, const char *method,
 	return 0;
 }
 
+static bool asus_check_pega_lucid(struct asus_laptop *asus)
+{
+	return !strcmp(asus->name, DEVICE_NAME_PEGA) &&
+	   !acpi_check_handle(asus->handle, METHOD_PEGA_ENABLE, NULL) &&
+	   !acpi_check_handle(asus->handle, METHOD_PEGA_DISABLE, NULL) &&
+	   !acpi_check_handle(asus->handle, METHOD_PEGA_READ, NULL);
+}
+
 /* Generic LED function */
 static int asus_led_set(struct asus_laptop *asus, const char *method,
 			 int value)
@@ -1334,7 +1351,6 @@ static mode_t asus_sysfs_is_visible(struct kobject *kobj,
 		   attr == &dev_attr_ls_level.attr) {
 		supported = !acpi_check_handle(handle, METHOD_ALS_CONTROL, NULL) &&
 			    !acpi_check_handle(handle, METHOD_ALS_LEVEL, NULL);
-
 	} else if (attr == &dev_attr_gps.attr) {
 		supported = !acpi_check_handle(handle, METHOD_GPS_ON, NULL) &&
 			    !acpi_check_handle(handle, METHOD_GPS_OFF, NULL) &&
@@ -1579,9 +1595,10 @@ static int __devinit asus_acpi_add(struct acpi_device *device)
 		goto fail_platform;
 
 	/*
-	 * Register the platform device first.  It is used as a parent for the
-	 * sub-devices below.
+	 * Need platform type detection first, then the platform
+	 * device.  It is used as a parent for the sub-devices below.
 	 */
+	asus->have_pega_lucid = asus_check_pega_lucid(asus);
 	result = asus_platform_init(asus);
 	if (result)
 		goto fail_platform;
-- 
1.7.1

[PATCH 2/4] asus-laptop: Pegatron Lucid ALS sensor

[Andy Ross]

Ambient light sensor for Pegatron Lucid.  Supports pre-existing
ls_switch sysfs interface to en/disable automatic control, and exports
the brightness from the device as "ls_value".

Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
 drivers/platform/x86/asus-laptop.c |   70 +++++++++++++++++++++++++++++++++---
 1 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index ec46d71..6651d8c 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -214,9 +214,15 @@ static char *display_get_paths[] = {
 
 /* For Pegatron Lucid tablet */
 #define DEVICE_NAME_PEGA	"Lucid"
+
 #define METHOD_PEGA_ENABLE	"ENPR"
 #define METHOD_PEGA_DISABLE	"DAPR"
+#define PEGA_ALS	0x04
+#define PEGA_ALS_POWER	0x05
+
 #define METHOD_PEGA_READ	"RDLN"
+#define PEGA_READ_ALS_H	0x02
+#define PEGA_READ_ALS_L	0x03
 
 /*
  * Define a specific led structure to keep the main structure clean
@@ -378,6 +384,12 @@ static bool asus_check_pega_lucid(struct asus_laptop *asus)
 	   !acpi_check_handle(asus->handle, METHOD_PEGA_READ, NULL);
 }
 
+static int asus_pega_lucid_set(struct asus_laptop *asus, int unit, bool enable)
+{
+	char *method = enable ? METHOD_PEGA_ENABLE : METHOD_PEGA_DISABLE;
+	return write_acpi_int(asus->handle, method, unit);
+}
+
 /* Generic LED function */
 static int asus_led_set(struct asus_laptop *asus, const char *method,
 			 int value)
@@ -1046,7 +1058,15 @@ static ssize_t store_disp(struct device *dev, struct device_attribute *attr,
  */
 static void asus_als_switch(struct asus_laptop *asus, int value)
 {
-	if (write_acpi_int(asus->handle, METHOD_ALS_CONTROL, value))
+	int ret;
+	if (asus->have_pega_lucid) {
+		ret = asus_pega_lucid_set(asus, PEGA_ALS, value);
+		if (!ret)
+			ret = asus_pega_lucid_set(asus, PEGA_ALS_POWER, value);
+	} else {
+		ret = write_acpi_int(asus->handle, METHOD_ALS_CONTROL, value);
+	}
+	if (ret)
 		pr_warning("Error setting light sensor switch\n");
 	asus->light_switch = value;
 }
@@ -1103,6 +1123,35 @@ static ssize_t store_lslvl(struct device *dev, struct device_attribute *attr,
 	return rv;
 }
 
+static int pega_int_read(struct asus_laptop *asus, int arg, int *result)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	int err = write_acpi_int_ret(asus->handle, METHOD_PEGA_READ, arg,
+				     &buffer);
+	if (!err) {
+		union acpi_object *obj = buffer.pointer;
+		if (obj && obj->type == ACPI_TYPE_INTEGER)
+			*result = obj->integer.value;
+		else
+			err = -EIO;
+	}
+	return err;
+}
+
+static ssize_t show_lsvalue(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct asus_laptop *asus = dev_get_drvdata(dev);
+	int err, hi, lo;
+
+	err = pega_int_read(asus, PEGA_READ_ALS_H, &hi);
+	if (!err)
+		err = pega_int_read(asus, PEGA_READ_ALS_L, &lo);
+	if (!err)
+		return sprintf(buf, "%d\n", 10 * hi + lo);
+	return err;
+}
+
 /*
  * GPS
  */
@@ -1300,6 +1349,7 @@ static DEVICE_ATTR(wimax, S_IRUGO | S_IWUSR, show_wimax, store_wimax);
 static DEVICE_ATTR(wwan, S_IRUGO | S_IWUSR, show_wwan, store_wwan);
 static DEVICE_ATTR(display, S_IRUGO | S_IWUSR, show_disp, store_disp);
 static DEVICE_ATTR(ledd, S_IRUGO | S_IWUSR, show_ledd, store_ledd);
+static DEVICE_ATTR(ls_value, S_IRUGO, show_lsvalue, NULL);
 static DEVICE_ATTR(ls_level, S_IRUGO | S_IWUSR, show_lslvl, store_lslvl);
 static DEVICE_ATTR(ls_switch, S_IRUGO | S_IWUSR, show_lssw, store_lssw);
 static DEVICE_ATTR(gps, S_IRUGO | S_IWUSR, show_gps, store_gps);
@@ -1312,6 +1362,7 @@ static struct attribute *asus_attributes[] = {
 	&dev_attr_wwan.attr,
 	&dev_attr_display.attr,
 	&dev_attr_ledd.attr,
+	&dev_attr_ls_value.attr,
 	&dev_attr_ls_level.attr,
 	&dev_attr_ls_switch.attr,
 	&dev_attr_gps.attr,
@@ -1349,8 +1400,15 @@ static mode_t asus_sysfs_is_visible(struct kobject *kobj,
 
 	} else if (attr == &dev_attr_ls_switch.attr ||
 		   attr == &dev_attr_ls_level.attr) {
-		supported = !acpi_check_handle(handle, METHOD_ALS_CONTROL, NULL) &&
-			    !acpi_check_handle(handle, METHOD_ALS_LEVEL, NULL);
+		if (asus->have_pega_lucid) {
+			/* no ls_level interface on the Lucid */
+			supported = attr == &dev_attr_ls_switch.attr;
+		} else {
+			supported = !acpi_check_handle(handle, METHOD_ALS_CONTROL, NULL) &&
+				    !acpi_check_handle(handle, METHOD_ALS_LEVEL, NULL);
+		}
+	} else if (attr == &dev_attr_ls_value.attr) {
+		supported = asus->have_pega_lucid;
 	} else if (attr == &dev_attr_gps.attr) {
 		supported = !acpi_check_handle(handle, METHOD_GPS_ON, NULL) &&
 			    !acpi_check_handle(handle, METHOD_GPS_OFF, NULL) &&
@@ -1562,8 +1620,10 @@ static int __devinit asus_acpi_init(struct asus_laptop *asus)
 	asus->light_switch = 0;	/* Default to light sensor disabled */
 	asus->light_level = 5;	/* level 5 for sensor sensitivity */
 
-	if (!acpi_check_handle(asus->handle, METHOD_ALS_CONTROL, NULL) &&
-	    !acpi_check_handle(asus->handle, METHOD_ALS_LEVEL, NULL)) {
+	if (asus->have_pega_lucid) {
+		asus_als_switch(asus, asus->light_switch);
+	} else if (!acpi_check_handle(asus->handle, METHOD_ALS_CONTROL, NULL) &&
+		   !acpi_check_handle(asus->handle, METHOD_ALS_LEVEL, NULL)) {
 		asus_als_switch(asus, asus->light_switch);
 		asus_als_level(asus, asus->light_level);
 	}
-- 
1.7.1

[PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Andy Ross]

Support the built-in accelerometer on the Lucid tablets as a standard
3-axis input device.

Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
 drivers/platform/x86/Kconfig       |    9 ++-
 drivers/platform/x86/asus-laptop.c |  137 +++++++++++++++++++++++++++++++++++-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b6f983e..43906f5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -67,10 +67,11 @@ config ASUS_LAPTOP
 	  This is a driver for Asus laptops and the Pegatron Lucid
 	  tablet. It may also support some MEDION, JVC or VICTOR
 	  laptops. It makes all the extra buttons generate standard
-	  ACPI events and input events. It also adds support for video
-	  output switching, LCD backlight control, Bluetooth and Wlan
-	  control, and most importantly, allows you to blink those
-	  fancy LEDs.
+	  ACPI events and input events, and on the Lucid the built-in
+	  accelerometer appears as an input device.  It also adds
+	  support for video output switching, LCD backlight control,
+	  Bluetooth and Wlan control, and most importantly, allows you
+	  to blink those fancy LEDs.
 
 	  For more information and a userspace daemon for handling the extra
 	  buttons see <http://acpi4asus.sf.net>.
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 6651d8c..9b07368 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -224,6 +224,14 @@ static char *display_get_paths[] = {
 #define PEGA_READ_ALS_H	0x02
 #define PEGA_READ_ALS_L	0x03
 
+#define PEGA_ACCEL_NAME "pega_accel"
+#define PEGA_ACCEL_DESC "Pegatron Lucid Tablet Accelerometer"
+#define METHOD_XLRX "XLRX"
+#define METHOD_XLRY "XLRY"
+#define METHOD_XLRZ "XLRZ"
+#define PEGA_ACC_CLAMP 512 /* 1G accel is reported as ~256, so clamp to 2G */
+#define PEGA_ACC_RETRIES 3
+
 /*
  * Define a specific led structure to keep the main structure clean
  */
@@ -249,6 +257,7 @@ struct asus_laptop {
 
 	struct input_dev *inputdev;
 	struct key_entry *keymap;
+	struct input_polled_dev *pega_accel_poll;
 
 	struct asus_led mled;
 	struct asus_led tled;
@@ -262,6 +271,10 @@ struct asus_laptop {
 	bool have_rsts;
 	bool have_pega_lucid;
 	int lcd_state;
+	bool pega_acc_live;
+	int pega_acc_x;
+	int pega_acc_y;
+	int pega_acc_z;
 
 	struct rfkill *gps_rfkill;
 
@@ -390,6 +403,99 @@ static int asus_pega_lucid_set(struct asus_laptop *asus, int unit, bool enable)
 	return write_acpi_int(asus->handle, method, unit);
 }
 
+static int pega_acc_axis(struct asus_laptop *asus, int curr, char *method)
+{
+	int i, delta;
+	unsigned long long val;
+	for (i = 0; i < PEGA_ACC_RETRIES; i++) {
+		acpi_evaluate_integer(asus->handle, method, NULL, &val);
+
+		/* The output is noisy.  From reading the ASL
+		 * dissassembly, timeout errors are returned with 1's
+		 * in the high word, and the lack of locking around
+		 * thei hi/lo byte reads means that a transition
+		 * between (for example) -1 and 0 could be read as
+		 * 0xff00 or 0x00ff. */
+		delta = abs(curr - (short)val);
+		if (delta < 128 && !(val & ~0xffff))
+			break;
+	}
+	return clamp_val((short)val, -PEGA_ACC_CLAMP, PEGA_ACC_CLAMP);
+}
+
+static void pega_accel_poll(struct input_polled_dev *ipd)
+{
+	struct device *parent = ipd->input->dev.parent;
+	struct asus_laptop *asus = dev_get_drvdata(parent);
+
+	/* In some cases, the very first call to poll causes a
+	 * recursive fault under the polldev worker.  This is
+	 * apparently related to very early userspace access to the
+	 * device, and perhaps a firmware bug.  See related comments
+	 * in asus_platform_probe regarding the fragility of these
+	 * methods early in the boot.  Fake the first report. */
+	if (!asus->pega_acc_live) {
+		asus->pega_acc_live = true;
+		input_report_abs(ipd->input, ABS_X, 0);
+		input_report_abs(ipd->input, ABS_Y, 0);
+		input_report_abs(ipd->input, ABS_Z, 0);
+		input_sync(ipd->input);
+		return;
+	}
+
+	asus->pega_acc_x = pega_acc_axis(asus, asus->pega_acc_x, METHOD_XLRX);
+	asus->pega_acc_y = pega_acc_axis(asus, asus->pega_acc_y, METHOD_XLRY);
+	asus->pega_acc_z = pega_acc_axis(asus, asus->pega_acc_z, METHOD_XLRZ);
+
+	/* Note transform, convert to "right/up/out" in the native
+	 * landscape orientation (i.e. the vector is the direction of
+	 * "real up" in the device's cartiesian coordinates). */
+	input_report_abs(ipd->input, ABS_X, -asus->pega_acc_x);
+	input_report_abs(ipd->input, ABS_Y, -asus->pega_acc_y);
+	input_report_abs(ipd->input, ABS_Z,  asus->pega_acc_z);
+	input_sync(ipd->input);
+}
+
+static void pega_accel_probe(struct asus_laptop *asus)
+{
+	int err;
+	struct input_polled_dev *ipd;
+
+	if (!asus->have_pega_lucid ||
+	    acpi_check_handle(asus->handle, METHOD_XLRX, NULL) ||
+	    acpi_check_handle(asus->handle, METHOD_XLRY, NULL) ||
+	    acpi_check_handle(asus->handle, METHOD_XLRZ, NULL))
+		return;
+
+	ipd = input_allocate_polled_device();
+	if (!ipd)
+		return;
+
+	ipd->poll = pega_accel_poll;
+	ipd->poll_interval = 125;
+	ipd->poll_interval_min = 50;
+	ipd->poll_interval_max = 2000;
+
+	ipd->input->name = PEGA_ACCEL_DESC;
+	ipd->input->phys = PEGA_ACCEL_NAME "/input0";
+	ipd->input->dev.parent = &asus->platform_device->dev;
+	ipd->input->id.bustype = BUS_HOST;
+
+	set_bit(EV_ABS, ipd->input->evbit);
+	input_set_abs_params(ipd->input, ABS_X,
+			     -PEGA_ACC_CLAMP, PEGA_ACC_CLAMP, 0, 0);
+	input_set_abs_params(ipd->input, ABS_Y,
+			     -PEGA_ACC_CLAMP, PEGA_ACC_CLAMP, 0, 0);
+	input_set_abs_params(ipd->input, ABS_Z,
+			     -PEGA_ACC_CLAMP, PEGA_ACC_CLAMP, 0, 0);
+
+	err = input_register_polled_device(ipd);
+	if (err)
+		input_free_polled_device(ipd);
+	else
+		asus->pega_accel_poll = ipd;
+}
+
 /* Generic LED function */
 static int asus_led_set(struct asus_laptop *asus, const char *method,
 			 int value)
@@ -1459,11 +1565,40 @@ static void asus_platform_exit(struct asus_laptop *asus)
 	platform_device_unregister(asus->platform_device);
 }
 
+static int asus_platform_probe(struct platform_device *pd)
+{
+	struct asus_laptop *asus = dev_get_drvdata(&pd->dev);
+
+	/* This is instantiated during platform driver initialization
+	 * becuase if it's done from underneath asus_acpi_add(), the
+	 * resulting input device can be grabbed by an early userspace
+	 * reader before ACPI initialization is finished and something
+	 * oopses underneath the acpi_evaluate_integer() call out of
+	 * pega_accel_poll().  Firmware bug? */
+	pega_accel_probe(asus);
+
+	return 0;
+}
+
+static int __devexit asus_platform_remove(struct platform_device *pd)
+{
+	struct asus_laptop *asus = dev_get_drvdata(&pd->dev);
+	if (asus->pega_accel_poll) {
+		input_unregister_polled_device(asus->pega_accel_poll);
+		input_free_polled_device(asus->pega_accel_poll);
+	}
+	asus->pega_accel_poll = NULL;
+	return 0;
+}
+
+
 static struct platform_driver platform_driver = {
 	.driver = {
 		.name = ASUS_LAPTOP_FILE,
 		.owner = THIS_MODULE,
-	}
+	},
+	.probe  = asus_platform_probe,
+	.remove = asus_platform_remove,
 };
 
 static int asus_handle_init(char *name, acpi_handle * handle,
-- 
1.7.1

[PATCH 4/4] asus-laptop: allow boot time control of Pegatron ALS sensor

[Andy Ross]

Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
 drivers/platform/x86/asus-laptop.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 9b07368..1d32457 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -85,6 +85,7 @@ static int wlan_status = 1;
 static int bluetooth_status = 1;
 static int wimax_status = -1;
 static int wwan_status = -1;
+static int als_status = -1;
 
 module_param(wlan_status, int, 0444);
 MODULE_PARM_DESC(wlan_status, "Set the wireless status on boot "
@@ -106,6 +107,11 @@ MODULE_PARM_DESC(wwan_status, "Set the wireless status on boot "
 		 "(0 = disabled, 1 = enabled, -1 = don't do anything). "
 		 "default is 1");
 
+module_param(als_status, int, 0444);
+MODULE_PARM_DESC(als_status, "Set the ALS status on boot "
+		 "(0 = disabled, 1 = enabled, -1 = don't do anything). "
+		 "default is 0");
+
 /*
  * Some events we use, same for all Asus
  */
@@ -1752,7 +1758,7 @@ static int __devinit asus_acpi_init(struct asus_laptop *asus)
 	asus->ledd_status = 0xFFF;
 
 	/* Set initial values of light sensor and level */
-	asus->light_switch = 0;	/* Default to light sensor disabled */
+	asus->light_switch = als_status >= 0 ? !!als_status : 0;
 	asus->light_level = 5;	/* level 5 for sensor sensitivity */
 
 	if (asus->have_pega_lucid) {
-- 
1.7.1

Re: [PATCH 2/4] asus-laptop: Pegatron Lucid ALS sensor

[Corentin Chary]

On Thu, Mar 24, 2011 at 11:02 PM, Andy Ross <andy.ross@windriver.com> wrote:
> Ambient light sensor for Pegatron Lucid.  Supports pre-existing
> ls_switch sysfs interface to en/disable automatic control, and exports
> the brightness from the device as "ls_value".
>
> Signed-off-by: Andy Ross <andy.ross@windriver.com>
> ---
>  drivers/platform/x86/asus-laptop.c |   70 +++++++++++++++++++++++++++++++++---
>  1 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index ec46d71..6651d8c 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -214,9 +214,15 @@ static char *display_get_paths[] = {
>
>  /* For Pegatron Lucid tablet */
>  #define DEVICE_NAME_PEGA       "Lucid"
> +
>  #define METHOD_PEGA_ENABLE     "ENPR"
>  #define METHOD_PEGA_DISABLE    "DAPR"
> +#define PEGA_ALS       0x04
> +#define PEGA_ALS_POWER 0x05
> +
>  #define METHOD_PEGA_READ       "RDLN"
> +#define PEGA_READ_ALS_H        0x02
> +#define PEGA_READ_ALS_L        0x03
>
>  /*
>  * Define a specific led structure to keep the main structure clean
> @@ -378,6 +384,12 @@ static bool asus_check_pega_lucid(struct asus_laptop *asus)
>           !acpi_check_handle(asus->handle, METHOD_PEGA_READ, NULL);
>  }
>
> +static int asus_pega_lucid_set(struct asus_laptop *asus, int unit, bool enable)
> +{
> +       char *method = enable ? METHOD_PEGA_ENABLE : METHOD_PEGA_DISABLE;
> +       return write_acpi_int(asus->handle, method, unit);
> +}
> +
>  /* Generic LED function */
>  static int asus_led_set(struct asus_laptop *asus, const char *method,
>                         int value)
> @@ -1046,7 +1058,15 @@ static ssize_t store_disp(struct device *dev, struct device_attribute *attr,
>  */
>  static void asus_als_switch(struct asus_laptop *asus, int value)
>  {
> -       if (write_acpi_int(asus->handle, METHOD_ALS_CONTROL, value))
> +       int ret;
> +       if (asus->have_pega_lucid) {
> +               ret = asus_pega_lucid_set(asus, PEGA_ALS, value);
> +               if (!ret)
> +                       ret = asus_pega_lucid_set(asus, PEGA_ALS_POWER, value);
> +       } else {
> +               ret = write_acpi_int(asus->handle, METHOD_ALS_CONTROL, value);
> +       }
> +       if (ret)
>                pr_warning("Error setting light sensor switch\n");
>        asus->light_switch = value;
>  }
> @@ -1103,6 +1123,35 @@ static ssize_t store_lslvl(struct device *dev, struct device_attribute *attr,
>        return rv;
>  }
>
> +static int pega_int_read(struct asus_laptop *asus, int arg, int *result)
> +{
> +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       int err = write_acpi_int_ret(asus->handle, METHOD_PEGA_READ, arg,
> +                                    &buffer);
> +       if (!err) {
> +               union acpi_object *obj = buffer.pointer;

I think a kfree(buffer.pointer); is missing here.

> +               if (obj && obj->type == ACPI_TYPE_INTEGER)
> +                       *result = obj->integer.value;
> +               else
> +                       err = -EIO;
> +       }
> +       return err;
> +}
> +
> +static ssize_t show_lsvalue(struct device *dev,
> +                           struct device_attribute *attr, char *buf)
> +{
> +       struct asus_laptop *asus = dev_get_drvdata(dev);
> +       int err, hi, lo;
> +
> +       err = pega_int_read(asus, PEGA_READ_ALS_H, &hi);
> +       if (!err)
> +               err = pega_int_read(asus, PEGA_READ_ALS_L, &lo);
> +       if (!err)
> +               return sprintf(buf, "%d\n", 10 * hi + lo);
> +       return err;
> +}
> +
>  /*
>  * GPS
>  */
> @@ -1300,6 +1349,7 @@ static DEVICE_ATTR(wimax, S_IRUGO | S_IWUSR, show_wimax, store_wimax);
>  static DEVICE_ATTR(wwan, S_IRUGO | S_IWUSR, show_wwan, store_wwan);
>  static DEVICE_ATTR(display, S_IRUGO | S_IWUSR, show_disp, store_disp);
>  static DEVICE_ATTR(ledd, S_IRUGO | S_IWUSR, show_ledd, store_ledd);
> +static DEVICE_ATTR(ls_value, S_IRUGO, show_lsvalue, NULL);

Could you document that new file in
Documentation/ABI/testing/sysfs-platform-asus-laptop ?

>  static DEVICE_ATTR(ls_level, S_IRUGO | S_IWUSR, show_lslvl, store_lslvl);
>  static DEVICE_ATTR(ls_switch, S_IRUGO | S_IWUSR, show_lssw, store_lssw);
>  static DEVICE_ATTR(gps, S_IRUGO | S_IWUSR, show_gps, store_gps);
> @@ -1312,6 +1362,7 @@ static struct attribute *asus_attributes[] = {
>        &dev_attr_wwan.attr,
>        &dev_attr_display.attr,
>        &dev_attr_ledd.attr,
> +       &dev_attr_ls_value.attr,
>        &dev_attr_ls_level.attr,
>        &dev_attr_ls_switch.attr,
>        &dev_attr_gps.attr,
> @@ -1349,8 +1400,15 @@ static mode_t asus_sysfs_is_visible(struct kobject *kobj,
>
>        } else if (attr == &dev_attr_ls_switch.attr ||
>                   attr == &dev_attr_ls_level.attr) {
> -               supported = !acpi_check_handle(handle, METHOD_ALS_CONTROL, NULL) &&
> -                           !acpi_check_handle(handle, METHOD_ALS_LEVEL, NULL);
> +               if (asus->have_pega_lucid) {
> +                       /* no ls_level interface on the Lucid */
> +                       supported = attr == &dev_attr_ls_switch.attr;
> +               } else {
> +                       supported = !acpi_check_handle(handle, METHOD_ALS_CONTROL, NULL) &&
> +                                   !acpi_check_handle(handle, METHOD_ALS_LEVEL, NULL);
> +               }
> +       } else if (attr == &dev_attr_ls_value.attr) {
> +               supported = asus->have_pega_lucid;
>        } else if (attr == &dev_attr_gps.attr) {
>                supported = !acpi_check_handle(handle, METHOD_GPS_ON, NULL) &&
>                            !acpi_check_handle(handle, METHOD_GPS_OFF, NULL) &&
> @@ -1562,8 +1620,10 @@ static int __devinit asus_acpi_init(struct asus_laptop *asus)
>        asus->light_switch = 0; /* Default to light sensor disabled */
>        asus->light_level = 5;  /* level 5 for sensor sensitivity */
>
> -       if (!acpi_check_handle(asus->handle, METHOD_ALS_CONTROL, NULL) &&
> -           !acpi_check_handle(asus->handle, METHOD_ALS_LEVEL, NULL)) {
> +       if (asus->have_pega_lucid) {
> +               asus_als_switch(asus, asus->light_switch);
> +       } else if (!acpi_check_handle(asus->handle, METHOD_ALS_CONTROL, NULL) &&
> +                  !acpi_check_handle(asus->handle, METHOD_ALS_LEVEL, NULL)) {
>                asus_als_switch(asus, asus->light_switch);
>                asus_als_level(asus, asus->light_level);
>        }
> --
> 1.7.1
>
>



-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Corentin Chary]

Ccing Dmitry and Matthew, they may want to comment that one.

On Thu, Mar 24, 2011 at 11:02 PM, Andy Ross <andy.ross@windriver.com> wrote:
> Support the built-in accelerometer on the Lucid tablets as a standard
> 3-axis input device.
>
> Signed-off-by: Andy Ross <andy.ross@windriver.com>
> ---
>  drivers/platform/x86/Kconfig       |    9 ++-
>  drivers/platform/x86/asus-laptop.c |  137 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 141 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index b6f983e..43906f5 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -67,10 +67,11 @@ config ASUS_LAPTOP
>          This is a driver for Asus laptops and the Pegatron Lucid
>          tablet. It may also support some MEDION, JVC or VICTOR
>          laptops. It makes all the extra buttons generate standard
> -         ACPI events and input events. It also adds support for video
> -         output switching, LCD backlight control, Bluetooth and Wlan
> -         control, and most importantly, allows you to blink those
> -         fancy LEDs.
> +         ACPI events and input events, and on the Lucid the built-in
> +         accelerometer appears as an input device.  It also adds
> +         support for video output switching, LCD backlight control,
> +         Bluetooth and Wlan control, and most importantly, allows you
> +         to blink those fancy LEDs.
>
>          For more information and a userspace daemon for handling the extra
>          buttons see <http://acpi4asus.sf.net>.
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index 6651d8c..9b07368 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -224,6 +224,14 @@ static char *display_get_paths[] = {
>  #define PEGA_READ_ALS_H        0x02
>  #define PEGA_READ_ALS_L        0x03
>
> +#define PEGA_ACCEL_NAME "pega_accel"
> +#define PEGA_ACCEL_DESC "Pegatron Lucid Tablet Accelerometer"
> +#define METHOD_XLRX "XLRX"
> +#define METHOD_XLRY "XLRY"
> +#define METHOD_XLRZ "XLRZ"
> +#define PEGA_ACC_CLAMP 512 /* 1G accel is reported as ~256, so clamp to 2G */
> +#define PEGA_ACC_RETRIES 3
> +
>  /*
>  * Define a specific led structure to keep the main structure clean
>  */
> @@ -249,6 +257,7 @@ struct asus_laptop {
>
>        struct input_dev *inputdev;
>        struct key_entry *keymap;
> +       struct input_polled_dev *pega_accel_poll;
>
>        struct asus_led mled;
>        struct asus_led tled;
> @@ -262,6 +271,10 @@ struct asus_laptop {
>        bool have_rsts;
>        bool have_pega_lucid;
>        int lcd_state;
> +       bool pega_acc_live;
> +       int pega_acc_x;
> +       int pega_acc_y;
> +       int pega_acc_z;
>
>        struct rfkill *gps_rfkill;
>
> @@ -390,6 +403,99 @@ static int asus_pega_lucid_set(struct asus_laptop *asus, int unit, bool enable)
>        return write_acpi_int(asus->handle, method, unit);
>  }
>
> +static int pega_acc_axis(struct asus_laptop *asus, int curr, char *method)
> +{
> +       int i, delta;
> +       unsigned long long val;
> +       for (i = 0; i < PEGA_ACC_RETRIES; i++) {
> +               acpi_evaluate_integer(asus->handle, method, NULL, &val);
> +
> +               /* The output is noisy.  From reading the ASL
> +                * dissassembly, timeout errors are returned with 1's
> +                * in the high word, and the lack of locking around
> +                * thei hi/lo byte reads means that a transition
> +                * between (for example) -1 and 0 could be read as
> +                * 0xff00 or 0x00ff. */
> +               delta = abs(curr - (short)val);
> +               if (delta < 128 && !(val & ~0xffff))
> +                       break;
> +       }
> +       return clamp_val((short)val, -PEGA_ACC_CLAMP, PEGA_ACC_CLAMP);
> +}

Wow :/ How long is an acpi_evaluate_integer call here ?

> +static int asus_platform_probe(struct platform_device *pd)
> +{
> +       struct asus_laptop *asus = dev_get_drvdata(&pd->dev);
> +
> +       /* This is instantiated during platform driver initialization
> +        * becuase if it's done from underneath asus_acpi_add(), the
> +        * resulting input device can be grabbed by an early userspace
> +        * reader before ACPI initialization is finished and something
> +        * oopses underneath the acpi_evaluate_integer() call out of
> +        * pega_accel_poll().  Firmware bug? */
> +       pega_accel_probe(asus);
> +
> +       return 0;
> +}

When is asus_platform_probe called exactly ? Because I'd say it's
called during asus_platform_probe(), and that doesn't fix your issue
right ?

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Dmitry Torokhov]

On Fri, Mar 25, 2011 at 12:21:20PM +0100, Corentin Chary wrote:
> Ccing Dmitry and Matthew, they may want to comment that one.
> 
> On Thu, Mar 24, 2011 at 11:02 PM, Andy Ross <andy.ross@windriver.com> wrote:
> > +static int asus_platform_probe(struct platform_device *pd)
> > +{
> > +       struct asus_laptop *asus = dev_get_drvdata(&pd->dev);
> > +
> > +       /* This is instantiated during platform driver initialization
> > +        * becuase if it's done from underneath asus_acpi_add(), the
> > +        * resulting input device can be grabbed by an early userspace
> > +        * reader before ACPI initialization is finished and something
> > +        * oopses underneath the acpi_evaluate_integer() call out of
> > +        * pega_accel_poll().  Firmware bug? */
> > +       pega_accel_probe(asus);
> > +
> > +       return 0;
> > +}
> 
> When is asus_platform_probe called exactly ? Because I'd say it's
> called during asus_platform_probe(), and that doesn't fix your issue
> right ?

This I do not quite understand... Do we bind acpi drivers to devices
before ACPI initialization is done? Then this should be fixed in ACPI
layer, but I doubt even early userspace is active at the time ACPI core
is being initialized. Also, input polling is done in a separate thread
so you not moving the poll out of ACPI binding thread but delay poll
execution by a few microseconds...

-- 
Dmitry

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Corentin Chary]

On Fri, Mar 25, 2011 at 12:21 PM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> Ccing Dmitry and Matthew, they may want to comment that one.
>
> On Thu, Mar 24, 2011 at 11:02 PM, Andy Ross <andy.ross@windriver.com> wrote:
>> Support the built-in accelerometer on the Lucid tablets as a standard
>> 3-axis input device.
>>
>> Signed-off-by: Andy Ross <andy.ross@windriver.com>
>> ---
>>  drivers/platform/x86/Kconfig       |    9 ++-
>>  drivers/platform/x86/asus-laptop.c |  137 +++++++++++++++++++++++++++++++++++-
>>  2 files changed, 141 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index b6f983e..43906f5 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -67,10 +67,11 @@ config ASUS_LAPTOP
>>          This is a driver for Asus laptops and the Pegatron Lucid
>>          tablet. It may also support some MEDION, JVC or VICTOR
>>          laptops. It makes all the extra buttons generate standard
>> -         ACPI events and input events. It also adds support for video
>> -         output switching, LCD backlight control, Bluetooth and Wlan
>> -         control, and most importantly, allows you to blink those
>> -         fancy LEDs.
>> +         ACPI events and input events, and on the Lucid the built-in
>> +         accelerometer appears as an input device.  It also adds
>> +         support for video output switching, LCD backlight control,
>> +         Bluetooth and Wlan control, and most importantly, allows you
>> +         to blink those fancy LEDs.
>>
>>          For more information and a userspace daemon for handling the extra
>>          buttons see <http://acpi4asus.sf.net>.
>> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
>> index 6651d8c..9b07368 100644
>> --- a/drivers/platform/x86/asus-laptop.c
>> +++ b/drivers/platform/x86/asus-laptop.c
>> @@ -224,6 +224,14 @@ static char *display_get_paths[] = {
>>  #define PEGA_READ_ALS_H        0x02
>>  #define PEGA_READ_ALS_L        0x03
>>
>> +#define PEGA_ACCEL_NAME "pega_accel"
>> +#define PEGA_ACCEL_DESC "Pegatron Lucid Tablet Accelerometer"
>> +#define METHOD_XLRX "XLRX"
>> +#define METHOD_XLRY "XLRY"
>> +#define METHOD_XLRZ "XLRZ"
>> +#define PEGA_ACC_CLAMP 512 /* 1G accel is reported as ~256, so clamp to 2G */
>> +#define PEGA_ACC_RETRIES 3
>> +
>>  /*
>>  * Define a specific led structure to keep the main structure clean
>>  */
>> @@ -249,6 +257,7 @@ struct asus_laptop {
>>
>>        struct input_dev *inputdev;
>>        struct key_entry *keymap;
>> +       struct input_polled_dev *pega_accel_poll;
>>
>>        struct asus_led mled;
>>        struct asus_led tled;
>> @@ -262,6 +271,10 @@ struct asus_laptop {
>>        bool have_rsts;
>>        bool have_pega_lucid;
>>        int lcd_state;
>> +       bool pega_acc_live;
>> +       int pega_acc_x;
>> +       int pega_acc_y;
>> +       int pega_acc_z;
>>
>>        struct rfkill *gps_rfkill;
>>
>> @@ -390,6 +403,99 @@ static int asus_pega_lucid_set(struct asus_laptop *asus, int unit, bool enable)
>>        return write_acpi_int(asus->handle, method, unit);
>>  }
>>
>> +static int pega_acc_axis(struct asus_laptop *asus, int curr, char *method)
>> +{
>> +       int i, delta;
>> +       unsigned long long val;
>> +       for (i = 0; i < PEGA_ACC_RETRIES; i++) {
>> +               acpi_evaluate_integer(asus->handle, method, NULL, &val);
>> +
>> +               /* The output is noisy.  From reading the ASL
>> +                * dissassembly, timeout errors are returned with 1's
>> +                * in the high word, and the lack of locking around
>> +                * thei hi/lo byte reads means that a transition
>> +                * between (for example) -1 and 0 could be read as
>> +                * 0xff00 or 0x00ff. */
>> +               delta = abs(curr - (short)val);
>> +               if (delta < 128 && !(val & ~0xffff))
>> +                       break;
>> +       }
>> +       return clamp_val((short)val, -PEGA_ACC_CLAMP, PEGA_ACC_CLAMP);
>> +}
>
> Wow :/ How long is an acpi_evaluate_integer call here ?
>
>> +static int asus_platform_probe(struct platform_device *pd)
>> +{
>> +       struct asus_laptop *asus = dev_get_drvdata(&pd->dev);
>> +
>> +       /* This is instantiated during platform driver initialization
>> +        * becuase if it's done from underneath asus_acpi_add(), the
>> +        * resulting input device can be grabbed by an early userspace
>> +        * reader before ACPI initialization is finished and something
>> +        * oopses underneath the acpi_evaluate_integer() call out of
>> +        * pega_accel_poll().  Firmware bug? */
>> +       pega_accel_probe(asus);
>> +
>> +       return 0;
>> +}
>
> When is asus_platform_probe called exactly ? Because I'd say it's
> called during asus_platform_probe(), and that doesn't fix your issue
> right ?



On Mon, Mar 28, 2011 at 7:43 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Mar 25, 2011 at 12:21:20PM +0100, Corentin Chary wrote:
>> Ccing Dmitry and Matthew, they may want to comment that one.
>>
>> On Thu, Mar 24, 2011 at 11:02 PM, Andy Ross <andy.ross@windriver.com> wrote:
>> > +static int asus_platform_probe(struct platform_device *pd)
>> > +{
>> > +       struct asus_laptop *asus = dev_get_drvdata(&pd->dev);
>> > +
>> > +       /* This is instantiated during platform driver initialization
>> > +        * becuase if it's done from underneath asus_acpi_add(), the
>> > +        * resulting input device can be grabbed by an early userspace
>> > +        * reader before ACPI initialization is finished and something
>> > +        * oopses underneath the acpi_evaluate_integer() call out of
>> > +        * pega_accel_poll().  Firmware bug? */
>> > +       pega_accel_probe(asus);
>> > +
>> > +       return 0;
>> > +}
>>
>> When is asus_platform_probe called exactly ? Because I'd say it's
>> called during asus_platform_probe(), and that doesn't fix your issue
>> right ?

I meant during *asus_platform_init()* (which is called by
asus_acpi_add()), not *asus_platform_probe()*


-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Andy Ross]

[combined responses for clarity]

Corentin Chary wrote:
> Wow :/ How long is an acpi_evaluate_integer call here ?

Early on I benchmarked the per-axis ACPI calls about about 50us, which
is shockingly high to me.  But honestly that note there is just
pedantry: the hi/lo race (and the fact that "errors" live in the same
space with valid data values) is a real condition that lets me justify
the "retry on large deltas" behavior, which most definitely improves
data quality.

> When is asus_platform_probe called exactly ? Because I'd say it's
> called during asus_platform_probe(), and that doesn't fix your issue
> right ?

Admittedly this is voodoo.  Moving the input_polldev device
initialization to asus_laptop (originally in asus_acpi_add(), which is
called out of the ACPI framwwork) caused reliable panics on boot.  I
moved it to asus_platform_probe() to restore the structure of the
original pega_accel module, which did it in a platform probe.

I don't know enough about the platform driver model to say exactly,
but I strongly suspect that platform probe calls are not done
synchronously underneath the platform_{device|driver}_register()
calls.

Dmitry Torokhov wrote:
> This I do not quite understand... Do we bind acpi drivers to devices
> before ACPI initialization is done? Then this should be fixed in
> ACPI layer, but I doubt even early userspace is active at the time
> ACPI core is being initialized. Also, input polling is done in a
> separate thread so you not moving the poll out of ACPI binding
> thread but delay poll execution by a few microseconds...

See comments above.  The current model in asus-laptop is to register
its keyboard input device underneat the .add callback in the acpi
driver.  That was crashing with the accelerometer.

And there's definitely a userspace interaction: disabling the sensor
daemon (the distro in question is a MeeGo tablet image) at startup
avoided the crash (and in fact worked fine when started up manually
later).

Andy

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Corentin Chary]

test platform printk

On Mon, Mar 28, 2011 at 3:36 PM, Andy Ross <andy.ross@windriver.com> wrote:
> [combined responses for clarity]
>
> Corentin Chary wrote:
>> Wow :/ How long is an acpi_evaluate_integer call here ?
>
> Early on I benchmarked the per-axis ACPI calls about about 50us, which
> is shockingly high to me.  But honestly that note there is just
> pedantry: the hi/lo race (and the fact that "errors" live in the same
> space with valid data values) is a real condition that lets me justify
> the "retry on large deltas" behavior, which most definitely improves
> data quality.

Ok,

>> When is asus_platform_probe called exactly ? Because I'd say it's
>> called during asus_platform_probe(), and that doesn't fix your issue
>> right ?
>
> Admittedly this is voodoo.  Moving the input_polldev device
> initialization to asus_laptop (originally in asus_acpi_add(), which is
> called out of the ACPI framwwork) caused reliable panics on boot.  I
> moved it to asus_platform_probe() to restore the structure of the
> original pega_accel module, which did it in a platform probe.
>
> I don't know enough about the platform driver model to say exactly,
> but I strongly suspect that platform probe calls are not done
> synchronously underneath the platform_{device|driver}_register()
> calls.

I'll check that this weekend.

> Dmitry Torokhov wrote:
>> This I do not quite understand... Do we bind acpi drivers to devices
>> before ACPI initialization is done? Then this should be fixed in
>> ACPI layer, but I doubt even early userspace is active at the time
>> ACPI core is being initialized. Also, input polling is done in a
>> separate thread so you not moving the poll out of ACPI binding
>> thread but delay poll execution by a few microseconds...
>
> See comments above.  The current model in asus-laptop is to register
> its keyboard input device underneat the .add callback in the acpi
> driver.  That was crashing with the accelerometer.
>
> And there's definitely a userspace interaction: disabling the sensor
> daemon (the distro in question is a MeeGo tablet image) at startup
> avoided the crash (and in fact worked fine when started up manually
> later).

Just to try to fix the crash in a more cleaner way, can you (re-)tell
us more about it ? It's a kernel crash, or the hardware just start
doing stupid things ? Any traces ? Does it also happens when you load
the model *after* boot (blacklist it, then modprobe) ? Could you also
re-send me the dsdt ? I think I lost it.

Thanks,

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Corentin Chary]

On Thu, Mar 31, 2011 at 7:23 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> test platform printk
>
> On Mon, Mar 28, 2011 at 3:36 PM, Andy Ross <andy.ross@windriver.com> wrote:
>> [combined responses for clarity]
>>
>> Corentin Chary wrote:
>>> Wow :/ How long is an acpi_evaluate_integer call here ?
>>
>> Early on I benchmarked the per-axis ACPI calls about about 50us, which
>> is shockingly high to me.  But honestly that note there is just
>> pedantry: the hi/lo race (and the fact that "errors" live in the same
>> space with valid data values) is a real condition that lets me justify
>> the "retry on large deltas" behavior, which most definitely improves
>> data quality.
>
> Ok,
>
>>> When is asus_platform_probe called exactly ? Because I'd say it's
>>> called during asus_platform_probe(), and that doesn't fix your issue
>>> right ?
>>
>> Admittedly this is voodoo.  Moving the input_polldev device
>> initialization to asus_laptop (originally in asus_acpi_add(), which is
>> called out of the ACPI framwwork) caused reliable panics on boot.  I
>> moved it to asus_platform_probe() to restore the structure of the
>> original pega_accel module, which did it in a platform probe.
>>
>> I don't know enough about the platform driver model to say exactly,
>> but I strongly suspect that platform probe calls are not done
>> synchronously underneath the platform_{device|driver}_register()
>> calls.
>
> I'll check that this weekend.

I didn't have any asus-laptop to test it, but here is a call trace for hp-wmi:

init
register
alloc
add
------------[ cut here ]------------
WARNING: at drivers/platform/x86/hp-wmi.c:901 hp_wmi_init+0x1db/0x234 [hp_wmi]()
Hardware name: HP Compaq 8710p
Modules linked in: hp_wmi(+) nvidia(P) hp_accel lis3lv02d
input_polldev ext2 sdhci_pci sdhci mmc_core thermal button ehci_hcd
btusb usbhid joydev sparse_keymap hid e1000e wmi bluetooth [last
unloaded: hp_wmi]
Pid: 25890, comm: insmod Tainted: P            2.6.39-rc1-00184-g78fca1b9 #4
Call Trace:
 [<ffffffff8103becb>] ? warn_slowpath_common+0x7b/0xc0
 [<ffffffffa00621db>] ? hp_wmi_init+0x1db/0x234 [hp_wmi]
 [<ffffffffa0062000>] ? 0xffffffffa0061fff
 [<ffffffff810002ba>] ? do_one_initcall+0x3a/0x170
 [<ffffffff81070e39>] ? sys_init_module+0xb9/0x200
 [<ffffffff815b733b>] ? system_call_fastpath+0x16/0x1b
---[ end trace 9b2a16f0f1ed60e4 ]---
bios setup begin
------------[ cut here ]------------
WARNING: at drivers/platform/x86/hp-wmi.c:783
hp_wmi_bios_setup+0x2a/0x2df [hp_wmi]()
Hardware name: HP Compaq 8710p
Modules linked in: hp_wmi(+) nvidia(P) hp_accel lis3lv02d
input_polldev ext2 sdhci_pci sdhci mmc_core thermal button ehci_hcd
btusb usbhid joydev sparse_keymap hid e1000e wmi bluetooth [last
unloaded: hp_wmi]
Pid: 25890, comm: insmod Tainted: P        W   2.6.39-rc1-00184-g78fca1b9 #4
Call Trace:
 [<ffffffff8103becb>] ? warn_slowpath_common+0x7b/0xc0
 [<ffffffffa003ad2d>] ? hp_wmi_bios_setup+0x2a/0x2df [hp_wmi]
 [<ffffffff81339efe>] ? pm_runtime_barrier+0x5e/0xc0
 [<ffffffff81331b46>] ? driver_probe_device+0x96/0x1b0
 [<ffffffff81331d00>] ? __driver_attach+0xa0/0xa0
 [<ffffffff8133087c>] ? bus_for_each_drv+0x5c/0x90
 [<ffffffff813319ef>] ? device_attach+0x8f/0xb0
 [<ffffffff8133121d>] ? bus_probe_device+0x2d/0x50
 [<ffffffff8132f546>] ? device_add+0x566/0x630
 [<ffffffff8132e18f>] ? dev_set_name+0x3f/0x50
 [<ffffffff813338b8>] ? platform_device_add+0xf8/0x1c0
 [<ffffffffa00621e7>] ? hp_wmi_init+0x1e7/0x234 [hp_wmi]
 [<ffffffffa0062000>] ? 0xffffffffa0061fff
 [<ffffffff810002ba>] ? do_one_initcall+0x3a/0x170
 [<ffffffff81070e39>] ? sys_init_module+0xb9/0x200
 [<ffffffff815b733b>] ? system_call_fastpath+0x16/0x1b
---[ end trace 9b2a16f0f1ed60e5 ]---
bios setup end
done

So, the call is clearly synchroneous.

Where did you put the call exactly to make it crash ?

>> Dmitry Torokhov wrote:
>>> This I do not quite understand... Do we bind acpi drivers to devices
>>> before ACPI initialization is done? Then this should be fixed in
>>> ACPI layer, but I doubt even early userspace is active at the time
>>> ACPI core is being initialized. Also, input polling is done in a
>>> separate thread so you not moving the poll out of ACPI binding
>>> thread but delay poll execution by a few microseconds...
>>
>> See comments above.  The current model in asus-laptop is to register
>> its keyboard input device underneat the .add callback in the acpi
>> driver.  That was crashing with the accelerometer.
>>
>> And there's definitely a userspace interaction: disabling the sensor
>> daemon (the distro in question is a MeeGo tablet image) at startup
>> avoided the crash (and in fact worked fine when started up manually
>> later).
>
> Just to try to fix the crash in a more cleaner way, can you (re-)tell
> us more about it ? It's a kernel crash, or the hardware just start
> doing stupid things ? Any traces ? Does it also happens when you load
> the model *after* boot (blacklist it, then modprobe) ? Could you also
> re-send me the dsdt ? I think I lost it.

ping ?

Thanks,
-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Anisse Astier]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Mon, 4 Apr 2011 17:16:05 +0000, Corentin Chary <corentin.chary@gmail.com> wrote :

> On Thu, Mar 31, 2011 at 7:23 AM, Corentin Chary
> <corentin.chary@gmail.com> wrote:
<snip>
> > Just to try to fix the crash in a more cleaner way, can you (re-)tell
> > us more about it ? It's a kernel crash, or the hardware just start
> > doing stupid things ? Any traces ? Does it also happens when you load
> > the model *after* boot (blacklist it, then modprobe) ? Could you also
> > re-send me the dsdt ? I think I lost it.
> 
> ping ?

I have a Pegatron Lucid. Please find the DSDT attached.

Regards,

Anisse

[-- Attachment #2: DSDT-pegatron-lucid --]
[-- Type: application/octet-stream, Size: 35041 bytes --]

Re: [PATCH 3/4] asus-laptop: Pegatron Lucid accelerometer

[Corentin Chary]

On Thu, Apr 7, 2011 at 5:56 PM, Anisse Astier <anisse@astier.eu> wrote:
> On Mon, 4 Apr 2011 17:16:05 +0000, Corentin Chary <corentin.chary@gmail.com> wrote :
>
>> On Thu, Mar 31, 2011 at 7:23 AM, Corentin Chary
>> <corentin.chary@gmail.com> wrote:
> <snip>
>> > Just to try to fix the crash in a more cleaner way, can you (re-)tell
>> > us more about it ? It's a kernel crash, or the hardware just start
>> > doing stupid things ? Any traces ? Does it also happens when you load
>> > the model *after* boot (blacklist it, then modprobe) ? Could you also
>> > re-send me the dsdt ? I think I lost it.
>>
>> ping ?
>
> I have a Pegatron Lucid. Please find the DSDT attached.

Thanks for the DSDT. After a quick read I didn't find anything in it
that could produce the crash.
I'll wait for Andy's answers.


-- 
Corentin Chary
http://xf.iksaif.net