[RFC][PATCH 0/2] LIS3LV02D I2C driver

[RFC][PATCH 0/2] LIS3LV02D I2C driver

[Samu Onkalo]

These patches add I2C bus support for LIS3LV02D accelerometer driver
and possibility to use platform configurability for
remapping of device axes.

Changes are tested on linux-omap environment.
Applies to 2.6.32-rc1.

Samu Onkalo (2):
  LIS3LV02D: axis remap, irq and resource setup / release added to
    platform data
  LIS3LV02D: I2C support

 drivers/hwmon/Kconfig         |   17 ++++
 drivers/hwmon/Makefile        |    1 +
 drivers/hwmon/lis3lv02d.h     |    1 +
 drivers/hwmon/lis3lv02d_i2c.c |  183 +++++++++++++++++++++++++++++++++++++++++
 include/linux/lis3lv02d.h     |   13 +++
 5 files changed, 215 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/lis3lv02d_i2c.c

[PATCH 1/2] LIS3LV02D: axis remap, irq and resource setup / release added to platform data

[Samu Onkalo]

Adds axis remapping and second IRQ request to platform data.
Second IRQ request doesn't exists in all lis3xxx variants.

Setup and release functions added to support
platform specific operations.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/hwmon/lis3lv02d.h |    1 +
 include/linux/lis3lv02d.h |   13 +++++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 3e1ff46..b89f465 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -207,6 +207,7 @@ struct lis3lv02d {
 	struct axis_conversion	ac;        /* hw -> logical axis */
 
 	u32			irq;       /* IRQ number */
+	u32			irq2;      /* IRQ number (LIS302DL) */
 	struct fasync_struct	*async_queue; /* queue for the misc device */
 	wait_queue_head_t	misc_wait; /* Wait queue for the misc device */
 	unsigned long		misc_opened; /* bit0: whether the device is open */
diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
index dbe0702..7ac1e56 100644
--- a/include/linux/lis3lv02d.h
+++ b/include/linux/lis3lv02d.h
@@ -43,6 +43,19 @@ struct lis3lv02d_platform_data {
 #define LIS3_WAKEUP_Z_HI	(1 << 5)
 	unsigned char wakeup_flags;
 	unsigned char wakeup_thresh;
+#define LIS3_NO_MAP		0
+#define LIS3_DEV_X		1
+#define LIS3_DEV_Y		2
+#define LIS3_DEV_Z		3
+#define LIS3_INV_DEV_X	       -1
+#define LIS3_INV_DEV_Y	       -2
+#define LIS3_INV_DEV_Z	       -3
+	s8 axis_x;
+	s8 axis_y;
+	s8 axis_z;
+	u32 irq2;
+	int (*setup_resources)(void);
+	int (*release_resources)(void);
 };
 
 #endif /* __LIS3LV02D_H_ */
-- 
1.5.6.3

[PATCH 2/2] LIS3LV02D: I2C support

[Samu Onkalo]

I2C bus support for lis3lv02d and variant accelerometer chips

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/hwmon/Kconfig         |   17 ++++
 drivers/hwmon/Makefile        |    1 +
 drivers/hwmon/lis3lv02d_i2c.c |  183 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/lis3lv02d_i2c.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 69222bc..3d8e1b7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -992,6 +992,23 @@ config SENSORS_LIS3_SPI
 	  will be called lis3lv02d and a specific module for the SPI transport
 	  is called lis3lv02d_spi.
 
+config SENSORS_LIS3_I2C
+	tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer (I2C)"
+	depends on I2C && INPUT
+	select INPUT_POLLDEV
+	default n
+	help
+	  This driver provides support for the LIS3LV02Dx accelerometer connected
+	  via I2C. The accelerometer data is readable via
+	  /sys/devices/platform/lis3lv02d.
+
+	  This driver also provides an absolute input class device, allowing
+	  the device 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 I2C transport
+	  is called lis3lv02d_I2C.
+
 config SENSORS_APPLESMC
 	tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
 	depends on INPUT && X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4a34375..699e372 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -55,6 +55,7 @@ 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_LIS3_I2C)	+= lis3lv02d.o lis3lv02d_i2c.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c
