2009-03-01 13:30:11

by Daniel Mack

[permalink] [raw]
Subject: lis3's ACPI dependency

Hi Pavel, Eric,

are there any plans to free the lis3 driver from its ACPI dependency?
In fact, this device is I2C/SPI connected which the ACPI layer seems to
hide from the driver, but to use it on embedded devices, the bus drivers
must be used directly and the dependeny seems entirely unnecessary
anyway.

Also I got the feeling that using a globally exported 'adev' symbol all
over the different layers is not the best practice - I suspect all the
occasions could be solved with private pointers and/or container_of().
Isn't there any cleanup pending?

Thanks and best regards,
Daniel


2009-03-01 19:51:41

by Robert Hancock

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

Daniel Mack wrote:
> Hi Pavel, Eric,
>
> are there any plans to free the lis3 driver from its ACPI dependency?
> In fact, this device is I2C/SPI connected which the ACPI layer seems to
> hide from the driver, but to use it on embedded devices, the bus drivers
> must be used directly and the dependeny seems entirely unnecessary
> anyway.

If ACPI AML code attempts to access the device (which it obviously does,
since we're presumably using the same code to access it), then we
cannot be accessing it directly at the same time, otherwise the two will
stomp on each other.

>
> Also I got the feeling that using a globally exported 'adev' symbol all
> over the different layers is not the best practice - I suspect all the
> occasions could be solved with private pointers and/or container_of().
> Isn't there any cleanup pending?
>
> Thanks and best regards,
> Daniel
>

2009-03-01 21:59:09

by Éric Piel

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

Daniel Mack schreef:
> Hi Pavel, Eric,
>
> are there any plans to free the lis3 driver from its ACPI dependency?
> In fact, this device is I2C/SPI connected which the ACPI layer seems to
> hide from the driver, but to use it on embedded devices, the bus drivers
> must be used directly and the dependeny seems entirely unnecessary
> anyway.
>
> Also I got the feeling that using a globally exported 'adev' symbol all
> over the different layers is not the best practice - I suspect all the
> occasions could be solved with private pointers and/or container_of().
> Isn't there any cleanup pending?
Hi Daniel,
Actually, the hp_accel.c file is dedicated to the acpi part. The
lis3lv02d.c file is supposed to be bus agnostic. In practice I agree
it's not completely true, but this is mainly because for now there is
only one bus supported (ACPI) and therefore the work for making it
completely bus-independent is not yet worthy. As soon as someone is
interested in writing a I?C/SPI interface it will become much more
apparent what has to be generalized.

Indeed, as you noticed, the main work left to do is to generalize
acpi_lis3lv02d (adev) to something ACPI independent. I'd say this could
be done simply by changing the signature of the four callback functions
and having device being just a void*.

If you are willing to write one of the I?C/SPI interfaces, I'll be happy
to support you.

See you,
Eric

2009-03-02 00:50:43

by Daniel Mack

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

On Sun, Mar 01, 2009 at 01:51:25PM -0600, Robert Hancock wrote:
>> are there any plans to free the lis3 driver from its ACPI dependency?
>> In fact, this device is I2C/SPI connected which the ACPI layer seems to
>> hide from the driver, but to use it on embedded devices, the bus drivers
>> must be used directly and the dependeny seems entirely unnecessary
>> anyway.
>
> If ACPI AML code attempts to access the device (which it obviously does,
> since we're presumably using the same code to access it), then we
> cannot be accessing it directly at the same time, otherwise the two will
> stomp on each other.

It's not about accessing the same device in more than one way one one
particular platform but about keeping the driver abstract enough so it
can be hooked up to different bus types. At the moment, it isn't
completely seperated from ACPI, that's why I'm asking.

Daniel

2009-03-02 00:56:18

by Daniel Mack

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

On Sun, Mar 01, 2009 at 07:28:01PM +0100, ?ric Piel wrote:
> > are there any plans to free the lis3 driver from its ACPI dependency?
> > In fact, this device is I2C/SPI connected which the ACPI layer seems to
> > hide from the driver, but to use it on embedded devices, the bus drivers
> > must be used directly and the dependeny seems entirely unnecessary
> > anyway.
> >
> > Also I got the feeling that using a globally exported 'adev' symbol all
> > over the different layers is not the best practice - I suspect all the
> > occasions could be solved with private pointers and/or container_of().
> > Isn't there any cleanup pending?
> Hi Daniel,
> Actually, the hp_accel.c file is dedicated to the acpi part. The
> lis3lv02d.c file is supposed to be bus agnostic. In practice I agree
> it's not completely true, but this is mainly because for now there is
> only one bus supported (ACPI) and therefore the work for making it
> completely bus-independent is not yet worthy. As soon as someone is
> interested in writing a I?C/SPI interface it will become much more
> apparent what has to be generalized.

Well, it's clear already - there must be no occurance of apci specific
structs and calls in lis3lv02d.c and all the calls to the bus type need
to go thru a generic interface which is - as you say - bus independent.

> Indeed, as you noticed, the main work left to do is to generalize
> acpi_lis3lv02d (adev) to something ACPI independent. I'd say this could
> be done simply by changing the signature of the four callback functions
> and having device being just a void*.
>
> If you are willing to write one of the I?C/SPI interfaces, I'll be happy
> to support you.

Ok, I'll start and seperate things then but will need you help with
testing as I don't have the hardware the driver was written for
originally. I do have, in turn, a custom board with this chip connected
via SPI.

Is there any patch pending on top of Linus' current git head that I
should base my patches on?

Thanks,
Daniel

2009-03-02 10:17:32

by Éric Piel

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

Daniel Mack schreef:

>
> Ok, I'll start and seperate things then but will need you help with
> testing as I don't have the hardware the driver was written for
> originally. I do have, in turn, a custom board with this chip connected
> via SPI.
Very good.

> Is there any patch pending on top of Linus' current git head that I
> should base my patches on?
There are a couple of clean-up patches from Pavel hanging around in the
-mm tree. See hp_accel* in http://userweb.kernel.org/~akpm/mmotm/broken-out/

Eric

2009-03-02 14:32:21

by Daniel Mack

[permalink] [raw]
Subject:

From: Pavel Machek <[email protected]>

Fix english in Documentation, add "how to test" description.

Signed-off-by: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
Cc: Vladimir Botka <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
This patch is already in the -mm tree, posted as reference only.

Documentation/hwmon/lis3lv02d | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff -puN Documentation/hwmon/lis3lv02d~hp_accel-small-documentation-updates Documentation/hwmon/lis3lv02d
--- a/Documentation/hwmon/lis3lv02d~hp_accel-small-documentation-updates
+++ a/Documentation/hwmon/lis3lv02d
@@ -1,11 +1,11 @@
Kernel driver lis3lv02d
-==================
+=======================

Supported chips:

* STMicroelectronics LIS3LV02DL and LIS3LV02DQ

-Author:
+Authors:
Yan Burman <[email protected]>
Eric Piel <[email protected]>

@@ -15,7 +15,7 @@ Description

This driver provides support for the accelerometer found in various HP
laptops sporting the feature officially called "HP Mobile Data
-Protection System 3D" or "HP 3D DriveGuard". It detect automatically
+Protection System 3D" or "HP 3D DriveGuard". It detects automatically
laptops with this sensor. Known models (for now the HP 2133, nc6420,
nc2510, nc8510, nc84x0, nw9440 and nx9420) will have their axis
automatically oriented on standard way (eg: you can directly play
@@ -27,7 +27,7 @@ position - 3D position that the accelero
calibrate - read: values (x, y, z) that are used as the base for input
class device operation.
write: forces the base to be recalibrated with the current
- position.
+ position.
rate - reports the sampling rate of the accelerometer device in HZ

This driver also provides an absolute input class device, allowing
@@ -48,7 +48,7 @@ For better compatibility between the var
the accelerometer are converted into a "standard" organisation of the axes
(aka "can play neverball out of the box"):
* When the laptop is horizontal the position reported is about 0 for X and Y
-and a positive value for Z
+ and a positive value for Z
* If the left side is elevated, X increases (becomes positive)
* If the front side (where the touchpad is) is elevated, Y decreases
(becomes negative)
@@ -59,3 +59,13 @@ email to the authors to add it to the da
laptop, please include the output of "dmidecode" plus the value of
/sys/devices/platform/lis3lv02d/position in these four cases.

+Q&A
+---
+
+Q: How do I safely simulate freefall? I have an HP "portable
+workstation" which has about 3.5kg and a plastic case, so letting it
+fall to the ground is out of question...
+
+A: The sensor is pretty sensitive, so your hands can do it. Lift it
+into free space, follow the fall with your hands for like 10
+centimeters. That should be enough to trigger the detection.
_

2009-03-02 14:32:34

by Daniel Mack

[permalink] [raw]
Subject:

From: Pavel Machek <[email protected]>

As Andrew noted, adev is pretty poor name for symbol being exported.
Rename it to lis3.

Signed-off-by: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
Cc: Vladimir Botka <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
This patch is already in the -mm tree, posted as reference only.

drivers/hwmon/hp_accel.c | 48 ++++-----
drivers/hwmon/lis3lv02d.c | 176 +++++++++++++++++++-----------------
drivers/hwmon/lis3lv02d.h | 2
3 files changed, 118 insertions(+), 108 deletions(-)

diff -puN drivers/hwmon/hp_accel.c~hp_accel-adev-is-poor-name-of-exported-symbol drivers/hwmon/hp_accel.c
--- a/drivers/hwmon/hp_accel.c~hp_accel-adev-is-poor-name-of-exported-symbol
+++ a/drivers/hwmon/hp_accel.c
@@ -140,7 +140,7 @@ acpi_status lis3lv02d_acpi_write(acpi_ha

static int lis3lv02d_dmi_matched(const struct dmi_system_id *dmi)
{
- adev.ac = *((struct axis_conversion *)dmi->driver_data);
+ lis3_dev.ac = *((struct axis_conversion *)dmi->driver_data);
printk(KERN_INFO DRIVER_NAME ": hardware type %s found.\n", dmi->ident);

return 1;
@@ -214,7 +214,7 @@ static struct dmi_system_id lis3lv02d_dm

static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness value)
{
- acpi_handle handle = adev.device->handle;
+ acpi_handle handle = lis3_dev.device->handle;
unsigned long long ret; /* Not used when writing */
union acpi_object in_obj[1];
struct acpi_object_list args = { 1, in_obj };
@@ -254,7 +254,7 @@ static void lis3lv02d_enum_resources(str
acpi_status status;

status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
- lis3lv02d_get_resource, &adev.irq);
+ lis3lv02d_get_resource, &lis3_dev.irq);
if (ACPI_FAILURE(status))
printk(KERN_DEBUG DRIVER_NAME ": Error getting resources\n");
}
@@ -263,8 +263,8 @@ static s16 lis3lv02d_read_16(acpi_handle
{
u8 lo, hi;

- adev.read(handle, reg - 1, &lo);
- adev.read(handle, reg, &hi);
+ lis3_dev.read(handle, reg - 1, &lo);
+ lis3_dev.read(handle, reg, &hi);
/* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
return (s16)((hi << 8) | lo);
}
@@ -272,7 +272,7 @@ static s16 lis3lv02d_read_16(acpi_handle
static s16 lis3lv02d_read_8(acpi_handle handle, int reg)
{
s8 lo;
- adev.read(handle, reg, &lo);
+ lis3_dev.read(handle, reg, &lo);
return lo;
}

@@ -283,29 +283,29 @@ static int lis3lv02d_add(struct acpi_dev
if (!device)
return -EINVAL;

- adev.device = device;
- adev.init = lis3lv02d_acpi_init;
- adev.read = lis3lv02d_acpi_read;
- adev.write = lis3lv02d_acpi_write;
+ lis3_dev.device = device;
+ lis3_dev.init = lis3lv02d_acpi_init;
+ lis3_dev.read = lis3lv02d_acpi_read;
+ lis3_dev.write = lis3lv02d_acpi_write;
strcpy(acpi_device_name(device), DRIVER_NAME);
strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
- device->driver_data = &adev;
+ device->driver_data = &lis3_dev;

- lis3lv02d_acpi_read(device->handle, WHO_AM_I, &adev.whoami);
- switch (adev.whoami) {
+ lis3lv02d_acpi_read(device->handle, WHO_AM_I, &lis3_dev.whoami);
+ switch (lis3_dev.whoami) {
case LIS_DOUBLE_ID:
printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
- adev.read_data = lis3lv02d_read_16;
- adev.mdps_max_val = 2048;
+ lis3_dev.read_data = lis3lv02d_read_16;
+ lis3_dev.mdps_max_val = 2048;
break;
case LIS_SINGLE_ID:
printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
- adev.read_data = lis3lv02d_read_8;
- adev.mdps_max_val = 128;
+ lis3_dev.read_data = lis3lv02d_read_8;
+ lis3_dev.mdps_max_val = 128;
break;
default:
printk(KERN_ERR DRIVER_NAME
- ": unknown sensor type 0x%X\n", adev.whoami);
+ ": unknown sensor type 0x%X\n", lis3_dev.whoami);
return -EINVAL;
}

@@ -313,7 +313,7 @@ static int lis3lv02d_add(struct acpi_dev
if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
printk(KERN_INFO DRIVER_NAME ": laptop model unknown, "
"using default axes configuration\n");
- adev.ac = lis3lv02d_axis_normal;
+ lis3_dev.ac = lis3lv02d_axis_normal;
}

INIT_WORK(&hpled_led.work, delayed_set_status_worker);
@@ -322,9 +322,9 @@ static int lis3lv02d_add(struct acpi_dev
return ret;

/* obtain IRQ number of our device from ACPI */
- lis3lv02d_enum_resources(adev.device);
+ lis3lv02d_enum_resources(lis3_dev.device);

- ret = lis3lv02d_init_device(&adev);
+ ret = lis3lv02d_init_device(&lis3_dev);
if (ret) {
flush_work(&hpled_led.work);
led_classdev_unregister(&hpled_led.led_classdev);
@@ -360,12 +360,12 @@ static int lis3lv02d_suspend(struct acpi
static int lis3lv02d_resume(struct acpi_device *device)
{
/* put back the device in the right state (ACPI might turn it on) */
- mutex_lock(&adev.lock);
- if (adev.usage > 0)
+ mutex_lock(&lis3_dev.lock);
+ if (lis3_dev.usage > 0)
lis3lv02d_poweron(device->handle);
else
lis3lv02d_poweroff(device->handle);
- mutex_unlock(&adev.lock);
+ mutex_unlock(&lis3_dev.lock);
return 0;
}
#else
diff -puN drivers/hwmon/lis3lv02d.c~hp_accel-adev-is-poor-name-of-exported-symbol drivers/hwmon/lis3lv02d.c
--- a/drivers/hwmon/lis3lv02d.c~hp_accel-adev-is-poor-name-of-exported-symbol
+++ a/drivers/hwmon/lis3lv02d.c
@@ -53,14 +53,24 @@
* joystick.
*/

-struct acpi_lis3lv02d adev = {
- .misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(adev.misc_wait),
+struct acpi_lis3lv02d lis3_dev = {
+ .misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
};

-EXPORT_SYMBOL_GPL(adev);
+EXPORT_SYMBOL_GPL(lis3_dev);

static int lis3lv02d_add_fs(struct acpi_device *device);

+static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
+{
+ u8 lo, hi;
+
+ lis3_dev.read(handle, reg, &lo);
+ lis3_dev.read(handle, reg + 1, &hi);
+ /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
+ return (s16)((hi << 8) | lo);
+}
+
/**
* lis3lv02d_get_axis - For the given axis, give the value converted
* @axis: 1,2,3 - can also be negative
@@ -89,25 +99,25 @@ static void lis3lv02d_get_xyz(acpi_handl
{
int position[3];

- position[0] = adev.read_data(handle, OUTX);
- position[1] = adev.read_data(handle, OUTY);
- position[2] = adev.read_data(handle, OUTZ);
-
- *x = lis3lv02d_get_axis(adev.ac.x, position);
- *y = lis3lv02d_get_axis(adev.ac.y, position);
- *z = lis3lv02d_get_axis(adev.ac.z, position);
+ position[0] = lis3_dev.read_data(handle, OUTX);
+ position[1] = lis3_dev.read_data(handle, OUTY);
+ position[2] = lis3_dev.read_data(handle, OUTZ);
+
+ *x = lis3lv02d_get_axis(lis3_dev.ac.x, position);
+ *y = lis3lv02d_get_axis(lis3_dev.ac.y, position);
+ *z = lis3lv02d_get_axis(lis3_dev.ac.z, position);
}

void lis3lv02d_poweroff(acpi_handle handle)
{
- adev.is_on = 0;
+ lis3_dev.is_on = 0;
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweroff);

void lis3lv02d_poweron(acpi_handle handle)
{
- adev.is_on = 1;
- adev.init(handle);
+ lis3_dev.is_on = 1;
+ lis3_dev.init(handle);
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

@@ -147,10 +157,10 @@ static irqreturn_t lis302dl_interrupt(in
* the lid is closed. This leads to interrupts as soon as a little move
* is done.
*/
- atomic_inc(&adev.count);
+ atomic_inc(&lis3_dev.count);

- wake_up_interruptible(&adev.misc_wait);
- kill_fasync(&adev.async_queue, SIGIO, POLL_IN);
+ wake_up_interruptible(&lis3_dev.misc_wait);
+ kill_fasync(&lis3_dev.async_queue, SIGIO, POLL_IN);
return IRQ_HANDLED;
}

@@ -158,10 +168,10 @@ static int lis3lv02d_misc_open(struct in
{
int ret;

- if (test_and_set_bit(0, &adev.misc_opened))
+ if (test_and_set_bit(0, &lis3_dev.misc_opened))
return -EBUSY; /* already open */

- atomic_set(&adev.count, 0);
+ atomic_set(&lis3_dev.count, 0);

/*
* The sensor can generate interrupts for free-fall and direction
@@ -174,25 +184,25 @@ static int lis3lv02d_misc_open(struct in
* io-apic is not configurable (and generates a warning) but I keep it
* in case of support for other hardware.
*/
- ret = request_irq(adev.irq, lis302dl_interrupt, IRQF_TRIGGER_RISING,
- DRIVER_NAME, &adev);
+ ret = request_irq(lis3_dev.irq, lis302dl_interrupt, IRQF_TRIGGER_RISING,
+ DRIVER_NAME, &lis3_dev);

if (ret) {
- clear_bit(0, &adev.misc_opened);
- printk(KERN_ERR DRIVER_NAME ": IRQ%d allocation failed\n", adev.irq);
+ clear_bit(0, &lis3_dev.misc_opened);
+ printk(KERN_ERR DRIVER_NAME ": IRQ%d allocation failed\n", lis3_dev.irq);
return -EBUSY;
}
- lis3lv02d_increase_use(&adev);
- printk("lis3: registered interrupt %d\n", adev.irq);
+ lis3lv02d_increase_use(&lis3_dev);
+ printk("lis3: registered interrupt %d\n", lis3_dev.irq);
return 0;
}

static int lis3lv02d_misc_release(struct inode *inode, struct file *file)
{
- fasync_helper(-1, file, 0, &adev.async_queue);
- lis3lv02d_decrease_use(&adev);
- free_irq(adev.irq, &adev);
- clear_bit(0, &adev.misc_opened); /* release the device */
+ fasync_helper(-1, file, 0, &lis3_dev.async_queue);
+ lis3lv02d_decrease_use(&lis3_dev);
+ free_irq(lis3_dev.irq, &lis3_dev);
+ clear_bit(0, &lis3_dev.misc_opened); /* release the device */
return 0;
}

@@ -207,10 +217,10 @@ static ssize_t lis3lv02d_misc_read(struc
if (count < 1)
return -EINVAL;

- add_wait_queue(&adev.misc_wait, &wait);
+ add_wait_queue(&lis3_dev.misc_wait, &wait);
while (true) {
set_current_state(TASK_INTERRUPTIBLE);
- data = atomic_xchg(&adev.count, 0);
+ data = atomic_xchg(&lis3_dev.count, 0);
if (data)
break;

@@ -240,22 +250,22 @@ static ssize_t lis3lv02d_misc_read(struc

out:
__set_current_state(TASK_RUNNING);
- remove_wait_queue(&adev.misc_wait, &wait);
+ remove_wait_queue(&lis3_dev.misc_wait, &wait);

return retval;
}

static unsigned int lis3lv02d_misc_poll(struct file *file, poll_table *wait)
{
- poll_wait(file, &adev.misc_wait, wait);
- if (atomic_read(&adev.count))
+ poll_wait(file, &lis3_dev.misc_wait, wait);
+ if (atomic_read(&lis3_dev.count))
return POLLIN | POLLRDNORM;
return 0;
}

static int lis3lv02d_misc_fasync(int fd, struct file *file, int on)
{
- return fasync_helper(fd, file, on, &adev.async_queue);
+ return fasync_helper(fd, file, on, &lis3_dev.async_queue);
}

static const struct file_operations lis3lv02d_misc_fops = {
@@ -283,12 +293,12 @@ static int lis3lv02d_joystick_kthread(vo
int x, y, z;

while (!kthread_should_stop()) {
- lis3lv02d_get_xyz(adev.device->handle, &x, &y, &z);
- input_report_abs(adev.idev, ABS_X, x - adev.xcalib);
- input_report_abs(adev.idev, ABS_Y, y - adev.ycalib);
- input_report_abs(adev.idev, ABS_Z, z - adev.zcalib);
+ lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ input_report_abs(lis3_dev.idev, ABS_X, x - lis3_dev.xcalib);
+ input_report_abs(lis3_dev.idev, ABS_Y, y - lis3_dev.ycalib);
+ input_report_abs(lis3_dev.idev, ABS_Z, z - lis3_dev.zcalib);

- input_sync(adev.idev);
+ input_sync(lis3_dev.idev);

try_to_freeze();
msleep_interruptible(MDPS_POLL_INTERVAL);
@@ -299,11 +309,11 @@ static int lis3lv02d_joystick_kthread(vo

static int lis3lv02d_joystick_open(struct input_dev *input)
{
- lis3lv02d_increase_use(&adev);
- adev.kthread = kthread_run(lis3lv02d_joystick_kthread, NULL, "klis3lv02d");
- if (IS_ERR(adev.kthread)) {
- lis3lv02d_decrease_use(&adev);
- return PTR_ERR(adev.kthread);
+ lis3lv02d_increase_use(&lis3_dev);
+ lis3_dev.kthread = kthread_run(lis3lv02d_joystick_kthread, NULL, "klis3lv02d");
+ if (IS_ERR(lis3_dev.kthread)) {
+ lis3lv02d_decrease_use(&lis3_dev);
+ return PTR_ERR(lis3_dev.kthread);
}

return 0;
@@ -311,45 +321,45 @@ static int lis3lv02d_joystick_open(struc

static void lis3lv02d_joystick_close(struct input_dev *input)
{
- kthread_stop(adev.kthread);
- lis3lv02d_decrease_use(&adev);
+ kthread_stop(lis3_dev.kthread);
+ lis3lv02d_decrease_use(&lis3_dev);
}

static inline void lis3lv02d_calibrate_joystick(void)
{
- lis3lv02d_get_xyz(adev.device->handle, &adev.xcalib, &adev.ycalib, &adev.zcalib);
+ lis3lv02d_get_xyz(lis3_dev.device->handle, &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
}

int lis3lv02d_joystick_enable(void)
{
int err;

- if (adev.idev)
+ if (lis3_dev.idev)
return -EINVAL;

- adev.idev = input_allocate_device();
- if (!adev.idev)
+ lis3_dev.idev = input_allocate_device();
+ if (!lis3_dev.idev)
return -ENOMEM;

lis3lv02d_calibrate_joystick();

- adev.idev->name = "ST LIS3LV02DL Accelerometer";
- adev.idev->phys = DRIVER_NAME "/input0";
- adev.idev->id.bustype = BUS_HOST;
- adev.idev->id.vendor = 0;
- adev.idev->dev.parent = &adev.pdev->dev;
- adev.idev->open = lis3lv02d_joystick_open;
- adev.idev->close = lis3lv02d_joystick_close;
-
- set_bit(EV_ABS, adev.idev->evbit);
- input_set_abs_params(adev.idev, ABS_X, -adev.mdps_max_val, adev.mdps_max_val, 3, 3);
- input_set_abs_params(adev.idev, ABS_Y, -adev.mdps_max_val, adev.mdps_max_val, 3, 3);
- input_set_abs_params(adev.idev, ABS_Z, -adev.mdps_max_val, adev.mdps_max_val, 3, 3);
+ lis3_dev.idev->name = "ST LIS3LV02DL Accelerometer";
+ lis3_dev.idev->phys = DRIVER_NAME "/input0";
+ lis3_dev.idev->id.bustype = BUS_HOST;
+ lis3_dev.idev->id.vendor = 0;
+ lis3_dev.idev->dev.parent = &lis3_dev.pdev->dev;
+ lis3_dev.idev->open = lis3lv02d_joystick_open;
+ lis3_dev.idev->close = lis3lv02d_joystick_close;
+
+ set_bit(EV_ABS, lis3_dev.idev->evbit);
+ input_set_abs_params(lis3_dev.idev, ABS_X, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
+ input_set_abs_params(lis3_dev.idev, ABS_Y, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
+ input_set_abs_params(lis3_dev.idev, ABS_Z, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);

- err = input_register_device(adev.idev);
+ err = input_register_device(lis3_dev.idev);
if (err) {
- input_free_device(adev.idev);
- adev.idev = NULL;
+ input_free_device(lis3_dev.idev);
+ lis3_dev.idev = NULL;
}

return err;
@@ -358,12 +368,12 @@ EXPORT_SYMBOL_GPL(lis3lv02d_joystick_ena

void lis3lv02d_joystick_disable(void)
{
- if (!adev.idev)
+ if (!lis3_dev.idev)
return;

misc_deregister(&lis3lv02d_misc_device);
- input_unregister_device(adev.idev);
- adev.idev = NULL;
+ input_unregister_device(lis3_dev.idev);
+ lis3_dev.idev = NULL;
}
EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);

@@ -404,25 +414,25 @@ static ssize_t lis3lv02d_position_show(s
{
int x, y, z;

- lis3lv02d_increase_use(&adev);
- lis3lv02d_get_xyz(adev.device->handle, &x, &y, &z);
- lis3lv02d_decrease_use(&adev);
+ lis3lv02d_increase_use(&lis3_dev);
+ lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_decrease_use(&lis3_dev);
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}

static ssize_t lis3lv02d_calibrate_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "(%d,%d,%d)\n", adev.xcalib, adev.ycalib, adev.zcalib);
+ return sprintf(buf, "(%d,%d,%d)\n", lis3_dev.xcalib, lis3_dev.ycalib, lis3_dev.zcalib);
}

static ssize_t lis3lv02d_calibrate_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- lis3lv02d_increase_use(&adev);
+ lis3lv02d_increase_use(&lis3_dev);
lis3lv02d_calibrate_joystick();
- lis3lv02d_decrease_use(&adev);
+ lis3lv02d_decrease_use(&lis3_dev);
return count;
}

@@ -434,9 +444,9 @@ static ssize_t lis3lv02d_rate_show(struc
u8 ctrl;
int val;

- lis3lv02d_increase_use(&adev);
- adev.read(adev.device->handle, CTRL_REG1, &ctrl);
- lis3lv02d_decrease_use(&adev);
+ lis3lv02d_increase_use(&lis3_dev);
+ lis3_dev.read(lis3_dev.device->handle, CTRL_REG1, &ctrl);
+ lis3lv02d_decrease_use(&lis3_dev);
val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
}
@@ -460,17 +470,17 @@ static struct attribute_group lis3lv02d_

static int lis3lv02d_add_fs(struct acpi_device *device)
{
- adev.pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
- if (IS_ERR(adev.pdev))
- return PTR_ERR(adev.pdev);
+ lis3_dev.pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
+ if (IS_ERR(lis3_dev.pdev))
+ return PTR_ERR(lis3_dev.pdev);

- return sysfs_create_group(&adev.pdev->dev.kobj, &lis3lv02d_attribute_group);
+ return sysfs_create_group(&lis3_dev.pdev->dev.kobj, &lis3lv02d_attribute_group);
}

int lis3lv02d_remove_fs(void)
{
- sysfs_remove_group(&adev.pdev->dev.kobj, &lis3lv02d_attribute_group);
- platform_device_unregister(adev.pdev);
+ sysfs_remove_group(&lis3_dev.pdev->dev.kobj, &lis3lv02d_attribute_group);
+ platform_device_unregister(lis3_dev.pdev);
return 0;
}
EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
diff -puN drivers/hwmon/lis3lv02d.h~hp_accel-adev-is-poor-name-of-exported-symbol drivers/hwmon/lis3lv02d.h
--- a/drivers/hwmon/lis3lv02d.h~hp_accel-adev-is-poor-name-of-exported-symbol
+++ a/drivers/hwmon/lis3lv02d.h
@@ -194,4 +194,4 @@ void lis3lv02d_poweroff(acpi_handle hand
void lis3lv02d_poweron(acpi_handle handle);
int lis3lv02d_remove_fs(void);

-extern struct acpi_lis3lv02d adev;
+extern struct acpi_lis3lv02d lis3_dev;
_

2009-03-02 14:32:50

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 3/5] lis3: reorder functions to make forward decl obsolete

moved lis3lv02d_init_device() down so that the forward declaration of
lis3lv02d_add_fs() becomes unnecessary.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 64 +++++++++++++++++++++-----------------------
1 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 4888ac5..eeae7c9 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -59,8 +59,6 @@ struct acpi_lis3lv02d lis3_dev = {

EXPORT_SYMBOL_GPL(lis3_dev);

-static int lis3lv02d_add_fs(struct acpi_device *device);
-
static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
{
u8 lo, hi;
@@ -377,37 +375,6 @@ void lis3lv02d_joystick_disable(void)
}
EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);

-/*
- * Initialise the accelerometer and the various subsystems.
- * Should be rather independant of the bus system.
- */
-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
-{
- mutex_init(&dev->lock);
- lis3lv02d_add_fs(dev->device);
- lis3lv02d_increase_use(dev);
-
- if (lis3lv02d_joystick_enable())
- printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
-
- printk("lis3_init_device: irq %d\n", dev->irq);
-
- /* if we did not get an IRQ from ACPI - we have nothing more to do */
- if (!dev->irq) {
- printk(KERN_ERR DRIVER_NAME
- ": No IRQ in ACPI. Disabling /dev/freefall\n");
- goto out;
- }
-
- printk("lis3: registering device\n");
- if (misc_register(&lis3lv02d_misc_device))
- printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
-out:
- lis3lv02d_decrease_use(dev);
- return 0;
-}
-EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
-
/* Sysfs stuff */
static ssize_t lis3lv02d_position_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -485,6 +452,37 @@ int lis3lv02d_remove_fs(void)
}
EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);

+/*
+ * Initialise the accelerometer and the various subsystems.
+ * Should be rather independant of the bus system.
+ */
+int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
+{
+ mutex_init(&dev->lock);
+ lis3lv02d_add_fs(dev->device);
+ lis3lv02d_increase_use(dev);
+
+ if (lis3lv02d_joystick_enable())
+ printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
+
+ printk("lis3_init_device: irq %d\n", dev->irq);
+
+ /* if we did not get an IRQ from ACPI - we have nothing more to do */
+ if (!dev->irq) {
+ printk(KERN_ERR DRIVER_NAME
+ ": No IRQ in ACPI. Disabling /dev/freefall\n");
+ goto out;
+ }
+
+ printk("lis3: registering device\n");
+ if (misc_register(&lis3lv02d_misc_device))
+ printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
+out:
+ lis3lv02d_decrease_use(dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
+
MODULE_DESCRIPTION("ST LIS3LV02Dx three-axis digital accelerometer driver");
MODULE_AUTHOR("Yan Burman, Eric Piel, Pavel Machek");
MODULE_LICENSE("GPL");
--
1.6.1.3

2009-03-02 14:33:11

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 4/5] lis3: solve dependency between core and ACPI

This solves the dependency between lis3lv02d.[ch] and ACPI specific
methods. It introduces a ->bus_priv pointer to the device struct which
is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
accordingly.

This also moves the read_8() and read_16() routines from hp_accel.c to
lis3lv02d.c as they are not specific to ACPI.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
---
drivers/hwmon/hp_accel.c | 100 +++++++++++++++++----------------------------
drivers/hwmon/lis3lv02d.c | 86 +++++++++++++++++++++++++-------------
drivers/hwmon/lis3lv02d.h | 20 +++++-----
3 files changed, 105 insertions(+), 101 deletions(-)

diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index ee1ed42..39e5011 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -85,25 +85,31 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids);

/**
* lis3lv02d_acpi_init - ACPI _INI method: initialize the device.
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_init(acpi_handle handle)
+int lis3lv02d_acpi_init(struct lis3lv02d *lis3)
{
- return acpi_evaluate_object(handle, METHOD_NAME__INI, NULL, NULL);
+ struct acpi_device *dev = lis3->bus_priv;
+ if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI,
+ NULL, NULL) != AE_OK)
+ return -EINVAL;
+
+ return 0;
}

/**
* lis3lv02d_acpi_read - ACPI ALRD method: read a register
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
* @reg: the register to read
* @ret: result of the operation
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret)
+int lis3lv02d_acpi_read(struct lis3lv02d *lis3, int reg, u8 *ret)
{
+ struct acpi_device *dev = lis3->bus_priv;
union acpi_object arg0 = { ACPI_TYPE_INTEGER };
struct acpi_object_list args = { 1, &arg0 };
unsigned long long lret;
@@ -111,21 +117,22 @@ acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret)

arg0.integer.value = reg;

- status = acpi_evaluate_integer(handle, "ALRD", &args, &lret);
+ status = acpi_evaluate_integer(dev->handle, "ALRD", &args, &lret);
*ret = lret;
- return status;
+ return (status != AE_OK) ? -EINVAL : 0;
}

/**
* lis3lv02d_acpi_write - ACPI ALWR method: write to a register
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
* @reg: the register to write to
* @val: the value to write
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val)
+int lis3lv02d_acpi_write(struct lis3lv02d *lis3, int reg, u8 val)
{
+ struct acpi_device *dev = lis3->bus_priv;
unsigned long long ret; /* Not used when writting */
union acpi_object in_obj[2];
struct acpi_object_list args = { 2, in_obj };
@@ -135,7 +142,10 @@ acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val)
in_obj[1].type = ACPI_TYPE_INTEGER;
in_obj[1].integer.value = val;

- return acpi_evaluate_integer(handle, "ALWR", &args, &ret);
+ if (acpi_evaluate_integer(dev->handle, "ALWR", &args, &ret) != AE_OK)
+ return -EINVAL;
+
+ return 0;
}

static int lis3lv02d_dmi_matched(const struct dmi_system_id *dmi)
@@ -214,7 +224,7 @@ static struct dmi_system_id lis3lv02d_dmi_ids[] = {

static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness value)
{
- acpi_handle handle = lis3_dev.device->handle;
+ struct acpi_device *dev = lis3_dev.bus_priv;
unsigned long long ret; /* Not used when writing */
union acpi_object in_obj[1];
struct acpi_object_list args = { 1, in_obj };
@@ -222,7 +232,7 @@ static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness
in_obj[0].type = ACPI_TYPE_INTEGER;
in_obj[0].integer.value = !!value;

- acpi_evaluate_integer(handle, "ALED", &args, &ret);
+ acpi_evaluate_integer(dev->handle, "ALED", &args, &ret);
}

static struct delayed_led_classdev hpled_led = {
@@ -259,23 +269,6 @@ static void lis3lv02d_enum_resources(struct acpi_device *device)
printk(KERN_DEBUG DRIVER_NAME ": Error getting resources\n");
}

-static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
-{
- u8 lo, hi;
-
- lis3_dev.read(handle, reg - 1, &lo);
- lis3_dev.read(handle, reg, &hi);
- /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
- return (s16)((hi << 8) | lo);
-}
-
-static s16 lis3lv02d_read_8(acpi_handle handle, int reg)
-{
- s8 lo;
- lis3_dev.read(handle, reg, &lo);
- return lo;
-}
-
static int lis3lv02d_add(struct acpi_device *device)
{
int ret;
@@ -283,7 +276,7 @@ static int lis3lv02d_add(struct acpi_device *device)
if (!device)
return -EINVAL;

- lis3_dev.device = device;
+ lis3_dev.bus_priv = device;
lis3_dev.init = lis3lv02d_acpi_init;
lis3_dev.read = lis3lv02d_acpi_read;
lis3_dev.write = lis3lv02d_acpi_write;
@@ -291,23 +284,9 @@ static int lis3lv02d_add(struct acpi_device *device)
strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
device->driver_data = &lis3_dev;

- lis3lv02d_acpi_read(device->handle, WHO_AM_I, &lis3_dev.whoami);
- switch (lis3_dev.whoami) {
- case LIS_DOUBLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
- lis3_dev.read_data = lis3lv02d_read_16;
- lis3_dev.mdps_max_val = 2048;
- break;
- case LIS_SINGLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
- lis3_dev.read_data = lis3lv02d_read_8;
- lis3_dev.mdps_max_val = 128;
- break;
- default:
- printk(KERN_ERR DRIVER_NAME
- ": unknown sensor type 0x%X\n", lis3_dev.whoami);
- return -EINVAL;
- }
+ ret = lis3lv02d_init_device(&lis3_dev);
+ if (ret)
+ return ret;

/* If possible use a "standard" axes order */
if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
@@ -318,19 +297,16 @@ static int lis3lv02d_add(struct acpi_device *device)

INIT_WORK(&hpled_led.work, delayed_set_status_worker);
ret = led_classdev_register(NULL, &hpled_led.led_classdev);
- if (ret)
- return ret;
-
- /* obtain IRQ number of our device from ACPI */
- lis3lv02d_enum_resources(lis3_dev.device);
-
- ret = lis3lv02d_init_device(&lis3_dev);
if (ret) {
+ lis3lv02d_joystick_disable();
+ lis3lv02d_poweroff(&lis3_dev);
flush_work(&hpled_led.work);
- led_classdev_unregister(&hpled_led.led_classdev);
return ret;
}

+ /* obtain IRQ number of our device from ACPI */
+ lis3lv02d_enum_resources(device);
+
return ret;
}

@@ -340,7 +316,7 @@ static int lis3lv02d_remove(struct acpi_device *device, int type)
return -EINVAL;

lis3lv02d_joystick_disable();
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);

flush_work(&hpled_led.work);
led_classdev_unregister(&hpled_led.led_classdev);
@@ -353,7 +329,7 @@ static int lis3lv02d_remove(struct acpi_device *device, int type)
static int lis3lv02d_suspend(struct acpi_device *device, pm_message_t state)
{
/* make sure the device is off when we suspend */
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);
return 0;
}

@@ -362,9 +338,9 @@ static int lis3lv02d_resume(struct acpi_device *device)
/* put back the device in the right state (ACPI might turn it on) */
mutex_lock(&lis3_dev.lock);
if (lis3_dev.usage > 0)
- lis3lv02d_poweron(device->handle);
+ lis3lv02d_poweron(&lis3_dev);
else
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);
mutex_unlock(&lis3_dev.lock);
return 0;
}
diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index eeae7c9..7c6c4ca 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -36,7 +36,6 @@
#include <linux/freezer.h>
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
-#include <acpi/acpi_drivers.h>
#include <asm/atomic.h>
#include "lis3lv02d.h"

@@ -53,18 +52,27 @@
* joystick.
*/

-struct acpi_lis3lv02d lis3_dev = {
+struct lis3lv02d lis3_dev = {
.misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
};

EXPORT_SYMBOL_GPL(lis3_dev);

-static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
+static s16 lis3lv02d_read_8(struct lis3lv02d *lis3, int reg)
+{
+ s8 lo;
+ if (lis3->read(lis3, reg, &lo) < 0)
+ return 0;
+
+ return lo;
+}
+
+static s16 lis3lv02d_read_16(struct lis3lv02d *lis3, int reg)
{
u8 lo, hi;

- lis3_dev.read(handle, reg, &lo);
- lis3_dev.read(handle, reg + 1, &hi);
+ lis3->read(lis3, reg, &lo);
+ lis3->read(lis3, reg + 1, &hi);
/* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
return (s16)((hi << 8) | lo);
}
@@ -86,36 +94,36 @@ static inline int lis3lv02d_get_axis(s8 axis, int hw_values[3])

/**
* lis3lv02d_get_xyz - Get X, Y and Z axis values from the accelerometer
- * @handle: the handle to the device
- * @x: where to store the X axis value
- * @y: where to store the Y axis value
- * @z: where to store the Z axis value
+ * @lis3: pointer to the device struct
+ * @x: where to store the X axis value
+ * @y: where to store the Y axis value
+ * @z: where to store the Z axis value
*
* Note that 40Hz input device can eat up about 10% CPU at 800MHZ
*/
-static void lis3lv02d_get_xyz(acpi_handle handle, int *x, int *y, int *z)
+static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
{
int position[3];

- position[0] = lis3_dev.read_data(handle, OUTX);
- position[1] = lis3_dev.read_data(handle, OUTY);
- position[2] = lis3_dev.read_data(handle, OUTZ);
+ position[0] = lis3_dev.read_data(lis3, OUTX);
+ position[1] = lis3_dev.read_data(lis3, OUTY);
+ position[2] = lis3_dev.read_data(lis3, OUTZ);

*x = lis3lv02d_get_axis(lis3_dev.ac.x, position);
*y = lis3lv02d_get_axis(lis3_dev.ac.y, position);
*z = lis3lv02d_get_axis(lis3_dev.ac.z, position);
}

-void lis3lv02d_poweroff(acpi_handle handle)
+void lis3lv02d_poweroff(struct lis3lv02d *lis3)
{
lis3_dev.is_on = 0;
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweroff);

-void lis3lv02d_poweron(acpi_handle handle)
+void lis3lv02d_poweron(struct lis3lv02d *lis3)
{
lis3_dev.is_on = 1;
- lis3_dev.init(handle);
+ lis3_dev.init(lis3);
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

@@ -124,13 +132,13 @@ EXPORT_SYMBOL_GPL(lis3lv02d_poweron);
* device will always be on until a call to lis3lv02d_decrease_use(). Not to be
* used from interrupt context.
*/
-static void lis3lv02d_increase_use(struct acpi_lis3lv02d *dev)
+static void lis3lv02d_increase_use(struct lis3lv02d *dev)
{
mutex_lock(&dev->lock);
dev->usage++;
if (dev->usage == 1) {
if (!dev->is_on)
- lis3lv02d_poweron(dev->device->handle);
+ lis3lv02d_poweron(dev);
}
mutex_unlock(&dev->lock);
}
@@ -139,12 +147,12 @@ static void lis3lv02d_increase_use(struct acpi_lis3lv02d *dev)
* To be called whenever a usage of the device is stopped.
* It will make sure to turn off the device when there is not usage.
*/
-static void lis3lv02d_decrease_use(struct acpi_lis3lv02d *dev)
+static void lis3lv02d_decrease_use(struct lis3lv02d *dev)
{
mutex_lock(&dev->lock);
dev->usage--;
if (dev->usage == 0)
- lis3lv02d_poweroff(dev->device->handle);
+ lis3lv02d_poweroff(dev);
mutex_unlock(&dev->lock);
}

@@ -291,7 +299,7 @@ static int lis3lv02d_joystick_kthread(void *data)
int x, y, z;

while (!kthread_should_stop()) {
- lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
input_report_abs(lis3_dev.idev, ABS_X, x - lis3_dev.xcalib);
input_report_abs(lis3_dev.idev, ABS_Y, y - lis3_dev.ycalib);
input_report_abs(lis3_dev.idev, ABS_Z, z - lis3_dev.zcalib);
@@ -325,7 +333,8 @@ static void lis3lv02d_joystick_close(struct input_dev *input)

static inline void lis3lv02d_calibrate_joystick(void)
{
- lis3lv02d_get_xyz(lis3_dev.device->handle, &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
+ lis3lv02d_get_xyz(&lis3_dev,
+ &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
}

int lis3lv02d_joystick_enable(void)
@@ -382,7 +391,7 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
int x, y, z;

lis3lv02d_increase_use(&lis3_dev);
- lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
lis3lv02d_decrease_use(&lis3_dev);
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}
@@ -412,7 +421,7 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
int val;

lis3lv02d_increase_use(&lis3_dev);
- lis3_dev.read(lis3_dev.device->handle, CTRL_REG1, &ctrl);
+ lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
lis3lv02d_decrease_use(&lis3_dev);
val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
@@ -435,7 +444,7 @@ static struct attribute_group lis3lv02d_attribute_group = {
};


-static int lis3lv02d_add_fs(struct acpi_device *device)
+static int lis3lv02d_add_fs(struct lis3lv02d *lis3)
{
lis3_dev.pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(lis3_dev.pdev))
@@ -456,10 +465,29 @@ EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
* Initialise the accelerometer and the various subsystems.
* Should be rather independant of the bus system.
*/
-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
+int lis3lv02d_init_device(struct lis3lv02d *dev)
{
+ dev->whoami = lis3lv02d_read_8(dev, WHO_AM_I);
+
+ switch (dev->whoami) {
+ case LIS_DOUBLE_ID:
+ printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
+ dev->read_data = lis3lv02d_read_16;
+ dev->mdps_max_val = 2048;
+ break;
+ case LIS_SINGLE_ID:
+ printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
+ dev->read_data = lis3lv02d_read_8;
+ dev->mdps_max_val = 128;
+ break;
+ default:
+ printk(KERN_ERR DRIVER_NAME
+ ": unknown sensor type 0x%X\n", lis3_dev.whoami);
+ return -EINVAL;
+ }
+
mutex_init(&dev->lock);
- lis3lv02d_add_fs(dev->device);
+ lis3lv02d_add_fs(dev);
lis3lv02d_increase_use(dev);

if (lis3lv02d_joystick_enable())
@@ -467,10 +495,10 @@ int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)

printk("lis3_init_device: irq %d\n", dev->irq);

- /* if we did not get an IRQ from ACPI - we have nothing more to do */
+ /* bail if we did not get an IRQ from the bus layer */
if (!dev->irq) {
printk(KERN_ERR DRIVER_NAME
- ": No IRQ in ACPI. Disabling /dev/freefall\n");
+ ": No IRQ. Disabling /dev/freefall\n");
goto out;
}

diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 017fb2b..745ec96 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -159,14 +159,14 @@ struct axis_conversion {
s8 z;
};

-struct acpi_lis3lv02d {
- struct acpi_device *device; /* The ACPI device */
- acpi_status (*init) (acpi_handle handle);
- acpi_status (*write) (acpi_handle handle, int reg, u8 val);
- acpi_status (*read) (acpi_handle handle, int reg, u8 *ret);
+struct lis3lv02d {
+ void *bus_priv; /* used by the bus layer only */
+ int (*init) (struct lis3lv02d *lis3);
+ int (*write) (struct lis3lv02d *lis3, int reg, u8 val);
+ int (*read) (struct lis3lv02d *lis3, int reg, u8 *ret);

u8 whoami; /* 3Ah: 2-byte registries, 3Bh: 1-byte registries */
- s16 (*read_data) (acpi_handle handle, int reg);
+ s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;

struct input_dev *idev; /* input device */
@@ -187,11 +187,11 @@ struct acpi_lis3lv02d {
unsigned long misc_opened; /* bit0: whether the device is open */
};

-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev);
+int lis3lv02d_init_device(struct lis3lv02d *lis3);
int lis3lv02d_joystick_enable(void);
void lis3lv02d_joystick_disable(void);
-void lis3lv02d_poweroff(acpi_handle handle);
-void lis3lv02d_poweron(acpi_handle handle);
+void lis3lv02d_poweroff(struct lis3lv02d *lis3);
+void lis3lv02d_poweron(struct lis3lv02d *lis3);
int lis3lv02d_remove_fs(void);

-extern struct acpi_lis3lv02d lis3_dev;
+extern struct lis3lv02d lis3_dev;
--
1.6.1.3

2009-03-02 14:33:31

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 5/5] lis3: SPI transport layer

Make use of the new abstraction layer and add a new transport layer for
spi. Works fine on a PXA based board.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
---
drivers/hwmon/Kconfig | 16 ++++++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/lis3lv02d_spi.c | 113 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 130 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/lis3lv02d_spi.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b84bf06..9d78ab4 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -895,6 +895,22 @@ config SENSORS_LIS3LV02D
Say Y here if you have an applicable laptop and want to experience
the awesome power of lis3lv02d.

+config SENSORS_LIS3_SPI
+ tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer (SPI)"
+ depends on !ACPI && SPI_MASTER && INPUT
+ default n
+ help
+ This driver provides support for the LIS3LV02Dx accelerometer connected
+ via SPI. The accelerometer data is readable via
+ /sys/devices/platform/lis3lv02d.
+
+ This driver also provides an absolute input class device, allowing
+ the laptop to act as a pinball machine-esque joystick.
+
+ This driver can also be built as modules. If so, the core module
+ will be called lis3lv02d and a specific module for the SPI transport
+ is called lis3lv02d_spi.
+
config SENSORS_APPLESMC
tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
depends on INPUT && X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2e80f37..6f0f4b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
obj-$(CONFIG_SENSORS_IT87) += it87.o
obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o
obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
+obj-$(CONFIG_SENSORS_LIS3_SPI) += lis3lv02d.o lis3lv02d_spi.o
obj-$(CONFIG_SENSORS_LM63) += lm63.o
obj-$(CONFIG_SENSORS_LM70) += lm70.o
obj-$(CONFIG_SENSORS_LM75) += lm75.o
diff --git a/drivers/hwmon/lis3lv02d_spi.c b/drivers/hwmon/lis3lv02d_spi.c
new file mode 100644
index 0000000..14436e7
--- /dev/null
+++ b/drivers/hwmon/lis3lv02d_spi.c
@@ -0,0 +1,113 @@
+/*
+ * lis3lv02d_spi - SPI glue layer for lis3lv02d
+ *
+ * Copyright (c) 2009 Daniel Mack <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/spi/spi.h>
+
+#include "lis3lv02d.h"
+
+#define DRV_NAME "lis3lv02d_spi"
+#define LIS3_SPI_READ 0x80
+
+static int lis3_spi_read(struct lis3lv02d *lis3, int reg, u8 *v)
+{
+ struct spi_device *spi = lis3->bus_priv;
+ int ret = spi_w8r8(spi, reg | LIS3_SPI_READ);
+ if (ret < 0)
+ return -EINVAL;
+
+ *v = (u8) ret;
+ return 0;
+}
+
+static int lis3_spi_write(struct lis3lv02d *lis3, int reg, u8 val)
+{
+ u8 tmp[2] = { reg, val };
+ struct spi_device *spi = lis3->bus_priv;
+ return spi_write(spi, tmp, sizeof(tmp));
+}
+
+static int lis3_spi_init(struct lis3lv02d *lis3)
+{
+ u8 reg;
+ int ret;
+
+ ret = lis3->read(lis3, CTRL_REG1, &reg);
+ if (ret < 0)
+ return ret;
+
+ reg |= 0x40;
+ return lis3->write(lis3, CTRL_REG1, reg);
+}
+
+static struct axis_conversion lis3lv02d_axis_normal = { 1, 2, 3 };
+
+static int __devinit lis302dl_spi_probe(struct spi_device *spi)
+{
+ int ret;
+
+ spi->bits_per_word = 8;
+ spi->mode = SPI_MODE_0;
+ ret = spi_setup(spi);
+ if (ret < 0)
+ return ret;
+
+ lis3_dev.bus_priv = spi;
+ lis3_dev.init = lis3_spi_init;
+ lis3_dev.read = lis3_spi_read;
+ lis3_dev.write = lis3_spi_write;
+ lis3_dev.irq = spi->irq;
+ lis3_dev.ac = lis3lv02d_axis_normal;
+ spi_set_drvdata(spi, &lis3_dev);
+
+ ret = lis3lv02d_init_device(&lis3_dev);
+ return ret;
+}
+
+static int __devexit lis302dl_spi_remove(struct spi_device *spi)
+{
+ struct lis3lv02d *lis3 = spi_get_drvdata(spi);
+ lis3lv02d_joystick_disable();
+ lis3lv02d_poweroff(lis3);
+ return 0;
+}
+
+static struct spi_driver lis302dl_spi_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = lis302dl_spi_probe,
+ .remove = __devexit_p(lis302dl_spi_remove),
+};
+
+static int __init lis302dl_init(void)
+{
+ return spi_register_driver(&lis302dl_spi_driver);
+}
+
+static void __exit lis302dl_exit(void)
+{
+ spi_unregister_driver(&lis302dl_spi_driver);
+}
+
+module_init(lis302dl_init);
+module_exit(lis302dl_exit);
+
+MODULE_AUTHOR("Daniel Mack <[email protected]>");
+MODULE_DESCRIPTION("lis3lv02d SPI glue layer");
+MODULE_LICENSE("GPL");
+
--
1.6.1.3

2009-03-02 14:37:07

by Daniel Mack

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

Hi Eric,

On Mon, Mar 02, 2009 at 11:17:19AM +0100, ?ric Piel wrote:
> > Ok, I'll start and seperate things then but will need you help with
> > testing as I don't have the hardware the driver was written for
> > originally. I do have, in turn, a custom board with this chip connected
> > via SPI.
> Very good.

Ok, just send a series of 5 patches. For some reason, git send-email ate
up the subject lines of the first two, sorry for that.

All together they do what I want - I can use the device now on my PXA
board. There might be some more updates I'll come up with later. For
now, I'd like to make sure I didn't break anything for HP laptops :)

Thanks,
Daniel

2009-03-02 14:40:27

by Éric Piel

[permalink] [raw]
Subject: Re: lis3's ACPI dependency

Daniel Mack schreef:
> Hi Eric,
>
> On Mon, Mar 02, 2009 at 11:17:19AM +0100, ?ric Piel wrote:
>>> Ok, I'll start and seperate things then but will need you help with
>>> testing as I don't have the hardware the driver was written for
>>> originally. I do have, in turn, a custom board with this chip connected
>>> via SPI.
>> Very good.
>
> Ok, just send a series of 5 patches. For some reason, git send-email ate
> up the subject lines of the first two, sorry for that.
>
> All together they do what I want - I can use the device now on my PXA
> board. There might be some more updates I'll come up with later. For
> now, I'd like to make sure I didn't break anything for HP laptops :)
Will try to find some time tonight or tomorrow to test them.
At first sight they look good :-)

Eric

2009-03-02 14:55:27

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 3/5] lis3: reorder functions to make forward decl obsolete

On Mon 2009-03-02 15:31:48, Daniel Mack wrote:
> moved lis3lv02d_init_device() down so that the forward declaration of
> lis3lv02d_add_fs() becomes unnecessary.
>
> Signed-off-by: Daniel Mack <[email protected]>

Acked-by: Pavel Machek <[email protected]>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-03-02 15:07:32

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

Hi!

> This solves the dependency between lis3lv02d.[ch] and ACPI specific
> methods. It introduces a ->bus_priv pointer to the device struct which
> is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> accordingly.

Looks okay to me. I did the basic split but -- having only single
client -- did not really have motivation to finish it.

> This also moves the read_8() and read_16() routines from hp_accel.c to
> lis3lv02d.c as they are not specific to ACPI.

These would be slightly better done as separate patch, but I guess its okay...

> Signed-off-by: Daniel Mack <[email protected]>

Acked-by: Pavel Machek <[email protected]>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-03-02 15:09:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 5/5] lis3: SPI transport layer

On Mon 2009-03-02 15:31:50, Daniel Mack wrote:
> Make use of the new abstraction layer and add a new transport layer for
> spi. Works fine on a PXA based board.

Looks ok to me, and proves that previous patch was sane.

> Signed-off-by: Daniel Mack <[email protected]>

Acked-by: Pavel Machek <[email protected]>

Vlado, could you test the compelete series?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-03-03 19:55:14

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

Daniel Mack schreef:
> This solves the dependency between lis3lv02d.[ch] and ACPI specific
> methods. It introduces a ->bus_priv pointer to the device struct which
> is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> accordingly.
>
> This also moves the read_8() and read_16() routines from hp_accel.c to
> lis3lv02d.c as they are not specific to ACPI.
Hello,
I've tried the patch series on my laptop. This particular patch burst
the driver... need some work :-) The values are not read correctly and
the IRQ is not detected. See down in the code...


> @@ -291,23 +284,9 @@ static int lis3lv02d_add(struct acpi_device *device)
> strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
> device->driver_data = &lis3_dev;
>
> - lis3lv02d_acpi_read(device->handle, WHO_AM_I, &lis3_dev.whoami);
> - switch (lis3_dev.whoami) {
> - case LIS_DOUBLE_ID:
> - printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
> - lis3_dev.read_data = lis3lv02d_read_16;
> - lis3_dev.mdps_max_val = 2048;
> - break;
> - case LIS_SINGLE_ID:
> - printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
> - lis3_dev.read_data = lis3lv02d_read_8;
> - lis3_dev.mdps_max_val = 128;
> - break;
> - default:
> - printk(KERN_ERR DRIVER_NAME
> - ": unknown sensor type 0x%X\n", lis3_dev.whoami);
> - return -EINVAL;
> - }
> + ret = lis3lv02d_init_device(&lis3_dev);
> + if (ret)
> + return ret;
>
> /* If possible use a "standard" axes order */
> if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
> @@ -318,19 +297,16 @@ static int lis3lv02d_add(struct acpi_device *device)
>
> INIT_WORK(&hpled_led.work, delayed_set_status_worker);
> ret = led_classdev_register(NULL, &hpled_led.led_classdev);
> - if (ret)
> - return ret;
> -
> - /* obtain IRQ number of our device from ACPI */
> - lis3lv02d_enum_resources(lis3_dev.device);
> -
> - ret = lis3lv02d_init_device(&lis3_dev);
> if (ret) {
> + lis3lv02d_joystick_disable();
> + lis3lv02d_poweroff(&lis3_dev);
> flush_work(&hpled_led.work);
> - led_classdev_unregister(&hpled_led.led_classdev);
> return ret;
> }
>
> + /* obtain IRQ number of our device from ACPI */
> + lis3lv02d_enum_resources(device);
> +
> return ret;
> }
So now, we first try to set up the IRQ, in lis3lv02d_init_device() and
only after look for a IRQ number with lis3lv02d_enum_resources()... that
doesn't work.

>
> -static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
> -{
> - u8 lo, hi;
> -
> - lis3_dev.read(handle, reg - 1, &lo);
> - lis3_dev.read(handle, reg, &hi);
> - /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
> - return (s16)((hi << 8) | lo);
> -}
:
> +static s16 lis3lv02d_read_16(struct lis3lv02d *lis3, int reg)
> {
> u8 lo, hi;
>
> - lis3_dev.read(handle, reg, &lo);
> - lis3_dev.read(handle, reg + 1, &hi);
> + lis3->read(lis3, reg, &lo);
> + lis3->read(lis3, reg + 1, &hi);
> /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
> return (s16)((hi << 8) | lo);
> }
Looking at it I cannot beleive we ended up with two versions of
lis3lv02d_read_16()!
Anyway, you want the logic of the above version, the one with "- 1".

Eric

2009-03-03 19:59:20

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 5/5] lis3: SPI transport layer

Daniel Mack schreef:
> Make use of the new abstraction layer and add a new transport layer for
> spi. Works fine on a PXA based board.
>
Hi,
Not much to say about this part. Just this minor comment:
:
> +
> +static int lis3_spi_init(struct lis3lv02d *lis3)
> +{
> + u8 reg;
> + int ret;
> +
> + ret = lis3->read(lis3, CTRL_REG1, &reg);
> + if (ret < 0)
> + return ret;
> +
> + reg |= 0x40;
No magic please, use the constants defined in lis3lv02d.h : CTRL1_PD0.


> + return lis3->write(lis3, CTRL_REG1, reg);
> +}

Looks good otherwise :-)
Eric

2009-03-04 01:44:16

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

Hi,

On Tue, Mar 03, 2009 at 08:54:50PM +0100, ?ric Piel wrote:
> Daniel Mack schreef:
> > This solves the dependency between lis3lv02d.[ch] and ACPI specific
> > methods. It introduces a ->bus_priv pointer to the device struct which
> > is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> > accordingly.
> >
> > This also moves the read_8() and read_16() routines from hp_accel.c to
> > lis3lv02d.c as they are not specific to ACPI.
> Hello,
> I've tried the patch series on my laptop. This particular patch burst
> the driver... need some work :-) The values are not read correctly and
> the IRQ is not detected. See down in the code...

Ok, obvious fixes. I'll post a new set of the three topmost patches -
could you test them again, please?

Thanks and best regards,
Daniel

2009-03-04 01:45:28

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 3/5] lis3: reorder functions to make forward decl obsolete

moved lis3lv02d_init_device() down so that the forward declaration of
lis3lv02d_add_fs() becomes unnecessary.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 64 +++++++++++++++++++++-----------------------
1 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 4888ac5..eeae7c9 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -59,8 +59,6 @@ struct acpi_lis3lv02d lis3_dev = {

EXPORT_SYMBOL_GPL(lis3_dev);

-static int lis3lv02d_add_fs(struct acpi_device *device);
-
static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
{
u8 lo, hi;
@@ -377,37 +375,6 @@ void lis3lv02d_joystick_disable(void)
}
EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);

-/*
- * Initialise the accelerometer and the various subsystems.
- * Should be rather independant of the bus system.
- */
-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
-{
- mutex_init(&dev->lock);
- lis3lv02d_add_fs(dev->device);
- lis3lv02d_increase_use(dev);
-
- if (lis3lv02d_joystick_enable())
- printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
-
- printk("lis3_init_device: irq %d\n", dev->irq);
-
- /* if we did not get an IRQ from ACPI - we have nothing more to do */
- if (!dev->irq) {
- printk(KERN_ERR DRIVER_NAME
- ": No IRQ in ACPI. Disabling /dev/freefall\n");
- goto out;
- }
-
- printk("lis3: registering device\n");
- if (misc_register(&lis3lv02d_misc_device))
- printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
-out:
- lis3lv02d_decrease_use(dev);
- return 0;
-}
-EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
-
/* Sysfs stuff */
static ssize_t lis3lv02d_position_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -485,6 +452,37 @@ int lis3lv02d_remove_fs(void)
}
EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);

+/*
+ * Initialise the accelerometer and the various subsystems.
+ * Should be rather independant of the bus system.
+ */
+int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
+{
+ mutex_init(&dev->lock);
+ lis3lv02d_add_fs(dev->device);
+ lis3lv02d_increase_use(dev);
+
+ if (lis3lv02d_joystick_enable())
+ printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
+
+ printk("lis3_init_device: irq %d\n", dev->irq);
+
+ /* if we did not get an IRQ from ACPI - we have nothing more to do */
+ if (!dev->irq) {
+ printk(KERN_ERR DRIVER_NAME
+ ": No IRQ in ACPI. Disabling /dev/freefall\n");
+ goto out;
+ }
+
+ printk("lis3: registering device\n");
+ if (misc_register(&lis3lv02d_misc_device))
+ printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
+out:
+ lis3lv02d_decrease_use(dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
+
MODULE_DESCRIPTION("ST LIS3LV02Dx three-axis digital accelerometer driver");
MODULE_AUTHOR("Yan Burman, Eric Piel, Pavel Machek");
MODULE_LICENSE("GPL");
--
1.6.1.3

2009-03-04 01:45:44

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 4/5] lis3: solve dependency between core and ACPI

This solves the dependency between lis3lv02d.[ch] and ACPI specific
methods. It introduces a ->bus_priv pointer to the device struct which
is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
accordingly.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
---
- calling lis3lv02d_enum_resources() before lis3lv02d_init_device()
now so that we have an irq prior to doing the device init.

drivers/hwmon/hp_accel.c | 101 +++++++++++++++++---------------------------
drivers/hwmon/lis3lv02d.c | 86 +++++++++++++++++++++++++-------------
drivers/hwmon/lis3lv02d.h | 20 ++++----
3 files changed, 106 insertions(+), 101 deletions(-)

diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index ee1ed42..363220b 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -85,25 +85,31 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids);

/**
* lis3lv02d_acpi_init - ACPI _INI method: initialize the device.
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_init(acpi_handle handle)
+int lis3lv02d_acpi_init(struct lis3lv02d *lis3)
{
- return acpi_evaluate_object(handle, METHOD_NAME__INI, NULL, NULL);
+ struct acpi_device *dev = lis3->bus_priv;
+ if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI,
+ NULL, NULL) != AE_OK)
+ return -EINVAL;
+
+ return 0;
}

/**
* lis3lv02d_acpi_read - ACPI ALRD method: read a register
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
* @reg: the register to read
* @ret: result of the operation
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret)
+int lis3lv02d_acpi_read(struct lis3lv02d *lis3, int reg, u8 *ret)
{
+ struct acpi_device *dev = lis3->bus_priv;
union acpi_object arg0 = { ACPI_TYPE_INTEGER };
struct acpi_object_list args = { 1, &arg0 };
unsigned long long lret;
@@ -111,21 +117,22 @@ acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret)

arg0.integer.value = reg;

- status = acpi_evaluate_integer(handle, "ALRD", &args, &lret);
+ status = acpi_evaluate_integer(dev->handle, "ALRD", &args, &lret);
*ret = lret;
- return status;
+ return (status != AE_OK) ? -EINVAL : 0;
}

/**
* lis3lv02d_acpi_write - ACPI ALWR method: write to a register
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
* @reg: the register to write to
* @val: the value to write
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val)
+int lis3lv02d_acpi_write(struct lis3lv02d *lis3, int reg, u8 val)
{
+ struct acpi_device *dev = lis3->bus_priv;
unsigned long long ret; /* Not used when writting */
union acpi_object in_obj[2];
struct acpi_object_list args = { 2, in_obj };
@@ -135,7 +142,10 @@ acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val)
in_obj[1].type = ACPI_TYPE_INTEGER;
in_obj[1].integer.value = val;

- return acpi_evaluate_integer(handle, "ALWR", &args, &ret);
+ if (acpi_evaluate_integer(dev->handle, "ALWR", &args, &ret) != AE_OK)
+ return -EINVAL;
+
+ return 0;
}

static int lis3lv02d_dmi_matched(const struct dmi_system_id *dmi)
@@ -214,7 +224,7 @@ static struct dmi_system_id lis3lv02d_dmi_ids[] = {

static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness value)
{
- acpi_handle handle = lis3_dev.device->handle;
+ struct acpi_device *dev = lis3_dev.bus_priv;
unsigned long long ret; /* Not used when writing */
union acpi_object in_obj[1];
struct acpi_object_list args = { 1, in_obj };
@@ -222,7 +232,7 @@ static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness
in_obj[0].type = ACPI_TYPE_INTEGER;
in_obj[0].integer.value = !!value;

- acpi_evaluate_integer(handle, "ALED", &args, &ret);
+ acpi_evaluate_integer(dev->handle, "ALED", &args, &ret);
}

static struct delayed_led_classdev hpled_led = {
@@ -259,23 +269,6 @@ static void lis3lv02d_enum_resources(struct acpi_device *device)
printk(KERN_DEBUG DRIVER_NAME ": Error getting resources\n");
}

-static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
-{
- u8 lo, hi;
-
- lis3_dev.read(handle, reg - 1, &lo);
- lis3_dev.read(handle, reg, &hi);
- /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
- return (s16)((hi << 8) | lo);
-}
-
-static s16 lis3lv02d_read_8(acpi_handle handle, int reg)
-{
- s8 lo;
- lis3_dev.read(handle, reg, &lo);
- return lo;
-}
-
static int lis3lv02d_add(struct acpi_device *device)
{
int ret;
@@ -283,7 +276,7 @@ static int lis3lv02d_add(struct acpi_device *device)
if (!device)
return -EINVAL;

- lis3_dev.device = device;
+ lis3_dev.bus_priv = device;
lis3_dev.init = lis3lv02d_acpi_init;
lis3_dev.read = lis3lv02d_acpi_read;
lis3_dev.write = lis3lv02d_acpi_write;
@@ -291,23 +284,13 @@ static int lis3lv02d_add(struct acpi_device *device)
strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
device->driver_data = &lis3_dev;

- lis3lv02d_acpi_read(device->handle, WHO_AM_I, &lis3_dev.whoami);
- switch (lis3_dev.whoami) {
- case LIS_DOUBLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
- lis3_dev.read_data = lis3lv02d_read_16;
- lis3_dev.mdps_max_val = 2048;
- break;
- case LIS_SINGLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
- lis3_dev.read_data = lis3lv02d_read_8;
- lis3_dev.mdps_max_val = 128;
- break;
- default:
- printk(KERN_ERR DRIVER_NAME
- ": unknown sensor type 0x%X\n", lis3_dev.whoami);
- return -EINVAL;
- }
+ /* obtain IRQ number of our device from ACPI */
+ lis3lv02d_enum_resources(device);
+
+ /* call the core layer do its init */
+ ret = lis3lv02d_init_device(&lis3_dev);
+ if (ret)
+ return ret;

/* If possible use a "standard" axes order */
if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
@@ -318,16 +301,10 @@ static int lis3lv02d_add(struct acpi_device *device)

INIT_WORK(&hpled_led.work, delayed_set_status_worker);
ret = led_classdev_register(NULL, &hpled_led.led_classdev);
- if (ret)
- return ret;
-
- /* obtain IRQ number of our device from ACPI */
- lis3lv02d_enum_resources(lis3_dev.device);
-
- ret = lis3lv02d_init_device(&lis3_dev);
if (ret) {
+ lis3lv02d_joystick_disable();
+ lis3lv02d_poweroff(&lis3_dev);
flush_work(&hpled_led.work);
- led_classdev_unregister(&hpled_led.led_classdev);
return ret;
}

@@ -340,7 +317,7 @@ static int lis3lv02d_remove(struct acpi_device *device, int type)
return -EINVAL;

lis3lv02d_joystick_disable();
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);

flush_work(&hpled_led.work);
led_classdev_unregister(&hpled_led.led_classdev);
@@ -353,7 +330,7 @@ static int lis3lv02d_remove(struct acpi_device *device, int type)
static int lis3lv02d_suspend(struct acpi_device *device, pm_message_t state)
{
/* make sure the device is off when we suspend */
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);
return 0;
}

@@ -362,9 +339,9 @@ static int lis3lv02d_resume(struct acpi_device *device)
/* put back the device in the right state (ACPI might turn it on) */
mutex_lock(&lis3_dev.lock);
if (lis3_dev.usage > 0)
- lis3lv02d_poweron(device->handle);
+ lis3lv02d_poweron(&lis3_dev);
else
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);
mutex_unlock(&lis3_dev.lock);
return 0;
}
diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index eeae7c9..778eb77 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -36,7 +36,6 @@
#include <linux/freezer.h>
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
-#include <acpi/acpi_drivers.h>
#include <asm/atomic.h>
#include "lis3lv02d.h"

@@ -53,18 +52,27 @@
* joystick.
*/

-struct acpi_lis3lv02d lis3_dev = {
+struct lis3lv02d lis3_dev = {
.misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
};

EXPORT_SYMBOL_GPL(lis3_dev);

-static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
+static s16 lis3lv02d_read_8(struct lis3lv02d *lis3, int reg)
+{
+ s8 lo;
+ if (lis3->read(lis3, reg, &lo) < 0)
+ return 0;
+
+ return lo;
+}
+
+static s16 lis3lv02d_read_16(struct lis3lv02d *lis3, int reg)
{
u8 lo, hi;

- lis3_dev.read(handle, reg, &lo);
- lis3_dev.read(handle, reg + 1, &hi);
+ lis3->read(lis3, reg - 1, &lo);
+ lis3->read(lis3, reg, &hi);
/* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
return (s16)((hi << 8) | lo);
}
@@ -86,36 +94,36 @@ static inline int lis3lv02d_get_axis(s8 axis, int hw_values[3])

/**
* lis3lv02d_get_xyz - Get X, Y and Z axis values from the accelerometer
- * @handle: the handle to the device
- * @x: where to store the X axis value
- * @y: where to store the Y axis value
- * @z: where to store the Z axis value
+ * @lis3: pointer to the device struct
+ * @x: where to store the X axis value
+ * @y: where to store the Y axis value
+ * @z: where to store the Z axis value
*
* Note that 40Hz input device can eat up about 10% CPU at 800MHZ
*/
-static void lis3lv02d_get_xyz(acpi_handle handle, int *x, int *y, int *z)
+static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
{
int position[3];

- position[0] = lis3_dev.read_data(handle, OUTX);
- position[1] = lis3_dev.read_data(handle, OUTY);
- position[2] = lis3_dev.read_data(handle, OUTZ);
+ position[0] = lis3_dev.read_data(lis3, OUTX);
+ position[1] = lis3_dev.read_data(lis3, OUTY);
+ position[2] = lis3_dev.read_data(lis3, OUTZ);

*x = lis3lv02d_get_axis(lis3_dev.ac.x, position);
*y = lis3lv02d_get_axis(lis3_dev.ac.y, position);
*z = lis3lv02d_get_axis(lis3_dev.ac.z, position);
}

-void lis3lv02d_poweroff(acpi_handle handle)
+void lis3lv02d_poweroff(struct lis3lv02d *lis3)
{
lis3_dev.is_on = 0;
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweroff);

-void lis3lv02d_poweron(acpi_handle handle)
+void lis3lv02d_poweron(struct lis3lv02d *lis3)
{
lis3_dev.is_on = 1;
- lis3_dev.init(handle);
+ lis3_dev.init(lis3);
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

@@ -124,13 +132,13 @@ EXPORT_SYMBOL_GPL(lis3lv02d_poweron);
* device will always be on until a call to lis3lv02d_decrease_use(). Not to be
* used from interrupt context.
*/
-static void lis3lv02d_increase_use(struct acpi_lis3lv02d *dev)
+static void lis3lv02d_increase_use(struct lis3lv02d *dev)
{
mutex_lock(&dev->lock);
dev->usage++;
if (dev->usage == 1) {
if (!dev->is_on)
- lis3lv02d_poweron(dev->device->handle);
+ lis3lv02d_poweron(dev);
}
mutex_unlock(&dev->lock);
}
@@ -139,12 +147,12 @@ static void lis3lv02d_increase_use(struct acpi_lis3lv02d *dev)
* To be called whenever a usage of the device is stopped.
* It will make sure to turn off the device when there is not usage.
*/
-static void lis3lv02d_decrease_use(struct acpi_lis3lv02d *dev)
+static void lis3lv02d_decrease_use(struct lis3lv02d *dev)
{
mutex_lock(&dev->lock);
dev->usage--;
if (dev->usage == 0)
- lis3lv02d_poweroff(dev->device->handle);
+ lis3lv02d_poweroff(dev);
mutex_unlock(&dev->lock);
}

@@ -291,7 +299,7 @@ static int lis3lv02d_joystick_kthread(void *data)
int x, y, z;

while (!kthread_should_stop()) {
- lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
input_report_abs(lis3_dev.idev, ABS_X, x - lis3_dev.xcalib);
input_report_abs(lis3_dev.idev, ABS_Y, y - lis3_dev.ycalib);
input_report_abs(lis3_dev.idev, ABS_Z, z - lis3_dev.zcalib);
@@ -325,7 +333,8 @@ static void lis3lv02d_joystick_close(struct input_dev *input)

static inline void lis3lv02d_calibrate_joystick(void)
{
- lis3lv02d_get_xyz(lis3_dev.device->handle, &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
+ lis3lv02d_get_xyz(&lis3_dev,
+ &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
}

int lis3lv02d_joystick_enable(void)
@@ -382,7 +391,7 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
int x, y, z;

lis3lv02d_increase_use(&lis3_dev);
- lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
lis3lv02d_decrease_use(&lis3_dev);
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}
@@ -412,7 +421,7 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
int val;

lis3lv02d_increase_use(&lis3_dev);
- lis3_dev.read(lis3_dev.device->handle, CTRL_REG1, &ctrl);
+ lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
lis3lv02d_decrease_use(&lis3_dev);
val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
@@ -435,7 +444,7 @@ static struct attribute_group lis3lv02d_attribute_group = {
};


-static int lis3lv02d_add_fs(struct acpi_device *device)
+static int lis3lv02d_add_fs(struct lis3lv02d *lis3)
{
lis3_dev.pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(lis3_dev.pdev))
@@ -456,10 +465,29 @@ EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
* Initialise the accelerometer and the various subsystems.
* Should be rather independant of the bus system.
*/
-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
+int lis3lv02d_init_device(struct lis3lv02d *dev)
{
+ dev->whoami = lis3lv02d_read_8(dev, WHO_AM_I);
+
+ switch (dev->whoami) {
+ case LIS_DOUBLE_ID:
+ printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
+ dev->read_data = lis3lv02d_read_16;
+ dev->mdps_max_val = 2048;
+ break;
+ case LIS_SINGLE_ID:
+ printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
+ dev->read_data = lis3lv02d_read_8;
+ dev->mdps_max_val = 128;
+ break;
+ default:
+ printk(KERN_ERR DRIVER_NAME
+ ": unknown sensor type 0x%X\n", lis3_dev.whoami);
+ return -EINVAL;
+ }
+
mutex_init(&dev->lock);
- lis3lv02d_add_fs(dev->device);
+ lis3lv02d_add_fs(dev);
lis3lv02d_increase_use(dev);

if (lis3lv02d_joystick_enable())
@@ -467,10 +495,10 @@ int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)

printk("lis3_init_device: irq %d\n", dev->irq);

- /* if we did not get an IRQ from ACPI - we have nothing more to do */
+ /* bail if we did not get an IRQ from the bus layer */
if (!dev->irq) {
printk(KERN_ERR DRIVER_NAME
- ": No IRQ in ACPI. Disabling /dev/freefall\n");
+ ": No IRQ. Disabling /dev/freefall\n");
goto out;
}

diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 017fb2b..745ec96 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -159,14 +159,14 @@ struct axis_conversion {
s8 z;
};

-struct acpi_lis3lv02d {
- struct acpi_device *device; /* The ACPI device */
- acpi_status (*init) (acpi_handle handle);
- acpi_status (*write) (acpi_handle handle, int reg, u8 val);
- acpi_status (*read) (acpi_handle handle, int reg, u8 *ret);
+struct lis3lv02d {
+ void *bus_priv; /* used by the bus layer only */
+ int (*init) (struct lis3lv02d *lis3);
+ int (*write) (struct lis3lv02d *lis3, int reg, u8 val);
+ int (*read) (struct lis3lv02d *lis3, int reg, u8 *ret);

u8 whoami; /* 3Ah: 2-byte registries, 3Bh: 1-byte registries */
- s16 (*read_data) (acpi_handle handle, int reg);
+ s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;

struct input_dev *idev; /* input device */
@@ -187,11 +187,11 @@ struct acpi_lis3lv02d {
unsigned long misc_opened; /* bit0: whether the device is open */
};

-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev);
+int lis3lv02d_init_device(struct lis3lv02d *lis3);
int lis3lv02d_joystick_enable(void);
void lis3lv02d_joystick_disable(void);
-void lis3lv02d_poweroff(acpi_handle handle);
-void lis3lv02d_poweron(acpi_handle handle);
+void lis3lv02d_poweroff(struct lis3lv02d *lis3);
+void lis3lv02d_poweron(struct lis3lv02d *lis3);
int lis3lv02d_remove_fs(void);

-extern struct acpi_lis3lv02d lis3_dev;
+extern struct lis3lv02d lis3_dev;
--
1.6.1.3

2009-03-04 01:46:00

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 5/5] lis3: SPI transport layer

Make use of the new abstraction layer and add a new transport layer for
spi. Works fine on a PXA based board.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
---
- replaced 0x40 magic by CTRL1_PD0

drivers/hwmon/Kconfig | 16 ++++++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/lis3lv02d_spi.c | 114 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 131 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/lis3lv02d_spi.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b84bf06..9d78ab4 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -895,6 +895,22 @@ config SENSORS_LIS3LV02D
Say Y here if you have an applicable laptop and want to experience
the awesome power of lis3lv02d.

+config SENSORS_LIS3_SPI
+ tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer (SPI)"
+ depends on !ACPI && SPI_MASTER && INPUT
+ default n
+ help
+ This driver provides support for the LIS3LV02Dx accelerometer connected
+ via SPI. The accelerometer data is readable via
+ /sys/devices/platform/lis3lv02d.
+
+ This driver also provides an absolute input class device, allowing
+ the laptop to act as a pinball machine-esque joystick.
+
+ This driver can also be built as modules. If so, the core module
+ will be called lis3lv02d and a specific module for the SPI transport
+ is called lis3lv02d_spi.
+
config SENSORS_APPLESMC
tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
depends on INPUT && X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2e80f37..6f0f4b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
obj-$(CONFIG_SENSORS_IT87) += it87.o
obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o
obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
+obj-$(CONFIG_SENSORS_LIS3_SPI) += lis3lv02d.o lis3lv02d_spi.o
obj-$(CONFIG_SENSORS_LM63) += lm63.o
obj-$(CONFIG_SENSORS_LM70) += lm70.o
obj-$(CONFIG_SENSORS_LM75) += lm75.o
diff --git a/drivers/hwmon/lis3lv02d_spi.c b/drivers/hwmon/lis3lv02d_spi.c
new file mode 100644
index 0000000..07ae74b
--- /dev/null
+++ b/drivers/hwmon/lis3lv02d_spi.c
@@ -0,0 +1,114 @@
+/*
+ * lis3lv02d_spi - SPI glue layer for lis3lv02d
+ *
+ * Copyright (c) 2009 Daniel Mack <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/spi/spi.h>
+
+#include "lis3lv02d.h"
+
+#define DRV_NAME "lis3lv02d_spi"
+#define LIS3_SPI_READ 0x80
+
+static int lis3_spi_read(struct lis3lv02d *lis3, int reg, u8 *v)
+{
+ struct spi_device *spi = lis3->bus_priv;
+ int ret = spi_w8r8(spi, reg | LIS3_SPI_READ);
+ if (ret < 0)
+ return -EINVAL;
+
+ *v = (u8) ret;
+ return 0;
+}
+
+static int lis3_spi_write(struct lis3lv02d *lis3, int reg, u8 val)
+{
+ u8 tmp[2] = { reg, val };
+ struct spi_device *spi = lis3->bus_priv;
+ return spi_write(spi, tmp, sizeof(tmp));
+}
+
+static int lis3_spi_init(struct lis3lv02d *lis3)
+{
+ u8 reg;
+ int ret;
+
+ /* power up the device */
+ ret = lis3->read(lis3, CTRL_REG1, &reg);
+ if (ret < 0)
+ return ret;
+
+ reg |= CTRL1_PD0;
+ return lis3->write(lis3, CTRL_REG1, reg);
+}
+
+static struct axis_conversion lis3lv02d_axis_normal = { 1, 2, 3 };
+
+static int __devinit lis302dl_spi_probe(struct spi_device *spi)
+{
+ int ret;
+
+ spi->bits_per_word = 8;
+ spi->mode = SPI_MODE_0;
+ ret = spi_setup(spi);
+ if (ret < 0)
+ return ret;
+
+ lis3_dev.bus_priv = spi;
+ lis3_dev.init = lis3_spi_init;
+ lis3_dev.read = lis3_spi_read;
+ lis3_dev.write = lis3_spi_write;
+ lis3_dev.irq = spi->irq;
+ lis3_dev.ac = lis3lv02d_axis_normal;
+ spi_set_drvdata(spi, &lis3_dev);
+
+ ret = lis3lv02d_init_device(&lis3_dev);
+ return ret;
+}
+
+static int __devexit lis302dl_spi_remove(struct spi_device *spi)
+{
+ struct lis3lv02d *lis3 = spi_get_drvdata(spi);
+ lis3lv02d_joystick_disable();
+ lis3lv02d_poweroff(lis3);
+ return 0;
+}
+
+static struct spi_driver lis302dl_spi_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = lis302dl_spi_probe,
+ .remove = __devexit_p(lis302dl_spi_remove),
+};
+
+static int __init lis302dl_init(void)
+{
+ return spi_register_driver(&lis302dl_spi_driver);
+}
+
+static void __exit lis302dl_exit(void)
+{
+ spi_unregister_driver(&lis302dl_spi_driver);
+}
+
+module_init(lis302dl_init);
+module_exit(lis302dl_exit);
+
+MODULE_AUTHOR("Daniel Mack <[email protected]>");
+MODULE_DESCRIPTION("lis3lv02d SPI glue layer");
+MODULE_LICENSE("GPL");
+
--
1.6.1.3

2009-03-16 19:10:15

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

Hi,

On Wed, Mar 04, 2009 at 02:43:50AM +0100, Daniel Mack wrote:
> On Tue, Mar 03, 2009 at 08:54:50PM +0100, ?ric Piel wrote:
> > Daniel Mack schreef:
> > > This solves the dependency between lis3lv02d.[ch] and ACPI specific
> > > methods. It introduces a ->bus_priv pointer to the device struct which
> > > is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> > > accordingly.
> > >
> > > This also moves the read_8() and read_16() routines from hp_accel.c to
> > > lis3lv02d.c as they are not specific to ACPI.
> > Hello,
> > I've tried the patch series on my laptop. This particular patch burst
> > the driver... need some work :-) The values are not read correctly and
> > the IRQ is not detected. See down in the code...
>
> Ok, obvious fixes. I'll post a new set of the three topmost patches -
> could you test them again, please?

Could anyone test that on hardware which has a lis3 chip? I'd like to
avoid missing the next merge window for this :)

Thanks,
Daniel

2009-03-16 21:28:03

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

Hi!

> > > > This solves the dependency between lis3lv02d.[ch] and ACPI specific
> > > > methods. It introduces a ->bus_priv pointer to the device struct which
> > > > is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> > > > accordingly.
> > > >
> > > > This also moves the read_8() and read_16() routines from hp_accel.c to
> > > > lis3lv02d.c as they are not specific to ACPI.
> > > Hello,
> > > I've tried the patch series on my laptop. This particular patch burst
> > > the driver... need some work :-) The values are not read correctly and
> > > the IRQ is not detected. See down in the code...
> >
> > Ok, obvious fixes. I'll post a new set of the three topmost patches -
> > could you test them again, please?
>
> Could anyone test that on hardware which has a lis3 chip? I'd like to
> avoid missing the next merge window for this :)

I no longer have the hardware, Vlado has some. As does HP :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-03-22 23:25:34

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

Daniel Mack schreef:
> This solves the dependency between lis3lv02d.[ch] and ACPI specific
> methods. It introduces a ->bus_priv pointer to the device struct which
> is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> accordingly.
Sorry for the long delay. Eventually I've found time to test the patch
and fix a bug: the axis conversion was set too late, preventing the
calibration to work.

The attached patch should make your modifications fully working with the
hp laptops :-)
I guess the best is to fold it into your patch and I'll ack the new
version.

