2010-02-04 08:25:34

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts

Lis3 accelerometer chip family changes for power management,
click and threshold event handling.

Patch set adds interrupt handlers for click/tap events and threshold
based events. Actual configuration which events are enabled
is done via platform data. All the features cannot be used in parallel.
Interrupts are implemented only for 8 bit device, since I'm not familiar
with other devices and I don't have suitable testing environment.

Changes:

lis3: Add missing constants for 8bit device
This is quite clear. Some click feature related register definitions
were missing.

lis3: Separate configuration function for 8 bit device
Move platformdata based configurations for 8 bit device to
separate function to keep common part little bit more readable.

lis3: Introduce platform data for second ff / wu unit
8 bit device has two freefall / wakeup detection blocks. Add possibility
to configure also the second unit. Change hipass filter configuration
to platform data. Change is compatible with existing platform data.

lis3: Power control for the chip
This kind of feature has been in the driver earlier. It was removed
because saving was so small in laptop environment. However, in smaller
devices, even a small saving need to be implemented. When driver detects
that no-one is really interested about the acceleration, chip is powered down.
Input device, freefall device and sysfs are controlling this. By default,
chip is powered on to keep functionality similar to current implementation.

lis3: Add skeletons for interrupt handlers
Interrupt handlers are added in two patches to keep changes cleaner.
This first patch adds two dummy threaded interrupt handlers for 8 bit device.

lis3: Interrupt handlers for 8bit wakeup and click events
This patch adds content to dummy handlers. Depending on the chip configuration,
either click or ff/wu handling is called. For click event, BTN input event is
sent separately for each axes. For threshold event, coordinates are updated
immediatelly to input device. This allows input device to be used either in
polled mode and / or interrupt driven mode. Polling can stopped from userspace
by via input device sysfs.

Patch set applies to 2.6.33-RC6 tree.
Tested with 2.6.32 environment for omap3

Samu Onkalo (6):
lis3: Add missing constants for 8bit device
lis3: Separate configuration function for 8 bit device
lis3: Introduce platform data for second ff / wu unit
lis3: Power control for the chip
lis3: Add skeletons for interrupt handlers
lis3: Interrupt handlers for 8bit wakeup and click events

drivers/hwmon/lis3lv02d.c | 308 +++++++++++++++++++++++++++++++++--------
drivers/hwmon/lis3lv02d.h | 12 ++
drivers/hwmon/lis3lv02d_i2c.c | 8 +-
drivers/hwmon/lis3lv02d_spi.c | 12 +-
include/linux/lis3lv02d.h | 12 ++
5 files changed, 287 insertions(+), 65 deletions(-)


2010-02-04 08:24:51

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 3/6] lis3: Introduce platform data for second ff / wu unit

8 bit device has two wakeup / free fall units. It was not possible
to configure the second unit. This patch introduces configuration
entry to the platform data and also corresponding changes to the
8 bit setup function.

