[PATCH 0/2] Pegatron Lucid tablet accelerometer

[PATCH 0/2] Pegatron Lucid tablet accelerometer

[Andy Ross]

Resend with fixes per comments: Driver for the ACPI interface to the
accelerometer on the Pegatron Lucid tablets (sold as WeTab and ExoPC).

Corentin Chary wrote:
> > +       int z =  acpi_s16("\\_SB.ATKD.XLRZ");
> 
> Using the absolute path (\\_SB.ATKD) may not work on new hardware, you
> should find the path of the device using the HID (AKT0100, or
> something like that). But we may not core in this case since it's pega
> specific.

I could pass the ACPI prefix (or a handle) through the platform driver
if you like, but it's a little more complexity and as you mention that
seems like overkill for a driver whose core is only three lines of
code.  Left unchanged.

Corentin Chary wrote:
> > +static int __devinit platform_probe(struct platform_device *pd)
> 
> You should check if the device exists here, before trying to do
> anything, (using the path or the HID, depending of what you'll do
> about my previous comment)

Detection is done in asus-laptop: this code won't be reached unless
the platform device is created, which is after the three needed
methods have already been detected with acpi_check_handle().

Dmitry Torokhov wrote:
> This is not a critical component; default should be 'n' (or just omit
> default statement).

Fixed.

Dmitry Torokhov wrote:
> Please keep Kconfig and Makefile sorted alphabetically.

Fixed in Makefile.  Kconfig is hopelessly out of order, but I did what
I could.

Dmitry Torokhov wrote:
> What's up with people loving to put filename into comments? It just
> makes renaming files more4 difficult.

No love here; I was trying to adhere to what looked like existing
convention.  Removed.

Dmitry Torokhov wrote:
> No need to initialize to 0, in fact, it is quite often dangerous as it
> masks compiler warnings.

True enough.  Fixed.

> > +	ipdev->input->dev.release = dev_noop_release;
> 
> WTH is this???

It's an attempt to avoid this from drivers/base/core.c:

      WARN(1, KERN_ERR "Device '%s' does not have a release() "
           "function, it is broken and must be fixed.\n",
           dev_name(dev));

...but it was in the wrong place.  The input device doesn't hit that
path, the platform device does.  Moved to where it belongs.

Andy

[PATCH 1/2] input: Pegatron Lucid accelerometer

[Andy Ross]

Add driver for the ACPI accelerometer interface on the Pegatron Lucid
tablet.

Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
 drivers/input/misc/Kconfig      |   10 +++
 drivers/input/misc/Makefile     |    2 +-
 drivers/input/misc/pega-accel.c |  130 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 1 deletions(-)
 create mode 100644 drivers/input/misc/pega-accel.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b99b8cb..e60af95 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -74,6 +74,16 @@ config INPUT_PCSPKR
 	  To compile this driver as a module, choose M here: the
 	  module will be called pcspkr.
 
+config INPUT_PEGA_ACCEL
+       tristate "Support Pegatron Lucid accelerometer"
+       depends on ASUS_LAPTOP
+       help
+         Say Y here if you want support for the built-in ACPI
+         accelerometer device on Pegatron Lucid tablet devices.
+
+	 To compile this driver as a module, choose M here: the module
+	 will be called pega_accel.
+
 config INPUT_SPARCSPKR
 	tristate "SPARC Speaker support"
 	depends on PCI && SPARC64
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 1fe1f6c..79aca4d 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
 obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
 obj-$(CONFIG_INPUT_PCSPKR)		+= pcspkr.o
+obj-$(CONFIG_INPUT_PEGA_ACCEL)		+= pega-accel.o
 obj-$(CONFIG_INPUT_POWERMATE)		+= powermate.o
 obj-$(CONFIG_INPUT_PWM_BEEPER)		+= pwm-beeper.o
 obj-$(CONFIG_INPUT_RB532_BUTTON)	+= rb532_button.o
@@ -42,4 +43,3 @@ obj-$(CONFIG_INPUT_WINBOND_CIR)		+= winbond-cir.o
 obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
 obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
 obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