new file mode 100644
index 0000000..344b240
--- /dev/null
+++ b/drivers/hwmon/lis3lv02d_i2c.c
@@ -0,0 +1,183 @@
+/*
+ * drivers/hwmon/lis3lv02d_i2c.c
+ *
+ * Implements I2C interface for lis3lv02d (STMicroelectronics) accelerometer.
+ * Driver is based on corresponding SPI driver written by Daniel Mack
+ * (lis3lv02d_spi.c (C) 2009 Daniel Mack <daniel@caiaq.de> ).
+ *
+ * Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: Samu Onkalo <samu.p.onkalo@nokia.com>
+ *
+ * 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.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include "lis3lv02d.h"
+
+#define DRV_NAME 	"lis3lv02d_i2c"
+
+static inline s32 lis3_i2c_write(struct lis3lv02d *lis3, int reg, u8 value)
+{
+	struct i2c_client *c = lis3->bus_priv;
+	return i2c_smbus_write_byte_data(c, reg, value);
+}
+
+static inline s32 lis3_i2c_read(struct lis3lv02d *lis3, int reg, u8 *v)
+{
+	struct i2c_client *c = lis3->bus_priv;
+	*v = i2c_smbus_read_byte_data(c, reg);
+	return 0;
+}
+
+static int lis3_i2c_init(struct lis3lv02d *lis3)
+{
+	u8 reg;
+	int ret;
+
+	/* power up the device */
+	ret = lis3->read(lis3, CTRL_REG1, &reg);
+	if (ret < 0)
+		return ret;
+
+	reg |= CTRL1_PD0;
+	return lis3->write(lis3, CTRL_REG1, reg);
+}
+
+static struct axis_conversion lis3lv02d_axis_map = { 1, 2, 3 };
+
+static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client,
+					const struct i2c_device_id *id)
+{
+	int ret = 0;
+	struct lis3lv02d_platform_data *pdata = client->dev.platform_data;
+
+	if (pdata) {
+		if (pdata->axis_x)
+			lis3lv02d_axis_map.x = pdata->axis_x;
+
+		if (pdata->axis_y)
+			lis3lv02d_axis_map.y = pdata->axis_y;
+
+		if (pdata->axis_z)
+			lis3lv02d_axis_map.z = pdata->axis_z;
+
+		lis3_dev.irq2 = pdata->irq2;
+
+		if (pdata->setup_resources)
+			ret = pdata->setup_resources();
+
+		if (ret)
+			goto fail;
+	}
+
+	lis3_dev.pdata	  = pdata;
+	lis3_dev.bus_priv = client;
+	lis3_dev.init	  = lis3_i2c_init;
+	lis3_dev.read	  = lis3_i2c_read;
+	lis3_dev.write	  = lis3_i2c_write;
+	lis3_dev.irq	  = client->irq;
+	lis3_dev.ac	  = lis3lv02d_axis_map;
+
+	i2c_set_clientdata(client, &lis3_dev);
+	ret = lis3lv02d_init_device(&lis3_dev);
+fail:
+	return ret;
+}
+
+static int __devexit lis3lv02d_i2c_remove(struct i2c_client *client)
+{
+	struct lis3lv02d *lis3 = i2c_get_clientdata(client);
+	struct lis3lv02d_platform_data *pdata = client->dev.platform_data;
+
+	if (pdata && pdata->release_resources)
+		pdata->release_resources();
+
+	lis3lv02d_joystick_disable();
+	lis3lv02d_poweroff(lis3);
+
+	return lis3lv02d_remove_fs(&lis3_dev);
+}
+
+#ifdef CONFIG_PM
+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)
+		lis3lv02d_poweroff(lis3);
+	return 0;
+}
+
+static int lis3lv02d_i2c_resume(struct i2c_client *client)
+{
+	struct lis3lv02d *lis3 = i2c_get_clientdata(client);
+
+	if (!lis3->pdata->wakeup_flags)
+		lis3lv02d_poweron(lis3);
+	return 0;
+}
+
+static void lis3lv02d_i2c_shutdown(struct i2c_client *client)
+{
+	lis3lv02d_i2c_suspend(client, PMSG_SUSPEND);
+}
+#else
+#define lis3lv02d_i2c_suspend	NULL
+#define lis3lv02d_i2c_resume	NULL
+#define lis3lv02d_i2c_shutdown	NULL
+#endif
+
+static const struct i2c_device_id lis3lv02d_id[] = {
+	{"lis3lv02d", 0 },
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, lis3lv02d_id);
+
+static struct i2c_driver lis3lv02d_i2c_driver = {
+	.driver	 = {
+		.name   = DRV_NAME,
+		.owner  = THIS_MODULE,
+	},
+	.suspend = lis3lv02d_i2c_suspend,
+	.shutdown = lis3lv02d_i2c_shutdown,
+	.resume = lis3lv02d_i2c_resume,
+	.probe	= lis3lv02d_i2c_probe,
+	.remove	= __devexit_p(lis3lv02d_i2c_remove),
+	.id_table = lis3lv02d_id,
+};
+
+static int __init lis3lv02d_init(void)
+{
+	return i2c_add_driver(&lis3lv02d_i2c_driver);
+}
+
+static void __exit lis3lv02d_exit(void)
+{
+	i2c_del_driver(&lis3lv02d_i2c_driver);
+}
+
+MODULE_AUTHOR("Nokia Corporation");
+MODULE_DESCRIPTION("lis3lv02d I2C interface");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("i2c:" DRV_NAME);
+
+module_init(lis3lv02d_init);
+module_exit(lis3lv02d_exit);
-- 
1.5.6.3