High pass filters were enabled by default. Patch introduces configuration
option for high pass filter cut off frequency and also possibility to
disable or enable the filter via platform data.
Since the control is a new one and default state was filter enabled,
new option is used to disable the filter. This way old platform data
is still compatible with the change.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 15 +++++++++++++--
include/linux/lis3lv02d.h | 9 +++++++++
2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 99fd45d..50f3123 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -524,6 +524,8 @@ EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
static void lis3lv02d_8b_configure(struct lis3lv02d *dev,
struct lis3lv02d_platform_data *p)
{
+ int ctrl2 = p->hipass_ctrl;
+
if (p->click_flags) {
dev->write(dev, CLICK_CFG, p->click_flags);
dev->write(dev, CLICK_TIMELIMIT, p->click_time_limit);
@@ -540,9 +542,18 @@ static void lis3lv02d_8b_configure(struct lis3lv02d *dev,
dev->write(dev, FF_WU_THS_1, p->wakeup_thresh & 0x7f);
/* default to 2.5ms for now */
dev->write(dev, FF_WU_DURATION_1, 1);
- /* enable high pass filter for both free-fall units */
- dev->write(dev, CTRL_REG2, HP_FF_WU1 | HP_FF_WU2);
+ ctrl2 ^= HP_FF_WU1; /* Xor to keep compatible with old pdata*/
+ }
+
+ if (p->wakeup_flags2) {
+ dev->write(dev, FF_WU_CFG_2, p->wakeup_flags2);
+ dev->write(dev, FF_WU_THS_2, p->wakeup_thresh2 & 0x7f);
+ /* default to 2.5ms for now */
+ dev->write(dev, FF_WU_DURATION_2, 1);
+ ctrl2 ^= HP_FF_WU2; /* Xor to keep compatible with old pdata*/
}
+ /* Configure hipass filters */
+ dev->write(dev, CTRL_REG2, ctrl2);
}

/*
diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
index f1ca0dc..d625199 100644
--- a/include/linux/lis3lv02d.h
+++ b/include/linux/lis3lv02d.h
@@ -43,6 +43,15 @@ struct lis3lv02d_platform_data {
#define LIS3_WAKEUP_Z_HI (1 << 5)
unsigned char wakeup_flags;
unsigned char wakeup_thresh;
+ unsigned char wakeup_flags2;
+ unsigned char wakeup_thresh2;
+#define LIS3_HIPASS_CUTFF_8HZ 0
+#define LIS3_HIPASS_CUTFF_4HZ 1
+#define LIS3_HIPASS_CUTFF_2HZ 2
+#define LIS3_HIPASS_CUTFF_1HZ 3
+#define LIS3_HIPASS1_DISABLE (1 << 2)
+#define LIS3_HIPASS2_DISABLE (1 << 3)
+ unsigned char hipass_ctrl;
#define LIS3_NO_MAP 0
#define LIS3_DEV_X 1
#define LIS3_DEV_Y 2
--
1.6.0.4

2010-02-04 08:25:01

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 6/6] lis3: Interrupt handlers for 8bit wakeup and click events

Content for the 8bit device threaded interrupt handlers.
Depending on the interrupt line and chip configuration,
either click or wakeup / freefall handler is called.
In case of click, BTN_ event is sent via input device.
In case of wakeup or freefall, input device ABS_ events
are updated immediatelly.

It is still possible to configure interrupt line 1 for fast freefall
detection and use the second line either for click or threshold
based interrupts. Or both lines can be used for click / threshold
interrupts.

Polled input device can be set to stopped state and still
get coordinate updates via input device using interrupt
based method. Polled mode and interrupt mode can also be used parallel.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 90 ++++++++++++++++++++++++++++++++++++++------
include/linux/lis3lv02d.h | 2 +
2 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 7de42f6..71ea645 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -121,11 +121,9 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
int position[3];
int i;

- mutex_lock(&lis3->mutex);
position[0] = lis3->read_data(lis3, OUTX);
position[1] = lis3->read_data(lis3, OUTY);
position[2] = lis3->read_data(lis3, OUTZ);
- mutex_unlock(&lis3->mutex);

for (i = 0; i < 3; i++)
position[i] = (position[i] * lis3->scale) / LIS3_ACCURACY;
@@ -274,6 +272,19 @@ static void lis3lv02d_joystick_close(struct input_polled_dev *pidev)
lis3lv02d_remove_users(&lis3_dev);
}

+static void lis3lv02d_joystick_poll(struct input_polled_dev *pidev)
+{
+ int x, y, z;
+
+ mutex_lock(&lis3_dev.mutex);
+ lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
+ input_report_abs(pidev->input, ABS_X, x);
+ input_report_abs(pidev->input, ABS_Y, y);
+ input_report_abs(pidev->input, ABS_Z, z);
+ input_sync(pidev->input);
+ mutex_unlock(&lis3_dev.mutex);
+}
+
static irqreturn_t lis302dl_interrupt(int irq, void *dummy)
{
if (!test_bit(0, &lis3_dev.misc_opened))
@@ -295,13 +306,71 @@ out:
return IRQ_HANDLED;
}

+static void lis302dl_interrupt_handle_click(struct lis3lv02d *lis3)
+{
+ struct input_dev *dev = lis3->idev->input;
+ u8 click_src;
+
+ mutex_lock(&lis3->mutex);
+ lis3->read(lis3, CLICK_SRC, &click_src);
+
+ if (click_src & CLICK_SINGLE_X) {
+ input_report_key(dev, BTN_X, 1);
+ input_report_key(dev, BTN_X, 0);
+ }
+
+ if (click_src & CLICK_SINGLE_Y) {
+ input_report_key(dev, BTN_Y, 1);
+ input_report_key(dev, BTN_Y, 0);
+ }
+
+ if (click_src & CLICK_SINGLE_Z) {
+ input_report_key(dev, BTN_Z, 1);
+ input_report_key(dev, BTN_Z, 0);
+ }
+ input_sync(dev);
+ mutex_unlock(&lis3->mutex);
+}
+
+static void lis302dl_interrupt_handle_ff_wu(struct lis3lv02d *lis3)
+{
+ u8 wu1_src;
+ u8 wu2_src;
+
+ lis3->read(lis3, FF_WU_SRC_1, &wu1_src);
+ lis3->read(lis3, FF_WU_SRC_2, &wu2_src);
+
+ wu1_src = wu1_src & FF_WU_SRC_IA ? wu1_src : 0;
+ wu2_src = wu2_src & FF_WU_SRC_IA ? wu2_src : 0;
+
+ /* joystick poll is internally protected by the lis3->mutex. */
+ if (wu1_src || wu2_src)
+ lis3lv02d_joystick_poll(lis3_dev.idev);
+}
+
static irqreturn_t lis302dl_interrupt_thread1_8b(int irq, void *data)
{
+
+ struct lis3lv02d *lis3 = data;
+
+ if ((lis3->pdata->irq_cfg & LIS3_IRQ1_MASK) == LIS3_IRQ1_CLICK)
+ lis302dl_interrupt_handle_click(lis3);
+ else
+ lis302dl_interrupt_handle_ff_wu(lis3);
+
return IRQ_HANDLED;
}

static irqreturn_t lis302dl_interrupt_thread2_8b(int irq, void *data)
{
+
+ struct lis3lv02d *lis3 = data;
+
+ if ((lis3->pdata->irq_cfg & LIS3_IRQ2_MASK) == LIS3_IRQ2_CLICK)
+ lis302dl_interrupt_handle_click(lis3);
+ else
+ lis302dl_interrupt_handle_ff_wu(lis3);
+
return IRQ_HANDLED;
}

@@ -401,17 +470,6 @@ static struct miscdevice lis3lv02d_misc_device = {
.fops = &lis3lv02d_misc_fops,
};

-static void lis3lv02d_joystick_poll(struct input_polled_dev *pidev)
-{
- int x, y, z;
-
- lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
- input_report_abs(pidev->input, ABS_X, x);
- input_report_abs(pidev->input, ABS_Y, y);
- input_report_abs(pidev->input, ABS_Z, z);
- input_sync(pidev->input);
-}
-
int lis3lv02d_joystick_enable(void)
{
struct input_dev *input_dev;
@@ -445,6 +503,10 @@ int lis3lv02d_joystick_enable(void)
input_set_abs_params(input_dev, ABS_Y, -max_val, max_val, fuzz, flat);
input_set_abs_params(input_dev, ABS_Z, -max_val, max_val, fuzz, flat);

+ input_set_capability(input_dev, EV_KEY, BTN_X);
+ input_set_capability(input_dev, EV_KEY, BTN_Y);
+ input_set_capability(input_dev, EV_KEY, BTN_Z);
+
err = input_register_polled_device(lis3_dev.idev);
if (err) {
input_free_polled_device(lis3_dev.idev);
@@ -493,7 +555,9 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
if (lis3_dev.active == 0)
return -ENODATA;

+ mutex_lock(&lis3_dev.mutex);
lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
+ mutex_unlock(&lis3_dev.mutex);
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}

diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
index fd289b1..0e8a346 100644
--- a/include/linux/lis3lv02d.h
+++ b/include/linux/lis3lv02d.h
@@ -25,12 +25,14 @@ struct lis3lv02d_platform_data {
#define LIS3_IRQ1_FF_WU_12 (3 << 0)
#define LIS3_IRQ1_DATA_READY (4 << 0)
#define LIS3_IRQ1_CLICK (7 << 0)
+#define LIS3_IRQ1_MASK (7 << 0)
#define LIS3_IRQ2_DISABLE (0 << 3)
#define LIS3_IRQ2_FF_WU_1 (1 << 3)
#define LIS3_IRQ2_FF_WU_2 (2 << 3)
#define LIS3_IRQ2_FF_WU_12 (3 << 3)
#define LIS3_IRQ2_DATA_READY (4 << 3)
#define LIS3_IRQ2_CLICK (7 << 3)
+#define LIS3_IRQ2_MASK (7 << 3)
#define LIS3_IRQ_OPEN_DRAIN (1 << 6)
#define LIS3_IRQ_ACTIVE_LOW (1 << 7)
unsigned char irq_cfg;
--
1.6.0.4

2010-02-04 08:25:17

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 2/6] lis3: Separate configuration function for 8 bit device

Separate configuration function for 8 bit version of the chip.
This way generic part of the init function stays little bit
more readable.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 45 ++++++++++++++++++++++++++-------------------
1 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index b2f2277..99fd45d 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -521,6 +521,30 @@ int lis3lv02d_remove_fs(struct lis3lv02d *lis3)
}
EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);