-
diff --git a/drivers/input/misc/pega-accel.c b/drivers/input/misc/pega-accel.c
new file mode 100644
index 0000000..7005b46
--- /dev/null
+++ b/drivers/input/misc/pega-accel.c
@@ -0,0 +1,130 @@
+/*
+ * Driver for accelerometer in Pegatron Lucid tablets
+ *
+ * Copyright (c) 2011 Wind River Systems
+ *
+ * Author: Andy Ross <andy.ross@windriver.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/input-polldev.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <acpi/acpi_bus.h>
+
+#define DRIVER_NAME "pega_accel"
+#define DRIVER_DESC "Pegatron Lucid Tablet Accelerometer"
+
+/* 1G accel is reported as ~256, so clamp to 2G */
+#define CLAMP 512
+
+static struct input_polled_dev *ipdev;
+
+/* FIXME: this mechanism is *very* slow: ~50ms to read the three
+ * values.  Pre-caching an acpi handle with acpi_get_handle has no
+ * effect, so the issue isn't in the parsing or tree walking... */
+static int acpi_s16(char *method)
+{
+	unsigned long long val = 0;
+	acpi_evaluate_integer(NULL, method, NULL, &val);
+	return (short)val;
+}
+
+static void pega_accel_poll(struct input_polled_dev *ipdev)
+{
+	/* 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).  FIXME:
+	 * is there a relevant convention to adhere to? */
+	int x = -acpi_s16("\\_SB.ATKD.XLRX");
+	int y = -acpi_s16("\\_SB.ATKD.XLRY");
+	int z =  acpi_s16("\\_SB.ATKD.XLRZ");
+
+	x = clamp_val(x, -CLAMP, CLAMP);
+	y = clamp_val(y, -CLAMP, CLAMP);
+	z = clamp_val(z, -CLAMP, CLAMP);
+
+	input_report_abs(ipdev->input, ABS_X, x);
+	input_report_abs(ipdev->input, ABS_Y, y);
+	input_report_abs(ipdev->input, ABS_Z, z);
+	input_sync(ipdev->input);
+}
+
+static int __devinit platform_probe(struct platform_device *pd)
+{
+	int err;
+
+	ipdev = input_allocate_polled_device();
+	if (!ipdev)
+		return -ENOMEM;
+
+	ipdev->poll = pega_accel_poll;
+	ipdev->poll_interval = 100;
+	ipdev->poll_interval_min = 10;
+	ipdev->poll_interval_max = 2000;
+
+	ipdev->input->dev.parent = &pd->dev;
+	ipdev->input->id.bustype = BUS_HOST;
+
+	ipdev->input->name = DRIVER_DESC;
+	ipdev->input->phys = DRIVER_NAME "/input0";
+
+	set_bit(EV_ABS, ipdev->input->evbit);
+	input_set_abs_params(ipdev->input, ABS_X, -CLAMP, CLAMP, 0, 0);
+	input_set_abs_params(ipdev->input, ABS_Y, -CLAMP, CLAMP, 0, 0);
+	input_set_abs_params(ipdev->input, ABS_Z, -CLAMP, CLAMP, 0, 0);
+
+	err = input_register_polled_device(ipdev);
+	if (err)
+		input_free_polled_device(ipdev);
+
+	return err;
+}
+
+static int __devexit platform_remove(struct platform_device *pd)
+{
+	input_unregister_polled_device(ipdev);
+	input_free_polled_device(ipdev);
+	ipdev = NULL;
+	return 0;
+}
+
+static struct platform_driver platform_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = DRIVER_NAME,
+	},
+	.probe  = platform_probe,
+	.remove = __devexit_p(platform_remove),
+};
+
+static int __init mod_init(void)
+{
+	return platform_driver_register(&platform_driver);
+}
+
+static void __exit mod_exit(void)
+{
+	platform_driver_unregister(&platform_driver);
+}
+
+module_init(mod_init);
+module_exit(mod_exit);
+
+MODULE_AUTHOR("Andy Ross <andy.ross@windriver.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_ALIAS("dmi:*:bvrLucid-CE-133:*");
-- 
1.7.1

Re: [PATCH 1/2] input: Pegatron Lucid accelerometer

[Shubhrajyoti]

Hi Andy,

On Monday 17 January 2011 11:26 PM, Andy Ross wrote:
> Add driver for the ACPI accelerometer interface on the Pegatron Lucid
> tablet.
>
> Signed-off-by: Andy Ross<andy.ross@windriver.com>
> ---
>   drivers/input/misc/Kconfig      |   10 +++
>   drivers/input/misc/Makefile     |    2 +-
>   drivers/input/misc/pega-accel.c |  130 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 141 insertions(+), 1 deletions(-)
>   create mode 100644 drivers/input/misc/pega-accel.c
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index b99b8cb..e60af95 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -74,6 +74,16 @@ config INPUT_PCSPKR
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called pcspkr.
>
> +config INPUT_PEGA_ACCEL
> +       tristate "Support Pegatron Lucid accelerometer"
> +       depends on ASUS_LAPTOP
> +       help
> +         Say Y here if you want support for the built-in ACPI
> +         accelerometer device on Pegatron Lucid tablet devices.
> +
> +	 To compile this driver as a module, choose M here: the module
> +	 will be called pega_accel.
> +
>   config INPUT_SPARCSPKR
>   	tristate "SPARC Speaker support"
>   	depends on PCI&&  SPARC64
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 1fe1f6c..79aca4d 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
>   obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
>   obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
>   obj-$(CONFIG_INPUT_PCSPKR)		+= pcspkr.o
> +obj-$(CONFIG_INPUT_PEGA_ACCEL)		+= pega-accel.o
>   obj-$(CONFIG_INPUT_POWERMATE)		+= powermate.o
>   obj-$(CONFIG_INPUT_PWM_BEEPER)		+= pwm-beeper.o
>   obj-$(CONFIG_INPUT_RB532_BUTTON)	+= rb532_button.o
> @@ -42,4 +43,3 @@ obj-$(CONFIG_INPUT_WINBOND_CIR)		+= winbond-cir.o
>   obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
>   obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
>   obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
> -
> diff --git a/drivers/input/misc/pega-accel.c b/drivers/input/misc/pega-accel.c
> new file mode 100644
> index 0000000..7005b46
> --- /dev/null
> +++ b/drivers/input/misc/pega-accel.c
> @@ -0,0 +1,130 @@
> +/*
> + * Driver for accelerometer in Pegatron Lucid tablets
> + *
> + * Copyright (c) 2011 Wind River Systems
> + *
> + * Author: Andy Ross<andy.ross@windriver.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include<linux/input-polldev.h>
> +#include<linux/module.h>
> +#include<linux/platform_device.h>
> +#include<acpi/acpi_bus.h>
> +
> +#define DRIVER_NAME "pega_accel"
> +#define DRIVER_DESC "Pegatron Lucid Tablet Accelerometer"
> +
> +/* 1G accel is reported as ~256, so clamp to 2G */
> +#define CLAMP 512
> +
> +static struct input_polled_dev *ipdev;
> +
> +/* FIXME: this mechanism is *very* slow: ~50ms to read the three
> + * values.  Pre-caching an acpi handle with acpi_get_handle has no
> + * effect, so the issue isn't in the parsing or tree walking... */
> +static int acpi_s16(char *method)
> +{
> +	unsigned long long val = 0;
> +	acpi_evaluate_integer(NULL, method, NULL,&val);
> +	return (short)val;
> +}
> +
> +static void pega_accel_poll(struct input_polled_dev *ipdev)
> +{
> +	/* 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).  FIXME:
> +	 * is there a relevant convention to adhere to? */
> +	int x = -acpi_s16("\\_SB.ATKD.XLRX");
> +	int y = -acpi_s16("\\_SB.ATKD.XLRY");
> +	int z =  acpi_s16("\\_SB.ATKD.XLRZ");
> +
> +	x = clamp_val(x, -CLAMP, CLAMP);
> +	y = clamp_val(y, -CLAMP, CLAMP);
> +	z = clamp_val(z, -CLAMP, CLAMP);
> +
> +	input_report_abs(ipdev->input, ABS_X, x);
> +	input_report_abs(ipdev->input, ABS_Y, y);
> +	input_report_abs(ipdev->input, ABS_Z, z);
> +	input_sync(ipdev->input);
> +}
> +
> +static int __devinit platform_probe(struct platform_device *pd)
> +{
> +	int err;
> +
> +	ipdev = input_allocate_polled_device();
> +	if (!ipdev)
> +		return -ENOMEM;
Could you explain why a polling is needed. Is no irq line connected.
> +
> +	ipdev->poll = pega_accel_poll;
> +	ipdev->poll_interval = 100;
> +	ipdev->poll_interval_min = 10;
> +	ipdev->poll_interval_max = 2000;
> +
> +	ipdev->input->dev.parent =&pd->dev;
> +	ipdev->input->id.bustype = BUS_HOST;
> +
> +	ipdev->input->name = DRIVER_DESC;
> +	ipdev->input->phys = DRIVER_NAME "/input0";
> +
> +	set_bit(EV_ABS, ipdev->input->evbit);
> +	input_set_abs_params(ipdev->input, ABS_X, -CLAMP, CLAMP, 0, 0);
> +	input_set_abs_params(ipdev->input, ABS_Y, -CLAMP, CLAMP, 0, 0);
> +	input_set_abs_params(ipdev->input, ABS_Z, -CLAMP, CLAMP, 0, 0);
> +
> +	err = input_register_polled_device(ipdev);
> +	if (err)
> +		input_free_polled_device(ipdev);
> +
> +	return err;
> +}
> +
> +static int __devexit platform_remove(struct platform_device *pd)
> +{
> +	input_unregister_polled_device(ipdev);
> +	input_free_polled_device(ipdev);
This may not be needed.
> +	ipdev = NULL;
> +	return 0;
> +}
> +
> +static struct platform_driver platform_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name  = DRIVER_NAME,
> +	},
> +	.probe  = platform_probe,
> +	.remove = __devexit_p(platform_remove),
Generally the name has the device name in the probe and remove.
However feel free to ignore such comments.
> +};
> +
> +static int __init mod_init(void)
> +{
> +	return platform_driver_register(&platform_driver);
> +}
> +
> +static void __exit mod_exit(void)
> +{
> +	platform_driver_unregister(&platform_driver);
> +}
> +
> +module_init(mod_init);
> +module_exit(mod_exit);
> +
> +MODULE_AUTHOR("Andy Ross<andy.ross@windriver.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_ALIAS("dmi:*:bvrLucid-CE-133:*");

Re: [PATCH 1/2] input: Pegatron Lucid accelerometer

[Dmitry Torokhov]

On Tue, Jan 18, 2011 at 11:18:17AM +0530, Shubhrajyoti wrote:
> On Monday 17 January 2011 11:26 PM, Andy Ross wrote:
> >+
> >+static int __devexit platform_remove(struct platform_device *pd)
> >+{
> >+	input_unregister_polled_device(ipdev);
> >+	input_free_polled_device(ipdev);
> This may not be needed.

Yes this is needed (for polled input devices only).

-- 
Dmitry

Re: [PATCH 1/2] input: Pegatron Lucid accelerometer

[Andy Ross]

On 01/17/2011 09:48 PM, Shubhrajyoti wrote:
> Could you explain why a polling is needed. Is no irq line connected.

It's just an ACPI method interface, so no dedicated IRQ certainly.
The firmware does issue ACPI events on coarse orientation changes
(i.e. which edge is up) that we're catching with acpid, so there's no
reason this needs to poll all the time in typical usage.

Andy

Re: [PATCH 1/2] input: Pegatron Lucid accelerometer

[Matthew Garrett]

On Tue, Jan 18, 2011 at 09:21:31AM -0800, Andy Ross wrote:

> It's just an ACPI method interface, so no dedicated IRQ certainly.
> The firmware does issue ACPI events on coarse orientation changes
> (i.e. which edge is up) that we're catching with acpid, so there's no
> reason this needs to poll all the time in typical usage.

It'd be good to provide those via some more generally-usable mechanism 
than raw ACPI events.

-- 
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org

------------------------------------------------------------------------------
Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)!
Finally, a world-class log management solution at an even better price-free!
Download using promo code Free_Logger_4_Dev2Dev. Offer expires 
February 28th, so secure your free ArcSight Logger TODAY! 
http://p.sf.net/sfu/arcsight-sfd2d

Re: [PATCH 1/2] input: Pegatron Lucid accelerometer

[Anisse Astier]

On Mon, 31 Jan 2011 21:30:57 +0000, Matthew Garrett <mjg59@srcf.ucam.org> wrote :

> On Tue, Jan 18, 2011 at 09:21:31AM -0800, Andy Ross wrote:
> 
> > It's just an ACPI method interface, so no dedicated IRQ certainly.
> > The firmware does issue ACPI events on coarse orientation changes
> > (i.e. which edge is up) that we're catching with acpid, so there's no
> > reason this needs to poll all the time in typical usage.
> 
> It'd be good to provide those via some more generally-usable mechanism 
> than raw ACPI events.
> 
Something like that ?


From: Anisse Astier <anisse@astier.eu>
Subject: [PATCH] asus-laptop: Send input event when orientation changes on Pegatron Lucid

Pegatron Lucid tablet sends an ACPI event on coarse orientation changes.
Translate this into KEY_DIRECTION input event.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 drivers/platform/x86/asus-laptop.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 75c84f8..d32395c 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -312,6 +312,8 @@ static const struct key_entry asus_keymap[] = {
 	{KE_KEY, 0xc4, { KEY_KBDILLUMUP } },
 	{KE_KEY, 0xc5, { KEY_KBDILLUMDOWN } },
 	{KE_KEY, 0xb5, { KEY_CALC } },
+	/* Pegatron Lucid tablet specific */
+	{KE_KEY, 0xEA, { KEY_DIRECTION } }, /* Orientation changed */
 	{KE_END, 0},
 };
 
-- 
1.7.3.2

[PATCH 2/2] asus-laptop: Support pega_accel driver

[Andy Ross]

Add device detecton for the ACPI accelerometer interface on Pegatron
Lucid tablets.

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

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index d235f44..6ee7eca 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
@@ -31,6 +32,7 @@
  *  Josh Green     - Light Sens support
  *  Thomas Tuttle  - His first patch for led support was very helpfull
  *  Sam Lin        - GPS support
+ *  Andy Ross      - Pegatron Lucid accelerometer
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -246,6 +248,7 @@ struct asus_laptop {
 
 	int wireless_status;
 	bool have_rsts;
+	bool have_pega_accel;
 	int lcd_state;
 
 	struct rfkill *gps_rfkill;
@@ -1352,6 +1355,7 @@ static const struct attribute_group asus_attr_group = {
 	.attrs		= asus_attributes,
 };
 
+
 static int asus_platform_init(struct asus_laptop *asus)
 {
 	int result;
@@ -1556,6 +1560,29 @@ static int __devinit asus_acpi_init(struct asus_laptop *asus)
 	return result;
 }
 
+static void pega_accel_noop_release(struct device *d) { }
+
+static struct platform_device pega_accel_dev = {
+	.name = "pega_accel",
+	.id   = -1,
+	.dev = {
+		.release = pega_accel_noop_release,
+	},
+};
+
+static void __devinit asus_pega_accel_init(struct asus_laptop *asus)
+{
+	/* Pegatron Lucid tablets expose their accelerometer through ACPI.
+	 * Check for XLR{X,Y,Z} methods */
+	if (acpi_check_handle(asus->handle, "XLRX", NULL) ||
+	    acpi_check_handle(asus->handle, "XLRY", NULL) ||
+	    acpi_check_handle(asus->handle, "XLRZ", NULL))
+		return;
+
+	if (!platform_device_register(&pega_accel_dev))
+		asus->have_pega_accel = true;
+}
+
 static bool asus_device_present;
 
 static int __devinit asus_acpi_add(struct acpi_device *device)
@@ -1605,6 +1632,8 @@ static int __devinit asus_acpi_add(struct acpi_device *device)
 	if (result)
 		goto fail_rfkill;
 
+	asus_pega_accel_init(asus);
+
 	asus_device_present = true;
 	return 0;
 
@@ -1627,6 +1656,9 @@ static int asus_acpi_remove(struct acpi_device *device, int type)
 {
 	struct asus_laptop *asus = acpi_driver_data(device);
 
+	if (asus->have_pega_accel)
+		platform_device_unregister(&pega_accel_dev);
+
 	asus_backlight_exit(asus);
 	asus_rfkill_exit(asus);
 	asus_led_exit(asus);
-- 
1.7.1

Re: [PATCH 0/2] Pegatron Lucid tablet accelerometer

[Dmitry Torokhov]

On Mon, Jan 17, 2011 at 09:56:49AM -0800, Andy Ross wrote:
> 
> > > +	ipdev->input->dev.release = dev_noop_release;
> > 
> > WTH is this???
> 
> It's an attempt to avoid this from drivers/base/core.c:
> 
>       WARN(1, KERN_ERR "Device '%s' does not have a release() "
>            "function, it is broken and must be fixed.\n",
>            dev_name(dev));
> 
> ...but it was in the wrong place.  The input device doesn't hit that
> path, the platform device does.  Moved to where it belongs.
> 

It does not belong anywhere. Please create platform device dynamically
(for example using platfrom_device_register_simple) and destroy it when
done.

The only time when static devices are [semi-]allowed (Greg has differing
opioon on this I believe) is in platform/arch code for devices that can
never be destroyed.

Thanks.

-- 
Dmitry

Re: [PATCH 0/2] Pegatron Lucid tablet accelerometer

[Andy Ross]

On 01/17/2011 10:49 AM, Dmitry Torokhov wrote:
> It does not belong anywhere. Please create platform device dynamically
> (for example using platfrom_device_register_simple) and destroy it when
> done.
>
> The only time when static devices are [semi-]allowed (Greg has differing
> opioon on this I believe) is in platform/arch code for devices that can
> never be destroyed.

OK, didn't know that rule.  I honestly thought avoiding the heap
allocation for this driver (which is is stateless: the "device" is
just a single bit of information indicating the presence of three acpi
methods) would be desirable.

Fixed patch 2/2 below.

Andy

>From 83c679bc5687d293c12f566c348871fc4e55896c Mon Sep 17 00:00:00 2001
From: Andy Ross <andy.ross@windriver.com>
Date: Mon, 17 Jan 2011 09:19:58 -0800
Subject: [PATCH 2/2] asus-laptop: Support pega_accel driver

Add device detecton for the ACPI accelerometer interface on Pegatron
Lucid tablets.

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

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index d235f44..73f8ca0 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
@@ -31,6 +32,7 @@
  *  Josh Green     - Light Sens support
  *  Thomas Tuttle  - His first patch for led support was very helpfull
  *  Sam Lin        - GPS support
+ *  Andy Ross      - Pegatron Lucid accelerometer
  */

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -247,6 +249,7 @@ struct asus_laptop {
 	int wireless_status;
 	bool have_rsts;
 	int lcd_state;
+	struct platform_device *pega_accel;

 	struct rfkill *gps_rfkill;

@@ -1556,6 +1559,19 @@ static int __devinit asus_acpi_init(struct asus_laptop *asus)
 	return result;
 }

+static void __devinit asus_pega_accel_init(struct asus_laptop *asus)
+{
+	/* Pegatron Lucid tablets expose their accelerometer through ACPI.
+	 * Check for XLR{X,Y,Z} methods */
+	if (acpi_check_handle(asus->handle, "XLRX", NULL) ||
+	    acpi_check_handle(asus->handle, "XLRY", NULL) ||
+	    acpi_check_handle(asus->handle, "XLRZ", NULL))
+		return;
+
+	asus->pega_accel = platform_device_register_simple("pega_accel", -1,
+							   NULL, 0);
+}
+
 static bool asus_device_present;

 static int __devinit asus_acpi_add(struct acpi_device *device)
@@ -1605,6 +1621,8 @@ static int __devinit asus_acpi_add(struct acpi_device *device)
 	if (result)
 		goto fail_rfkill;

+	asus_pega_accel_init(asus);
+
 	asus_device_present = true;
 	return 0;

@@ -1627,6 +1645,9 @@ static int asus_acpi_remove(struct acpi_device *device, int type)
 {
 	struct asus_laptop *asus = acpi_driver_data(device);

+	if (asus->pega_accel)
+		platform_device_unregister(asus->pega_accel);
+
 	asus_backlight_exit(asus);
 	asus_rfkill_exit(asus);
 	asus_led_exit(asus);
-- 
1.7.1