login
Search
HP Mobile Data Protection System 4D ACPI driver. Similar to hdaps and applesmc in functionality. This driver provides 3 kinds of functionality: 1) Creates a misc device /dev/accel that acts similar to /dev/rtc and unblocks the process reading from it when the device detects free-fall interrupt 2) Functions as an input class device to provide similar functionality to hdaps, in order to be able to use the laptop as a joystick 3) Makes it possible to power the device off. Changes from previous post: Clean up all checkpatch.pl warnings Remove hdaps "compatibility" that made the drive disguise itself as hdaps Always provide 3D sensor readings instead of selecting 2D or 3D operation Make the driver load automatically for supported hardware Identify laptop models based on DMI and adjust axis information accordingly The previous version of the driver seems to be used by quite a few people it seems based on the feedback I'm getting by mail, so perhaps it can be considered for inclusion in the kernel. Signed-off-by: Yan Burman <burman.yan@gmail.com> Signed-off-by: Eric Piel <eric.piel@tremplin-utc.net> diff -Nrubp linux-2.6.25.4_orig/Documentation/hwmon/mdps linux-2.6.25.4/Documentation/hwmon/mdps --- linux-2.6.25.4_orig/Documentation/hwmon/mdps 1970-01-01 02:00:00.000000000 +0200 +++ linux-2.6.25.4/Documentation/hwmon/mdps 2008-05-28 11:15:33.000000000 +0300 @@ -0,0 +1,96 @@ +Kernel driver mdps +================== + +Supported chips: + + * STMicroelectronics LIS3LV02DL and LIS3LV02DQ + +Author: + Yan Burman <burman.yan@gmail.com> + Eric Piel <eric.piel@tremplin-utc.net> + + +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". HP nc6420, nc2510, nw9440 and nx9420 are supported right now, but +it should work on other models as well. The accelerometer data is readable ...
Hello, We haven't received any review for this driver since last week's post by Yan :-( That's why I'm adding explicitly as receivers of this email all the people who have reviewed the first two previous takes. Apologies for the possible multiple receptions. It would be _really really kind_ if some of you could take some time and comment (or simply ACK ;-) ) this code. It adds support for hardware which can be found in mostly every HP Compaq laptop less than three years old. It's quite worthy. Mark, as maintainer of the hwmon tree, would accept this driver? Let us know if there are things we need to change. Thanks, Eric --
Hi Eric, If you didn't receive any answer, my guess is that it's because you're trying to add this driver to the wrong tree. Why would this driver go in drivers/hwmon and be reviewed by hwmon folks, when it doesn't expose a single hwmon attribute to user-space? I guess you thought putting the driver there would be fine because that's where the hdaps and ams drivers are, but IMHO the hdaps and ams drivers should never have been placed in drivers/hwmon. These devices really aren't hardware monitoring chips is the traditional sense of the term. They are different chips, serving different purposes and deserving a totally different interface. They'd better live in a different subsystem and be maintainer by a different crew with interest in these devices and hardware to test the drivers. This discussion thread might sched some light on a possible approach: http://marc.info/?l=lm-sensors&m=121127824726050&w=2 So my advice is that you don't wait for a review from the hwmon people, because apparently we don't have much interest in this type of device, so you'll be waiting forever. You're much better adding the mdps driver to drivers/misc through Andrew Morton, at least until someone takes care of creating a subsystem for this type of devices and move all existing drivers there. Would someone object to moving the hdaps and ams (and possibly applesmc) drivers to drivers/misc? -- Jean Delvare --
Hi Jean, I understand your argumentation. Indeed, this driver has nothing specially related to hwmon, excepted that all the other accelerometers so far have been put in hwmon. I've got nothing against redoing the Yeah, applesmc seems a bit less obvious because it also exposes really some hardware sensors. Any idea how to handle this? Just leave it in hwmon for now? So, the rational about moving all these drivers to drivers/misc/ would be that it shows they have no relation with hwmon, and they are maintained separately. This would be a temporary place until the "accelerometer" subsystem exists by it own, right? If everyone is happy with this idea, I can submit a separate patch to move them. See you, Eric --
Hi Eric, There are already other drivers in this case: drivers/thermal/thermal_sys.c, drivers/misc/eeepc-laptop.c, drivers/misc/thinkpad_acpi.c and drivers/input/touchscreen/ads7846.c. All these drivers have multiple functions, one of them is hardware monitoring but that's not the main one, so they didn't go under drivers/hwmon. The applesmc driver would simply be one more driver in this case. Note that drivers/mfd is a possible alternative to drivers/misc for I'm fine with moving the drivers right now, yes. -- Jean Delvare --
Just create drivers/accel ... accelerometers are quite common these days. Going to drivers/misc is okay, too, I guess -- moves are easy with git ;-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
Yes, why not do one little step to the correct direction now and create drivers/accel, it will likely be the chosen name anyway! That will not yet be a "real" subsystem, just a place to put all the related drivers already present together. Anyone against this idea? Eric --
Well, I would expect all these accelerometer drivers to have a common, standardized interface, so this should most probably become a real I am in favor of it. -- Jean Delvare --
The upshot of the discussion Jean referenced was that the there were a number of other devices / sensors that will form part of the same system. For an initial bash I'm throwing in a couple of accelerometers, ADC's and probably one or two other sensors as well. Haven't come up with a good name as yet, but the point is that accelerometers would be just part of the relevant subsystem. Based on one suggestion (and current layout in my source tree), drivers/industrialio/accelerometer drivers/industrialio/adc and crucially for all those weird and wonderful sensors, drivers/industrialio/misc The reason for making it an io system is that many chips combine ADCs and DACs. Also suggestions for alternative names would be most welcome. Still, it may take some time to iron out the various kinks in this system and get it fully tested, so may be worth putting stuff elsewhere for now with the intention to move it as and when the more unified stuff has been written. Also, the LI3L02DQ (very similar to the chip you are using) is one of my test chips, so combining the two drivers should be straight forward. Also the driver I have is SPI only, so this may give us an easy way to test a combined driver. Jonathan --
Great to hear that you have started the new subsystem! It's also nice that you are still hesitant about the name because "industrialio" sounds for me too... let's say... industrial ;-) For accelerometers found in laptops and mobile phones, it will sound strange (although a name is just a name). Maybe "physicsio" (physics as "the Yes, so I'll keep my plan of first submitting the driver with the current interface, and latter updating the interface to the new subsystem when you have stabilised its interface. For now I'll go back to the drawing board (with Yan) in order to address the comments received, all very insightful. I'll also go around the other versions of the driver to pick up the nice parts. Looking at the openmoko driver, I finally understood how to avoid polling. It will likely be worthy :-) In addition, I'll try to make the main functions as independent as possible of the platform, hoping that we can then easily merge the SPI and the ACPI versions together. See you, Eric --
From: Pau Oliva Fora <pau@eslack.org> Add support for HTC Shift UMPC G-Sensor LIS3LV02DL accelerometer. Signed-off-by: Pau Oliva Fora <pau@eslack.org> --- Here's an i2c version of the driver for the LIS3LV02DL accelerometer, please try to combine it too, so we'll end up having ACPI, SPI and I2C. Patch against linux-2.6.26-rc5, should apply clean on other versions too. diff -uprN linux-2.6.26-rc5/drivers/input/joystick/htcgsensor.c linux/drivers/input/joystick/htcgsensor.c --- linux-2.6.26-rc5/drivers/input/joystick/htcgsensor.c 1970-01-01 01:00:00.000000000 +0100 +++ linux/drivers/input/joystick/htcgsensor.c 2008-06-05 23:56:58.000000000 +0200 @@ -0,0 +1,303 @@ +/* + * HTC Shift G-Sensor Joystick driver + * The G-Sensor chip is a STMicroelectronics LIS3LV02DL connected via i2c + * + * Copyright (C) 2008 Pau Oliva Fora <pof@eslack.org> + * + * Based on: + * mdps.c - HP Mobile Data Protection System 3D ACPI driver + * ams.c - Apple Motion Sensor driver + * + * 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 published + * by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/errno.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/input-polldev.h> +#include <linux/dmi.h> + +#define GSENSOR_WHO_AM_I 0x0F /*r 00111010 */ +#define GSENSOR_CTRL_REG1 0x20 /*rw 00000111 */ +#define GSENSOR_CTRL_REG2 0x21 /*rw 00000000 */ +#define GSENSOR_CTRL_REG3 0x22 /*rw 00001000 */ +#define GSENSOR_OUTX_L 0x28 /*r */ +#define GSENSOR_OUTX_H 0x29 /*r */ +#define GSENSOR_OUTY_L 0x2A /*r */ +#define GSENSOR_OUTY_H 0x2B /*r */ +#define GSENSOR_OUTZ_L 0x2C /*r */ +#define GSENSOR_OUTZ_H 0x2D /*r ...
Hi, The input piece looks good, however it seens that the input is the only functionality that this driver exports so I don't see any reason to make input device registration optional. Or are you planning on adding more functions to it? Thanks. -- Dmitry --
I wasn't planning on adding more functions, I posted it so that Éric can have a look at the i2c part and integrate it in his driver. Pau Oliva --
Whaoo, you went fast! As there is a dmi_match, that shouldn't be too hard to incorporate it nicely :-) BTW, do you really need the invert_x and invert_y and swap_xy parameters? Don't all the HTC shift laptops have the sensors soldered the same way? We've worked hard to remove those parameters in mdps ;-) See you, Eric
Yes, you can remove the module params, and use the same approach you have based in DMI table. Cheers, Pau Oliva --
Not just yet. (doesn't all work as of yet and I need to get at least a couple of devices working with it be sure I haven't written something too device specific) I'll post to the relevant mailing lists when it reaches a more reasonable state and the 'arguments' can begin ;) Jonathan --
But this part is what I don't understand. powering off does not seem to belong there. Why can't you just use acpi to power down? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
Not sure what you mean. Are you saying acpi has already a user interface to power up/down a device? If so, I'm not aware of it, is it in /sys ? Or did you mean that there is already a kernel api to turn on/off a device via acpi, no need to access the device directly? That said, this special /sys interface to power the device off is probably going to be used rarely. It was put there because it is costly to wake the device up each time we want to read it. What I could do is put a timer to turn the device off after 10s of non-usage, and power it on automatically again as soon as the user wants to read some info. That would permit to remove this interface. Eric --
I think he means power down the g-sensors and related filters and A/D circuitry. The out-of-tree version of the HDAPS driver can do it as well. For what is it worth, I think drivers/accel is a good idea, and I think the other accel drivers should be moved there. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh --
Agreed. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
Hi, I would recommend converting the input piece to input_polled_device instead of managing the polling thread by yourselves. -- Dmitry --
Yes, you are right, I had forgotten about this one! That said, for now I'm working on removing the polling and relying only the interrupts. So if it works that shouldn't be needed :-) See you, Eric --
Oh, great, this is even better. -- Dmitry --
I think going to interrupt-only approach will cause a problem in the HTC Shift: In CTRL_REG2 (21h) if you set to 1 the Interrupt ENable (IEN) field the device shuts down. I guess HTC took the "polling" approach in the windows driver, and they enable the interrupt when the sensor reports a "free fall" event, to have the disk parked and protect the hardware from being damaged. Maybe you can keep the input_polled_device together with interrupts and select one or the other depending on DMI information (same as for swaping the axis, etc.) Cheers, Pau Oliva --
Patch looks reasonable, although my head's spinning a bit over the Beware that the version becomes essentially meaningless once the code is merged into mainline. It cannot be used to identify the exact source code which a user is running - this is done via the main kernel version instead. All these comments you have sort-of look like kerneldoc only they aren't. I suggest that either they be converted to kerneldoc or the /** and This wakes up every 30 milliseconds which will make powertop users unhappy. These games with mdps.available are awkward and the above looks racy. It will work out better if it is converted to test_and_set_bit() and Please remove the nasty-looking cast. dmi_system_id.driver_data is already void*, and the cast will suppress compiler warnings which might It is preferable to do this at compile-time, which can be done with This is an unneeded initialisation to shut the compiler up. Problems are a) it takes a lot of staring for the reader to work out why this initialisation is here and b) it generates extra code. --
These chips are connected to either I2C or SPI - This is the 4th driver for (apparently) these same chips: http://docwiki.gumstix.org/Lis3lv02dq_spi.c http://svn.openmoko.org/branches/src/target/kernel/2.6.24.x/patches/lis302dl.patch Perhaps it would make more sense implement support for SPI bus on the laptop and use the SPI interface directly instead or --
Hi! Thanks a lot for these links (which I was not aware of). They indeed seem to handle the same hardware (all via spi). To be even more complete, here is a link we received off-list for accessing the same hardware via I²C: http://pof.eslack.org/HTC/shift/i2c-gsensor.tar.gz Getting rid of ACPI could be nice, as it tends to be rather slow. However, so far we've stick to it because it ensures that for the ~15 different models of HP laptop, we can access the hardware exactly the same way. I have the gut feeling that if HP spent some time to add an interface in ACPI, there was some kind of reason. However, I know nothing about SPI. Maybe you'll tell that if this chip is on the SPI bus, it will always be accessed the same way, located at the same address... or whatever that can ensure us that from the moment we know this device is in the laptop (and that's easy via HPQ0004) we cannot mess up. In that case, going to SPI would be definitely worthy. Eric --