+static void lis3lv02d_8b_configure(struct lis3lv02d *dev,
+ struct lis3lv02d_platform_data *p)
+{
+ if (p->click_flags) {
+ dev->write(dev, CLICK_CFG, p->click_flags);
+ dev->write(dev, CLICK_TIMELIMIT, p->click_time_limit);
+ dev->write(dev, CLICK_LATENCY, p->click_latency);
+ dev->write(dev, CLICK_WINDOW, p->click_window);
+ dev->write(dev, CLICK_THSZ, p->click_thresh_z & 0xf);
+ dev->write(dev, CLICK_THSY_X,
+ (p->click_thresh_x & 0xf) |
+ (p->click_thresh_y << 4));
+ }
+
+ if (p->wakeup_flags) {
+ dev->write(dev, FF_WU_CFG_1, p->wakeup_flags);
+ dev->write(dev, FF_WU_THS_1, p->wakeup_thresh & 0x7f);
+ /* default to 2.5ms for now */
+ dev->write(dev, FF_WU_DURATION_1, 1);
+ /* enable high pass filter for both free-fall units */
+ dev->write(dev, CTRL_REG2, HP_FF_WU1 | HP_FF_WU2);
+ }
+}
+
/*
* Initialise the accelerometer and the various subsystems.
* Should be rather independent of the bus system.
@@ -567,25 +591,8 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
if (dev->pdata) {
struct lis3lv02d_platform_data *p = dev->pdata;

- if (p->click_flags && (dev->whoami == WAI_8B)) {
- dev->write(dev, CLICK_CFG, p->click_flags);
- dev->write(dev, CLICK_TIMELIMIT, p->click_time_limit);
- dev->write(dev, CLICK_LATENCY, p->click_latency);
- dev->write(dev, CLICK_WINDOW, p->click_window);
- dev->write(dev, CLICK_THSZ, p->click_thresh_z & 0xf);
- dev->write(dev, CLICK_THSY_X,
- (p->click_thresh_x & 0xf) |
- (p->click_thresh_y << 4));
- }
-
- if (p->wakeup_flags && (dev->whoami == WAI_8B)) {
- dev->write(dev, FF_WU_CFG_1, p->wakeup_flags);
- dev->write(dev, FF_WU_THS_1, p->wakeup_thresh & 0x7f);
- /* default to 2.5ms for now */
- dev->write(dev, FF_WU_DURATION_1, 1);
- /* enable high pass filter for both free-fall units */
- dev->write(dev, CTRL_REG2, HP_FF_WU1 | HP_FF_WU2);
- }
+ if (dev->whoami == WAI_8B)
+ lis3lv02d_8b_configure(dev, p);

if (p->irq_cfg)
dev->write(dev, CTRL_REG3, p->irq_cfg);
--
1.6.0.4

2010-02-04 08:25:39

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 4/6] lis3: Power control for the chip

Chip operational state is controlled by number of the users.
When no-one is really using the chip, it is powered down.
Typical saving for 8bit device is 300 uA based on chip datasheet.
Not much, but in small devices everything counts.

New entry is added to sysfs: active
It tells when position entry in sysfs is in operational state.
Initial setting is chip active to keep functionality similar as
before. When "active" is set to 0, position entry return error code.

Regardless of the active sysfs entry state, chip is also powered on / off
according to /dev/freefall or /dev/input/eventx / /dev/input/jsx
use. If some of the device handles is open, chip is on operational state.

Chip is in powerdown:
- sysfs active == 0 and
- /dev/freefall is not open and
- /dev/input/... is not open

Chip is in active mode:
- active == 1 or
- /dev/freefall is open or
- /dev/input/.... is open

Initial state:
active == 1

When chip is powered on, it takes 30-125 ms depending on the chip type and
current sampling rate.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 75 +++++++++++++++++++++++++++++++++++++++--
drivers/hwmon/lis3lv02d.h | 2 +
drivers/hwmon/lis3lv02d_i2c.c | 8 +++-
drivers/hwmon/lis3lv02d_spi.c | 12 ++++---
4 files changed, 87 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 50f3123..5948a3a 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -248,6 +248,31 @@ void lis3lv02d_poweron(struct lis3lv02d *lis3)
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

+static void lis3lv02d_add_users(struct lis3lv02d *lis3)
+{
+ mutex_lock(&lis3->mutex);
+ if (!lis3->users++)
+ lis3lv02d_poweron(lis3);
+ mutex_unlock(&lis3->mutex);
+}
+
+static void lis3lv02d_remove_users(struct lis3lv02d *lis3)
+{
+ mutex_lock(&lis3->mutex);
+ if (!--lis3->users)
+ lis3lv02d_poweroff(lis3);
+ mutex_unlock(&lis3->mutex);
+}
+
+static void lis3lv02d_joystick_open(struct input_polled_dev *pidev)
+{
+ lis3lv02d_add_users(&lis3_dev);
+}
+
+static void lis3lv02d_joystick_close(struct input_polled_dev *pidev)
+{
+ lis3lv02d_remove_users(&lis3_dev);
+}