Eric

=
Must put the axis info before using it.
---
drivers/hwmon/hp_accel.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index 363220b..b6c32c5 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -287,11 +287,6 @@ static int lis3lv02d_add(struct acpi_device *device)
/* obtain IRQ number of our device from ACPI */
lis3lv02d_enum_resources(device);

- /* call the core layer do its init */
- ret = lis3lv02d_init_device(&lis3_dev);
- if (ret)
- return ret;
-
/* If possible use a "standard" axes order */
if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
printk(KERN_INFO DRIVER_NAME ": laptop model unknown, "
@@ -299,6 +294,11 @@ static int lis3lv02d_add(struct acpi_device *device)
lis3_dev.ac = lis3lv02d_axis_normal;
}

+ /* call the core layer do its init */
+ ret = lis3lv02d_init_device(&lis3_dev);
+ if (ret)
+ return ret;
+
INIT_WORK(&hpled_led.work, delayed_set_status_worker);
ret = led_classdev_register(NULL, &hpled_led.led_classdev);
if (ret) {
--
1.6.2.1

2009-03-22 23:43:07

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 4/5] lis3: solve dependency between core and ACPI

On Mon, Mar 23, 2009 at 12:25:18AM +0100, ?ric Piel wrote:
> > This solves the dependency between lis3lv02d.[ch] and ACPI specific
> > methods. It introduces a ->bus_priv pointer to the device struct which
> > is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> > accordingly.
> Sorry for the long delay. Eventually I've found time to test the patch
> and fix a bug: the axis conversion was set too late, preventing the
> calibration to work.
>
> The attached patch should make your modifications fully working with the
> hp laptops :-)
> I guess the best is to fold it into your patch and I'll ack the new
> version.

