lis3&#39;s ACPI dependency

Daniel Mack schreef:

Quoted 8 lines

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.

Quoted 4 lines

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

Daniel Mack

author

2009-03-02 00:56 UTC #4 lore raw

Quoted 10 lines · Éric Piel

Quoted 10 lines
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?

Quoted 8 lines

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.

Quoted 7 lines

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

Éric Piel

2009-03-02 10:17 UTC #5 lore raw

Daniel Mack schreef:

Quoted 5 lines

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.

Quoted 2 lines

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

Daniel Mack

author

2009-03-02 14:32 UTC #6 lore raw

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(-)

drivers/hwmon/hp_accel.c renamed from drivers/hwmon/hp_accel.c~hp_accel-adev-is-poor-name-of-exported-symbol +24 ~~-24~~

diff -puN drivers/hwmon/hp_accel.c~hp_accel-adev-is-poor-name-of-exported-symbol 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

drivers/hwmon/lis3lv02d.c renamed from drivers/hwmon/lis3lv02d.c~hp_accel-adev-is-poor-name-of-exported-symbol +93 ~~-83~~

340 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h renamed from drivers/hwmon/lis3lv02d.h~hp_accel-adev-is-poor-name-of-exported-symbol +1 -1

diff -puN drivers/hwmon/lis3lv02d.h~hp_accel-adev-is-poor-name-of-exported-symbol 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;

Daniel Mack

author

2009-03-02 14:32 UTC #7 lore raw

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(-)

Documentation/hwmon/lis3lv02d renamed from Documentation/hwmon/lis3lv02d~hp_accel-small-documentation-updates +15 -5

diff -puN Documentation/hwmon/lis3lv02d~hp_accel-small-documentation-updates 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.

Daniel Mack

author

2009-03-02 14:32 UTC #8 lore raw

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(-)

drivers/hwmon/lis3lv02d.c +31 ~~-33~~

index 4888ac5..eeae7c9 100644
@@ -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");

Signature · 1 line

1.6.1.3

Daniel Mack

author

2009-03-02 14:33 UTC #9 lore raw

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(-)

drivers/hwmon/hp_accel.c +38 ~~-62~~

index ee1ed42..39e5011 100644
@@ -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;
 }

drivers/hwmon/lis3lv02d.c +57 ~~-29~~

198 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +10 ~~-10~~

index 017fb2b..745ec96 100644
@@ -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;

Signature · 1 line

1.6.1.3

Daniel Mack

author

2009-03-02 14:33 UTC #10 lore raw

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

drivers/hwmon/Kconfig +16 -0

index b84bf06..9d78ab4 100644
@@ -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

drivers/hwmon/Makefile +1 -0

index 2e80f37..6f0f4b3 100644
@@ -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

drivers/hwmon/lis3lv02d_spi.c new file +113 -0

113 lines not on this page · raw message

Signature · 1 line

1.6.1.3

Pavel Machek

Acked-by on v1 5/5

2009-03-02 15:09 UTC #11 lore raw

Re: [PATCH 5/5] lis3: SPI transport layer

Quoted 2 lines · Daniel Mack

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.

Quoted 1 line

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

Vlado, could you test the compelete series?
Pavel

Signature · 2 lines

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

Éric Piel

2009-03-03 19:59 UTC #12 lore raw

Re: [PATCH 5/5] lis3: SPI transport layer

Daniel Mack schreef:

Quoted 3 lines

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:
:

drivers/hwmon/lis3lv02d_spi.c line 52

Quoted 11 lines

drivers/hwmon/lis3lv02d_spi.c new file +11 -0

+
+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.

Review

Quoted 2 lines

(quoted diff) +2 -0

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

Looks good otherwise :-)
Eric

Pavel Machek

Acked-by on v1 4/5

2009-03-02 15:07 UTC #13 lore raw

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

Hi!

Quoted 4 lines

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.

Quoted 2 lines

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...

Quoted 1 line

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

Signature · 2 lines

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

Éric Piel

2009-03-03 19:55 UTC #14 lore raw

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

Daniel Mack schreef:

Quoted 7 lines

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...

Quoted 41 lines

drivers/hwmon/hp_accel.c +5 ~~-24~~

@@ -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);

drivers/hwmon/hp_accel.c static int lis3lv02d_add(struct acpi_device *device) · line 311

Quoted 10 lines

drivers/hwmon/hp_accel.c +3 -1

 		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.

drivers/hwmon/hp_accel.c static void lis3lv02d_enum_resources(struct acpi_device *device) · line 270

Quoted 10 lines

drivers/hwmon/hp_accel.c +0 -9

-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);
-}

drivers/hwmon/lis3lv02d.c line 78

Quoted 11 lines

drivers/hwmon/lis3lv02d.c +3 -2

+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

Daniel Mack

author

2009-03-04 01:44 UTC #15 lore raw

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

Hi,

Quoted 6 lines · Éric Piel

Daniel Mack schreef:
Quoted 5 lines
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.

Quoted 6 lines

Quoted 2 lines
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

Daniel Mack

author

2009-03-16 19:10 UTC #16 lore raw

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

Hi,

Quoted 9 lines · Daniel Mack

Quoted 8 lines · Éric Piel
Daniel Mack schreef:
Quoted 7 lines
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.

Quoted 7 lines

Quoted 4 lines
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

Pavel Machek

2009-03-16 21:28 UTC #17 lore raw

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

Hi!

Quoted 12 lines

Quoted 12 lines
Quoted 11 lines
Quoted 7 lines
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...

Quoted 5 lines

Quoted 2 lines
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

Signature · 2 lines

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

Daniel Mack

author