static irqreturn_t lis302dl_interrupt(int irq, void *dummy)
{
@@ -270,6 +295,7 @@ static int lis3lv02d_misc_open(struct inode *inode, struct file *file)
if (test_and_set_bit(0, &lis3_dev.misc_opened))
return -EBUSY; /* already open */

+ lis3lv02d_add_users(&lis3_dev);
atomic_set(&lis3_dev.count, 0);

/*
@@ -299,6 +325,7 @@ static int lis3lv02d_misc_release(struct inode *inode, struct file *file)
fasync_helper(-1, file, 0, &lis3_dev.async_queue);
free_irq(lis3_dev.irq, &lis3_dev);
clear_bit(0, &lis3_dev.misc_opened); /* release the device */
+ lis3lv02d_remove_users(&lis3_dev);
return 0;
}

@@ -404,8 +431,10 @@ int lis3lv02d_joystick_enable(void)
if (!lis3_dev.idev)
return -ENOMEM;

- lis3_dev.idev->poll = lis3lv02d_joystick_poll;
- lis3_dev.idev->poll_interval = MDPS_POLL_INTERVAL;
+ lis3_dev.idev->poll = lis3lv02d_joystick_poll;
+ lis3_dev.idev->open = lis3lv02d_joystick_open;
+ lis3_dev.idev->close = lis3lv02d_joystick_close;
+ lis3_dev.idev->poll_interval = MDPS_POLL_INTERVAL;
input_dev = lis3_dev.idev->input;

input_dev->name = "ST LIS3LV02DL Accelerometer";
@@ -462,6 +491,9 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
{
int x, y, z;

+ if (lis3_dev.active == 0)
+ return -ENODATA;
+
lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}
@@ -487,15 +519,51 @@ static ssize_t lis3lv02d_rate_set(struct device *dev,
return count;
}

+static ssize_t lis3lv02d_active_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", lis3_dev.active);
+}
+
+static ssize_t lis3lv02d_active_set(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ unsigned long active;
+
+ if (strict_strtoul(buf, 0, &active))
+ return -EINVAL;
+
+ if (active > 1)
+ return -EINVAL;
+
+ mutex_lock(&lis3_dev.mutex);
+ if (active == lis3_dev.active) {
+ mutex_unlock(&lis3_dev.mutex);
+ return count;
+ }
+ lis3_dev.active = active;
+ mutex_unlock(&lis3_dev.mutex);
+
+ if (active)
+ lis3lv02d_add_users(&lis3_dev);
+ else
+ lis3lv02d_remove_users(&lis3_dev);
+ return count;
+}
+
static DEVICE_ATTR(selftest, S_IRUSR, lis3lv02d_selftest_show, NULL);
static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
static DEVICE_ATTR(rate, S_IRUGO | S_IWUSR, lis3lv02d_rate_show,
lis3lv02d_rate_set);
+static DEVICE_ATTR(active, S_IRUGO | S_IWUSR, lis3lv02d_active_show,
+ lis3lv02d_active_set);

static struct attribute *lis3lv02d_attributes[] = {
&dev_attr_selftest.attr,
&dev_attr_position.attr,
&dev_attr_rate.attr,
+ &dev_attr_active.attr,
NULL
};

@@ -592,7 +660,8 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
mutex_init(&dev->mutex);

lis3lv02d_add_fs(dev);
- lis3lv02d_poweron(dev);
+ lis3lv02d_add_users(dev);
+ lis3_dev.active = 1; /* By default, chip is in operational state */