Ok, thanks :)

Three patches following this mail - I left out the first two I was
repeatedly sending previously as they're in Andrew's tree already.

Daniel

2009-03-22 23:52:46

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 1/3] lis3: reorder functions to make forward decl obsolete

moved lis3lv02d_init_device() down so that the forward declaration of
lis3lv02d_add_fs() becomes unnecessary.

Signed-off-by: Daniel Mack <[email protected]>
Acked-by: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
Cc: Andrew Morton <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 64 +++++++++++++++++++++-----------------------
1 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 4888ac5..eeae7c9 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -59,8 +59,6 @@ struct acpi_lis3lv02d lis3_dev = {

EXPORT_SYMBOL_GPL(lis3_dev);

-static int lis3lv02d_add_fs(struct acpi_device *device);
-
static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
{
u8 lo, hi;
@@ -377,37 +375,6 @@ void lis3lv02d_joystick_disable(void)
}
EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);

-/*
- * Initialise the accelerometer and the various subsystems.
- * Should be rather independant of the bus system.
- */
-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
-{
- mutex_init(&dev->lock);
- lis3lv02d_add_fs(dev->device);
- lis3lv02d_increase_use(dev);
-
- if (lis3lv02d_joystick_enable())
- printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
-
- printk("lis3_init_device: irq %d\n", dev->irq);
-
- /* if we did not get an IRQ from ACPI - we have nothing more to do */
- if (!dev->irq) {
- printk(KERN_ERR DRIVER_NAME
- ": No IRQ in ACPI. Disabling /dev/freefall\n");
- goto out;
- }
-
- printk("lis3: registering device\n");
- if (misc_register(&lis3lv02d_misc_device))
- printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
-out:
- lis3lv02d_decrease_use(dev);
- return 0;
-}
-EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
-
/* Sysfs stuff */
static ssize_t lis3lv02d_position_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -485,6 +452,37 @@ int lis3lv02d_remove_fs(void)
}
EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);