Re: [PATCH 2/2] LIS3LV02D: I2C support

[Éric Piel]

Op 01-10-09 12:06, Samu Onkalo schreef:
> I2C bus support for lis3lv02d and variant accelerometer chips
> 
Looks very good in general. Just a few comments.

> Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> ---
>  drivers/hwmon/Kconfig         |   17 ++++
>  drivers/hwmon/Makefile        |    1 +
>  drivers/hwmon/lis3lv02d_i2c.c |  183 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 201 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hwmon/lis3lv02d_i2c.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 69222bc..3d8e1b7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -992,6 +992,23 @@ config SENSORS_LIS3_SPI
>  	  will be called lis3lv02d and a specific module for the SPI transport
>  	  is called lis3lv02d_spi.
>  

You put the config just after SPI, but SPI had a "!ACPI" which allowed
it to look no too weird compared to the option for ACPI/HP. In your
case, some people will see both LIS3_I2C and LIS3LV02D and it will start
to look really messy. Ok, not too much a big deal, but I think
eventually it would be better to have an generic option for lis3, and
sub-options for ACPI/HP, SPI, and I²C. So unless someone complains, I
propose you let it like this, and I'll send later on a patch which merge
the three options together.
> +config SENSORS_LIS3_I2C
> +	tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer (I2C)"
> +	depends on I2C && INPUT
> +	select INPUT_POLLDEV
> +	default n
> +	help
> +	  This driver provides support for the LIS3LV02Dx accelerometer connected
> +	  via I2C. The accelerometer data is readable via
> +	  /sys/devices/platform/lis3lv02d.
> +
> +	  This driver also provides an absolute input class device, allowing
> +	  the device 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 I2C transport
> +	  is called lis3lv02d_I2C.
You meant "lis3lv02d_i2c", right?