if (lis3lv02d_joystick_enable())
printk(KERN_ERR DRIVER_NAME ": joystick initialization failed\n");
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 692e244..72d8660 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -221,9 +221,11 @@ struct lis3lv02d {
int *odrs; /* Supported output data rates */
u8 odr_mask; /* ODR bit mask */
u8 whoami; /* indicates measurement precision */
+ u8 active;
s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;
int pwron_delay;
+ int users;
int scale; /*
* relationship between 1 LBS and mG
* (1/1000th of earth gravity)
diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c
index dc1f540..5ffcaba 100644
--- a/drivers/hwmon/lis3lv02d_i2c.c
+++ b/drivers/hwmon/lis3lv02d_i2c.c
@@ -121,8 +121,10 @@ static int lis3lv02d_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
{
struct lis3lv02d *lis3 = i2c_get_clientdata(client);

- if (!lis3->pdata->wakeup_flags)
+ mutex_lock(&lis3->mutex);
+ if (!lis3->pdata->wakeup_flags && lis3->users)
lis3lv02d_poweroff(lis3);
+ mutex_unlock(&lis3->mutex);
return 0;
}

@@ -130,8 +132,10 @@ static int lis3lv02d_i2c_resume(struct i2c_client *client)
{
struct lis3lv02d *lis3 = i2c_get_clientdata(client);

- if (!lis3->pdata->wakeup_flags)
+ mutex_lock(&lis3->mutex);
+ if (!lis3->pdata->wakeup_flags && lis3->users)
lis3lv02d_poweron(lis3);
+ mutex_unlock(&lis3->mutex);
return 0;
}

diff --git a/drivers/hwmon/lis3lv02d_spi.c b/drivers/hwmon/lis3lv02d_spi.c
index 82b1680..c48c523 100644
--- a/drivers/hwmon/lis3lv02d_spi.c
+++ b/drivers/hwmon/lis3lv02d_spi.c
@@ -92,9 +92,10 @@ static int lis3lv02d_spi_suspend(struct spi_device *spi, pm_message_t mesg)
{
struct lis3lv02d *lis3 = spi_get_drvdata(spi);

- if (!lis3->pdata->wakeup_flags)
- lis3lv02d_poweroff(&lis3_dev);
-
+ mutex_lock(&lis3->mutex);
+ if (!lis3->pdata->wakeup_flags && lis3->users)
+ lis3lv02d_poweroff(lis3);
+ mutex_unlock(&lis3->mutex);
return 0;
}

@@ -102,9 +103,10 @@ static int lis3lv02d_spi_resume(struct spi_device *spi)
{
struct lis3lv02d *lis3 = spi_get_drvdata(spi);

- if (!lis3->pdata->wakeup_flags)
+ mutex_lock(&lis3->mutex);
+ if (!lis3->pdata->wakeup_flags && lis3->users)
lis3lv02d_poweron(lis3);
-
+ mutex_unlock(&lis3->mutex);
return 0;
}

--
1.6.0.4

2010-02-04 08:26:04

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 1/6] lis3: Add missing constants for 8bit device

Definitions for click were missing.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.h | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index e6a01f4..692e244 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -196,6 +196,16 @@ enum lis3lv02d_dd_src {
DD_SRC_IA = 0x40,
};

+enum lis3lv02d_click_src_8b {
+ CLICK_SINGLE_X = 0x01,
+ CLICK_DOUBLE_X = 0x02,
+ CLICK_SINGLE_Y = 0x04,
+ CLICK_DOUBLE_Y = 0x08,
+ CLICK_SINGLE_Z = 0x10,
+ CLICK_DOUBLE_Z = 0x20,
+ CLICK_IA = 0x40,
+};
+
struct axis_conversion {
s8 x;
s8 y;
--
1.6.0.4

2010-02-04 08:26:29

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH 5/6] lis3: Add skeletons for interrupt handlers

Original lis3 driver didn't provide interrupt handler(s) for
click or threshold event handling. This patch adds threaded handlers
for one or two interrupt lines for 8 bit device.
Actual content for interrupt handling is provided in the
separate patch.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 87 +++++++++++++++++++++++++++++++++------------
include/linux/lis3lv02d.h | 1 +
2 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 5948a3a..7de42f6 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -276,6 +276,9 @@ static void lis3lv02d_joystick_close(struct input_polled_dev *pidev)

static irqreturn_t lis302dl_interrupt(int irq, void *dummy)
{
+ if (!test_bit(0, &lis3_dev.misc_opened))
+ goto out;
+
/*
* Be careful: on some HP laptops the bios force DD when on battery and
* the lid is closed. This leads to interrupts as soon as a little move
@@ -285,45 +288,36 @@ static irqreturn_t lis302dl_interrupt(int irq, void *dummy)

wake_up_interruptible(&lis3_dev.misc_wait);
kill_fasync(&lis3_dev.async_queue, SIGIO, POLL_IN);
+out:
+ if (lis3_dev.whoami == WAI_8B && lis3_dev.idev &&
+ lis3_dev.idev->input->users)
+ return IRQ_WAKE_THREAD;
return IRQ_HANDLED;
}

-static int lis3lv02d_misc_open(struct inode *inode, struct file *file)
+static irqreturn_t lis302dl_interrupt_thread1_8b(int irq, void *data)
{
- int ret;
+ return IRQ_HANDLED;
+}

+static irqreturn_t lis302dl_interrupt_thread2_8b(int irq, void *data)
+{
+ return IRQ_HANDLED;
+}
+
+static int lis3lv02d_misc_open(struct inode *inode, struct file *file)
+{
if (test_and_set_bit(0, &lis3_dev.misc_opened))
return -EBUSY; /* already open */

lis3lv02d_add_users(&lis3_dev);
atomic_set(&lis3_dev.count, 0);
-
- /*
- * The sensor can generate interrupts for free-fall and direction
- * detection (distinguishable with FF_WU_SRC and DD_SRC) but to keep
- * the things simple and _fast_ we activate it only for free-fall, so
- * no need to read register (very slow with ACPI). For the same reason,
- * we forbid shared interrupts.
- *
- * IRQF_TRIGGER_RISING seems pointless on HP laptops because the
- * io-apic is not configurable (and generates a warning) but I keep it
- * in case of support for other hardware.
- */
- ret = request_irq(lis3_dev.irq, lis302dl_interrupt, IRQF_TRIGGER_RISING,
- DRIVER_NAME, &lis3_dev);
-
- if (ret) {
- clear_bit(0, &lis3_dev.misc_opened);
- printk(KERN_ERR DRIVER_NAME ": IRQ%d allocation failed\n", lis3_dev.irq);
- return -EBUSY;
- }
return 0;
}