+/*
+ * Initialise the accelerometer and the various subsystems.
+ * Should be rather independant of the bus system.
+ */
+int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
+{
+ mutex_init(&dev->lock);
+ lis3lv02d_add_fs(dev->device);
+ lis3lv02d_increase_use(dev);
+
+ if (lis3lv02d_joystick_enable())
+ printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
+
+ printk("lis3_init_device: irq %d\n", dev->irq);
+
+ /* if we did not get an IRQ from ACPI - we have nothing more to do */
+ if (!dev->irq) {
+ printk(KERN_ERR DRIVER_NAME
+ ": No IRQ in ACPI. Disabling /dev/freefall\n");
+ goto out;
+ }
+
+ printk("lis3: registering device\n");
+ if (misc_register(&lis3lv02d_misc_device))
+ printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
+out:
+ lis3lv02d_decrease_use(dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
+
MODULE_DESCRIPTION("ST LIS3LV02Dx three-axis digital accelerometer driver");
MODULE_AUTHOR("Yan Burman, Eric Piel, Pavel Machek");
MODULE_LICENSE("GPL");
--
1.6.2

2009-03-22 23:53:03

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 2/3] lis3: solve dependency between core and ACPI

This solves the dependency between lis3lv02d.[ch] and ACPI specific
methods. It introduces a ->bus_priv pointer to the device struct which
is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
accordingly.

Signed-off-by: Daniel Mack <[email protected]>
Acked-by: Pavel Machek <[email protected]>
Cc: Eric Piel <[email protected]>
Cc: Andrew Morton <[email protected]>
---
drivers/hwmon/hp_accel.c | 99 +++++++++++++++++---------------------------
drivers/hwmon/lis3lv02d.c | 86 ++++++++++++++++++++++++++-------------
drivers/hwmon/lis3lv02d.h | 20 +++++-----
3 files changed, 105 insertions(+), 100 deletions(-)

diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index ee1ed42..b6c32c5 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -85,25 +85,31 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids);