> +
>  config SENSORS_APPLESMC
>  	tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
>  	depends on INPUT && X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4a34375..699e372 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -55,6 +55,7 @@ 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_LIS3_I2C)	+= lis3lv02d.o lis3lv02d_i2c.o
>  obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
>  obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
>  obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
> diff --git a/drivers/hwmon/lis3lv02d_i2c.c b/drivers/hwmon/lis3lv02d_i2c.c
> new file mode 100644
> index 0000000..344b240
> --- /dev/null
> +++ b/drivers/hwmon/lis3lv02d_i2c.c
> @@ -0,0 +1,183 @@
> +/*
> + * drivers/hwmon/lis3lv02d_i2c.c
> + *
> + * Implements I2C interface for lis3lv02d (STMicroelectronics) accelerometer.
> + * Driver is based on corresponding SPI driver written by Daniel Mack
> + * (lis3lv02d_spi.c (C) 2009 Daniel Mack <daniel@caiaq.de> ).
> + *
> + * Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
> + *
> + * Contact: Samu Onkalo <samu.p.onkalo@nokia.com>
> + *
> + * 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.
> + *
> + * 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., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include "lis3lv02d.h"
> +
> +#define DRV_NAME 	"lis3lv02d_i2c"
> +
> +static inline s32 lis3_i2c_write(struct lis3lv02d *lis3, int reg, u8 value)
> +{
> +	struct i2c_client *c = lis3->bus_priv;
> +	return i2c_smbus_write_byte_data(c, reg, value);
> +}
> +
> +static inline s32 lis3_i2c_read(struct lis3lv02d *lis3, int reg, u8 *v)
> +{
> +	struct i2c_client *c = lis3->bus_priv;
> +	*v = i2c_smbus_read_byte_data(c, reg);
> +	return 0;
> +}
> +
> +static int lis3_i2c_init(struct lis3lv02d *lis3)
> +{
> +	u8 reg;
> +	int ret;
> +
> +	/* power up the device */
> +	ret = lis3->read(lis3, CTRL_REG1, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	reg |= CTRL1_PD0;
> +	return lis3->write(lis3, CTRL_REG1, reg);
> +}
> +
> +static struct axis_conversion lis3lv02d_axis_map = { 1, 2, 3 };
Could you add a comment like:
/* Default axis mapping, but it will be overridden by platform data */


> +
> +static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client,
> +					const struct i2c_device_id *id)
> +{
> +	int ret = 0;
> +	struct lis3lv02d_platform_data *pdata = client->dev.platform_data;
> +
> +	if (pdata) {
> +		if (pdata->axis_x)
> +			lis3lv02d_axis_map.x = pdata->axis_x;
> +
> +		if (pdata->axis_y)
> +			lis3lv02d_axis_map.y = pdata->axis_y;
> +
> +		if (pdata->axis_z)
> +			lis3lv02d_axis_map.z = pdata->axis_z;
> +
> +		lis3_dev.irq2 = pdata->irq2;
Could you explain what is irq2 used for in the lis3 ship?
And where in your code is it used? If you are not using it in this set
of patches, it would be better to drop it completely. You'll bring it
back when you actually use it :-)

> +
> +		if (pdata->setup_resources)
> +			ret = pdata->setup_resources();
> +
> +		if (ret)
> +			goto fail;
> +	}
> +
> +	lis3_dev.pdata	  = pdata;
> +	lis3_dev.bus_priv = client;
> +	lis3_dev.init	  = lis3_i2c_init;
> +	lis3_dev.read	  = lis3_i2c_read;
> +	lis3_dev.write	  = lis3_i2c_write;
> +	lis3_dev.irq	  = client->irq;
> +	lis3_dev.ac	  = lis3lv02d_axis_map;
> +
> +	i2c_set_clientdata(client, &lis3_dev);
> +	ret = lis3lv02d_init_device(&lis3_dev);
> +fail:
> +	return ret;
> +}
> +
> +static int __devexit lis3lv02d_i2c_remove(struct i2c_client *client)
> +{
> +	struct lis3lv02d *lis3 = i2c_get_clientdata(client);
> +	struct lis3lv02d_platform_data *pdata = client->dev.platform_data;
> +
> +	if (pdata && pdata->release_resources)
> +		pdata->release_resources();
> +
> +	lis3lv02d_joystick_disable();
> +	lis3lv02d_poweroff(lis3);
> +
> +	return lis3lv02d_remove_fs(&lis3_dev);
> +}
> +
> +#ifdef CONFIG_PM
> +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)
> +		lis3lv02d_poweroff(lis3);
> +	return 0;
> +}
> +
> +static int lis3lv02d_i2c_resume(struct i2c_client *client)
> +{
> +	struct lis3lv02d *lis3 = i2c_get_clientdata(client);
> +
> +	if (!lis3->pdata->wakeup_flags)
> +		lis3lv02d_poweron(lis3);
> +	return 0;
> +}
> +
> +static void lis3lv02d_i2c_shutdown(struct i2c_client *client)
> +{
> +	lis3lv02d_i2c_suspend(client, PMSG_SUSPEND);
> +}
> +#else
> +#define lis3lv02d_i2c_suspend	NULL
> +#define lis3lv02d_i2c_resume	NULL
> +#define lis3lv02d_i2c_shutdown	NULL
> +#endif
> +
> +static const struct i2c_device_id lis3lv02d_id[] = {
> +	{"lis3lv02d", 0 },
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, lis3lv02d_id);
> +
> +static struct i2c_driver lis3lv02d_i2c_driver = {
> +	.driver	 = {
> +		.name   = DRV_NAME,
> +		.owner  = THIS_MODULE,
> +	},
> +	.suspend = lis3lv02d_i2c_suspend,
> +	.shutdown = lis3lv02d_i2c_shutdown,
> +	.resume = lis3lv02d_i2c_resume,
> +	.probe	= lis3lv02d_i2c_probe,
> +	.remove	= __devexit_p(lis3lv02d_i2c_remove),
> +	.id_table = lis3lv02d_id,
> +};
> +
> +static int __init lis3lv02d_init(void)
> +{
> +	return i2c_add_driver(&lis3lv02d_i2c_driver);
> +}
> +
> +static void __exit lis3lv02d_exit(void)
> +{
> +	i2c_del_driver(&lis3lv02d_i2c_driver);
> +}
> +
> +MODULE_AUTHOR("Nokia Corporation");
> +MODULE_DESCRIPTION("lis3lv02d I2C interface");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("i2c:" DRV_NAME);
> +
> +module_init(lis3lv02d_init);
> +module_exit(lis3lv02d_exit);


