Hello,
I think I talked too fast: it seems impossible to have both drivers
(leds-hp-disk and lis3lv02d) working at the same time. Only the first
driver loaded is used.

After a little look at it, I think it comes from the fact that both
drivers are assigned to the same MODALIAS (HPQ0004). The ACPI PNP
(through the generic bus infrastructure) only declare the device to one
of the drivers supporting it, not all of them.

How can I tell to ACPI that it should load both drivers for the same PNP
ID match?

Eric
--

[ message continues ]

I can reproduce it here and it obviously needs fixing. OTOH  it should

I'll take a look if I can figure something out...

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

[ message continues ]

But for long-term the HPQ0004 specific things in the lids3v driver should get 
merged with your HP leds driver also registering for HPQ0004 and the lids3v 
specific things should get a separate driver which HPQ0004 driver makes use 
of?
Kay may know whether this should work and how.
IMO having several drivers registering for the same HID should get avoided if 
possible, it's confusing.

     Thomas

Pavel: I am also not sure whether it's a good idea to mis-use the HP's LED for 
mail notification or similar or to expose this one as a general LED. This LED 
is intended to be used with the disk parking feature by the vendor?
I don't know the LED interface well, but if it's now possible that every mail 
or other app can reprogram the disk parking LED for it's own purposes, this 
sounds wrong. The LED can still be accessed and activated, e.g. when disk 
gets parked behind the OS'es back. If this is a nice hack to e.g. use LED for 
suspend debugging or similar, then it should be well hidden to the outside 
world.
--

[ message continues ]

There is currently no driver core support to bind two "struct device"
to one parent "struct device" from different drivers.

You can work around that by creating a custom "struct bus_type",
create the several function devices there, and let them bind different
drivers. This is what matches a lot of hardware like custom devices
all hiding behind a single PCI device.

Or the code for the two functions must live in the same driver, and
create class devices, so it looks to the core like a single binding.

There is ongoing work in the driver core to allow
multi-driver-binding, but it's not ready to use today.

Kay
--

[ message continues ]

That's how LED interface works. This _is_ general LED.

Check permissions in /sys; root  permissons should be needed to

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

[ message continues ]

Yep... and this is the step in that direction. Relative to my previous
patch...

Later led stuff will get merged to hp_accel.c...