/**
* lis3lv02d_acpi_init - ACPI _INI method: initialize the device.
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_init(acpi_handle handle)
+int lis3lv02d_acpi_init(struct lis3lv02d *lis3)
{
- return acpi_evaluate_object(handle, METHOD_NAME__INI, NULL, NULL);
+ struct acpi_device *dev = lis3->bus_priv;
+ if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI,
+ NULL, NULL) != AE_OK)
+ return -EINVAL;
+
+ return 0;
}

/**
* lis3lv02d_acpi_read - ACPI ALRD method: read a register
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
* @reg: the register to read
* @ret: result of the operation
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret)
+int lis3lv02d_acpi_read(struct lis3lv02d *lis3, int reg, u8 *ret)
{
+ struct acpi_device *dev = lis3->bus_priv;
union acpi_object arg0 = { ACPI_TYPE_INTEGER };
struct acpi_object_list args = { 1, &arg0 };
unsigned long long lret;
@@ -111,21 +117,22 @@ acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret)

arg0.integer.value = reg;

- status = acpi_evaluate_integer(handle, "ALRD", &args, &lret);
+ status = acpi_evaluate_integer(dev->handle, "ALRD", &args, &lret);
*ret = lret;
- return status;
+ return (status != AE_OK) ? -EINVAL : 0;
}

/**
* lis3lv02d_acpi_write - ACPI ALWR method: write to a register
- * @handle: the handle of the device
+ * @lis3: pointer to the device struct
* @reg: the register to write to
* @val: the value to write
*
- * Returns AE_OK on success.
+ * Returns 0 on success.
*/
-acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val)
+int lis3lv02d_acpi_write(struct lis3lv02d *lis3, int reg, u8 val)
{
+ struct acpi_device *dev = lis3->bus_priv;
unsigned long long ret; /* Not used when writting */
union acpi_object in_obj[2];
struct acpi_object_list args = { 2, in_obj };
@@ -135,7 +142,10 @@ acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val)
in_obj[1].type = ACPI_TYPE_INTEGER;
in_obj[1].integer.value = val;