Eric

Re: [RFC][PATCH 0/2] LIS3LV02D I2C driver

[Jonathan Cameron]

Just a quick heads up wrt overlapping work.

For the i2c support Kalhan Trisal has been posting patches for i2c
support for this
driver to the lm-sensors list for some time and the latest version of that
set is also pretty clean.

The changes in these two patch sets should probably be reconciled before
merging.

> These patches add I2C bus support for LIS3LV02D accelerometer driver
> and possibility to use platform configurability for
> remapping of device axes.
>
> Changes are tested on linux-omap environment.
> Applies to 2.6.32-rc1.
>
> Samu Onkalo (2):
>   LIS3LV02D: axis remap, irq and resource setup / release added to
>     platform data
>   LIS3LV02D: I2C support
>
>  drivers/hwmon/Kconfig         |   17 ++++
>  drivers/hwmon/Makefile        |    1 +
>  drivers/hwmon/lis3lv02d.h     |    1 +
>  drivers/hwmon/lis3lv02d_i2c.c |  183 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/lis3lv02d.h     |   13 +++
>  5 files changed, 215 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hwmon/lis3lv02d_i2c.c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>   

Re: [RFC][PATCH 0/2] LIS3LV02D I2C driver

[Éric Piel]

Op 07-10-09 18:31, Jonathan Cameron schreef:
> Just a quick heads up wrt overlapping work.
> 
> For the i2c support Kalhan Trisal has been posting patches for i2c
> support for this
> driver to the lm-sensors list for some time and the latest version of that
> set is also pretty clean.
You mean this post, right?
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-August/026505.html
"Accelerometer driver for STMicroeletronics-LIS331DL-three-axis-digital"
Thanks for the heads up, I had never heard of this driver before.

Well, it's for the LIS331DL, not the LIS3LV02DL (12 bits) or the
LIS302DL (8 bits) (both supported by lis3lv02d driver). That said,
according to the specs, it seems to be _very_ similar to the LIS302DL,
they even report the same "who_am_i" value. Just some special features
are different (free fall detection for instance).
> 
> The changes in these two patch sets should probably be reconciled before
> merging.

Yes, at least from a 5 minutes look, it seems it should be possible to
merge both drivers. Samu's i2c implementation is pretty clean as well
and IMHO it seems the Right Thing to have all the bus specific parts
(I²C, SPI, ACPI/HP) "inheriting" from a generic driver. So I think we
should try to merge everything in the lis3lv02d driver. Kalhan, would
you mind having a look at the patch from Samu, and see what is required
to get your LIS331DL working with the lis3lv02d driver? It should be
very little and avoid duplication of efforts :-)