static int lis3lv02d_misc_release(struct inode *inode, struct file *file)
{
fasync_helper(-1, file, 0, &lis3_dev.async_queue);
- free_irq(lis3_dev.irq, &lis3_dev);
clear_bit(0, &lis3_dev.misc_opened); /* release the device */
lis3lv02d_remove_users(&lis3_dev);
return 0;
@@ -463,6 +457,11 @@ EXPORT_SYMBOL_GPL(lis3lv02d_joystick_enable);

void lis3lv02d_joystick_disable(void)
{
+ if (lis3_dev.irq)
+ free_irq(lis3_dev.irq, &lis3_dev);
+ if (lis3_dev.pdata && lis3_dev.pdata->irq2)
+ free_irq(lis3_dev.pdata->irq2, &lis3_dev);
+
if (!lis3_dev.idev)
return;

@@ -592,6 +591,7 @@ EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
static void lis3lv02d_8b_configure(struct lis3lv02d *dev,
struct lis3lv02d_platform_data *p)
{
+ int err;
int ctrl2 = p->hipass_ctrl;

if (p->click_flags) {
@@ -622,6 +622,18 @@ static void lis3lv02d_8b_configure(struct lis3lv02d *dev,
}
/* Configure hipass filters */
dev->write(dev, CTRL_REG2, ctrl2);
+
+ if (p->irq2) {
+ err = request_threaded_irq(p->irq2,
+ NULL,
+ lis302dl_interrupt_thread2_8b,
+ IRQF_TRIGGER_RISING |
+ IRQF_ONESHOT,
+ DRIVER_NAME, &lis3_dev);
+ if (err < 0)
+ printk(KERN_ERR DRIVER_NAME
+ "No second IRQ. Limited functionality\n");
+ }
}

/*
@@ -630,6 +642,9 @@ static void lis3lv02d_8b_configure(struct lis3lv02d *dev,
*/
int lis3lv02d_init_device(struct lis3lv02d *dev)
{
+ int err;
+ irq_handler_t thread_fn;
+
dev->whoami = lis3lv02d_read_8(dev, WHO_AM_I);

switch (dev->whoami) {
@@ -685,6 +700,32 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
goto out;
}

+ /*
+ * The sensor can generate interrupts for free-fall and direction
+ * detection (distinguishable with FF_WU_SRC and DD_SRC) but to keep
+ * the things simple and _fast_ we activate it only for free-fall, so
+ * no need to read register (very slow with ACPI). For the same reason,
+ * we forbid shared interrupts.
+ *
+ * IRQF_TRIGGER_RISING seems pointless on HP laptops because the
+ * io-apic is not configurable (and generates a warning) but I keep it
+ * in case of support for other hardware.
+ */
+ if (dev->whoami == WAI_8B)
+ thread_fn = lis302dl_interrupt_thread1_8b;
+ else
+ thread_fn = NULL;
+
+ err = request_threaded_irq(dev->irq, lis302dl_interrupt,
+ thread_fn,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ DRIVER_NAME, &lis3_dev);
+
+ if (err < 0) {
+ printk(KERN_ERR DRIVER_NAME "Cannot get IRQ\n");
+ goto out;
+ }
+
if (misc_register(&lis3lv02d_misc_device))
printk(KERN_ERR DRIVER_NAME ": misc_register failed\n");
out:
diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
index d625199..fd289b1 100644
--- a/include/linux/lis3lv02d.h
+++ b/include/linux/lis3lv02d.h
@@ -67,6 +67,7 @@ struct lis3lv02d_platform_data {
/* Limits for selftest are specified in chip data sheet */
s16 st_min_limits[3]; /* min pass limit x, y, z */
s16 st_max_limits[3]; /* max pass limit x, y, z */
+ int irq2;
};

#endif /* __LIS3LV02D_H_ */
--
1.6.0.4

2010-02-05 02:12:43

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts

On Thu, Feb 04, 2010 at 10:24:02AM +0200, Samu Onkalo wrote:
> Lis3 accelerometer chip family changes for power management,
> click and threshold event handling.
>
> Patch set adds interrupt handlers for click/tap events and threshold
> based events. Actual configuration which events are enabled
> is done via platform data. All the features cannot be used in parallel.
> Interrupts are implemented only for 8 bit device, since I'm not familiar
> with other devices and I don't have suitable testing environment.

I'll give these patches a test on my boards next week and get back to
you.

Thanks,
Daniel


> Changes:
>
> lis3: Add missing constants for 8bit device
> This is quite clear. Some click feature related register definitions
> were missing.
>
> lis3: Separate configuration function for 8 bit device
> Move platformdata based configurations for 8 bit device to
> separate function to keep common part little bit more readable.
>
> lis3: Introduce platform data for second ff / wu unit
> 8 bit device has two freefall / wakeup detection blocks. Add possibility
> to configure also the second unit. Change hipass filter configuration
> to platform data. Change is compatible with existing platform data.
>
> lis3: Power control for the chip
> This kind of feature has been in the driver earlier. It was removed
> because saving was so small in laptop environment. However, in smaller
> devices, even a small saving need to be implemented. When driver detects
> that no-one is really interested about the acceleration, chip is powered down.
> Input device, freefall device and sysfs are controlling this. By default,
> chip is powered on to keep functionality similar to current implementation.
>
> lis3: Add skeletons for interrupt handlers
> Interrupt handlers are added in two patches to keep changes cleaner.
> This first patch adds two dummy threaded interrupt handlers for 8 bit device.
>
> lis3: Interrupt handlers for 8bit wakeup and click events
> This patch adds content to dummy handlers. Depending on the chip configuration,
> either click or ff/wu handling is called. For click event, BTN input event is
> sent separately for each axes. For threshold event, coordinates are updated
> immediatelly to input device. This allows input device to be used either in
> polled mode and / or interrupt driven mode. Polling can stopped from userspace
> by via input device sysfs.
>
> Patch set applies to 2.6.33-RC6 tree.
> Tested with 2.6.32 environment for omap3
>
> Samu Onkalo (6):
> lis3: Add missing constants for 8bit device
> lis3: Separate configuration function for 8 bit device
> lis3: Introduce platform data for second ff / wu unit
> lis3: Power control for the chip
> lis3: Add skeletons for interrupt handlers
> lis3: Interrupt handlers for 8bit wakeup and click events
>
> drivers/hwmon/lis3lv02d.c | 308 +++++++++++++++++++++++++++++++++--------
> drivers/hwmon/lis3lv02d.h | 12 ++
> drivers/hwmon/lis3lv02d_i2c.c | 8 +-
> drivers/hwmon/lis3lv02d_spi.c | 12 +-
> include/linux/lis3lv02d.h | 12 ++
> 5 files changed, 287 insertions(+), 65 deletions(-)
>

2010-02-11 19:01:58

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts

On Thu, Feb 04, 2010 at 10:24:02AM +0200, Samu Onkalo wrote:
> Lis3 accelerometer chip family changes for power management,
> click and threshold event handling.
>
> Patch set adds interrupt handlers for click/tap events and threshold
> based events. Actual configuration which events are enabled
> is done via platform data. All the features cannot be used in parallel.
> Interrupts are implemented only for 8 bit device, since I'm not familiar
> with other devices and I don't have suitable testing environment.

I tested that with an PXA3xx based board featuring a 8bit sensor, and at
least I cannot see any regression :)

You can take this as an

Tested-by: Daniel Mack <[email protected]>


Thanks,
Daniel


> Changes:
>
> lis3: Add missing constants for 8bit device
> This is quite clear. Some click feature related register definitions
> were missing.
>
> lis3: Separate configuration function for 8 bit device
> Move platformdata based configurations for 8 bit device to
> separate function to keep common part little bit more readable.
>
> lis3: Introduce platform data for second ff / wu unit
> 8 bit device has two freefall / wakeup detection blocks. Add possibility
> to configure also the second unit. Change hipass filter configuration
> to platform data. Change is compatible with existing platform data.
>
> lis3: Power control for the chip
> This kind of feature has been in the driver earlier. It was removed
> because saving was so small in laptop environment. However, in smaller
> devices, even a small saving need to be implemented. When driver detects
> that no-one is really interested about the acceleration, chip is powered down.
> Input device, freefall device and sysfs are controlling this. By default,
> chip is powered on to keep functionality similar to current implementation.
>
> lis3: Add skeletons for interrupt handlers
> Interrupt handlers are added in two patches to keep changes cleaner.
> This first patch adds two dummy threaded interrupt handlers for 8 bit device.
>
> lis3: Interrupt handlers for 8bit wakeup and click events
> This patch adds content to dummy handlers. Depending on the chip configuration,
> either click or ff/wu handling is called. For click event, BTN input event is
> sent separately for each axes. For threshold event, coordinates are updated
> immediatelly to input device. This allows input device to be used either in
> polled mode and / or interrupt driven mode. Polling can stopped from userspace
> by via input device sysfs.
>
> Patch set applies to 2.6.33-RC6 tree.
> Tested with 2.6.32 environment for omap3
>
> Samu Onkalo (6):
> lis3: Add missing constants for 8bit device
> lis3: Separate configuration function for 8 bit device
> lis3: Introduce platform data for second ff / wu unit
> lis3: Power control for the chip
> lis3: Add skeletons for interrupt handlers
> lis3: Interrupt handlers for 8bit wakeup and click events
>
> drivers/hwmon/lis3lv02d.c | 308 +++++++++++++++++++++++++++++++++--------
> drivers/hwmon/lis3lv02d.h | 12 ++
> drivers/hwmon/lis3lv02d_i2c.c | 8 +-
> drivers/hwmon/lis3lv02d_spi.c | 12 +-
> include/linux/lis3lv02d.h | 12 ++
> 5 files changed, 287 insertions(+), 65 deletions(-)
>

2010-02-11 19:51:24

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts

Op 04-02-10 09:24, Samu Onkalo schreef:
> Lis3 accelerometer chip family changes for power management,
> click and threshold event handling.
>
> Patch set adds interrupt handlers for click/tap events and threshold
> based events. Actual configuration which events are enabled
> is done via platform data. All the features cannot be used in parallel.
> Interrupts are implemented only for 8 bit device, since I'm not familiar
> with other devices and I don't have suitable testing environment.
>
> Changes:
>
> lis3: Add missing constants for 8bit device
> This is quite clear. Some click feature related register definitions
> were missing.
>
> lis3: Separate configuration function for 8 bit device
> Move platformdata based configurations for 8 bit device to
> separate function to keep common part little bit more readable.
>
> lis3: Introduce platform data for second ff / wu unit
> 8 bit device has two freefall / wakeup detection blocks. Add possibility
> to configure also the second unit. Change hipass filter configuration
> to platform data. Change is compatible with existing platform data.
>
> lis3: Power control for the chip
> This kind of feature has been in the driver earlier. It was removed
> because saving was so small in laptop environment. However, in smaller
> devices, even a small saving need to be implemented. When driver detects
> that no-one is really interested about the acceleration, chip is powered down.
> Input device, freefall device and sysfs are controlling this. By default,
> chip is powered on to keep functionality similar to current implementation.
>
> lis3: Add skeletons for interrupt handlers
> Interrupt handlers are added in two patches to keep changes cleaner.
> This first patch adds two dummy threaded interrupt handlers for 8 bit device.
>
> lis3: Interrupt handlers for 8bit wakeup and click events
> This patch adds content to dummy handlers. Depending on the chip configuration,
> either click or ff/wu handling is called. For click event, BTN input event is
> sent separately for each axes. For threshold event, coordinates are updated
> immediatelly to input device. This allows input device to be used either in
> polled mode and / or interrupt driven mode. Polling can stopped from userspace
> by via input device sysfs.
>
> Patch set applies to 2.6.33-RC6 tree.
> Tested with 2.6.32 environment for omap3
Hello,
Sorry, I haven't had time to fully review the patches. Nevertheless,
I've tested them on my HP laptop with a 12bit device, and can confirm
it's all fine.

The only hiccup is in patch 6: it declares the joystick with 3 buttons,
although on this hardware it's impossible to have this feature. So, it
would be nice if input_set_capability() could be conditionned.

I also fully agree with the comments from others that it would be better
if it could use the special runtime power-management framework from
Rafael. BTW, what is exactly the usecase for the "active" sysfs
attribute? To me, it looks like it just prevents userspace from using
the position attribute, while the joystick interface is still
accessible, so it is not very effective!

So, for the first four patches, here is my
Acked-by: Éric Piel <[email protected]>

Cheers,
Eric

2010-02-12 06:32:01

by Samu Onkalo

[permalink] [raw]
Subject: RE: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts

>-----Original Message-----
>From: ext Éric Piel [mailto:[email protected]]
>Sent: 11 February, 2010 21:18
>To: Onkalo Samu.P (Nokia-D/Tampere)
>Cc: [email protected]; [email protected]; [email protected]; linux-
>[email protected]
>Subject: Re: [PATCH 0/6] lis3lv02d: Power management, click and
>threshold interrupts
>
>Op 04-02-10 09:24, Samu Onkalo schreef:
>> Lis3 accelerometer chip family changes for power management,
>> click and threshold event handling.
>>
>> Patch set adds interrupt handlers for click/tap events and threshold
>> based events. Actual configuration which events are enabled
>> is done via platform data. All the features cannot be used in
>parallel.
>> Interrupts are implemented only for 8 bit device, since I'm not
>familiar
>> with other devices and I don't have suitable testing environment.
>>
>> Changes:
>>
>> lis3: Add missing constants for 8bit device
>> This is quite clear. Some click feature related register definitions
>> were missing.
>>
>> lis3: Separate configuration function for 8 bit device
>> Move platformdata based configurations for 8 bit device to
>> separate function to keep common part little bit more readable.
>>
>> lis3: Introduce platform data for second ff / wu unit
>> 8 bit device has two freefall / wakeup detection blocks. Add
>possibility
>> to configure also the second unit. Change hipass filter configuration
>> to platform data. Change is compatible with existing platform data.
>>
>> lis3: Power control for the chip
>> This kind of feature has been in the driver earlier. It was removed
>> because saving was so small in laptop environment. However, in smaller
>> devices, even a small saving need to be implemented. When driver
>detects
>> that no-one is really interested about the acceleration, chip is
>powered down.
>> Input device, freefall device and sysfs are controlling this. By
>default,
>> chip is powered on to keep functionality similar to current
>implementation.
>>
>> lis3: Add skeletons for interrupt handlers
>> Interrupt handlers are added in two patches to keep changes cleaner.
>> This first patch adds two dummy threaded interrupt handlers for 8 bit
>device.
>>
>> lis3: Interrupt handlers for 8bit wakeup and click events
>> This patch adds content to dummy handlers. Depending on the chip
>configuration,
>> either click or ff/wu handling is called. For click event, BTN input
>event is
>> sent separately for each axes. For threshold event, coordinates are
>updated
>> immediatelly to input device. This allows input device to be used
>either in
>> polled mode and / or interrupt driven mode. Polling can stopped from
>userspace
>> by via input device sysfs.
>>
>> Patch set applies to 2.6.33-RC6 tree.
>> Tested with 2.6.32 environment for omap3
>Hello,
>Sorry, I haven't had time to fully review the patches. Nevertheless,
>I've tested them on my HP laptop with a 12bit device, and can confirm
>it's all fine.
>
>The only hiccup is in patch 6: it declares the joystick with 3 buttons,
>although on this hardware it's impossible to have this feature. So, it
>would be nice if input_set_capability() could be conditionned.

I'll add configuration option to platform data or do you prefer some other
way to enable this?

>
>I also fully agree with the comments from others that it would be better
>if it could use the special runtime power-management framework from
>Rafael.

It looks like the corrent way to do that. However, I don't have suitable environment
to do that right now. I need to study this more if I can somehow verify it.

BTW, what is exactly the usecase for the "active" sysfs
>attribute? To me, it looks like it just prevents userspace from using
>the position attribute, while the joystick interface is still
>accessible, so it is not very effective!

That is exactly what it does. Input and freefall interfaces control the chip state
separately. So, this is only for sysfs interface since driver doesn't know
if somebody is using sysfs interface.

So this is the logic:
- If application wants to use sysfs, it must set 'active' to 1 (which is the default to
maintain backward compatibility).
Otherwise chip state is controlled by the number of users in
input and freefall device handles.
When 'active' is 0, only input / freefall device handles controls
the chip state.

When active is 0 but chip is enabled via input / freefall entries,
position entry returns error code. This is done to prevent the case where
chip is suddenly powered down when input / freefall devices are closed.
So position entry is working only when it is separately enabled.

When there are no users at all, chip is powered down.

>
>So, for the first four patches, here is my
>Acked-by: Éric Piel <[email protected]>

Thanks,
Samu

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2010-02-12 09:57:18

by Samu Onkalo

[permalink] [raw]
Subject: RE: [lm-sensors] [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts


>>Hello,
>>Sorry, I haven't had time to fully review the patches. Nevertheless,
>>I've tested them on my HP laptop with a 12bit device, and can confirm
>>it's all fine.
>>
>>The only hiccup is in patch 6: it declares the joystick with 3 buttons,
>>although on this hardware it's impossible to have this feature. So, it
>>would be nice if input_set_capability() could be conditionned.
>
>I'll add configuration option to platform data or do you prefer some
>other
>way to enable this?
>

You obviously meant that enable those only for 8 bit device, since
12 bit doesn't have click detection. I can do that.

-Samu



????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2010-02-12 12:21:21

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts

Op 12-02-10 07:31, [email protected] schreef:
>> The only hiccup is in patch 6: it declares the joystick with 3 buttons,
>> although on this hardware it's impossible to have this feature. So, it
>> would be nice if input_set_capability() could be conditionned.
>
> I'll add configuration option to platform data or do you prefer some other
> way to enable this?

> You obviously meant that enable those only for 8 bit device, since
> 12 bit doesn't have click detection. I can do that.
>
Perfect!

>> I also fully agree with the comments from others that it would be better
>> if it could use the special runtime power-management framework from
>> Rafael.
>
> It looks like the corrent way to do that. However, I don't have suitable environment
> to do that right now. I need to study this more if I can somehow verify it.
That would be great :-)

> BTW, what is exactly the usecase for the "active" sysfs
>> attribute? To me, it looks like it just prevents userspace from using
>> the position attribute, while the joystick interface is still
>> accessible, so it is not very effective!
>
> That is exactly what it does. Input and freefall interfaces control the chip state
> separately. So, this is only for sysfs interface since driver doesn't know
> if somebody is using sysfs interface.
>
> So this is the logic:
> - If application wants to use sysfs, it must set 'active' to 1 (which is the default to
> maintain backward compatibility).
> Otherwise chip state is controlled by the number of users in
> input and freefall device handles.
> When 'active' is 0, only input / freefall device handles controls
> the chip state.
>
> When active is 0 but chip is enabled via input / freefall entries,
> position entry returns error code. This is done to prevent the case where
> chip is suddenly powered down when input / freefall devices are closed.
> So position entry is working only when it is separately enabled.
>
> When there are no users at all, chip is powered down.
>
I understand. The main problem is that powering up and down the chip
every time someone opens the sysfs position attribute is very expensive.

Having an explicit attribute to turn on/off the powersaving means that
the userspace must be aware of it. This is a burden, and especially if
you run your userspace application as a normal user, while the active
attribute obviously requires root level.

Previously, when I implemented the auto power down feature, I had put a
timer after 30s of usage to work around this. It's obviously not
optimal, but it has the advantage of hiding completely the powersaving
feature from userspace. Do you think it would be fine it implement it
this way?

See you,
Eric