(Not for andrew, yet. I'd like basic support to get merged, first)
diff -ur clean-mm/drivers/hwmon/hp_accel.c linux-mm/drivers/hwmon/hp_accel.c
--- clean-mm/drivers/hwmon/hp_accel.c	2008-11-06 09:18:16.000000000 +0100
+++ linux-mm/drivers/hwmon/hp_accel.c	2008-11-06 12:26:03.000000000 +0100
@@ -163,6 +163,9 @@
 		return -EINVAL;
 
 	adev.device = device;
+	adev.init = lis3lv02d_acpi_init;
+	adev.read = lis3lv02d_acpi_read;
+	adev.write = lis3lv02d_acpi_write;
 	strcpy(acpi_device_name(device), DRIVER_NAME);
 	strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
 	device->driver_data = &adev;
@@ -259,3 +262,5 @@
 
 module_init(lis3lv02d_init_module);
 module_exit(lis3lv02d_exit_module);
+
+
diff -ur clean-mm/drivers/hwmon/lis3lv02d.c linux-mm/drivers/hwmon/lis3lv02d.c
--- clean-mm/drivers/hwmon/lis3lv02d.c	2008-11-06 09:18:16.000000000 +0100
+++ linux-mm/drivers/hwmon/lis3lv02d.c	2008-11-06 12:27:27.000000000 +0100
@@ -68,8 +68,8 @@
 {
 	u8 lo, hi;
 
-	lis3lv02d_acpi_read(handle, reg, &lo);
-	lis3lv02d_acpi_read(handle, reg + 1, &hi);
+	adev.read(handle, reg, &lo);
+	adev.read(handle, reg + 1, &hi);
 	/* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
 	return (s16)((hi << 8) | lo);
 }
@@ -115,7 +115,7 @@
 {
 	adev.is_on = 0;
 	/* disable X,Y,Z axis and power down */
-	lis3lv02d_acpi_write(handle, CTRL_REG1, 0x00);
+	adev.write(handle, CTRL_REG1, 0x00);
 }
 
 void lis3lv02d_poweron(acpi_handle handle)
@@ -123,16 +123,16 @@
 	u8 val;
 
 	adev.is_on = 1;
-	lis3lv02d_acpi_init(handle);
-	lis3lv02d_acpi_write(handle, FF_WU_CFG, 0);
+	adev.init(handle);
+	adev.write(handle, FF_WU_CFG, 0);
 	/*
 	 * BDU: LSB and MSB values are not updated until both have been read.
 	 *      So the value read will always be correct.
 	 * IEN: Interrupt for free-fall and DD, not for data-ready.
 	 ...
[ message continues ]

Hi Pavel,
Just to say I like this approach. It should be completely compatible 
with the idea of allowing different buses (SPI and I²C) as well. Of 
course, I wish it could be possible to have the led and accelerometer 
drivers separated because they have no common point but the fact that 
they are advertised by the same PNP ID. But I can deal with that.

Andrew, is there anything preventing the merge of the lis3lv02d driver 
from your tree to Linus' one? I think it's still worth to integrate it 
as is, and later on this refactoring can take place.

See you,
Eric
--

[ message continues ]

On Thu, 06 Nov 2008 22:45:05 +0100

Nope.  I have a number of shiny new drivers which I plan to send in for
2.6.28 (I sent an fbdev one this morning).  I need to get down and do a
sweep to round tham all up.
--

[ message continues ]

Ok, I guess lis3lv.c should be split into hw-dependend and
acpi-dependend parts, and then we can merge led driver and the acpi
parts....?

This is version that compiles :-)... relative to latest -mm.

								Pavel

diff -ur clean-mm/drivers/hwmon/Makefile linux-mm/drivers/hwmon/Makefile
--- clean-mm/drivers/hwmon/Makefile	2008-11-05 23:48:31.000000000 +0100
+++ linux-mm/drivers/hwmon/Makefile	2008-11-05 23:22:15.000000000 +0100
@@ -48,7 +48,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
+obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
diff -ur clean-mm/drivers/hwmon/hp_accel.c linux-mm/drivers/hwmon/hp_accel.c
--- clean-mm/drivers/hwmon/hp_accel.c	2008-11-05 23:53:06.000000000 +0100
+++ linux-mm/drivers/hwmon/hp_accel.c	2008-11-05 23:49:16.000000000 +0100
@@ -0,0 +1,261 @@
+/*
+ *  hp_accel.c - Interface between LIS3LV02DL driver and HP ACPI BIOS
+ *
+ *  Copyright (C) 2007-2008 Yan Burman
+ *  Copyright (C) 2008 Eric Piel
+ *  Copyright (C) 2008 Pavel Machek
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include ...
[ message continues ]

Here's patch that moves both to ec_accel driver, so LED and
accelerometer should work at the same time. Testing would be welcome,
relative to linux-mm.

(Incremental version relative to last version I posted is attached; it
is easier to read).

									Pavel
diff -ur clean-real-mm/drivers/hwmon/Makefile linux-mm/drivers/hwmon/Makefile
--- clean-real-mm/drivers/hwmon/Makefile	2008-11-10 10:48:57.000000000 +0100
+++ linux-mm/drivers/hwmon/Makefile	2008-11-06 09:18:16.000000000 +0100
@@ -48,7 +48,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
+obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
diff -ur clean-real-mm/drivers/hwmon/lis3lv02d.c linux-mm/drivers/hwmon/lis3lv02d.c
--- clean-real-mm/drivers/hwmon/lis3lv02d.c	2008-11-10 10:48:57.000000000 +0100
+++ linux-mm/drivers/hwmon/lis3lv02d.c	2008-11-06 12:27:27.000000000 +0100
@@ -3,6 +3,7 @@
  *
  *  Copyright (C) 2007-2008 Yan Burman
  *  Copyright (C) 2008 Eric Piel
+ *  Copyright (C) 2008 Pavel Machek
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -42,6 +43,7 @@
 #define DRIVER_NAME     "lis3lv02d"
 #define ACPI_MDPS_CLASS "accelerometer"
 
+
 /* joystick device poll interval in milliseconds */
 #define MDPS_POLL_INTERVAL 50
 /*
@@ -56,100 +58,18 @@
 /* Maximum value our axis may get for the input device (signed 12 bits) */
 #define MDPS_MAX_VAL 2048
 
-struct axis_conversion {
-	s8	x;
-	s8	y;
-	s8	z;
-};
 
-struct acpi_lis3lv02d {
-	struct acpi_device	*device;   /* The ACPI device */
-	struct input_dev	*idev;     /* input device */
-	struct task_struct	*kthread;  /* kthread for input */
-	struct mutex            lock;
-	struct platform_device	*pdev;  ...
[ message continues ]