See you,
Eric

Re: [RFC][PATCH 0/2] LIS3LV02D I2C driver

[Jonathan Cameron]

Éric Piel wrote:
> Op 07-10-09 18:31, Jonathan Cameron schreef:
>   
>> Just a quick heads up wrt overlapping work.
>>
>> For the i2c support Kalhan Trisal has been posting patches for i2c
>> support for this
>> driver to the lm-sensors list for some time and the latest version of that
>> set is also pretty clean.
>>     
> You mean this post, right?
> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-August/026505.html
> "Accelerometer driver for STMicroeletronics-LIS331DL-three-axis-digital"
> Thanks for the heads up, I had never heard of this driver before.
>   
I think that was the original post. (sorry should have added references!)

> Well, it's for the LIS331DL, not the LIS3LV02DL (12 bits) or the
> LIS302DL (8 bits) (both supported by lis3lv02d driver). That said,
> according to the specs, it seems to be _very_ similar to the LIS302DL,
> they even report the same "who_am_i" value. Just some special features
> are different (free fall detection for instance).
>   
Yes, that's why he was advised to combine it with your driver leading
via about 3 revisions
to
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-October/026840.html
(removed a few pointless headers from previous version
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-September/026706.html
.)


which at first glance is pretty much the same as Samu's driver.
(hence real issue with overlapping work).

I do wonder if it is worth adding a mailing list entry to the driver
MAINTAINERS
entry (unless lm-sensors is where you do want anything relevant to be
discussed?).
Problem here is this evolved in a thread not initially related to your
driver and hence I guess no one noticed you weren't cc'd when it became
relevant.

Jonathan

p.s. Kalhan, btw, just noticed a typo in the patch message :
Macroelectronics->Microelectronics.

Re: [RFC][PATCH 0/2] LIS3LV02D I2C driver

[Éric Piel]

Op 07-10-09 19:01, Éric Piel schreef:
> Op 07-10-09 18:31, Jonathan Cameron schreef:
>> Just a quick heads up wrt overlapping work.
>>
>> For the i2c support Kalhan Trisal has been posting patches for i2c
>> support for this
>> driver to the lm-sensors list for some time and the latest version of that
>> set is also pretty clean.
> You mean this post, right?
> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-August/026505.html
> "Accelerometer driver for STMicroeletronics-LIS331DL-three-axis-digital"
> Thanks for the heads up, I had never heard of this driver before.

:
> Kalhan, would
> you mind having a look at the patch from Samu, and see what is required
> to get your LIS331DL working with the lis3lv02d driver? It should be
> very little and avoid duplication of efforts :-)
Ah, bah,
I see you have actually already more or less done this!
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-October/026840.html
"I2C glue layer for lis3lv02d STMicroelectronics digital accelerometer"

The main thing you forgot to do with this patch was to CC the maintainer
of the driver (AKA me) ;-)

Anyway, this version is _very_ close to the one from Samu. It has just
all the basics, really clean! Samu has already added some nifty things
(like changing the axis conversion, support for suspend...). So it
should _really_ easy to converge :-) As Samu's patch is already in
Andrew's queue, I think it's simpler to leave it as it, and just to keep
my request to you, Kalhan, to check that it works for your hardware as
well.

See you,
Eric
PS: in a previous iteration, you had fixed the rate reading function for
the 8 bits version of the hardware. Would you mind sending this as a
independent patch, or just letting me "rip off" your fix?

Re: [RFC][PATCH 0/2] LIS3LV02D I2C driver

[Onkalo Samu]