2009-03-04 01:45 UTC #18 lore raw

PATCH 3/5 lis3: reorder functions to make forward decl obsolete

In reply to Éric Piel

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(-)

drivers/hwmon/lis3lv02d.c +31 ~~-33~~

index 4888ac5..eeae7c9 100644
@@ -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");

Signature · 1 line

1.6.1.3

Daniel Mack

author

2009-03-04 01:45 UTC #19 lore raw

PATCH 4/5 lis3: solve dependency between core and ACPI

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(-)

drivers/hwmon/hp_accel.c +39 ~~-62~~

index ee1ed42..363220b 100644
@@ -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;
 }

drivers/hwmon/lis3lv02d.c +57 ~~-29~~

198 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +10 ~~-10~~

index 017fb2b..745ec96 100644
@@ -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;

Signature · 1 line

1.6.1.3

Daniel Mack

author

2009-03-04 01:46 UTC #20 lore raw

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

drivers/hwmon/Kconfig +16 -0

index b84bf06..9d78ab4 100644
@@ -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

drivers/hwmon/Makefile +1 -0

index 2e80f37..6f0f4b3 100644
@@ -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

drivers/hwmon/lis3lv02d_spi.c new file +114 -0

114 lines not on this page · raw message

Signature · 1 line

1.6.1.3

Éric Piel

Acked-by on v1 5/5

2009-03-23 15:28 UTC #21 lore raw

Re: [PATCH 5/5] lis3: SPI transport layer

Daniel Mack schreef:

Quoted 2 lines

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.

Quoted 3 lines

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

Eric

Daniel Mack

author

2009-03-23 15:41 UTC #22 lore raw

Re: [PATCH 5/5] lis3: SPI transport layer

Quoted 9 lines · Éric Piel

Daniel Mack schreef:
Quoted 2 lines
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.
Quoted 3 lines
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

Éric Piel

2009-03-22 23:25 UTC #23 lore raw

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

Daniel Mack schreef:

Quoted 4 lines

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(-)

drivers/hwmon/hp_accel.c +5 -5

index 363220b..b6c32c5 100644
@@ -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) {

Signature · 1 line

1.6.2.1

Daniel Mack

author

2009-03-22 23:43 UTC #24 lore raw

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

Quoted 4 lines · Éric Piel

Quoted 4 lines
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.

Quoted 8 lines

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

Daniel Mack

author

2009-03-22 23:52 UTC #25 lore raw

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]>

Cc Eric Piel <[email protected]>

Cc Andrew Morton <[email protected]>

---
 drivers/hwmon/lis3lv02d.c |   64 +++++++++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 33 deletions(-)

drivers/hwmon/lis3lv02d.c +31 ~~-33~~

index 4888ac5..eeae7c9 100644
@@ -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");

Signature · 1 line

1.6.2

Daniel Mack

author

2009-03-22 23:53 UTC #26 lore raw

PATCH 2/3 lis3: solve dependency between core and ACPI

Signed-off-by Daniel Mack <[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(-)

drivers/hwmon/hp_accel.c +38 ~~-61~~

index ee1ed42..b6c32c5 100644
@@ -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;
 }

drivers/hwmon/lis3lv02d.c +57 ~~-29~~

198 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +10 ~~-10~~

index 017fb2b..745ec96 100644
@@ -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;

Signature · 1 line

1.6.2

Daniel Mack

author

2009-03-22 23:53 UTC #27 lore raw

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]>

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

drivers/hwmon/Kconfig +16 -0

index b84bf06..9d78ab4 100644
@@ -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

drivers/hwmon/Makefile +1 -0

index 2e80f37..6f0f4b3 100644
@@ -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

drivers/hwmon/lis3lv02d_spi.c new file +114 -0

114 lines not on this page · raw message

Signature · 1 line

1.6.2

Éric Piel

Acked-by on v1 3/3

2009-03-23 15:48 UTC #28 lore raw

Re: [PATCH 3/3] lis3: SPI transport layer

Daniel Mack schreef:

Quoted 7 lines

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]>

Eric

Geert Uytterhoeven

2016-08-10 10:31 UTC #29 lore raw

Re: [PATCH 3/3] lis3: SPI transport layer

Hi Daniel,

drivers/hwmon/Kconfig

Quoted 18 lines · Daniel Mack

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 <akpm-de/[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
drivers/hwmon/Kconfig +0 -0
index b84bf06..9d78ab4 100644

In the mean time, this has moved to drivers/misc/lis3lv02d/Kconfig

drivers/hwmon/Kconfig config SENSORS_LIS3LV02D · line 900

Quoted 7 lines

drivers/hwmon/Kconfig +3 -0

@@ -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

Is there any specific reason this option depends on !ACPI? This hurts multi-platform kernels, supporting both OF and ACPI.

drivers/hwmon/Kconfig config SENSORS_LIS3LV02D · line 913

Quoted 13 lines

drivers/hwmon/Kconfig +13 -0

+       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.
+

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Éric Piel

Acked-by on v1 2/3

2009-03-23 15:48 UTC #30 lore raw

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

Daniel Mack schreef:

Quoted 9 lines

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]>

Eric

Éric Piel

Acked-by on v1 1/3

2009-03-23 15:48 UTC #31 lore raw

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

Daniel Mack schreef:

Quoted 7 lines

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]>

Eric

Pavel Machek

Acked-by on v1 3/5

2009-03-02 14:55 UTC #32 lore raw

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

Quoted 4 lines · Daniel Mack

moved lis3lv02d_init_device() down so that the forward declaration of lis3lv02d_add_fs() becomes unnecessary.
Signed-off-by Daniel Mack <[email protected]>