- return acpi_evaluate_integer(handle, "ALWR", &args, &ret);
+ if (acpi_evaluate_integer(dev->handle, "ALWR", &args, &ret) != AE_OK)
+ return -EINVAL;
+
+ return 0;
}

static int lis3lv02d_dmi_matched(const struct dmi_system_id *dmi)
@@ -214,7 +224,7 @@ static struct dmi_system_id lis3lv02d_dmi_ids[] = {

static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness value)
{
- acpi_handle handle = lis3_dev.device->handle;
+ struct acpi_device *dev = lis3_dev.bus_priv;
unsigned long long ret; /* Not used when writing */
union acpi_object in_obj[1];
struct acpi_object_list args = { 1, in_obj };
@@ -222,7 +232,7 @@ static void hpled_set(struct delayed_led_classdev *led_cdev, enum led_brightness
in_obj[0].type = ACPI_TYPE_INTEGER;
in_obj[0].integer.value = !!value;

- acpi_evaluate_integer(handle, "ALED", &args, &ret);
+ acpi_evaluate_integer(dev->handle, "ALED", &args, &ret);
}

static struct delayed_led_classdev hpled_led = {
@@ -259,23 +269,6 @@ static void lis3lv02d_enum_resources(struct acpi_device *device)
printk(KERN_DEBUG DRIVER_NAME ": Error getting resources\n");
}

-static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
-{
- u8 lo, hi;
-
- lis3_dev.read(handle, reg - 1, &lo);
- lis3_dev.read(handle, reg, &hi);
- /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
- return (s16)((hi << 8) | lo);
-}
-
-static s16 lis3lv02d_read_8(acpi_handle handle, int reg)
-{
- s8 lo;
- lis3_dev.read(handle, reg, &lo);
- return lo;
-}
-
static int lis3lv02d_add(struct acpi_device *device)
{
int ret;
@@ -283,7 +276,7 @@ static int lis3lv02d_add(struct acpi_device *device)
if (!device)
return -EINVAL;

- lis3_dev.device = device;
+ lis3_dev.bus_priv = device;
lis3_dev.init = lis3lv02d_acpi_init;
lis3_dev.read = lis3lv02d_acpi_read;
lis3_dev.write = lis3lv02d_acpi_write;
@@ -291,23 +284,8 @@ static int lis3lv02d_add(struct acpi_device *device)
strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
device->driver_data = &lis3_dev;

- lis3lv02d_acpi_read(device->handle, WHO_AM_I, &lis3_dev.whoami);
- switch (lis3_dev.whoami) {
- case LIS_DOUBLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
- lis3_dev.read_data = lis3lv02d_read_16;
- lis3_dev.mdps_max_val = 2048;
- break;
- case LIS_SINGLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
- lis3_dev.read_data = lis3lv02d_read_8;
- lis3_dev.mdps_max_val = 128;
- break;
- default:
- printk(KERN_ERR DRIVER_NAME
- ": unknown sensor type 0x%X\n", lis3_dev.whoami);
- return -EINVAL;
- }
+ /* obtain IRQ number of our device from ACPI */
+ lis3lv02d_enum_resources(device);

/* If possible use a "standard" axes order */
if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
@@ -316,18 +294,17 @@ static int lis3lv02d_add(struct acpi_device *device)
lis3_dev.ac = lis3lv02d_axis_normal;
}

- INIT_WORK(&hpled_led.work, delayed_set_status_worker);
- ret = led_classdev_register(NULL, &hpled_led.led_classdev);
+ /* call the core layer do its init */
+ ret = lis3lv02d_init_device(&lis3_dev);
if (ret)
return ret;

- /* obtain IRQ number of our device from ACPI */
- lis3lv02d_enum_resources(lis3_dev.device);
-
- ret = lis3lv02d_init_device(&lis3_dev);
+ INIT_WORK(&hpled_led.work, delayed_set_status_worker);
+ ret = led_classdev_register(NULL, &hpled_led.led_classdev);
if (ret) {
+ lis3lv02d_joystick_disable();
+ lis3lv02d_poweroff(&lis3_dev);
flush_work(&hpled_led.work);
- led_classdev_unregister(&hpled_led.led_classdev);
return ret;
}

@@ -340,7 +317,7 @@ static int lis3lv02d_remove(struct acpi_device *device, int type)
return -EINVAL;

lis3lv02d_joystick_disable();
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);