On Wed, 2009-10-07 at 19:26 +0200, ext Éric Piel wrote:
> Op 07-10-09 19:01, Éric Piel schreef:
> > Op 07-10-09 18:31, Jonathan Cameron schreef:
> >> Just a quick heads up wrt overlapping work.
> >>
> >> For the i2c support Kalhan Trisal has been posting patches for i2c
> >> support for this
> >> driver to the lm-sensors list for some time and the latest version of that
> >> set is also pretty clean.
> > You mean this post, right?
> > http://lists.lm-sensors.org/pipermail/lm-sensors/2009-August/026505.html
> > "Accelerometer driver for STMicroeletronics-LIS331DL-three-axis-digital"
> > Thanks for the heads up, I had never heard of this driver before.
> 
> :
> > Kalhan, would
> > you mind having a look at the patch from Samu, and see what is required
> > to get your LIS331DL working with the lis3lv02d driver? It should be
> > very little and avoid duplication of efforts :-)
> Ah, bah,
> I see you have actually already more or less done this!
> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-October/026840.html
> "I2C glue layer for lis3lv02d STMicroelectronics digital accelerometer"
> 
> The main thing you forgot to do with this patch was to CC the maintainer
> of the driver (AKA me) ;-)
> 
> Anyway, this version is _very_ close to the one from Samu. It has just
> all the basics, really clean! Samu has already added some nifty things
> (like changing the axis conversion, support for suspend...). So it
> should _really_ easy to converge :-) As Samu's patch is already in
> Andrew's queue, I think it's simpler to leave it as it, and just to keep
> my request to you, Kalhan, to check that it works for your hardware as
> well.

Hi Kalhan,

Have you been able to test my patch if it works also for your HW?

http://marc.info/?l=linux-kernel&m=125472767421625&w=2

http://marc.info/?l=linux-kernel&m=125472767421629&w=2

http://marc.info/?l=linux-kernel&m=125472767421639&w=2

Br,
Samu

RE: [RFC][PATCH 0/2] LIS3LV02D I2C driver

[Trisal, Kalhan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2555 bytes --]



-----Original Message-----
From: Onkalo Samu [mailto:samu.p.onkalo@nokia.com] 
Sent: Tuesday, October 13, 2009 3:44 PM
To: ext Éric Piel; Trisal, Kalhan
Cc: Jonathan Cameron; linux-kernel@vger.kernel.org; LM Sensors; Jean Delvare; Andrew Morton
Subject: Re: [RFC][PATCH 0/2] LIS3LV02D I2C driver

On Wed, 2009-10-07 at 19:26 +0200, ext Éric Piel wrote:
> Op 07-10-09 19:01, Éric Piel schreef:
> > Op 07-10-09 18:31, Jonathan Cameron schreef:
> >> Just a quick heads up wrt overlapping work.
> >>
> >> For the i2c support Kalhan Trisal has been posting patches for i2c
> >> support for this
> >> driver to the lm-sensors list for some time and the latest version of that
> >> set is also pretty clean.
> > You mean this post, right?
> > http://lists.lm-sensors.org/pipermail/lm-sensors/2009-August/026505.html
> > "Accelerometer driver for STMicroeletronics-LIS331DL-three-axis-digital"
> > Thanks for the heads up, I had never heard of this driver before.
> 
> :
> > Kalhan, would
> > you mind having a look at the patch from Samu, and see what is required
> > to get your LIS331DL working with the lis3lv02d driver? It should be
> > very little and avoid duplication of efforts :-)
> Ah, bah,
> I see you have actually already more or less done this!
> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-October/026840.html
> "I2C glue layer for lis3lv02d STMicroelectronics digital accelerometer"
> 
> The main thing you forgot to do with this patch was to CC the maintainer
> of the driver (AKA me) ;-)
> 
> Anyway, this version is _very_ close to the one from Samu. It has just
> all the basics, really clean! Samu has already added some nifty things
> (like changing the axis conversion, support for suspend...). So it
> should _really_ easy to converge :-) As Samu's patch is already in
> Andrew's queue, I think it's simpler to leave it as it, and just to keep
> my request to you, Kalhan, to check that it works for your hardware as
> well.

Hi Kalhan,

Have you been able to test my patch if it works also for your HW?

http://marc.info/?l=linux-kernel&m=125472767421625&w=2

http://marc.info/?l=linux-kernel&m=125472767421629&w=2

http://marc.info/?l=linux-kernel&m=125472767421639&w=2
 
Br,
Samu


Hi Samu, 
  I haven't tested your patch yet but it will work.
   I have seen the patch, the glue layer is exactly same 
   what I have also tested part of my driver.

Br
Kalhan

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