flush_work(&hpled_led.work);
led_classdev_unregister(&hpled_led.led_classdev);
@@ -353,7 +330,7 @@ static int lis3lv02d_remove(struct acpi_device *device, int type)
static int lis3lv02d_suspend(struct acpi_device *device, pm_message_t state)
{
/* make sure the device is off when we suspend */
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);
return 0;
}

@@ -362,9 +339,9 @@ static int lis3lv02d_resume(struct acpi_device *device)
/* put back the device in the right state (ACPI might turn it on) */
mutex_lock(&lis3_dev.lock);
if (lis3_dev.usage > 0)
- lis3lv02d_poweron(device->handle);
+ lis3lv02d_poweron(&lis3_dev);
else
- lis3lv02d_poweroff(device->handle);
+ lis3lv02d_poweroff(&lis3_dev);
mutex_unlock(&lis3_dev.lock);
return 0;
}
diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index eeae7c9..778eb77 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -36,7 +36,6 @@
#include <linux/freezer.h>
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
-#include <acpi/acpi_drivers.h>
#include <asm/atomic.h>
#include "lis3lv02d.h"

@@ -53,18 +52,27 @@
* joystick.
*/

-struct acpi_lis3lv02d lis3_dev = {
+struct lis3lv02d lis3_dev = {
.misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
};

EXPORT_SYMBOL_GPL(lis3_dev);

-static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
+static s16 lis3lv02d_read_8(struct lis3lv02d *lis3, int reg)
+{
+ s8 lo;
+ if (lis3->read(lis3, reg, &lo) < 0)
+ return 0;
+
+ return lo;
+}
+
+static s16 lis3lv02d_read_16(struct lis3lv02d *lis3, int reg)
{
u8 lo, hi;

- lis3_dev.read(handle, reg, &lo);
- lis3_dev.read(handle, reg + 1, &hi);
+ lis3->read(lis3, reg - 1, &lo);
+ lis3->read(lis3, reg, &hi);
/* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
return (s16)((hi << 8) | lo);
}
@@ -86,36 +94,36 @@ static inline int lis3lv02d_get_axis(s8 axis, int hw_values[3])

/**
* lis3lv02d_get_xyz - Get X, Y and Z axis values from the accelerometer
- * @handle: the handle to the device
- * @x: where to store the X axis value
- * @y: where to store the Y axis value
- * @z: where to store the Z axis value
+ * @lis3: pointer to the device struct
+ * @x: where to store the X axis value
+ * @y: where to store the Y axis value
+ * @z: where to store the Z axis value
*
* Note that 40Hz input device can eat up about 10% CPU at 800MHZ
*/
-static void lis3lv02d_get_xyz(acpi_handle handle, int *x, int *y, int *z)
+static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
{
int position[3];

- position[0] = lis3_dev.read_data(handle, OUTX);
- position[1] = lis3_dev.read_data(handle, OUTY);
- position[2] = lis3_dev.read_data(handle, OUTZ);
+ position[0] = lis3_dev.read_data(lis3, OUTX);
+ position[1] = lis3_dev.read_data(lis3, OUTY);
+ position[2] = lis3_dev.read_data(lis3, OUTZ);

*x = lis3lv02d_get_axis(lis3_dev.ac.x, position);
*y = lis3lv02d_get_axis(lis3_dev.ac.y, position);
*z = lis3lv02d_get_axis(lis3_dev.ac.z, position);
}

-void lis3lv02d_poweroff(acpi_handle handle)
+void lis3lv02d_poweroff(struct lis3lv02d *lis3)
{
lis3_dev.is_on = 0;
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweroff);

-void lis3lv02d_poweron(acpi_handle handle)
+void lis3lv02d_poweron(struct lis3lv02d *lis3)
{
lis3_dev.is_on = 1;
- lis3_dev.init(handle);
+ lis3_dev.init(lis3);
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

@@ -124,13 +132,13 @@ EXPORT_SYMBOL_GPL(lis3lv02d_poweron);
* device will always be on until a call to lis3lv02d_decrease_use(). Not to be
* used from interrupt context.
*/
-static void lis3lv02d_increase_use(struct acpi_lis3lv02d *dev)
+static void lis3lv02d_increase_use(struct lis3lv02d *dev)
{
mutex_lock(&dev->lock);
dev->usage++;
if (dev->usage == 1) {
if (!dev->is_on)
- lis3lv02d_poweron(dev->device->handle);
+ lis3lv02d_poweron(dev);
}
mutex_unlock(&dev->lock);
}
@@ -139,12 +147,12 @@ static void lis3lv02d_increase_use(struct acpi_lis3lv02d *dev)
* To be called whenever a usage of the device is stopped.
* It will make sure to turn off the device when there is not usage.
*/
-static void lis3lv02d_decrease_use(struct acpi_lis3lv02d *dev)
+static void lis3lv02d_decrease_use(struct lis3lv02d *dev)
{
mutex_lock(&dev->lock);
dev->usage--;
if (dev->usage == 0)
- lis3lv02d_poweroff(dev->device->handle);
+ lis3lv02d_poweroff(dev);
mutex_unlock(&dev->lock);
}

@@ -291,7 +299,7 @@ static int lis3lv02d_joystick_kthread(void *data)
int x, y, z;

while (!kthread_should_stop()) {
- lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
input_report_abs(lis3_dev.idev, ABS_X, x - lis3_dev.xcalib);
input_report_abs(lis3_dev.idev, ABS_Y, y - lis3_dev.ycalib);
input_report_abs(lis3_dev.idev, ABS_Z, z - lis3_dev.zcalib);
@@ -325,7 +333,8 @@ static void lis3lv02d_joystick_close(struct input_dev *input)

static inline void lis3lv02d_calibrate_joystick(void)
{
- lis3lv02d_get_xyz(lis3_dev.device->handle, &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
+ lis3lv02d_get_xyz(&lis3_dev,
+ &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
}

int lis3lv02d_joystick_enable(void)
@@ -382,7 +391,7 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
int x, y, z;

lis3lv02d_increase_use(&lis3_dev);
- lis3lv02d_get_xyz(lis3_dev.device->handle, &x, &y, &z);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
lis3lv02d_decrease_use(&lis3_dev);
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}
@@ -412,7 +421,7 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
int val;

lis3lv02d_increase_use(&lis3_dev);
- lis3_dev.read(lis3_dev.device->handle, CTRL_REG1, &ctrl);
+ lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
lis3lv02d_decrease_use(&lis3_dev);
val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
@@ -435,7 +444,7 @@ static struct attribute_group lis3lv02d_attribute_group = {
};


-static int lis3lv02d_add_fs(struct acpi_device *device)
+static int lis3lv02d_add_fs(struct lis3lv02d *lis3)
{
lis3_dev.pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
if (IS_ERR(lis3_dev.pdev))
@@ -456,10 +465,29 @@ EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
* Initialise the accelerometer and the various subsystems.
* Should be rather independant of the bus system.
*/
-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)
+int lis3lv02d_init_device(struct lis3lv02d *dev)
{
+ dev->whoami = lis3lv02d_read_8(dev, WHO_AM_I);
+
+ switch (dev->whoami) {
+ case LIS_DOUBLE_ID:
+ printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
+ dev->read_data = lis3lv02d_read_16;
+ dev->mdps_max_val = 2048;
+ break;
+ case LIS_SINGLE_ID:
+ printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
+ dev->read_data = lis3lv02d_read_8;
+ dev->mdps_max_val = 128;
+ break;
+ default:
+ printk(KERN_ERR DRIVER_NAME
+ ": unknown sensor type 0x%X\n", lis3_dev.whoami);
+ return -EINVAL;
+ }
+
mutex_init(&dev->lock);
- lis3lv02d_add_fs(dev->device);
+ lis3lv02d_add_fs(dev);
lis3lv02d_increase_use(dev);

if (lis3lv02d_joystick_enable())
@@ -467,10 +495,10 @@ int lis3lv02d_init_device(struct acpi_lis3lv02d *dev)

printk("lis3_init_device: irq %d\n", dev->irq);

- /* if we did not get an IRQ from ACPI - we have nothing more to do */
+ /* bail if we did not get an IRQ from the bus layer */
if (!dev->irq) {
printk(KERN_ERR DRIVER_NAME
- ": No IRQ in ACPI. Disabling /dev/freefall\n");
+ ": No IRQ. Disabling /dev/freefall\n");
goto out;
}

diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 017fb2b..745ec96 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -159,14 +159,14 @@ struct axis_conversion {
s8 z;
};

-struct acpi_lis3lv02d {
- struct acpi_device *device; /* The ACPI device */
- acpi_status (*init) (acpi_handle handle);
- acpi_status (*write) (acpi_handle handle, int reg, u8 val);
- acpi_status (*read) (acpi_handle handle, int reg, u8 *ret);
+struct lis3lv02d {
+ void *bus_priv; /* used by the bus layer only */
+ int (*init) (struct lis3lv02d *lis3);
+ int (*write) (struct lis3lv02d *lis3, int reg, u8 val);
+ int (*read) (struct lis3lv02d *lis3, int reg, u8 *ret);

u8 whoami; /* 3Ah: 2-byte registries, 3Bh: 1-byte registries */
- s16 (*read_data) (acpi_handle handle, int reg);
+ s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;

struct input_dev *idev; /* input device */
@@ -187,11 +187,11 @@ struct acpi_lis3lv02d {
unsigned long misc_opened; /* bit0: whether the device is open */
};

-int lis3lv02d_init_device(struct acpi_lis3lv02d *dev);
+int lis3lv02d_init_device(struct lis3lv02d *lis3);
int lis3lv02d_joystick_enable(void);
void lis3lv02d_joystick_disable(void);
-void lis3lv02d_poweroff(acpi_handle handle);
-void lis3lv02d_poweron(acpi_handle handle);
+void lis3lv02d_poweroff(struct lis3lv02d *lis3);
+void lis3lv02d_poweron(struct lis3lv02d *lis3);
int lis3lv02d_remove_fs(void);

-extern struct acpi_lis3lv02d lis3_dev;
+extern struct lis3lv02d lis3_dev;
--
1.6.2

2009-03-22 23:53:29

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 3/3] lis3: SPI transport layer

Make use of the new abstraction layer and add a new transport layer for
spi. Works fine on a PXA based board.

Signed-off-by: Daniel Mack <[email protected]>
Acked-by: Pavel Machek <[email protected]>
Cc: Éric Piel <[email protected]>
Cc: Andrew Morton <[email protected]>
---
drivers/hwmon/Kconfig | 16 ++++++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/lis3lv02d_spi.c | 114 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 131 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/lis3lv02d_spi.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b84bf06..9d78ab4 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -895,6 +895,22 @@ config SENSORS_LIS3LV02D
Say Y here if you have an applicable laptop and want to experience
the awesome power of lis3lv02d.

+config SENSORS_LIS3_SPI
+ tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer (SPI)"
+ depends on !ACPI && SPI_MASTER && INPUT
+ default n
+ help
+ This driver provides support for the LIS3LV02Dx accelerometer connected
+ via SPI. The accelerometer data is readable via
+ /sys/devices/platform/lis3lv02d.
+
+ This driver also provides an absolute input class device, allowing
+ the laptop to act as a pinball machine-esque joystick.
+
+ This driver can also be built as modules. If so, the core module
+ will be called lis3lv02d and a specific module for the SPI transport
+ is called lis3lv02d_spi.
+
config SENSORS_APPLESMC
tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
depends on INPUT && X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2e80f37..6f0f4b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
obj-$(CONFIG_SENSORS_IT87) += it87.o
obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o
obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
+obj-$(CONFIG_SENSORS_LIS3_SPI) += lis3lv02d.o lis3lv02d_spi.o
obj-$(CONFIG_SENSORS_LM63) += lm63.o
obj-$(CONFIG_SENSORS_LM70) += lm70.o
obj-$(CONFIG_SENSORS_LM75) += lm75.o
diff --git a/drivers/hwmon/lis3lv02d_spi.c b/drivers/hwmon/lis3lv02d_spi.c
new file mode 100644
index 0000000..07ae74b
--- /dev/null
+++ b/drivers/hwmon/lis3lv02d_spi.c
@@ -0,0 +1,114 @@
+/*
+ * lis3lv02d_spi - SPI glue layer for lis3lv02d
+ *
+ * Copyright (c) 2009 Daniel Mack <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/spi/spi.h>
+
+#include "lis3lv02d.h"
+
+#define DRV_NAME "lis3lv02d_spi"
+#define LIS3_SPI_READ 0x80
+
+static int lis3_spi_read(struct lis3lv02d *lis3, int reg, u8 *v)
+{
+ struct spi_device *spi = lis3->bus_priv;
+ int ret = spi_w8r8(spi, reg | LIS3_SPI_READ);
+ if (ret < 0)
+ return -EINVAL;
+
+ *v = (u8) ret;
+ return 0;
+}
+
+static int lis3_spi_write(struct lis3lv02d *lis3, int reg, u8 val)
+{
+ u8 tmp[2] = { reg, val };
+ struct spi_device *spi = lis3->bus_priv;
+ return spi_write(spi, tmp, sizeof(tmp));
+}
+
+static int lis3_spi_init(struct lis3lv02d *lis3)
+{
+ u8 reg;
+ int ret;
+
+ /* power up the device */
+ ret = lis3->read(lis3, CTRL_REG1, &reg);
+ if (ret < 0)
+ return ret;
+
+ reg |= CTRL1_PD0;
+ return lis3->write(lis3, CTRL_REG1, reg);
+}
+
+static struct axis_conversion lis3lv02d_axis_normal = { 1, 2, 3 };
+
+static int __devinit lis302dl_spi_probe(struct spi_device *spi)
+{
+ int ret;
+
+ spi->bits_per_word = 8;
+ spi->mode = SPI_MODE_0;
+ ret = spi_setup(spi);
+ if (ret < 0)
+ return ret;
+
+ lis3_dev.bus_priv = spi;
+ lis3_dev.init = lis3_spi_init;
+ lis3_dev.read = lis3_spi_read;
+ lis3_dev.write = lis3_spi_write;
+ lis3_dev.irq = spi->irq;
+ lis3_dev.ac = lis3lv02d_axis_normal;
+ spi_set_drvdata(spi, &lis3_dev);
+
+ ret = lis3lv02d_init_device(&lis3_dev);
+ return ret;
+}
+
+static int __devexit lis302dl_spi_remove(struct spi_device *spi)
+{
+ struct lis3lv02d *lis3 = spi_get_drvdata(spi);
+ lis3lv02d_joystick_disable();
+ lis3lv02d_poweroff(lis3);
+ return 0;
+}
+
+static struct spi_driver lis302dl_spi_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = lis302dl_spi_probe,
+ .remove = __devexit_p(lis302dl_spi_remove),
+};
+
+static int __init lis302dl_init(void)
+{
+ return spi_register_driver(&lis302dl_spi_driver);
+}
+
+static void __exit lis302dl_exit(void)
+{
+ spi_unregister_driver(&lis302dl_spi_driver);
+}
+
+module_init(lis302dl_init);
+module_exit(lis302dl_exit);
+
+MODULE_AUTHOR("Daniel Mack <[email protected]>");
+MODULE_DESCRIPTION("lis3lv02d SPI glue layer");
+MODULE_LICENSE("GPL");
+
--
1.6.2

2009-03-23 15:28:56

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 5/5] lis3: SPI transport layer

Daniel Mack schreef:
> Make use of the new abstraction layer and add a new transport layer for
> spi. Works fine on a PXA based board.
Tested with a HP laptop, no problem on this part.

> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Eric Piel <[email protected]>
Acked-by: Eric Piel <[email protected]>

Eric

2009-03-23 15:41:35

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 5/5] lis3: SPI transport layer

On Mon, Mar 23, 2009 at 12:31:16AM +0100, ?ric Piel wrote:
> Daniel Mack schreef:
> > Make use of the new abstraction layer and add a new transport layer for
> > spi. Works fine on a PXA based board.
> Tested with a HP laptop, no problem on this part.
>
> > Signed-off-by: Daniel Mack <[email protected]>
> > Cc: Pavel Machek <[email protected]>
> > Cc: Eric Piel <[email protected]>
> Acked-by: Eric Piel <[email protected]>

Thanks Eric! However, I guess you wanted to ack the latest version
rather than the one you replied to ;)

Just to make sure, these are the references to the patches we want:

http://lkml.org/lkml/2009/3/22/194
http://lkml.org/lkml/2009/3/22/195
http://lkml.org/lkml/2009/3/22/196

Andrew, could you add them to your tree?

Thanks,
Daniel

2009-03-23 15:48:18

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 1/3] lis3: reorder functions to make forward decl obsolete

Daniel Mack schreef:
> moved lis3lv02d_init_device() down so that the forward declaration of
> lis3lv02d_add_fs() becomes unnecessary.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Acked-by: Pavel Machek <[email protected]>
> Cc: Eric Piel <[email protected]>
> Cc: Andrew Morton <[email protected]>
Acked-by: Eric Piel <[email protected]>

Eric

2009-03-23 15:48:32

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 2/3] lis3: solve dependency between core and ACPI

Daniel Mack schreef:
> This solves the dependency between lis3lv02d.[ch] and ACPI specific
> methods. It introduces a ->bus_priv pointer to the device struct which
> is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> accordingly.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Acked-by: Pavel Machek <[email protected]>
> Cc: Eric Piel <[email protected]>
> Cc: Andrew Morton <[email protected]>
Acked-by: Eric Piel <[email protected]>

Eric

2009-03-23 15:48:48

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 3/3] lis3: SPI transport layer

Daniel Mack schreef:
> Make use of the new abstraction layer and add a new transport layer for
> spi. Works fine on a PXA based board.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Acked-by: Pavel Machek <[email protected]>
> Cc: Éric Piel <[email protected]>
> Cc: Andrew Morton <[email protected]>
Acked-by: Eric Piel <[email protected]>

Eric