2010-04-14 13:24:10

by Alan

[permalink] [raw]
Subject: [PATCH 0/4] Various intel small device drivers

These have been through my clean up processing, in particular the accelerometers
are now using the input layer infrastructure.


---

Kalhan Trisal (4):
emc1403: thermal sensor support
liss331d1: accelerometer driver
isl29020: ambient light sensor
hmc6352: Add driver for the HMC6352 compass


drivers/hwmon/Kconfig | 34 ++++
drivers/hwmon/Makefile | 4
drivers/hwmon/emc1403.c | 437 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/hmc6352.c | 235 +++++++++++++++++++++++++
drivers/hwmon/isl29020.c | 243 ++++++++++++++++++++++++++
drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++
6 files changed, 1239 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/emc1403.c
create mode 100644 drivers/hwmon/hmc6352.c
create mode 100644 drivers/hwmon/isl29020.c
create mode 100644 drivers/hwmon/lis331dl.c

--


2010-04-14 13:24:27

by Alan

[permalink] [raw]
Subject: [PATCH 2/4] isl29020: ambient light sensor

From: Kalhan Trisal <[email protected]>

The LS driver will read the latest Lux measurement based upon the
light brightness and will report the LUX output through sysfs interface.

Signed-off-by: Kalhan Trisal <[email protected]>
Signed-off-by: Alan Cox <[email protected]>
---

drivers/hwmon/Kconfig | 9 ++
drivers/hwmon/Makefile | 1
drivers/hwmon/isl29020.c | 243 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 253 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/isl29020.c


diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 74f672d..1fa2533 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1095,6 +1095,15 @@ config SENSORS_HMC6352
This driver provides support for the Honeywell HMC6352 compass,
providing configuration and heading data via sysfs.

+config SENSORS_ISL29020
+ tristate "Intersil ISL29020 ALS"
+ depends on I2C
+ help
+ If you say yes here you get support for the ALS Devices
+ Ambient Light Sensor monitoring chip.
+ Range values can be configured using sysfs.
+ Lux data is accessible via sysfs.
+
if ACPI

comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index ad2ed36..13d6832 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_SENSORS_HMC6352) += hmc6352.o
obj-$(CONFIG_SENSORS_I5K_AMB) += i5k_amb.o
obj-$(CONFIG_SENSORS_IBMAEM) += ibmaem.o
obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
+obj-$(CONFIG_SENSORS_ISL29020) += isl29020.o
obj-$(CONFIG_SENSORS_IT87) += it87.o
obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o
obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.o
diff --git a/drivers/hwmon/isl29020.c b/drivers/hwmon/isl29020.c
new file mode 100644
index 0000000..458140d
--- /dev/null
+++ b/drivers/hwmon/isl29020.c
@@ -0,0 +1,243 @@
+/*
+ * isl29020.c - Intersil ALS Driver
+ *
+ * Copyright (C) 2008 Intel Corp
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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 <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/hwmon-vid.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+
+
+#define ALS_MIN_RANGE_VAL 0
+#define ALS_MAX_RANGE_VAL 5
+
+struct als_data {
+ struct device *hwmon_dev;
+};
+
+static unsigned int i2c_write_current_data(struct i2c_client *client,
+ unsigned int reg, unsigned int value)
+{
+ int ret_val;
+
+ ret_val = i2c_smbus_write_byte_data(client, reg, value);
+ return ret_val;
+}
+
+static ssize_t als_sensing_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int val;
+
+ val = i2c_smbus_read_byte_data(client, 0x00);
+ return sprintf(buf, "%d000\n", 1 << (2 * (val & 3)));
+
+}
+
+static ssize_t als_lux_output_data_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned int ret_val, val;
+ unsigned long int lux, max_count;
+ int tempv1, tempv2;
+
+ max_count = 65535;
+ tempv1 = i2c_smbus_read_byte_data(client, 0x02); /* MSB data */
+ tempv2 = i2c_smbus_read_byte_data(client, 0x01); /* LSB data */
+ ret_val = tempv1;
+ ret_val = (ret_val << 8 | tempv2);
+ val = i2c_smbus_read_byte_data(client, 0x00);
+ lux = ((((1 << (2 * (val & 3))))*1000) * ret_val) / max_count;
+ return sprintf(buf, "%ld\n", lux);
+}
+
+static ssize_t als_sensing_range_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned int ret_val, set_val = 0;
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+ ret_val = i2c_smbus_read_byte_data(client, 0x00);
+ ret_val = ret_val & 0xFC; /*reset the bit before setting them */
+ if (val == 1)
+ set_val = (ret_val | 0x00); /* setting the 1:0 bit */
+ else if (val == 2)
+ set_val = (ret_val | 0x01);
+ else if (val == 3)
+ set_val = (ret_val | 0x02);
+ else if (val == 4)
+ set_val = (ret_val | 0x03);
+ else
+ goto invarg;
+ i2c_write_current_data(client, 0x00, set_val);
+ return count;
+invarg:
+ return -EINVAL;
+}
+
+static ssize_t als_power_status_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val;
+
+ ret_val = i2c_smbus_read_byte_data(client, 0x00);
+ ret_val = ret_val & 0x80;
+ if (ret_val == 0x80)
+ ret_val = 1;
+ return sprintf(buf, "%x", ret_val);
+}
+
+static ssize_t als_power_status_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val = 0;
+ char curr_val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ curr_val = i2c_smbus_read_byte_data(client, 0x00);
+ if (val == 1)
+ curr_val = curr_val | 0x80;
+ else if (val == 0)
+ curr_val = curr_val & 0x7F;
+ else
+ return -EINVAL;
+ i2c_write_current_data(client, 0x00, curr_val);
+ return count;
+}
+
+static DEVICE_ATTR(sensing_range, S_IRUGO | S_IWUSR,
+ als_sensing_range_show, als_sensing_range_store);
+static DEVICE_ATTR(lux_output, S_IRUGO, als_lux_output_data_show, NULL);
+static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+ als_power_status_show, als_power_status_store);
+
+static struct attribute *mid_att_als[] = {
+ &dev_attr_sensing_range.attr,
+ &dev_attr_lux_output.attr,
+ &dev_attr_power_state.attr,
+ NULL
+};
+
+static struct attribute_group m_als_gr = {
+ .name = "isl29020",
+ .attrs = mid_att_als
+};
+
+static void als_set_default_config(struct i2c_client *client)
+{
+ i2c_write_current_data(client, 0x00, 0xc0);
+}
+
+static int isl29020_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int res;
+ struct als_data *data;
+
+ data = kzalloc(sizeof(struct als_data), GFP_KERNEL);
+ if (data == NULL) {
+ printk(KERN_WARNING " isl29020: out of memory");
+ return -ENOMEM;
+ }
+ i2c_set_clientdata(client, data);
+
+ res = sysfs_create_group(&client->dev.kobj, &m_als_gr);
+ if (res) {
+ printk(KERN_WARNING "isl29020: device create file failed!!\n");
+ goto als_error1;
+ }
+ data->hwmon_dev = hwmon_device_register(&client->dev);
+ if (IS_ERR(data->hwmon_dev)) {
+ res = PTR_ERR(data->hwmon_dev);
+ data->hwmon_dev = NULL;
+ sysfs_remove_group(&client->dev.kobj, &m_als_gr);
+ printk(KERN_ERR "isl29020:unable to register hwmon device\n");
+ goto als_error1;
+ }
+ dev_info(&client->dev, "%s isl29020: ALS chip found\n", client->name);
+ als_set_default_config(client);
+ return res;
+
+als_error1:
+ i2c_set_clientdata(client, NULL);
+ kfree(data);
+ return res;
+}
+
+static int isl29020_remove(struct i2c_client *client)
+{
+ struct als_data *data = i2c_get_clientdata(client);
+
+ hwmon_device_unregister(data->hwmon_dev);
+ sysfs_remove_group(&client->dev.kobj, &m_als_gr);
+ kfree(data);
+ return 0;
+}
+
+static struct i2c_device_id isl29020_id[] = {
+ { "i2c_als", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, isl29020_id);
+
+static struct i2c_driver isl29020_driver = {
+ .driver = {
+ .name = "isl29020",
+ },
+ .probe = isl29020_probe,
+ .remove = isl29020_remove,
+ .id_table = isl29020_id,
+};
+
+static int __init sensor_isl29020_init(void)
+{
+ return i2c_add_driver(&isl29020_driver);
+}
+
+static void __exit sensor_isl29020_exit(void)
+{
+ i2c_del_driver(&isl29020_driver);
+}
+
+module_init(sensor_isl29020_init);
+module_exit(sensor_isl29020_exit);
+
+MODULE_AUTHOR("Kalhan Trisal <[email protected]");
+MODULE_DESCRIPTION("Intersil isl29020 ALS Driver");
+MODULE_LICENSE("GPL v2");

2010-04-14 13:24:20

by Alan

[permalink] [raw]
Subject: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

From: Kalhan Trisal <[email protected]>

This driver will report the heading values in degrees to the sysfs interface.
The values returned are headings . e.g. 245.6

(Some cleanups from Alan Cox)

Signed-off-by: Kalhan Trisal <[email protected]>
Signed-off-by: Alan Cox <[email protected]>
---

drivers/hwmon/Kconfig | 7 +
drivers/hwmon/Makefile | 1
drivers/hwmon/hmc6352.c | 235 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 243 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/hmc6352.c


diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d38447f..74f672d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1088,6 +1088,13 @@ config SENSORS_MC13783_ADC
help
Support for the A/D converter on MC13783 PMIC.

+config SENSORS_HMC6352
+ tristate "Honeywell HMC6352 compass"
+ depends on I2C
+ help
+ This driver provides support for the Honeywell HMC6352 compass,
+ providing configuration and heading data via sysfs.
+
if ACPI

comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4aa1a3d..ad2ed36 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o
+obj-$(CONFIG_SENSORS_HMC6352) += hmc6352.o
obj-$(CONFIG_SENSORS_I5K_AMB) += i5k_amb.o
obj-$(CONFIG_SENSORS_IBMAEM) += ibmaem.o
obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
diff --git a/drivers/hwmon/hmc6352.c b/drivers/hwmon/hmc6352.c
new file mode 100644
index 0000000..926982f
--- /dev/null
+++ b/drivers/hwmon/hmc6352.c
@@ -0,0 +1,235 @@
+/*
+ * hmc6352.c - Honeywell Compass Driver
+ *
+ * Copyright (C) 2009 Intel Corp
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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 <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/hwmon-vid.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+
+struct compass_data {
+ struct device *hwmon_dev;
+};
+
+static ssize_t compass_calibration_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret;
+ unsigned long val;
+ char cmd[] = {0x43};
+ char cmd1[] = {0x45};
+ struct i2c_msg msg[] = {
+ { client->addr, 0, 1, cmd },
+ };
+ struct i2c_msg msg1[] = {
+ { client->addr, 0, 1, cmd1 },
+ };
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+ if (val == 1) {
+ client->addr = 0x21;
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352_comp : i2c callib start cmd failed\n");
+ return ret;
+ }
+ } else if (val == 2) {
+ client->addr = 0x21;
+ ret = i2c_transfer(client->adapter, msg1, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352_comp : i2c callib stop cmd failed\n");
+ return ret;
+ }
+ } else
+ return -EINVAL;
+
+ return count;
+}
+
+static ssize_t compass_heading_data_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+
+ struct i2c_client *client = to_i2c_client(dev);
+ static char cmd[] = { 0x41 };
+ unsigned char i2c_data[2];
+ unsigned int ret, ret_val;
+ struct i2c_msg msg[] = {
+ { client->addr, 0, 1, cmd },
+ };
+ struct i2c_msg msg1[] = {
+ { client->addr, I2C_M_RD, 2, i2c_data },
+ };
+
+ client->addr = 0x21;
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352 : i2c cmd 0x41 failed\n");
+ return ret;
+ }
+ msleep(10); /* sending 0x41 cmd we need to wait for 7-10 milli second*/
+ ret = i2c_transfer(client->adapter, msg1, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352 : i2c read data cmd failed\n");
+ return ret;
+ }
+ ret_val = i2c_data[0];
+ ret_val = ((ret_val << 8) | i2c_data[1]);
+ return sprintf(buf, "%d.%d\n", ret_val/10, ret_val%10);
+}
+
+static ssize_t compass_power_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val;
+ unsigned int ret;
+ static char cmd[] = { 0x53 };
+ static char cmd1[] = { 0x57 };
+ struct i2c_msg msg[] = {
+ { client->addr, 0, 1, cmd },
+ };
+ struct i2c_msg msg1[] = {
+ { client->addr, 0, 1, cmd1 },
+ };
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ if (val == 0) {
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret != 1)
+ dev_warn(dev, "hmc6352: i2c cmd sleep mode failed\n");
+ } else if (val == 1) {
+ ret = i2c_transfer(client->adapter, msg1, 1);
+ if (ret != 1)
+ dev_warn(dev, "hmc6352: i2c cmd active mode failed\n");
+ } else
+ return -EINVAL;
+
+ return count;
+}
+
+static DEVICE_ATTR(heading, S_IRUGO, compass_heading_data_show, NULL);
+static DEVICE_ATTR(calibration, S_IWUSR, NULL, compass_calibration_store);
+static DEVICE_ATTR(power_state, S_IWUSR, NULL, compass_power_mode_store);
+
+static struct attribute *mid_att_compass[] = {
+ &dev_attr_heading.attr,
+ &dev_attr_calibration.attr,
+ &dev_attr_power_state.attr,
+ NULL
+};
+
+static struct attribute_group m_compass_gr = {
+ .name = "hmc6352",
+ .attrs = mid_att_compass
+};
+
+static int hmc6352_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int res;
+ struct compass_data *data;
+
+ data = kzalloc(sizeof(struct compass_data), GFP_KERNEL);
+ if (data == NULL) {
+ dev_err(&client->dev, "hmc6352: out of memory");
+ return -ENOMEM;
+ }
+ i2c_set_clientdata(client, data);
+
+ res = sysfs_create_group(&client->dev.kobj, &m_compass_gr);
+ if (res) {
+ dev_err(&client->dev, "hmc6352: device_create_file failed\n");
+ goto compass_error1;
+ }
+ data->hwmon_dev = hwmon_device_register(&client->dev);
+ if (IS_ERR(data->hwmon_dev)) {
+ res = PTR_ERR(data->hwmon_dev);
+ data->hwmon_dev = NULL;
+ dev_err(&client->dev, "hmc6352: fail to register hwmon device\n");
+ sysfs_remove_group(&client->dev.kobj, &m_compass_gr);
+ goto compass_error1;
+ }
+ dev_info(&client->dev, "%s HMC6352 compass chip found\n",
+ client->name);
+ return res;
+
+compass_error1:
+ i2c_set_clientdata(client, NULL);
+ kfree(data);
+ return res;
+}
+
+static int hmc6352_remove(struct i2c_client *client)
+{
+ struct compass_data *data = i2c_get_clientdata(client);
+
+ hwmon_device_unregister(data->hwmon_dev);
+ sysfs_remove_group(&client->dev.kobj, &m_compass_gr);
+ kfree(data);
+ return 0;
+}
+
+static struct i2c_device_id hmc6352_id[] = {
+ { "i2c_compass", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, hmc6352_id);
+
+static struct i2c_driver hmc6352_driver = {
+ .driver = {
+ .name = "hmc6352",
+ },
+ .probe = hmc6352_probe,
+ .remove = hmc6352_remove,
+ .id_table = hmc6352_id,
+};
+
+static int __init sensor_hmc6352_init(void)
+{
+ return i2c_add_driver(&hmc6352_driver);
+}
+
+static void __exit sensor_hmc6352_exit(void)
+{
+ i2c_del_driver(&hmc6352_driver);
+}
+
+module_init(sensor_hmc6352_init);
+module_exit(sensor_hmc6352_exit);
+
+MODULE_AUTHOR("Kalhan Trisal <[email protected]");
+MODULE_DESCRIPTION("hmc6352 Compass Driver");
+MODULE_LICENSE("GPL v2");

2010-04-14 13:24:38

by Alan

[permalink] [raw]
Subject: [PATCH 3/4] liss331d1: accelerometer driver

From: Kalhan Trisal <[email protected]>

The acceleremeter driver reads the x,y,z coordinate registers and provides
the information to user through the input layer

Conversion to input device, clean up and porting of retry fixes needed
for AAVA done by Alan Cox.

Signed-off-by: Kalhan Trisal <[email protected]>
Signed-off-by: Alan Cox <[email protected]>
---

drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1
drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 295 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/lis331dl.c


diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 1fa2533..6868b9d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1104,6 +1104,14 @@ config SENSORS_ISL29020
Range values can be configured using sysfs.
Lux data is accessible via sysfs.

+config SENSORS_LIS331DL
+ tristate "STMicroeletronics LIS331DL three-axis digital accelerometer"
+ depends on I2C
+ help
+ If you say yes here you get support for the Accelerometer Devices
+ Device can be configured using sysfs.
+ x y Z data can be accessible via sysfs.
+
if ACPI

comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 13d6832..ebeb2a2 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.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_LIS331DL) += lis331dl.o
obj-$(CONFIG_SENSORS_LM63) += lm63.o
obj-$(CONFIG_SENSORS_LM70) += lm70.o
obj-$(CONFIG_SENSORS_LM73) += lm73.o
diff --git a/drivers/hwmon/lis331dl.c b/drivers/hwmon/lis331dl.c
new file mode 100644
index 0000000..34009eb
--- /dev/null
+++ b/drivers/hwmon/lis331dl.c
@@ -0,0 +1,286 @@
+/*
+ * lis331dl.c - ST LIS331DL Accelerometer Driver
+ *
+ * Copyright (C) 2009 Intel Corp
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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 <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/input-polldev.h>
+
+
+#define POLL_INTERVAL 50
+
+struct lis331dl {
+ struct input_polled_dev *idev;
+ struct i2c_client *client;
+ struct mutex update_lock;
+};
+
+static ssize_t data_rate_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int val;
+ int speed = 100;
+
+ val = i2c_smbus_read_byte_data(client, 0x20);
+ if (val & 0x80); /* 1= 400HZ 0= 100HZ */
+ speed = 400;
+ return sprintf(buf, "%d\n", speed);
+}
+
+static ssize_t power_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int val;
+
+ val = i2c_smbus_read_byte_data(client, 0x20) & 0x40;
+ if (val == 0x40)
+ val = 1;
+ return sprintf(buf, "%d\n", val);
+}
+
+static int read_with_retries(struct i2c_client *client, int r)
+{
+ int retries;
+ int ret;
+
+ for (retries = 0; retries < 5; retries++) {
+ ret = i2c_smbus_read_byte_data(client, r);
+ if (ret != -ETIMEDOUT)
+ break;
+ msleep(10);
+ }
+ return ret;
+}
+
+static void lis331dl_poll(struct input_polled_dev *idev)
+{
+ struct input_dev *input = idev->input;
+ struct lis331dl *data = idev->private;
+ int x, y, z;
+ struct i2c_client *client = data->client;
+
+ x = read_with_retries(client, 0x29);
+ y = read_with_retries(client, 0x2B);
+ z = read_with_retries(client, 0x2D);
+
+ input_report_abs(input, ABS_X, x);
+ input_report_abs(input, ABS_Y, y);
+ input_report_abs(input, ABS_Z, z);
+ input_sync(input);
+}
+
+static ssize_t data_rate_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lis331dl *data = i2c_get_clientdata(client);
+ unsigned int ret_val;
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ ret_val = i2c_smbus_read_byte_data(client, 0x20);
+ ret_val &= 0x7F;
+
+ switch (val) {
+ case 400:
+ ret_val |= 0x80;
+ case 100:
+ i2c_smbus_write_byte_data(client, 0x20, ret_val);
+ break;
+ default:
+ mutex_unlock(&data->update_lock);
+ return -EINVAL;
+ }
+ mutex_unlock(&data->update_lock);
+ return count;
+}
+
+static ssize_t power_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lis331dl *data = i2c_get_clientdata(client);
+ unsigned int ret_val;
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ ret_val = i2c_smbus_read_byte_data(client, 0x20);
+ ret_val &= 0xBF;
+
+ switch(val) {
+ case 1:
+ ret_val |= 0x40;
+ case 0:
+ i2c_smbus_write_byte_data(client, 0x20, ret_val);
+ break;
+ default:
+ mutex_unlock(&data->update_lock);
+ return -EINVAL;
+ }
+ mutex_unlock(&data->update_lock);
+ return count;
+}
+
+static DEVICE_ATTR(data_rate, S_IRUGO | S_IWUSR,
+ data_rate_show, data_rate_store);
+static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+ power_mode_show, power_mode_store);
+
+static struct attribute *mid_att_accelero[] = {
+ &dev_attr_data_rate.attr,
+ &dev_attr_power_state.attr,
+ NULL
+};
+
+static struct attribute_group m_accelero_gr = {
+ .name = "lis331dl",
+ .attrs = mid_att_accelero
+};
+
+static void accel_set_default_config(struct i2c_client *client)
+{
+ i2c_smbus_write_byte_data(client, 0x20, 0x47);
+}
+
+static int lis331dl_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int res;
+ struct lis331dl *data;
+ struct input_polled_dev *idev;
+ struct input_dev *input;
+
+ data = kzalloc(sizeof(struct lis331dl), GFP_KERNEL);
+ if (data == NULL) {
+ dev_err(&client->dev, "lis331dl: out of memory\n");
+ return -ENOMEM;
+ }
+ mutex_init(&data->update_lock);
+ i2c_set_clientdata(client, data);
+ data->client = client;
+
+ res = sysfs_create_group(&client->dev.kobj, &m_accelero_gr);
+ if (res) {
+ dev_err(&client->dev, "lis331dl: sysfs group failed\n");
+ goto accelero_error1;
+ }
+ idev = input_allocate_polled_device();
+ if (!idev) {
+ res = -ENOMEM;
+ dev_err(&client->dev,
+ "lis331dl: unable to register input device\n");
+ goto accelero_error2;
+ }
+ idev->poll = lis331dl_poll;
+ idev->poll_interval = POLL_INTERVAL;
+
+ idev->private = data;
+ data->idev = idev;
+
+ input = idev->input;
+ input->name = "lis331dl";
+ input->phys = "lis331dl/input0";
+ input->id.bustype = BUS_I2C;
+ input->dev.parent = &client->dev;
+ input->evbit[0] = BIT_MASK(EV_ABS);
+ input_set_abs_params(input, ABS_X, 0, 255, 2, 2);
+ input_set_abs_params(input, ABS_Y, 0, 255, 2, 2);
+ input_set_abs_params(input, ABS_Z, 0, 255, 2, 2);
+
+ res = input_register_polled_device(idev);
+ if (res == 0) {
+ dev_info(&client->dev,
+ "%s lis331dl: Accelerometer chip found",
+ client->name);
+ return 0;
+ }
+ input_free_polled_device(idev);
+accelero_error2:
+ sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
+accelero_error1:
+ i2c_set_clientdata(client, NULL);
+ kfree(data);
+ return res;
+}
+
+static int lis331dl_remove(struct i2c_client *client)
+{
+ struct lis331dl *data = i2c_get_clientdata(client);
+
+ input_unregister_polled_device(data->idev);
+ input_free_polled_device(data->idev);
+ sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
+ kfree(data);
+ return 0;
+}
+
+static struct i2c_device_id lis331dl_id[] = {
+ { "i2c_accel", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, lis331dl_id);
+
+static struct i2c_driver lis331dl_driver = {
+ .driver = {
+ .name = "lis331dl",
+ },
+ .probe = lis331dl_probe,
+ .remove = lis331dl_remove,
+ .id_table = lis331dl_id,
+};
+
+static int __init sensor_lis331dl_init(void)
+{
+ int res;
+
+ res = i2c_add_driver(&lis331dl_driver);
+ return res;
+}
+
+static void __exit sensor_lis331dl_exit(void)
+{
+ i2c_del_driver(&lis331dl_driver);
+}
+
+module_init(sensor_lis331dl_init);
+module_exit(sensor_lis331dl_exit);
+
+MODULE_AUTHOR("Kalhan Trisal <[email protected]>, Alan Cox");
+MODULE_DESCRIPTION("STMicroelectronics LIS331DL Accelerometer Driver");
+MODULE_LICENSE("GPL v2");

2010-04-14 13:24:46

by Alan

[permalink] [raw]
Subject: [PATCH 4/4] emc1403: thermal sensor support

From: Kalhan Trisal <kalhan.trisal at intel.com>

Provides support for the emc1403 thermal sensor. Only reporting of values
is supported. The various Moorestown specific extras to do with thermal
alerts and the like are not in this version of the driver.

Signed-off-by: Kalhan Trisal <kalhan.trisal at intel.com>
Signed-off-by: Alan Cox <[email protected]>
---

drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1
drivers/hwmon/emc1403.c | 437 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 448 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/emc1403.c


diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6868b9d..34ee302 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1112,6 +1112,16 @@ config SENSORS_LIS331DL
Device can be configured using sysfs.
x y Z data can be accessible via sysfs.

+config SENSORS_EMC1403
+ tristate "SMSC EMC1403 thermal sensor"
+ depends on I2C
+ help
+ If you say yes here you get support for the SMSC Devices
+ EMC1403 temperature monitoring chip.
+
+ Threshold values can be configured using sysfs.
+ Data from the different diode are accessible via sysfs.
+
if ACPI

comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index ebeb2a2..a7332ce 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
obj-$(CONFIG_SENSORS_DME1737) += dme1737.o
obj-$(CONFIG_SENSORS_DS1621) += ds1621.o
+obj-$(CONFIG_SENSORS_EMC1403) += emc1403.o
obj-$(CONFIG_SENSORS_F71805F) += f71805f.o
obj-$(CONFIG_SENSORS_F71882FG) += f71882fg.o
obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
new file mode 100644
index 0000000..b71cfa1
--- /dev/null
+++ b/drivers/hwmon/emc1403.c
@@ -0,0 +1,437 @@
+/*
+ * emc1403.c - SMSC Thermal Driver
+ *
+ * Copyright (C) 2008 Intel Corp
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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 <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/hwmon-vid.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+
+
+/* Limit status reg Therm/High/Low/Fault*/
+static const u8 THM_STAT_REG_TEMP[] = { 0x37, 0x35, 0x36, 0x1B, 0x02};
+
+/* Channel diode temp set */
+static const u8 THM_CHAN_TEMP[] = { 0x10, 0x08, 0x04, 0x02, 0x01 };
+
+/* Therm Limit reg store values */
+static const u8 THM_LIMIT_REG_TEMP[] = { 0x05, 0x06, 0x07, 0x08, 0x15, 0x16,
+ 0x19, 0x1A, 0x20, 0x21 };
+
+/* DATA REGISTERS */
+static const u8 THM_REG_CURR_TEMP[] = { 0x00, 0x01, 0x23 };
+
+#define THERMAL_PID_REG 0xfd
+#define THERMAL_SMSC_ID_REG 0xfe
+#define THERMAL_REVISION_REG 0xff
+#define THERMAL_ADC_UPDATE_BUSY 0x80
+#define I2C_THERMAL_SLAVE_ADDR 0x4C
+#define TEMP1 1
+#define TEMP2 2
+#define TEMP3 4
+#define IRQ_TYPE_MASK (1 << 15)
+#define HIGH_EVENT 1
+#define LOW_EVENT 2
+#define THERM_EVENT 3
+#define FAULT_EVENT 4
+#define ALERT_EVENT 1
+
+struct thermal_data {
+ struct i2c_client *client;
+ struct device *hwmon_dev;
+ int therm_irq;
+ int alert_irq;
+ struct work_struct therm_handler;
+ struct work_struct alert_handler;
+};
+
+static int calculate_offset(int type, int temp_ofs)
+{
+ int offset = 0;
+
+ switch (type) {
+ case TEMP1:
+ if (temp_ofs == 0)
+ offset = 1;
+ else if (temp_ofs == 1)
+ offset = 0;
+ else if (temp_ofs == 2)
+ offset = 8;
+ break;
+ case TEMP2:
+ if (temp_ofs == 0)
+ offset = 3;
+ else if (temp_ofs == 1)
+ offset = 2;
+ else if (temp_ofs == 2)
+ offset = 6;
+ break;
+ case TEMP3:
+ if (temp_ofs == 0)
+ offset = 5;
+ else if (temp_ofs == 1)
+ offset = 4;
+ else if (temp_ofs == 2)
+ offset = 7;
+ break;
+ default:
+ offset = -1;
+ break;
+ }
+ return offset;
+
+}
+
+
+static ssize_t show_temp_auto_offset(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct sensor_device_attribute_2 *s_attr = to_sensor_dev_attr_2(attr);
+ int temp_index = s_attr->index;
+ int temp_ofs = s_attr->nr;
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val;
+ int ret_offset;
+
+ ret_offset = calculate_offset(temp_index, temp_ofs);
+ if (ret_offset == -1)
+ return -EINVAL;
+ ret_val = i2c_smbus_read_byte_data(client,
+ THM_LIMIT_REG_TEMP[ret_offset]);
+ return sprintf(buf, "%d\n", ret_val);
+}
+
+static ssize_t store_temp_auto_offset(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct sensor_device_attribute_2 *s_attr = to_sensor_dev_attr_2(attr);
+ int temp_index = s_attr->index;
+ int temp_ofs = s_attr->nr;
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val;
+ int ret_offset;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+ ret_offset = calculate_offset(temp_index, temp_ofs);
+ if (ret_offset == -1)
+ return -EINVAL;
+
+ i2c_smbus_write_byte_data(client, THM_LIMIT_REG_TEMP[ret_offset], val);
+ return count;
+}
+
+static ssize_t show_temp_hyst(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ return sprintf(buf, "%d\n",
+ i2c_smbus_read_byte_data(client, THM_LIMIT_REG_TEMP[9]));
+}
+
+static ssize_t store_temp_hyst(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+ i2c_smbus_write_byte_data(client, THM_LIMIT_REG_TEMP[9], val);
+ return count;
+}
+
+static ssize_t show_temp1_curr_temp(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val;
+
+ ret_val = i2c_smbus_read_byte_data(client, THM_REG_CURR_TEMP[0]);
+ return sprintf(buf, "%d\n", ret_val);
+}
+
+static ssize_t show_temp2_curr_temp(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val;
+
+ ret_val = i2c_smbus_read_byte_data(client, THM_REG_CURR_TEMP[1]);
+ return sprintf(buf, "%d\n", ret_val);
+}
+
+static ssize_t show_temp3_curr_temp(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val;
+
+ ret_val = i2c_smbus_read_byte_data(client, THM_REG_CURR_TEMP[2]);
+ return sprintf(buf, "%d\n", ret_val);
+}
+
+static ssize_t show_status_reg(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int r1, r2, r3, r4;
+
+ r1 = i2c_smbus_read_byte_data(client, 0x1F);
+ r2 = i2c_smbus_read_byte_data(client, 0x35);
+ r3 = i2c_smbus_read_byte_data(client, 0x36);
+ r4 = i2c_smbus_read_byte_data(client, 0x37);
+ return sprintf(buf, "alarm=%x,High=%x,Low=%x,Therm=%x \n",
+ r1, r2, r3, r4);
+}
+
+static ssize_t show_power_state(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val = i2c_smbus_read_byte_data(client, 0x03);
+ return sprintf(buf, "%x", (ret_val >> 6) & 1);
+}
+
+static ssize_t store_power_state(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val;
+ char curr_val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ curr_val = i2c_smbus_read_byte_data(client, 0x03);
+ if (val == 1)
+ curr_val &= 0xBF;
+ else if (val == 0)
+ curr_val |= 0x40;
+ else
+ return -EINVAL;
+ i2c_smbus_write_byte_data(client, 0x03, curr_val);
+ return count;
+}
+
+static ssize_t show_mode(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val = i2c_smbus_read_byte_data(client, 0x03);
+ return sprintf(buf, "%x", (ret_val >> 7) & 1);
+}
+
+static ssize_t store_mode(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val;
+ int curr_val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ curr_val = i2c_smbus_read_byte_data(client, 0x03);
+ if (val == 1)
+ curr_val &= 0x7F;
+ else if (val == 0)
+ curr_val |= 0x80;
+ else
+ return -EINVAL;
+ i2c_smbus_write_byte_data(client, 0x03, curr_val);
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR_2(temp1_min, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 0, 1);
+static SENSOR_DEVICE_ATTR_2(temp1_max, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 1, 1);
+static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 2, 1);
+static DEVICE_ATTR(temp1_curr, S_IRUGO, show_temp1_curr_temp, NULL);
+
+static SENSOR_DEVICE_ATTR_2(temp2_min, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 0, 2);
+static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 1, 2);
+static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 2, 2);
+static DEVICE_ATTR(temp2_curr, S_IRUGO, show_temp2_curr_temp, NULL);
+
+static SENSOR_DEVICE_ATTR_2(temp3_min, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 0, 4);
+static SENSOR_DEVICE_ATTR_2(temp3_max, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 1, 4);
+static SENSOR_DEVICE_ATTR_2(temp3_crit, S_IRUGO | S_IWUSR,
+ show_temp_auto_offset, store_temp_auto_offset, 2, 4);
+static DEVICE_ATTR(temp3_curr, S_IRUGO, show_temp3_curr_temp, NULL);
+
+static DEVICE_ATTR(hyster, S_IRUGO | S_IWUSR, show_temp_hyst, store_temp_hyst);
+static DEVICE_ATTR(status, S_IRUGO, show_status_reg, NULL);
+
+static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+ show_power_state, store_power_state);
+static DEVICE_ATTR(mode, S_IRUGO | S_IWUSR, show_mode, store_mode);
+
+static struct attribute *mid_att_thermal[] = {
+ &sensor_dev_attr_temp1_min.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit.dev_attr.attr,
+ &dev_attr_temp1_curr.attr,
+ &sensor_dev_attr_temp2_min.dev_attr.attr,
+ &sensor_dev_attr_temp2_max.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit.dev_attr.attr,
+ &dev_attr_temp2_curr.attr,
+ &sensor_dev_attr_temp3_min.dev_attr.attr,
+ &sensor_dev_attr_temp3_max.dev_attr.attr,
+ &sensor_dev_attr_temp3_crit.dev_attr.attr,
+ &dev_attr_temp3_curr.attr,
+ &dev_attr_hyster.attr,
+ &dev_attr_status.attr,
+ &dev_attr_power_state.attr,
+ &dev_attr_mode.attr,
+ NULL
+};
+
+static struct attribute_group m_thermal_gr = {
+ .name = "emc1403",
+ .attrs = mid_att_thermal
+};
+
+static void emc1403_set_default_config(struct i2c_client *client)
+{
+ i2c_smbus_write_byte_data(client, 0x03, 0x00);
+ i2c_smbus_write_byte_data(client, 0x04, 0x02);
+ i2c_smbus_write_byte_data(client, 0x22, 0x00);
+}
+
+static int emc1403_probe(struct i2c_client *new_client,
+ const struct i2c_device_id *id)
+{
+ int res = 0;
+ struct thermal_data *data;
+ u16 pid, smsc_id, revision;
+
+ data = kzalloc(sizeof(struct thermal_data), GFP_KERNEL);
+
+ if (data == NULL) {
+ printk(KERN_WARNING "emc1403: out of memory");
+ return -ENOMEM;
+ }
+ data->client = new_client;
+ i2c_set_clientdata(new_client, data);
+
+ /* Check if thermal chip is SMSC and EMC1403 */
+ smsc_id = i2c_smbus_read_byte_data(new_client,
+ THERMAL_SMSC_ID_REG);
+ if (smsc_id != 0x5d) {
+ printk(KERN_WARNING "emc1403: vendor id mismatch\n");
+ goto thermal_error1;
+ }
+ pid = i2c_smbus_read_byte_data(new_client, THERMAL_PID_REG);
+ if (pid != 0x21) {
+ printk(KERN_WARNING "emc1403: product id mismatch\n");
+ goto thermal_error1;
+ }
+ revision = i2c_smbus_read_byte_data(new_client,
+ THERMAL_REVISION_REG);
+ if (revision != 0x01) {
+ printk(KERN_WARNING "emc1403: rev id mismatch (is %d)\n",
+ revision);
+ goto thermal_error1;
+ }
+ res = sysfs_create_group(&new_client->dev.kobj, &m_thermal_gr);
+ if (res) {
+ printk(KERN_WARNING "emc1403: create group failed\n");
+ hwmon_device_unregister(data->hwmon_dev);
+ goto thermal_error1;
+ }
+ data->hwmon_dev = hwmon_device_register(&new_client->dev);
+ if (IS_ERR(data->hwmon_dev)) {
+ res = PTR_ERR(data->hwmon_dev);
+ data->hwmon_dev = NULL;
+ printk(KERN_WARNING "emc1403:Register hwmon dev failed\n");
+ goto thermal_error2;
+ }
+ emc1403_set_default_config(new_client);
+ dev_info(&new_client->dev, "%s EMC1403 Thermal chip found\n",
+ new_client->name);
+ return res;
+thermal_error2:
+ sysfs_remove_group(&new_client->dev.kobj, &m_thermal_gr);
+thermal_error1:
+ i2c_set_clientdata(new_client, NULL);
+ kfree(data);
+ return res;
+}
+
+static int emc1403_remove(struct i2c_client *client)
+{
+ struct thermal_data *data = i2c_get_clientdata(client);
+
+ hwmon_device_unregister(data->hwmon_dev);
+ sysfs_remove_group(&client->dev.kobj, &m_thermal_gr);
+ kfree(data);
+ return 0;
+}
+
+static struct i2c_device_id emc1403_idtable[] = {
+ { "i2c_thermal", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
+
+static struct i2c_driver sensor_emc1403 = {
+ .driver = {
+ .name = "emc1403",
+ },
+ .probe = emc1403_probe,
+ .remove = emc1403_remove,
+ .id_table = emc1403_idtable,
+};
+
+static int __init sensor_emc1403_init(void)
+{
+ return i2c_add_driver(&sensor_emc1403);
+}
+
+static void __exit sensor_emc1403_exit(void)
+{
+ i2c_del_driver(&sensor_emc1403);
+}
+
+module_init(sensor_emc1403_init);
+module_exit(sensor_emc1403_exit);
+
+MODULE_AUTHOR("Kalhan Trisal <[email protected]");
+MODULE_DESCRIPTION("emc1403 Thermal Driver");
+MODULE_LICENSE("GPL v2");

2010-04-14 13:30:57

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH 0/4] Various intel small device drivers

Hi Alan,

On Wed, 14 Apr 2010 13:51:30 +0100, Alan Cox wrote:
> These have been through my clean up processing, in particular the accelerometers
> are now using the input layer infrastructure.
>
>
> ---
>
> Kalhan Trisal (4):
> emc1403: thermal sensor support
> liss331d1: accelerometer driver
> isl29020: ambient light sensor
> hmc6352: Add driver for the HMC6352 compass

The liss331d1, isl29020 and hmc6352 are not hardware monitoring drivers
and thus do not belong to drivers/hwmon.

BTW, when sending actual hardware monitoring drivers, the right mailing
list is lm-sensors, which you did not include. Please see MAINTAINERS.

> drivers/hwmon/Kconfig | 34 ++++
> drivers/hwmon/Makefile | 4
> drivers/hwmon/emc1403.c | 437 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/hmc6352.c | 235 +++++++++++++++++++++++++
> drivers/hwmon/isl29020.c | 243 ++++++++++++++++++++++++++
> drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++
> 6 files changed, 1239 insertions(+), 0 deletions(-)
> create mode 100644 drivers/hwmon/emc1403.c
> create mode 100644 drivers/hwmon/hmc6352.c
> create mode 100644 drivers/hwmon/isl29020.c
> create mode 100644 drivers/hwmon/lis331dl.c
>


--
Jean Delvare

2010-04-14 13:45:04

by Alan

[permalink] [raw]
Subject: Re: [PATCH 0/4] Various intel small device drivers

> > Kalhan Trisal (4):
> > emc1403: thermal sensor support
> > liss331d1: accelerometer driver
> > isl29020: ambient light sensor
> > hmc6352: Add driver for the HMC6352 compass
>
> The liss331d1, isl29020 and hmc6352 are not hardware monitoring drivers

Disagree somewhat. In fact on close grepping I find that there is another
related lis33 implementation in drivers/hwmon already 8)

Given all the accelerometers are in drivers/hwmon where do you think they
should be, and do you have pending patches to move the others ?

I'd also be interested where you think the compass fits if its not hwmon,
ditto the ambient light sensor ?

I'll bounce the emc1403 onto lm-sensors.

Thanks
Alan

2010-04-14 13:53:14

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 0/4] Various intel small device drivers

On 04/14/10 14:49, Alan Cox wrote:
>>> Kalhan Trisal (4):
>>> emc1403: thermal sensor support
>>> liss331d1: accelerometer driver
>>> isl29020: ambient light sensor
>>> hmc6352: Add driver for the HMC6352 compass
>>
>> The liss331d1, isl29020 and hmc6352 are not hardware monitoring drivers
>
> Disagree somewhat. In fact on close grepping I find that there is another
> related lis33 implementation in drivers/hwmon already 8)
>
> Given all the accelerometers are in drivers/hwmon where do you think they
> should be, and do you have pending patches to move the others ?
>
> I'd also be interested where you think the compass fits if its not hwmon,
> ditto the ambient light sensor ?

If you like, feel free to (re)start an argument with Linus on the ambient light
sensor front. Or see that thread for yet another round about discussion of
where these sensors should be.

http://lkml.org/lkml/2010/3/1/367

Basically if it is primarily an input device for human interaction, see if
Dmitry is willing to take it into input (though note he may quite rightly
take some convincing!)

Otherwise, I'm happy to take more general sensors into IIO, but obviously that
is still in staging and evolving reasonably quickly at the moment (large number
of abi clean up patches half way through cleanup at the mo.)

Otherwise, misc with the intent to sweep them all up when an agreed upon
subsystem is in place.

>
> I'll bounce the emc1403 onto lm-sensors.
>
> Thanks
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-04-14 14:09:24

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass


Hi Alan, Kalhan,

Couple of comments below.
On 04/14/10 13:51, Alan Cox wrote:
> From: Kalhan Trisal <[email protected]>
>
> This driver will report the heading values in degrees to the sysfs interface.
> The values returned are headings . e.g. 245.6
>
> (Some cleanups from Alan Cox)
>
> Signed-off-by: Kalhan Trisal <[email protected]>
> Signed-off-by: Alan Cox <[email protected]>
> ---
>
> drivers/hwmon/Kconfig | 7 +
> drivers/hwmon/Makefile | 1
> drivers/hwmon/hmc6352.c | 235 +++++++++++++++++++++++++++++++++++++++++++++++
This is in no way a hwmon chip. Surely misc is a better location for now
(pending the usual discussion about all singing all dancing sensors frameworks).

> 3 files changed, 243 insertions(+), 0 deletions(-)
> create mode 100644 drivers/hwmon/hmc6352.c
>
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index d38447f..74f672d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1088,6 +1088,13 @@ config SENSORS_MC13783_ADC
> help
> Support for the A/D converter on MC13783 PMIC.
>
> +config SENSORS_HMC6352
> + tristate "Honeywell HMC6352 compass"
> + depends on I2C
> + help
> + This driver provides support for the Honeywell HMC6352 compass,
> + providing configuration and heading data via sysfs.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4aa1a3d..ad2ed36 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
> obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
> obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
> obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o
> +obj-$(CONFIG_SENSORS_HMC6352) += hmc6352.o
> obj-$(CONFIG_SENSORS_I5K_AMB) += i5k_amb.o
> obj-$(CONFIG_SENSORS_IBMAEM) += ibmaem.o
> obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
> diff --git a/drivers/hwmon/hmc6352.c b/drivers/hwmon/hmc6352.c
> new file mode 100644
> index 0000000..926982f
> --- /dev/null
> +++ b/drivers/hwmon/hmc6352.c
> @@ -0,0 +1,235 @@
> +/*
> + * hmc6352.c - Honeywell Compass Driver
> + *
> + * Copyright (C) 2009 Intel Corp
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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; version 2 of the License.
> + *
> + * 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 <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>

Why these extra hwmon includes? At least at first glance I can't see
any uses of them. The only call to hwmon is to stick this in the
hwmon class.
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/hwmon-vid.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
or any mutex usage?
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>
> +

I guess this makes the driver look like many others, but why bother with
the wrapping structure? This is only used to keep track of the hwmon
device to be able to remove it later.
> +struct compass_data {
> + struct device *hwmon_dev;
> +};
> +
> +static ssize_t compass_calibration_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int ret;
> + unsigned long val;
Personally I'd have gone with a couple of chars then passed their address into
the i2c_msg initializations. Guess it doesn't matter either way though!
> + char cmd[] = {0x43};
> + char cmd1[] = {0x45};
> + struct i2c_msg msg[] = {
> + { client->addr, 0, 1, cmd },
> + };
> + struct i2c_msg msg1[] = {
> + { client->addr, 0, 1, cmd1 },
> + };
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> + if (val == 1) {
These address changes looking a little unusual to me. They may well be needed, but
if so can we have an explanatory comment?

> + client->addr = 0x21;
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352_comp : i2c callib start cmd failed\n");
> + return ret;
> + }
> + } else if (val == 2) {
> + client->addr = 0x21;
> + ret = i2c_transfer(client->adapter, msg1, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352_comp : i2c callib stop cmd failed\n");
> + return ret;
> + }
> + } else
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +static ssize_t compass_heading_data_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> +
> + struct i2c_client *client = to_i2c_client(dev);
> + static char cmd[] = { 0x41 };
> + unsigned char i2c_data[2];
> + unsigned int ret, ret_val;
> + struct i2c_msg msg[] = {
> + { client->addr, 0, 1, cmd },
> + };
> + struct i2c_msg msg1[] = {
> + { client->addr, I2C_M_RD, 2, i2c_data },
> + };
> +
> + client->addr = 0x21;
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352 : i2c cmd 0x41 failed\n");
> + return ret;
> + }
> + msleep(10); /* sending 0x41 cmd we need to wait for 7-10 milli second*/
> + ret = i2c_transfer(client->adapter, msg1, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352 : i2c read data cmd failed\n");
> + return ret;
> + }
> + ret_val = i2c_data[0];
> + ret_val = ((ret_val << 8) | i2c_data[1]);
> + return sprintf(buf, "%d.%d\n", ret_val/10, ret_val%10);
> +}
> +
> +static ssize_t compass_power_mode_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> +
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned long val;
> + unsigned int ret;
> + static char cmd[] = { 0x53 };
> + static char cmd1[] = { 0x57 };
> + struct i2c_msg msg[] = {
> + { client->addr, 0, 1, cmd },
> + };
> + struct i2c_msg msg1[] = {
> + { client->addr, 0, 1, cmd1 },
> + };
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val == 0) {
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret != 1)
> + dev_warn(dev, "hmc6352: i2c cmd sleep mode failed\n");
> + } else if (val == 1) {
> + ret = i2c_transfer(client->adapter, msg1, 1);
> + if (ret != 1)
> + dev_warn(dev, "hmc6352: i2c cmd active mode failed\n");
> + } else
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(heading, S_IRUGO, compass_heading_data_show, NULL);
> +static DEVICE_ATTR(calibration, S_IWUSR, NULL, compass_calibration_store);
> +static DEVICE_ATTR(power_state, S_IWUSR, NULL, compass_power_mode_store);
> +
> +static struct attribute *mid_att_compass[] = {
> + &dev_attr_heading.attr,
> + &dev_attr_calibration.attr,
> + &dev_attr_power_state.attr,
> + NULL
> +};
> +
> +static struct attribute_group m_compass_gr = {
> + .name = "hmc6352",
> + .attrs = mid_att_compass
> +};
> +
> +static int hmc6352_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int res;
> + struct compass_data *data;
> +
> + data = kzalloc(sizeof(struct compass_data), GFP_KERNEL);
> + if (data == NULL) {
> + dev_err(&client->dev, "hmc6352: out of memory");
> + return -ENOMEM;
> + }
> + i2c_set_clientdata(client, data);
> +
> + res = sysfs_create_group(&client->dev.kobj, &m_compass_gr);
> + if (res) {
> + dev_err(&client->dev, "hmc6352: device_create_file failed\n");
> + goto compass_error1;
> + }
> + data->hwmon_dev = hwmon_device_register(&client->dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + res = PTR_ERR(data->hwmon_dev);
> + data->hwmon_dev = NULL;
> + dev_err(&client->dev, "hmc6352: fail to register hwmon device\n");
> + sysfs_remove_group(&client->dev.kobj, &m_compass_gr);
> + goto compass_error1;
> + }
> + dev_info(&client->dev, "%s HMC6352 compass chip found\n",
> + client->name);
> + return res;
> +
> +compass_error1:
> + i2c_set_clientdata(client, NULL);
> + kfree(data);
> + return res;
> +}
> +
> +static int hmc6352_remove(struct i2c_client *client)
> +{
> + struct compass_data *data = i2c_get_clientdata(client);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + sysfs_remove_group(&client->dev.kobj, &m_compass_gr);
> + kfree(data);
> + return 0;
> +}
> +

Why i2c_compass for the name?
> +static struct i2c_device_id hmc6352_id[] = {
> + { "i2c_compass", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, hmc6352_id);
> +
> +static struct i2c_driver hmc6352_driver = {
> + .driver = {
> + .name = "hmc6352",
> + },
> + .probe = hmc6352_probe,
> + .remove = hmc6352_remove,
> + .id_table = hmc6352_id,
> +};
> +
> +static int __init _hmc6352_init(void)
> +{
> + return i2c_add_driver(&hmc6352_driver);
> +}
> +
> +static void __exit sensor_hmc6352_exit(void)
> +{
> + i2c_del_driver(&hmc6352_driver);
> +}
> +
> +module_init(sensor_hmc6352_init);
> +module_exit(sensor_hmc6352_exit);
> +
> +MODULE_AUTHOR("Kalhan Trisal <[email protected]");
> +MODULE_DESCRIPTION("hmc6352 Compass Driver");
> +MODULE_LICENSE("GPL v2");
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-04-14 14:15:20

by Alan

[permalink] [raw]
Subject: Re: [PATCH 0/4] Various intel small device drivers

> If you like, feel free to (re)start an argument with Linus on the ambient light
> sensor front. Or see that thread for yet another round about discussion of
> where these sensors should be.
>
> http://lkml.org/lkml/2010/3/1/367
>
> Basically if it is primarily an input device for human interaction, see if

Its a sensor on an embedded board, it might be managing the backlight or
measuring the dirt level in a sewage pipe depending upon application.

> Dmitry is willing to take it into input (though note he may quite rightly
> take some convincing!)

Its easy enough to make it an input device but that would then end up
with polled_input_dev which keeps waking up and costs battery. A lot of
the polled input model is somewhat broken in that regard but I can see
Linus point.

> Otherwise, misc with the intent to sweep them all up when an agreed upon
> subsystem is in place.

Looks like goes into misc for the moment with all the other light sensors
and I'll see if I can merge it with the 29007 support while I am at it.

Thanks

Alan

2010-04-14 14:28:49

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

> This is in no way a hwmon chip. Surely misc is a better location for now
> (pending the usual discussion about all singing all dancing sensors frameworks).

Yep it's moving at the moment


> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +#include <linux/hwmon.h>
>
> Why these extra hwmon includes? At least at first glance I can't see
> any uses of them. The only call to hwmon is to stick this in the
> hwmon class.

I inherited it for cleanup so these are good points (I've been staring at
piles of these for so long extra input is very useful - this is the tip
of the iceberg !)

> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/hwmon-vid.h>
> > +#include <linux/err.h>
> > +#include <linux/delay.h>
> or any mutex usage?
> > +#include <linux/mutex.h>
> > +#include <linux/sysfs.h>
> > +
>
> I guess this makes the driver look like many others, but why bother with
> the wrapping structure? This is only used to keep track of the hwmon
> device to be able to remove it later.

Should go - agreed will remove

> These address changes looking a little unusual to me. They may well be needed, but
> if so can we have an explanatory comment?

Will investigate.

2010-04-14 14:48:59

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

On 04/14/10 15:32, Alan Cox wrote:
>> This is in no way a hwmon chip. Surely misc is a better location for now
>> (pending the usual discussion about all singing all dancing sensors frameworks).
>
> Yep it's moving at the moment
>
>
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/hwmon.h>
>>
>> Why these extra hwmon includes? At least at first glance I can't see
>> any uses of them. The only call to hwmon is to stick this in the
>> hwmon class.
>
> I inherited it for cleanup so these are good points (I've been staring at
> piles of these for so long extra input is very useful - this is the tip
> of the iceberg !)
Cool, post away. Feel free to cc me in on anything in the category of general
sensors (accelerometers, magnetometers etc). If nothing else, I'm interested
to see them to get ideas for drivers in IIO etc.
>
>>> +#include <linux/hwmon-sysfs.h>
>>> +#include <linux/hwmon-vid.h>
>>> +#include <linux/err.h>
>>> +#include <linux/delay.h>
>> or any mutex usage?
>>> +#include <linux/mutex.h>
>>> +#include <linux/sysfs.h>
>>> +
>>
>> I guess this makes the driver look like many others, but why bother with
>> the wrapping structure? This is only used to keep track of the hwmon
>> device to be able to remove it later.
>
> Should go - agreed will remove
>
>> These address changes looking a little unusual to me. They may well be needed, but
>> if so can we have an explanatory comment?
>
> Will investigate.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-04-14 15:14:58

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

Ripping it out of hwmon and applying the hedge trimmers to everything not
needed and we get this for drivers/misc. I've also swapped the command
codes to characters as the datasheet specifies them in ascii.

--------------------------------8<------------------------------

hmc6352: Add driver for the HMC6352 compass

From: Kalhan Trisal <[email protected]>

This driver will report the heading values in degrees to the sysfs interface.
The values returned are headings . e.g. 245.6

(Some cleanups from Alan Cox)

Signed-off-by: Kalhan Trisal <[email protected]>
Signed-off-by: Alan Cox <[email protected]>
---

drivers/misc/Kconfig | 7 ++
drivers/misc/Makefile | 1
drivers/misc/hmc6352.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 207 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/hmc6352.c


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 2191c8d..e626bac 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -278,6 +278,13 @@ config SENSORS_TSL2550
This driver can also be built as a module. If so, the module
will be called tsl2550.

+config HMC6352
+ tristate "Honeywell HMC6352 compass"
+ depends on I2C
+ help
+ This driver provides support for the Honeywell HMC6352 compass,
+ providing configuration and heading data via sysfs.
+
config EP93XX_PWM
tristate "EP93xx PWM support"
depends on ARCH_EP93XX
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 208ae30..620cf0b 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -26,5 +26,6 @@ obj-$(CONFIG_DS1682) += ds1682.o
obj-$(CONFIG_TI_DAC7512) += ti_dac7512.o
obj-$(CONFIG_C2PORT) += c2port/
obj-$(CONFIG_IWMC3200TOP) += iwmc3200top/
+obj-$(CONFIG_HMC6352) += hmc6352.o
obj-y += eeprom/
obj-y += cb710/
diff --git a/drivers/misc/hmc6352.c b/drivers/misc/hmc6352.c
new file mode 100644
index 0000000..f4162ea
--- /dev/null
+++ b/drivers/misc/hmc6352.c
@@ -0,0 +1,199 @@
+/*
+ * hmc6352.c - Honeywell Compass Driver
+ *
+ * Copyright (C) 2009 Intel Corp
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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 <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/sysfs.h>
+
+static ssize_t compass_calibration_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret;
+ unsigned long val;
+ char cmd = 'C'; /* Calibrate */
+ char cmd1 = 'E'; /* Exit calibration mode */
+ struct i2c_msg msg[] = {
+ { client->addr, 0, 1, &cmd },
+ };
+ struct i2c_msg msg1[] = {
+ { client->addr, 0, 1, &cmd1 },
+ };
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+ if (val == 1) {
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352_comp: i2c calib start cmd failed\n");
+ return ret;
+ }
+ } else if (val == 2) {
+ ret = i2c_transfer(client->adapter, msg1, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352_comp: i2c calib stop cmd failed\n");
+ return ret;
+ }
+ } else
+ return -EINVAL;
+
+ return count;
+}
+
+static ssize_t compass_heading_data_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+
+ struct i2c_client *client = to_i2c_client(dev);
+ static char cmd = 'A'; /* Get Data */
+ unsigned char i2c_data[2];
+ unsigned int ret, ret_val;
+ struct i2c_msg msg[] = {
+ { client->addr, 0, 1, &cmd },
+ };
+ struct i2c_msg msg1[] = {
+ { client->addr, I2C_M_RD, 2, i2c_data },
+ };
+
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352: i2c cmd 0x41 failed\n");
+ return ret;
+ }
+ msleep(10); /* sending 0x41 cmd we need to wait for 7-10 milli second*/
+ ret = i2c_transfer(client->adapter, msg1, 1);
+ if (ret != 1) {
+ dev_warn(dev, "hmc6352: i2c read data cmd failed\n");
+ return ret;
+ }
+ ret_val = i2c_data[0];
+ ret_val = ((ret_val << 8) | i2c_data[1]);
+ return sprintf(buf, "%d.%d\n", ret_val/10, ret_val%10);
+}
+
+static ssize_t compass_power_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val;
+ unsigned int ret;
+ static char cmd = 'S'; /* Sleep mode */
+ static char cmd1 = 'W'; /* Wake up */
+ struct i2c_msg msg[] = {
+ { client->addr, 0, 1, &cmd },
+ };
+ struct i2c_msg msg1[] = {
+ { client->addr, 0, 1, &cmd1 },
+ };
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ if (val == 0) {
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret != 1)
+ dev_warn(dev, "hmc6352: i2c cmd sleep mode failed\n");
+ } else if (val == 1) {
+ ret = i2c_transfer(client->adapter, msg1, 1);
+ if (ret != 1)
+ dev_warn(dev, "hmc6352: i2c cmd active mode failed\n");
+ } else
+ return -EINVAL;
+
+ return count;
+}
+
+static DEVICE_ATTR(heading, S_IRUGO, compass_heading_data_show, NULL);
+static DEVICE_ATTR(calibration, S_IWUSR, NULL, compass_calibration_store);
+static DEVICE_ATTR(power_state, S_IWUSR, NULL, compass_power_mode_store);
+
+static struct attribute *mid_att_compass[] = {
+ &dev_attr_heading.attr,
+ &dev_attr_calibration.attr,
+ &dev_attr_power_state.attr,
+ NULL
+};
+
+static struct attribute_group m_compass_gr = {
+ .name = "hmc6352",
+ .attrs = mid_att_compass
+};
+
+static int hmc6352_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int res;
+
+ res = sysfs_create_group(&client->dev.kobj, &m_compass_gr);
+ if (res) {
+ dev_err(&client->dev, "hmc6352: device_create_file failed\n");
+ return res;
+ }
+ dev_info(&client->dev, "%s HMC6352 compass chip found\n",
+ client->name);
+ return 0;
+}
+
+static int hmc6352_remove(struct i2c_client *client)
+{
+ sysfs_remove_group(&client->dev.kobj, &m_compass_gr);
+ return 0;
+}
+
+static struct i2c_device_id hmc6352_id[] = {
+ { "hmc6352", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, hmc6352_id);
+
+static struct i2c_driver hmc6352_driver = {
+ .driver = {
+ .name = "hmc6352",
+ },
+ .probe = hmc6352_probe,
+ .remove = hmc6352_remove,
+ .id_table = hmc6352_id,
+};
+
+static int __init sensor_hmc6352_init(void)
+{
+ return i2c_add_driver(&hmc6352_driver);
+}
+
+static void __exit sensor_hmc6352_exit(void)
+{
+ i2c_del_driver(&hmc6352_driver);
+}
+
+module_init(sensor_hmc6352_init);
+module_exit(sensor_hmc6352_exit);
+
+MODULE_AUTHOR("Kalhan Trisal <[email protected]");
+MODULE_DESCRIPTION("hmc6352 Compass Driver");
+MODULE_LICENSE("GPL v2");

2010-04-14 16:36:53

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

Looks good to me,

Acked-by: Jonathan Cameron <[email protected]>

On 04/14/10 16:19, Alan Cox wrote:
> Ripping it out of hwmon and applying the hedge trimmers to everything not
> needed and we get this for drivers/misc. I've also swapped the command
> codes to characters as the datasheet specifies them in ascii.
>
> --------------------------------8<------------------------------
>
> hmc6352: Add driver for the HMC6352 compass
>
> From: Kalhan Trisal <[email protected]>
>
> This driver will report the heading values in degrees to the sysfs interface.
> The values returned are headings . e.g. 245.6
>
> (Some cleanups from Alan Cox)
>
> Signed-off-by: Kalhan Trisal <[email protected]>
> Signed-off-by: Alan Cox <[email protected]>
> ---
>
> drivers/misc/Kconfig | 7 ++
> drivers/misc/Makefile | 1
> drivers/misc/hmc6352.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 207 insertions(+), 0 deletions(-)
> create mode 100644 drivers/misc/hmc6352.c
>
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 2191c8d..e626bac 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -278,6 +278,13 @@ config SENSORS_TSL2550
> This driver can also be built as a module. If so, the module
> will be called tsl2550.
>
> +config HMC6352
> + tristate "Honeywell HMC6352 compass"
> + depends on I2C
> + help
> + This driver provides support for the Honeywell HMC6352 compass,
> + providing configuration and heading data via sysfs.
> +
> config EP93XX_PWM
> tristate "EP93xx PWM support"
> depends on ARCH_EP93XX
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 208ae30..620cf0b 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -26,5 +26,6 @@ obj-$(CONFIG_DS1682) += ds1682.o
> obj-$(CONFIG_TI_DAC7512) += ti_dac7512.o
> obj-$(CONFIG_C2PORT) += c2port/
> obj-$(CONFIG_IWMC3200TOP) += iwmc3200top/
> +obj-$(CONFIG_HMC6352) += hmc6352.o
> obj-y += eeprom/
> obj-y += cb710/
> diff --git a/drivers/misc/hmc6352.c b/drivers/misc/hmc6352.c
> new file mode 100644
> index 0000000..f4162ea
> --- /dev/null
> +++ b/drivers/misc/hmc6352.c
> @@ -0,0 +1,199 @@
> +/*
> + * hmc6352.c - Honeywell Compass Driver
> + *
> + * Copyright (C) 2009 Intel Corp
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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; version 2 of the License.
> + *
> + * 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 <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/sysfs.h>
> +
> +static ssize_t compass_calibration_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int ret;
> + unsigned long val;
> + char cmd = 'C'; /* Calibrate */
> + char cmd1 = 'E'; /* Exit calibration mode */
> + struct i2c_msg msg[] = {
> + { client->addr, 0, 1, &cmd },
> + };
> + struct i2c_msg msg1[] = {
> + { client->addr, 0, 1, &cmd1 },
> + };
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> + if (val == 1) {
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352_comp: i2c calib start cmd failed\n");
> + return ret;
> + }
> + } else if (val == 2) {
> + ret = i2c_transfer(client->adapter, msg1, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352_comp: i2c calib stop cmd failed\n");
> + return ret;
> + }
> + } else
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +static ssize_t compass_heading_data_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> +
> + struct i2c_client *client = to_i2c_client(dev);
> + static char cmd = 'A'; /* Get Data */
> + unsigned char i2c_data[2];
> + unsigned int ret, ret_val;
> + struct i2c_msg msg[] = {
> + { client->addr, 0, 1, &cmd },
> + };
> + struct i2c_msg msg1[] = {
> + { client->addr, I2C_M_RD, 2, i2c_data },
> + };
> +
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352: i2c cmd 0x41 failed\n");
> + return ret;
> + }
> + msleep(10); /* sending 0x41 cmd we need to wait for 7-10 milli second*/
> + ret = i2c_transfer(client->adapter, msg1, 1);
> + if (ret != 1) {
> + dev_warn(dev, "hmc6352: i2c read data cmd failed\n");
> + return ret;
> + }
> + ret_val = i2c_data[0];
> + ret_val = ((ret_val << 8) | i2c_data[1]);
> + return sprintf(buf, "%d.%d\n", ret_val/10, ret_val%10);
> +}
> +
> +static ssize_t compass_power_mode_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> +
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned long val;
> + unsigned int ret;
> + static char cmd = 'S'; /* Sleep mode */
> + static char cmd1 = 'W'; /* Wake up */
> + struct i2c_msg msg[] = {
> + { client->addr, 0, 1, &cmd },
> + };
> + struct i2c_msg msg1[] = {
> + { client->addr, 0, 1, &cmd1 },
> + };
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val == 0) {
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret != 1)
> + dev_warn(dev, "hmc6352: i2c cmd sleep mode failed\n");
> + } else if (val == 1) {
> + ret = i2c_transfer(client->adapter, msg1, 1);
> + if (ret != 1)
> + dev_warn(dev, "hmc6352: i2c cmd active mode failed\n");
> + } else
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(heading, S_IRUGO, compass_heading_data_show, NULL);
> +static DEVICE_ATTR(calibration, S_IWUSR, NULL, compass_calibration_store);
> +static DEVICE_ATTR(power_state, S_IWUSR, NULL, compass_power_mode_store);
> +
> +static struct attribute *mid_att_compass[] = {
> + &dev_attr_heading.attr,
> + &dev_attr_calibration.attr,
> + &dev_attr_power_state.attr,
> + NULL
> +};
> +
> +static struct attribute_group m_compass_gr = {
> + .name = "hmc6352",
> + .attrs = mid_att_compass
> +};
> +
> +static int hmc6352_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int res;
> +
> + res = sysfs_create_group(&client->dev.kobj, &m_compass_gr);
> + if (res) {
> + dev_err(&client->dev, "hmc6352: device_create_file failed\n");
> + return res;
> + }
> + dev_info(&client->dev, "%s HMC6352 compass chip found\n",
> + client->name);
> + return 0;
> +}
> +
> +static int hmc6352_remove(struct i2c_client *client)
> +{
> + sysfs_remove_group(&client->dev.kobj, &m_compass_gr);
> + return 0;
> +}
> +
> +static struct i2c_device_id hmc6352_id[] = {
> + { "hmc6352", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, hmc6352_id);
> +
> +static struct i2c_driver hmc6352_driver = {
> + .driver = {
> + .name = "hmc6352",
> + },
> + .probe = hmc6352_probe,
> + .remove = hmc6352_remove,
> + .id_table = hmc6352_id,
> +};
> +
> +static int __init sensor_hmc6352_init(void)
> +{
> + return i2c_add_driver(&hmc6352_driver);
> +}
> +
> +static void __exit sensor_hmc6352_exit(void)
> +{
> + i2c_del_driver(&hmc6352_driver);
> +}
> +
> +module_init(sensor_hmc6352_init);
> +module_exit(sensor_hmc6352_exit);
> +
> +MODULE_AUTHOR("Kalhan Trisal <[email protected]");
> +MODULE_DESCRIPTION("hmc6352 Compass Driver");
> +MODULE_LICENSE("GPL v2");
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-04-14 16:51:55

by Alan

[permalink] [raw]
Subject: isl29020: ALS driver as misc device

And this adds the isl29020 as a misc device per discussions

isl29020: ambient light sensor

From: Kalhan Trisal <[email protected]>

The LS driver will read the latest Lux measurement based upon the
light brightness and will report the LUX output through sysfs interface.

This hardware isn't quite the same as the ISL29003 so has a different driver.

Signed-off-by: Kalhan Trisal <[email protected]>
Signed-off-by: Alan Cox <[email protected]>
---

drivers/misc/Kconfig | 10 ++
drivers/misc/Makefile | 1
drivers/misc/isl29020.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 221 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/isl29020.c


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index e626bac..b3af3f4 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -268,6 +268,16 @@ config ISL29003
This driver can also be built as a module. If so, the module
will be called isl29003.

+config ISL29020
+ tristate "Intersil ISL29020 ambient light sensor"
+ depends on I2C
+ help
+ If you say yes here you get support for the Intersil ISL29020
+ ambient light sensor.
+
+ This driver can also be built as a module. If so, the module
+ will be called isl29020.
+
config SENSORS_TSL2550
tristate "Taos TSL2550 ambient light sensor"
depends on I2C && SYSFS
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 620cf0b..75c8b0e 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_SGI_GRU) += sgi-gru/
obj-$(CONFIG_CS5535_MFGPT) += cs5535-mfgpt.o
obj-$(CONFIG_HP_ILO) += hpilo.o
obj-$(CONFIG_ISL29003) += isl29003.o
+obj-$(CONFIG_ISL29020) += isl29020.o
obj-$(CONFIG_SENSORS_TSL2550) += tsl2550.o
obj-$(CONFIG_EP93XX_PWM) += ep93xx_pwm.o
obj-$(CONFIG_DS1682) += ds1682.o
diff --git a/drivers/misc/isl29020.c b/drivers/misc/isl29020.c
new file mode 100644
index 0000000..b8a6ed6
--- /dev/null
+++ b/drivers/misc/isl29020.c
@@ -0,0 +1,210 @@
+/*
+ * isl29020.c - Intersil ALS Driver
+ *
+ * Copyright (C) 2008 Intel Corp
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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 <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/sysfs.h>
+
+
+#define ALS_MIN_RANGE_VAL 0
+#define ALS_MAX_RANGE_VAL 5
+
+static unsigned int i2c_write_current_data(struct i2c_client *client,
+ unsigned int reg, unsigned int value)
+{
+ int ret_val;
+
+ ret_val = i2c_smbus_write_byte_data(client, reg, value);
+ return ret_val;
+}
+
+static ssize_t als_sensing_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int val;
+
+ val = i2c_smbus_read_byte_data(client, 0x00);
+ return sprintf(buf, "%d000\n", 1 << (2 * (val & 3)));
+
+}
+
+static ssize_t als_lux_output_data_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned int ret_val, val;
+ unsigned long int lux, max_count;
+ int tempv1, tempv2;
+
+ max_count = 65535;
+ tempv1 = i2c_smbus_read_byte_data(client, 0x02); /* MSB data */
+ tempv2 = i2c_smbus_read_byte_data(client, 0x01); /* LSB data */
+ ret_val = tempv1;
+ ret_val = (ret_val << 8 | tempv2);
+ val = i2c_smbus_read_byte_data(client, 0x00);
+ lux = ((((1 << (2 * (val & 3))))*1000) * ret_val) / max_count;
+ return sprintf(buf, "%ld\n", lux);
+}
+
+static ssize_t als_sensing_range_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned int ret_val, set_val = 0;
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+ ret_val = i2c_smbus_read_byte_data(client, 0x00);
+ ret_val = ret_val & 0xFC; /*reset the bit before setting them */
+ if (val == 1)
+ set_val = (ret_val | 0x00); /* setting the 1:0 bit */
+ else if (val == 2)
+ set_val = (ret_val | 0x01);
+ else if (val == 3)
+ set_val = (ret_val | 0x02);
+ else if (val == 4)
+ set_val = (ret_val | 0x03);
+ else
+ goto invarg;
+ i2c_write_current_data(client, 0x00, set_val);
+ return count;
+invarg:
+ return -EINVAL;
+}
+
+static ssize_t als_power_status_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ int ret_val;
+
+ ret_val = i2c_smbus_read_byte_data(client, 0x00);
+ ret_val = ret_val & 0x80;
+ if (ret_val == 0x80)
+ ret_val = 1;
+ return sprintf(buf, "%x", ret_val);
+}
+
+static ssize_t als_power_status_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val = 0;
+ char curr_val;
+
+ if (strict_strtoul(buf, 10, &val))
+ return -EINVAL;
+
+ curr_val = i2c_smbus_read_byte_data(client, 0x00);
+ if (val == 1)
+ curr_val = curr_val | 0x80;
+ else if (val == 0)
+ curr_val = curr_val & 0x7F;
+ else
+ return -EINVAL;
+ i2c_write_current_data(client, 0x00, curr_val);
+ return count;
+}
+
+static DEVICE_ATTR(sensing_range, S_IRUGO | S_IWUSR,
+ als_sensing_range_show, als_sensing_range_store);
+static DEVICE_ATTR(lux_output, S_IRUGO, als_lux_output_data_show, NULL);
+static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+ als_power_status_show, als_power_status_store);
+
+static struct attribute *mid_att_als[] = {
+ &dev_attr_sensing_range.attr,
+ &dev_attr_lux_output.attr,
+ &dev_attr_power_state.attr,
+ NULL
+};
+
+static struct attribute_group m_als_gr = {
+ .name = "isl29020",
+ .attrs = mid_att_als
+};
+
+static void als_set_default_config(struct i2c_client *client)
+{
+ i2c_write_current_data(client, 0x00, 0xc0);
+}
+
+static int isl29020_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int res = sysfs_create_group(&client->dev.kobj, &m_als_gr);
+ if (res) {
+ dev_err(&client->dev, "isl29020: device create file failed\n");
+ return res;
+ }
+ dev_info(&client->dev, "%s isl29020: ALS chip found\n", client->name);
+ als_set_default_config(client);
+ return res;
+}
+
+static int isl29020_remove(struct i2c_client *client)
+{
+ struct als_data *data = i2c_get_clientdata(client);
+ sysfs_remove_group(&client->dev.kobj, &m_als_gr);
+ kfree(data);
+ return 0;
+}
+
+static struct i2c_device_id isl29020_id[] = {
+ { "isl29020", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, isl29020_id);
+
+static struct i2c_driver isl29020_driver = {
+ .driver = {
+ .name = "isl29020",
+ },
+ .probe = isl29020_probe,
+ .remove = isl29020_remove,
+ .id_table = isl29020_id,
+};
+
+static int __init sensor_isl29020_init(void)
+{
+ return i2c_add_driver(&isl29020_driver);
+}
+
+static void __exit sensor_isl29020_exit(void)
+{
+ i2c_del_driver(&isl29020_driver);
+}
+
+module_init(sensor_isl29020_init);
+module_exit(sensor_isl29020_exit);
+
+MODULE_AUTHOR("Kalhan Trisal <[email protected]");
+MODULE_DESCRIPTION("Intersil isl29020 ALS Driver");
+MODULE_LICENSE("GPL v2");

2010-04-14 17:01:54

by Greg KH

[permalink] [raw]
Subject: Re: isl29020: ALS driver as misc device

On Wed, Apr 14, 2010 at 05:56:02PM +0100, Alan Cox wrote:
> And this adds the isl29020 as a misc device per discussions
>
> isl29020: ambient light sensor
>
> From: Kalhan Trisal <[email protected]>
>
> The LS driver will read the latest Lux measurement based upon the
> light brightness and will report the LUX output through sysfs interface.

Please document this sysfs interface with an addition to the
Documentatin/ABI/ directory.

thanks,

greg k-h

2010-04-14 21:36:37

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

Trivial neatening

Argument alignment
Spacing
Make chars static
Remove prefixes from dev_<level> logging

Signed-off-by: Joe Perches <[email protected]>
---
drivers/misc/hmc6352.c | 44 ++++++++++++++++++++++----------------------
1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/misc/hmc6352.c b/drivers/misc/hmc6352.c
index 1c1e974..2c12dc3 100644
--- a/drivers/misc/hmc6352.c
+++ b/drivers/misc/hmc6352.c
@@ -30,13 +30,14 @@
#include <linux/sysfs.h>

static ssize_t compass_calibration_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
+ struct device_attribute *attr,
+ const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
int ret;
unsigned long val;
- char cmd = 'C'; /* Calibrate */
- char cmd1 = 'E'; /* Exit calibration mode */
+ static char cmd = 'C'; /* Calibrate */
+ static char cmd1 = 'E'; /* Exit calibration mode */
struct i2c_msg msg[] = {
{ client->addr, 0, 1, &cmd },
};
@@ -49,13 +50,13 @@ static ssize_t compass_calibration_store(struct device *dev,
if (val == 1) {
ret = i2c_transfer(client->adapter, msg, 1);
if (ret != 1) {
- dev_warn(dev, "hmc6352_comp: i2c calib start cmd failed\n");
+ dev_warn(dev, "i2c calib start cmd failed\n");
return ret;
}
} else if (val == 2) {
ret = i2c_transfer(client->adapter, msg1, 1);
if (ret != 1) {
- dev_warn(dev, "hmc6352_comp: i2c calib stop cmd failed\n");
+ dev_warn(dev, "i2c calib stop cmd failed\n");
return ret;
}
} else
@@ -65,9 +66,9 @@ static ssize_t compass_calibration_store(struct device *dev,
}

static ssize_t compass_heading_data_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
-
struct i2c_client *client = to_i2c_client(dev);
static char cmd = 'A'; /* Get Data */
unsigned char i2c_data[2];
@@ -81,22 +82,22 @@ static ssize_t compass_heading_data_show(struct device *dev,

ret = i2c_transfer(client->adapter, msg, 1);
if (ret != 1) {
- dev_warn(dev, "hmc6352: i2c cmd 0x41 failed\n");
+ dev_warn(dev, "i2c cmd 0x41 failed\n");
return ret;
}
- msleep(10); /* sending 0x41 cmd we need to wait for 7-10 milli second*/
+ msleep(10); /* sending 0x41 cmd we need to wait for 7-10 milliseconds */
ret = i2c_transfer(client->adapter, msg1, 1);
if (ret != 1) {
- dev_warn(dev, "hmc6352: i2c read data cmd failed\n");
+ dev_warn(dev, "i2c read data cmd failed\n");
return ret;
}
- ret_val = i2c_data[0];
- ret_val = ((ret_val << 8) | i2c_data[1]);
+ ret_val = ((unsigned int)i2c_data[0] << 8) | i2c_data[1];
return sprintf(buf, "%d.%d\n", ret_val/10, ret_val%10);
}

static ssize_t compass_power_mode_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
+ struct device_attribute *attr,
+ const char *buf, size_t count)
{

struct i2c_client *client = to_i2c_client(dev);
@@ -117,11 +118,11 @@ static ssize_t compass_power_mode_store(struct device *dev,
if (val == 0) {
ret = i2c_transfer(client->adapter, msg, 1);
if (ret != 1)
- dev_warn(dev, "hmc6352: i2c cmd sleep mode failed\n");
+ dev_warn(dev, "i2c cmd sleep mode failed\n");
} else if (val == 1) {
ret = i2c_transfer(client->adapter, msg1, 1);
if (ret != 1)
- dev_warn(dev, "hmc6352: i2c cmd active mode failed\n");
+ dev_warn(dev, "i2c cmd active mode failed\n");
} else
return -EINVAL;

@@ -144,18 +145,17 @@ static struct attribute_group m_compass_gr = {
.attrs = mid_att_compass
};

-static int hmc6352_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int hmc6352_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
{
int res;

res = sysfs_create_group(&client->dev.kobj, &m_compass_gr);
if (res) {
- dev_err(&client->dev, "hmc6352: device_create_file failed\n");
+ dev_err(&client->dev, "sysfs_create_group failed\n");
return res;
}
- dev_info(&client->dev, "%s HMC6352 compass chip found\n",
- client->name);
+ dev_info(&client->dev, "%s HMC6352 compass chip found\n", client->name);
return 0;
}

@@ -174,7 +174,7 @@ MODULE_DEVICE_TABLE(i2c, hmc6352_id);

static struct i2c_driver hmc6352_driver = {
.driver = {
- .name = "hmc6352",
+ .name = "hmc6352",
},
.probe = hmc6352_probe,
.remove = hmc6352_remove,
@@ -186,7 +186,7 @@ static int __init sensor_hmc6352_init(void)
return i2c_add_driver(&hmc6352_driver);
}

-static void __exit sensor_hmc6352_exit(void)
+static void __exit sensor_hmc6352_exit(void)
{
i2c_del_driver(&hmc6352_driver);
}

2010-04-14 22:12:27

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

Op 14-04-10 14:52, Alan Cox schreef:
> From: Kalhan Trisal <[email protected]>
>
> The acceleremeter driver reads the x,y,z coordinate registers and provides
> the information to user through the input layer
>
> Conversion to input device, clean up and porting of retry fixes needed
> for AAVA done by Alan Cox.
Hello,

I wouldn't want to be too picky, if this driver works fine with your
hardware and it's tested, why not, but from looking at the spec of this
chip (and at the code of this driver), it looks like it could be
completely compatible with the lis3lv02d driver (with lis3lv02d_i2c).
Same registers, including the WHO_AM_I register which returns 0x3B
(which is a supported version).

Have you tried and there were some specific incompatibilities? To me, it
looks like the only thing not in the lis3lv02d is the read with multiple
retries, and it could be easily added if necessary for your hardware.

Cheers,
Eric


>
> Signed-off-by: Kalhan Trisal <[email protected]>
> Signed-off-by: Alan Cox <[email protected]>
> ---
>
> drivers/hwmon/Kconfig | 8 +
> drivers/hwmon/Makefile | 1
> drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 295 insertions(+), 0 deletions(-)
> create mode 100644 drivers/hwmon/lis331dl.c
>
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 1fa2533..6868b9d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1104,6 +1104,14 @@ config SENSORS_ISL29020
> Range values can be configured using sysfs.
> Lux data is accessible via sysfs.
>
> +config SENSORS_LIS331DL
> + tristate "STMicroeletronics LIS331DL three-axis digital accelerometer"
> + depends on I2C
> + help
> + If you say yes here you get support for the Accelerometer Devices
> + Device can be configured using sysfs.
> + x y Z data can be accessible via sysfs.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 13d6832..ebeb2a2 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.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_LIS331DL) += lis331dl.o
> obj-$(CONFIG_SENSORS_LM63) += lm63.o
> obj-$(CONFIG_SENSORS_LM70) += lm70.o
> obj-$(CONFIG_SENSORS_LM73) += lm73.o
> diff --git a/drivers/hwmon/lis331dl.c b/drivers/hwmon/lis331dl.c
> new file mode 100644
> index 0000000..34009eb
> --- /dev/null
> +++ b/drivers/hwmon/lis331dl.c
> @@ -0,0 +1,286 @@
> +/*
> + * lis331dl.c - ST LIS331DL Accelerometer Driver
> + *
> + * Copyright (C) 2009 Intel Corp
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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; version 2 of the License.
> + *
> + * 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 <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>
> +#include <linux/input-polldev.h>
> +
> +
> +#define POLL_INTERVAL 50
> +
> +struct lis331dl {
> + struct input_polled_dev *idev;
> + struct i2c_client *client;
> + struct mutex update_lock;
> +};
> +
> +static ssize_t data_rate_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int val;
> + int speed = 100;
> +
> + val = i2c_smbus_read_byte_data(client, 0x20);
> + if (val & 0x80); /* 1= 400HZ 0= 100HZ */
> + speed = 400;
> + return sprintf(buf, "%d\n", speed);
> +}
> +
> +static ssize_t power_mode_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int val;
> +
> + val = i2c_smbus_read_byte_data(client, 0x20) & 0x40;
> + if (val == 0x40)
> + val = 1;
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static int read_with_retries(struct i2c_client *client, int r)
> +{
> + int retries;
> + int ret;
> +
> + for (retries = 0; retries < 5; retries++) {
> + ret = i2c_smbus_read_byte_data(client, r);
> + if (ret != -ETIMEDOUT)
> + break;
> + msleep(10);
> + }
> + return ret;
> +}
> +
> +static void lis331dl_poll(struct input_polled_dev *idev)
> +{
> + struct input_dev *input = idev->input;
> + struct lis331dl *data = idev->private;
> + int x, y, z;
> + struct i2c_client *client = data->client;
> +
> + x = read_with_retries(client, 0x29);
> + y = read_with_retries(client, 0x2B);
> + z = read_with_retries(client, 0x2D);
> +
> + input_report_abs(input, ABS_X, x);
> + input_report_abs(input, ABS_Y, y);
> + input_report_abs(input, ABS_Z, z);
> + input_sync(input);
> +}
> +
> +static ssize_t data_rate_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lis331dl *data = i2c_get_clientdata(client);
> + unsigned int ret_val;
> + unsigned long val;
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + mutex_lock(&data->update_lock);
> +
> + ret_val = i2c_smbus_read_byte_data(client, 0x20);
> + ret_val &= 0x7F;
> +
> + switch (val) {
> + case 400:
> + ret_val |= 0x80;
> + case 100:
> + i2c_smbus_write_byte_data(client, 0x20, ret_val);
> + break;
> + default:
> + mutex_unlock(&data->update_lock);
> + return -EINVAL;
> + }
> + mutex_unlock(&data->update_lock);
> + return count;
> +}
> +
> +static ssize_t power_mode_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lis331dl *data = i2c_get_clientdata(client);
> + unsigned int ret_val;
> + unsigned long val;
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + mutex_lock(&data->update_lock);
> +
> + ret_val = i2c_smbus_read_byte_data(client, 0x20);
> + ret_val &= 0xBF;
> +
> + switch(val) {
> + case 1:
> + ret_val |= 0x40;
> + case 0:
> + i2c_smbus_write_byte_data(client, 0x20, ret_val);
> + break;
> + default:
> + mutex_unlock(&data->update_lock);
> + return -EINVAL;
> + }
> + mutex_unlock(&data->update_lock);
> + return count;
> +}
> +
> +static DEVICE_ATTR(data_rate, S_IRUGO | S_IWUSR,
> + data_rate_show, data_rate_store);
> +static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> + power_mode_show, power_mode_store);
> +
> +static struct attribute *mid_att_accelero[] = {
> + &dev_attr_data_rate.attr,
> + &dev_attr_power_state.attr,
> + NULL
> +};
> +
> +static struct attribute_group m_accelero_gr = {
> + .name = "lis331dl",
> + .attrs = mid_att_accelero
> +};
> +
> +static void accel_set_default_config(struct i2c_client *client)
> +{
> + i2c_smbus_write_byte_data(client, 0x20, 0x47);
> +}
> +
> +static int lis331dl_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int res;
> + struct lis331dl *data;
> + struct input_polled_dev *idev;
> + struct input_dev *input;
> +
> + data = kzalloc(sizeof(struct lis331dl), GFP_KERNEL);
> + if (data == NULL) {
> + dev_err(&client->dev, "lis331dl: out of memory\n");
> + return -ENOMEM;
> + }
> + mutex_init(&data->update_lock);
> + i2c_set_clientdata(client, data);
> + data->client = client;
> +
> + res = sysfs_create_group(&client->dev.kobj, &m_accelero_gr);
> + if (res) {
> + dev_err(&client->dev, "lis331dl: sysfs group failed\n");
> + goto accelero_error1;
> + }
> + idev = input_allocate_polled_device();
> + if (!idev) {
> + res = -ENOMEM;
> + dev_err(&client->dev,
> + "lis331dl: unable to register input device\n");
> + goto accelero_error2;
> + }
> + idev->poll = lis331dl_poll;
> + idev->poll_interval = POLL_INTERVAL;
> +
> + idev->private = data;
> + data->idev = idev;
> +
> + input = idev->input;
> + input->name = "lis331dl";
> + input->phys = "lis331dl/input0";
> + input->id.bustype = BUS_I2C;
> + input->dev.parent = &client->dev;
> + input->evbit[0] = BIT_MASK(EV_ABS);
> + input_set_abs_params(input, ABS_X, 0, 255, 2, 2);
> + input_set_abs_params(input, ABS_Y, 0, 255, 2, 2);
> + input_set_abs_params(input, ABS_Z, 0, 255, 2, 2);
> +
> + res = input_register_polled_device(idev);
> + if (res == 0) {
> + dev_info(&client->dev,
> + "%s lis331dl: Accelerometer chip found",
> + client->name);
> + return 0;
> + }
> + input_free_polled_device(idev);
> +accelero_error2:
> + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
> +accelero_error1:
> + i2c_set_clientdata(client, NULL);
> + kfree(data);
> + return res;
> +}
> +
> +static int lis331dl_remove(struct i2c_client *client)
> +{
> + struct lis331dl *data = i2c_get_clientdata(client);
> +
> + input_unregister_polled_device(data->idev);
> + input_free_polled_device(data->idev);
> + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
> + kfree(data);
> + return 0;
> +}
> +
> +static struct i2c_device_id lis331dl_id[] = {
> + { "i2c_accel", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, lis331dl_id);
> +
> +static struct i2c_driver lis331dl_driver = {
> + .driver = {
> + .name = "lis331dl",
> + },
> + .probe = lis331dl_probe,
> + .remove = lis331dl_remove,
> + .id_table = lis331dl_id,
> +};
> +
> +static int __init sensor_lis331dl_init(void)
> +{
> + int res;
> +
> + res = i2c_add_driver(&lis331dl_driver);
> + return res;
> +}
> +
> +static void __exit sensor_lis331dl_exit(void)
> +{
> + i2c_del_driver(&lis331dl_driver);
> +}
> +
> +module_init(sensor_lis331dl_init);
> +module_exit(sensor_lis331dl_exit);
> +
> +MODULE_AUTHOR("Kalhan Trisal <[email protected]>, Alan Cox");
> +MODULE_DESCRIPTION("STMicroelectronics LIS331DL Accelerometer Driver");
> +MODULE_LICENSE("GPL v2");
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-04-14 22:34:37

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

On Thu, Apr 15, 2010 at 12:12:21AM +0200, ?ric Piel wrote:
> Op 14-04-10 14:52, Alan Cox schreef:
> > From: Kalhan Trisal <[email protected]>
> >
> > The acceleremeter driver reads the x,y,z coordinate registers and provides
> > the information to user through the input layer
> >
> > Conversion to input device, clean up and porting of retry fixes needed
> > for AAVA done by Alan Cox.
> Hello,
>
> I wouldn't want to be too picky, if this driver works fine with your
> hardware and it's tested, why not, but from looking at the spec of this
> chip (and at the code of this driver), it looks like it could be
> completely compatible with the lis3lv02d driver (with lis3lv02d_i2c).
> Same registers, including the WHO_AM_I register which returns 0x3B
> (which is a supported version).
>
> Have you tried and there were some specific incompatibilities? To me, it
> looks like the only thing not in the lis3lv02d is the read with multiple
> retries, and it could be easily added if necessary for your hardware.

Yes, that would be better. Also because there's also a SPI version of
the lis331dl. If support for this chip would be incorporated in the
existing driver, they would automatically be supported as well.

Daniel


> > Signed-off-by: Kalhan Trisal <[email protected]>
> > Signed-off-by: Alan Cox <[email protected]>
> > ---
> >
> > drivers/hwmon/Kconfig | 8 +
> > drivers/hwmon/Makefile | 1
> > drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 295 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/hwmon/lis331dl.c
> >
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 1fa2533..6868b9d 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -1104,6 +1104,14 @@ config SENSORS_ISL29020
> > Range values can be configured using sysfs.
> > Lux data is accessible via sysfs.
> >
> > +config SENSORS_LIS331DL
> > + tristate "STMicroeletronics LIS331DL three-axis digital accelerometer"
> > + depends on I2C
> > + help
> > + If you say yes here you get support for the Accelerometer Devices
> > + Device can be configured using sysfs.
> > + x y Z data can be accessible via sysfs.
> > +
> > if ACPI
> >
> > comment "ACPI drivers"
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 13d6832..ebeb2a2 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.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_LIS331DL) += lis331dl.o
> > obj-$(CONFIG_SENSORS_LM63) += lm63.o
> > obj-$(CONFIG_SENSORS_LM70) += lm70.o
> > obj-$(CONFIG_SENSORS_LM73) += lm73.o
> > diff --git a/drivers/hwmon/lis331dl.c b/drivers/hwmon/lis331dl.c
> > new file mode 100644
> > index 0000000..34009eb
> > --- /dev/null
> > +++ b/drivers/hwmon/lis331dl.c
> > @@ -0,0 +1,286 @@
> > +/*
> > + * lis331dl.c - ST LIS331DL Accelerometer Driver
> > + *
> > + * Copyright (C) 2009 Intel Corp
> > + *
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + * 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; version 2 of the License.
> > + *
> > + * 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 <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +#include <linux/err.h>
> > +#include <linux/delay.h>
> > +#include <linux/mutex.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/input-polldev.h>
> > +
> > +
> > +#define POLL_INTERVAL 50
> > +
> > +struct lis331dl {
> > + struct input_polled_dev *idev;
> > + struct i2c_client *client;
> > + struct mutex update_lock;
> > +};
> > +
> > +static ssize_t data_rate_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + int val;
> > + int speed = 100;
> > +
> > + val = i2c_smbus_read_byte_data(client, 0x20);
> > + if (val & 0x80); /* 1= 400HZ 0= 100HZ */
> > + speed = 400;
> > + return sprintf(buf, "%d\n", speed);
> > +}
> > +
> > +static ssize_t power_mode_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + int val;
> > +
> > + val = i2c_smbus_read_byte_data(client, 0x20) & 0x40;
> > + if (val == 0x40)
> > + val = 1;
> > + return sprintf(buf, "%d\n", val);
> > +}
> > +
> > +static int read_with_retries(struct i2c_client *client, int r)
> > +{
> > + int retries;
> > + int ret;
> > +
> > + for (retries = 0; retries < 5; retries++) {
> > + ret = i2c_smbus_read_byte_data(client, r);
> > + if (ret != -ETIMEDOUT)
> > + break;
> > + msleep(10);
> > + }
> > + return ret;
> > +}
> > +
> > +static void lis331dl_poll(struct input_polled_dev *idev)
> > +{
> > + struct input_dev *input = idev->input;
> > + struct lis331dl *data = idev->private;
> > + int x, y, z;
> > + struct i2c_client *client = data->client;
> > +
> > + x = read_with_retries(client, 0x29);
> > + y = read_with_retries(client, 0x2B);
> > + z = read_with_retries(client, 0x2D);
> > +
> > + input_report_abs(input, ABS_X, x);
> > + input_report_abs(input, ABS_Y, y);
> > + input_report_abs(input, ABS_Z, z);
> > + input_sync(input);
> > +}
> > +
> > +static ssize_t data_rate_store(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct lis331dl *data = i2c_get_clientdata(client);
> > + unsigned int ret_val;
> > + unsigned long val;
> > +
> > + if (strict_strtoul(buf, 10, &val))
> > + return -EINVAL;
> > +
> > + mutex_lock(&data->update_lock);
> > +
> > + ret_val = i2c_smbus_read_byte_data(client, 0x20);
> > + ret_val &= 0x7F;
> > +
> > + switch (val) {
> > + case 400:
> > + ret_val |= 0x80;
> > + case 100:
> > + i2c_smbus_write_byte_data(client, 0x20, ret_val);
> > + break;
> > + default:
> > + mutex_unlock(&data->update_lock);
> > + return -EINVAL;
> > + }
> > + mutex_unlock(&data->update_lock);
> > + return count;
> > +}
> > +
> > +static ssize_t power_mode_store(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct lis331dl *data = i2c_get_clientdata(client);
> > + unsigned int ret_val;
> > + unsigned long val;
> > +
> > + if (strict_strtoul(buf, 10, &val))
> > + return -EINVAL;
> > +
> > + mutex_lock(&data->update_lock);
> > +
> > + ret_val = i2c_smbus_read_byte_data(client, 0x20);
> > + ret_val &= 0xBF;
> > +
> > + switch(val) {
> > + case 1:
> > + ret_val |= 0x40;
> > + case 0:
> > + i2c_smbus_write_byte_data(client, 0x20, ret_val);
> > + break;
> > + default:
> > + mutex_unlock(&data->update_lock);
> > + return -EINVAL;
> > + }
> > + mutex_unlock(&data->update_lock);
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(data_rate, S_IRUGO | S_IWUSR,
> > + data_rate_show, data_rate_store);
> > +static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> > + power_mode_show, power_mode_store);
> > +
> > +static struct attribute *mid_att_accelero[] = {
> > + &dev_attr_data_rate.attr,
> > + &dev_attr_power_state.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute_group m_accelero_gr = {
> > + .name = "lis331dl",
> > + .attrs = mid_att_accelero
> > +};
> > +
> > +static void accel_set_default_config(struct i2c_client *client)
> > +{
> > + i2c_smbus_write_byte_data(client, 0x20, 0x47);
> > +}
> > +
> > +static int lis331dl_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + int res;
> > + struct lis331dl *data;
> > + struct input_polled_dev *idev;
> > + struct input_dev *input;
> > +
> > + data = kzalloc(sizeof(struct lis331dl), GFP_KERNEL);
> > + if (data == NULL) {
> > + dev_err(&client->dev, "lis331dl: out of memory\n");
> > + return -ENOMEM;
> > + }
> > + mutex_init(&data->update_lock);
> > + i2c_set_clientdata(client, data);
> > + data->client = client;
> > +
> > + res = sysfs_create_group(&client->dev.kobj, &m_accelero_gr);
> > + if (res) {
> > + dev_err(&client->dev, "lis331dl: sysfs group failed\n");
> > + goto accelero_error1;
> > + }
> > + idev = input_allocate_polled_device();
> > + if (!idev) {
> > + res = -ENOMEM;
> > + dev_err(&client->dev,
> > + "lis331dl: unable to register input device\n");
> > + goto accelero_error2;
> > + }
> > + idev->poll = lis331dl_poll;
> > + idev->poll_interval = POLL_INTERVAL;
> > +
> > + idev->private = data;
> > + data->idev = idev;
> > +
> > + input = idev->input;
> > + input->name = "lis331dl";
> > + input->phys = "lis331dl/input0";
> > + input->id.bustype = BUS_I2C;
> > + input->dev.parent = &client->dev;
> > + input->evbit[0] = BIT_MASK(EV_ABS);
> > + input_set_abs_params(input, ABS_X, 0, 255, 2, 2);
> > + input_set_abs_params(input, ABS_Y, 0, 255, 2, 2);
> > + input_set_abs_params(input, ABS_Z, 0, 255, 2, 2);
> > +
> > + res = input_register_polled_device(idev);
> > + if (res == 0) {
> > + dev_info(&client->dev,
> > + "%s lis331dl: Accelerometer chip found",
> > + client->name);
> > + return 0;
> > + }
> > + input_free_polled_device(idev);
> > +accelero_error2:
> > + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
> > +accelero_error1:
> > + i2c_set_clientdata(client, NULL);
> > + kfree(data);
> > + return res;
> > +}
> > +
> > +static int lis331dl_remove(struct i2c_client *client)
> > +{
> > + struct lis331dl *data = i2c_get_clientdata(client);
> > +
> > + input_unregister_polled_device(data->idev);
> > + input_free_polled_device(data->idev);
> > + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
> > + kfree(data);
> > + return 0;
> > +}
> > +
> > +static struct i2c_device_id lis331dl_id[] = {
> > + { "i2c_accel", 0 },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(i2c, lis331dl_id);
> > +
> > +static struct i2c_driver lis331dl_driver = {
> > + .driver = {
> > + .name = "lis331dl",
> > + },
> > + .probe = lis331dl_probe,
> > + .remove = lis331dl_remove,
> > + .id_table = lis331dl_id,
> > +};
> > +
> > +static int __init sensor_lis331dl_init(void)
> > +{
> > + int res;
> > +
> > + res = i2c_add_driver(&lis331dl_driver);
> > + return res;
> > +}
> > +
> > +static void __exit sensor_lis331dl_exit(void)
> > +{
> > + i2c_del_driver(&lis331dl_driver);
> > +}
> > +
> > +module_init(sensor_lis331dl_init);
> > +module_exit(sensor_lis331dl_exit);
> > +
> > +MODULE_AUTHOR("Kalhan Trisal <[email protected]>, Alan Cox");
> > +MODULE_DESCRIPTION("STMicroelectronics LIS331DL Accelerometer Driver");
> > +MODULE_LICENSE("GPL v2");
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-04-14 22:45:51

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 2/4] isl29020: ambient light sensor

On Wed, Apr 14, 2010 at 01:51:49PM +0100, Alan Cox wrote:
> The LS driver will read the latest Lux measurement based upon the
> light brightness and will report the LUX output through sysfs interface.
>
> Signed-off-by: Kalhan Trisal <[email protected]>
> Signed-off-by: Alan Cox <[email protected]>
> ---
>
> drivers/hwmon/Kconfig | 9 ++
> drivers/hwmon/Makefile | 1
> drivers/hwmon/isl29020.c | 243 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 253 insertions(+), 0 deletions(-)
> create mode 100644 drivers/hwmon/isl29020.c

Would it be possible to make the existing driver for the ISL29003
support the ISL29020 as well?

Also note that there is a ALS (ambient light sensor) framework pending.
So patches against the 29003 driver should apply to this tree:

git://git.kernel.org/pub/scm/linux/kernel/git/jic23/als.git

I copied the maintainer to this mail. Jonathan, any plans when ALS will
be merged?

Thanks,
Daniel



> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 74f672d..1fa2533 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1095,6 +1095,15 @@ config SENSORS_HMC6352
> This driver provides support for the Honeywell HMC6352 compass,
> providing configuration and heading data via sysfs.
>
> +config SENSORS_ISL29020
> + tristate "Intersil ISL29020 ALS"
> + depends on I2C
> + help
> + If you say yes here you get support for the ALS Devices
> + Ambient Light Sensor monitoring chip.
> + Range values can be configured using sysfs.
> + Lux data is accessible via sysfs.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index ad2ed36..13d6832 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_SENSORS_HMC6352) += hmc6352.o
> obj-$(CONFIG_SENSORS_I5K_AMB) += i5k_amb.o
> obj-$(CONFIG_SENSORS_IBMAEM) += ibmaem.o
> obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
> +obj-$(CONFIG_SENSORS_ISL29020) += isl29020.o
> obj-$(CONFIG_SENSORS_IT87) += it87.o
> obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o
> obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.o
> diff --git a/drivers/hwmon/isl29020.c b/drivers/hwmon/isl29020.c
> new file mode 100644
> index 0000000..458140d
> --- /dev/null
> +++ b/drivers/hwmon/isl29020.c
> @@ -0,0 +1,243 @@
> +/*
> + * isl29020.c - Intersil ALS Driver
> + *
> + * Copyright (C) 2008 Intel Corp
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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; version 2 of the License.
> + *
> + * 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 <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/hwmon-vid.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>
> +
> +
> +#define ALS_MIN_RANGE_VAL 0
> +#define ALS_MAX_RANGE_VAL 5
> +
> +struct als_data {
> + struct device *hwmon_dev;
> +};
> +
> +static unsigned int i2c_write_current_data(struct i2c_client *client,
> + unsigned int reg, unsigned int value)
> +{
> + int ret_val;
> +
> + ret_val = i2c_smbus_write_byte_data(client, reg, value);
> + return ret_val;
> +}
> +
> +static ssize_t als_sensing_range_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int val;
> +
> + val = i2c_smbus_read_byte_data(client, 0x00);
> + return sprintf(buf, "%d000\n", 1 << (2 * (val & 3)));
> +
> +}
> +
> +static ssize_t als_lux_output_data_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned int ret_val, val;
> + unsigned long int lux, max_count;
> + int tempv1, tempv2;
> +
> + max_count = 65535;
> + tempv1 = i2c_smbus_read_byte_data(client, 0x02); /* MSB data */
> + tempv2 = i2c_smbus_read_byte_data(client, 0x01); /* LSB data */
> + ret_val = tempv1;
> + ret_val = (ret_val << 8 | tempv2);
> + val = i2c_smbus_read_byte_data(client, 0x00);
> + lux = ((((1 << (2 * (val & 3))))*1000) * ret_val) / max_count;
> + return sprintf(buf, "%ld\n", lux);
> +}
> +
> +static ssize_t als_sensing_range_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned int ret_val, set_val = 0;
> + unsigned long val;
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> + ret_val = i2c_smbus_read_byte_data(client, 0x00);
> + ret_val = ret_val & 0xFC; /*reset the bit before setting them */
> + if (val == 1)
> + set_val = (ret_val | 0x00); /* setting the 1:0 bit */
> + else if (val == 2)
> + set_val = (ret_val | 0x01);
> + else if (val == 3)
> + set_val = (ret_val | 0x02);
> + else if (val == 4)
> + set_val = (ret_val | 0x03);
> + else
> + goto invarg;
> + i2c_write_current_data(client, 0x00, set_val);
> + return count;
> +invarg:
> + return -EINVAL;
> +}
> +
> +static ssize_t als_power_status_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int ret_val;
> +
> + ret_val = i2c_smbus_read_byte_data(client, 0x00);
> + ret_val = ret_val & 0x80;
> + if (ret_val == 0x80)
> + ret_val = 1;
> + return sprintf(buf, "%x", ret_val);
> +}
> +
> +static ssize_t als_power_status_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned long val = 0;
> + char curr_val;
> +
> + if (strict_strtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + curr_val = i2c_smbus_read_byte_data(client, 0x00);
> + if (val == 1)
> + curr_val = curr_val | 0x80;
> + else if (val == 0)
> + curr_val = curr_val & 0x7F;
> + else
> + return -EINVAL;
> + i2c_write_current_data(client, 0x00, curr_val);
> + return count;
> +}
> +
> +static DEVICE_ATTR(sensing_range, S_IRUGO | S_IWUSR,
> + als_sensing_range_show, als_sensing_range_store);
> +static DEVICE_ATTR(lux_output, S_IRUGO, als_lux_output_data_show, NULL);
> +static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> + als_power_status_show, als_power_status_store);
> +
> +static struct attribute *mid_att_als[] = {
> + &dev_attr_sensing_range.attr,
> + &dev_attr_lux_output.attr,
> + &dev_attr_power_state.attr,
> + NULL
> +};
> +
> +static struct attribute_group m_als_gr = {
> + .name = "isl29020",
> + .attrs = mid_att_als
> +};
> +
> +static void als_set_default_config(struct i2c_client *client)
> +{
> + i2c_write_current_data(client, 0x00, 0xc0);
> +}
> +
> +static int isl29020_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int res;
> + struct als_data *data;
> +
> + data = kzalloc(sizeof(struct als_data), GFP_KERNEL);
> + if (data == NULL) {
> + printk(KERN_WARNING " isl29020: out of memory");
> + return -ENOMEM;
> + }
> + i2c_set_clientdata(client, data);
> +
> + res = sysfs_create_group(&client->dev.kobj, &m_als_gr);
> + if (res) {
> + printk(KERN_WARNING "isl29020: device create file failed!!\n");
> + goto als_error1;
> + }
> + data->hwmon_dev = hwmon_device_register(&client->dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + res = PTR_ERR(data->hwmon_dev);
> + data->hwmon_dev = NULL;
> + sysfs_remove_group(&client->dev.kobj, &m_als_gr);
> + printk(KERN_ERR "isl29020:unable to register hwmon device\n");
> + goto als_error1;
> + }
> + dev_info(&client->dev, "%s isl29020: ALS chip found\n", client->name);
> + als_set_default_config(client);
> + return res;
> +
> +als_error1:
> + i2c_set_clientdata(client, NULL);
> + kfree(data);
> + return res;
> +}
> +
> +static int isl29020_remove(struct i2c_client *client)
> +{
> + struct als_data *data = i2c_get_clientdata(client);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + sysfs_remove_group(&client->dev.kobj, &m_als_gr);
> + kfree(data);
> + return 0;
> +}
> +
> +static struct i2c_device_id isl29020_id[] = {
> + { "i2c_als", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, isl29020_id);
> +
> +static struct i2c_driver isl29020_driver = {
> + .driver = {
> + .name = "isl29020",
> + },
> + .probe = isl29020_probe,
> + .remove = isl29020_remove,
> + .id_table = isl29020_id,
> +};
> +
> +static int __init sensor_isl29020_init(void)
> +{
> + return i2c_add_driver(&isl29020_driver);
> +}
> +
> +static void __exit sensor_isl29020_exit(void)
> +{
> + i2c_del_driver(&isl29020_driver);
> +}
> +
> +module_init(sensor_isl29020_init);
> +module_exit(sensor_isl29020_exit);
> +
> +MODULE_AUTHOR("Kalhan Trisal <[email protected]");
> +MODULE_DESCRIPTION("Intersil isl29020 ALS Driver");
> +MODULE_LICENSE("GPL v2");
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-04-14 23:05:56

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

On Thu, 15 Apr 2010 00:34:29 +0200
Daniel Mack <[email protected]> wrote:

> On Thu, Apr 15, 2010 at 12:12:21AM +0200, Éric Piel wrote:
> > Op 14-04-10 14:52, Alan Cox schreef:
> > > From: Kalhan Trisal <[email protected]>
> > >
> > > The acceleremeter driver reads the x,y,z coordinate registers and provides
> > > the information to user through the input layer
> > >
> > > Conversion to input device, clean up and porting of retry fixes needed
> > > for AAVA done by Alan Cox.
> > Hello,
> >
> > I wouldn't want to be too picky, if this driver works fine with your
> > hardware and it's tested, why not, but from looking at the spec of this
> > chip (and at the code of this driver), it looks like it could be
> > completely compatible with the lis3lv02d driver (with lis3lv02d_i2c).
> > Same registers, including the WHO_AM_I register which returns 0x3B
> > (which is a supported version).
> >
> > Have you tried and there were some specific incompatibilities? To me, it
> > looks like the only thing not in the lis3lv02d is the read with multiple
> > retries, and it could be easily added if necessary for your hardware.
>
> Yes, that would be better. Also because there's also a SPI version of
> the lis331dl. If support for this chip would be incorporated in the
> existing driver, they would automatically be supported as well.
>

The Openmoko Freerunner has as lis302D which is an SPI version of a very
similar device, so making the one driver work on that was well would be nice
if possible.

The lis302d can detect threshold crossings and taps as well as simple
orientation and I found it useful to include support for those in the driver
as well.
In particular:
1/ I allowed the 'data_rate' that was programmed to be any number (including
fractions and 0) and the driver would report at that rate, using a timer
for rates below 50Hz.
When the app is only interested in long-term change, this can
significantly reduce the amount of data that has to be handled by
userspace.

2/ I allowed a 'threshold' to be set so that changes bigger than that get
notified promptly even if that is faster than the requested data rate

3/ I allowed taps to be detected and reported as BTN_X BTN_Y and BTN_Z
input events.

Would it be sensible to include that support in this driver? The
LIS331DL seem to have the same threshold and tap support.

NeilBrown



> Daniel
>
>
> > > Signed-off-by: Kalhan Trisal <[email protected]>
> > > Signed-off-by: Alan Cox <[email protected]>
> > > ---
> > >
> > > drivers/hwmon/Kconfig | 8 +
> > > drivers/hwmon/Makefile | 1
> > > drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 295 insertions(+), 0 deletions(-)
> > > create mode 100644 drivers/hwmon/lis331dl.c
> > >
> > >
> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > index 1fa2533..6868b9d 100644
> > > --- a/drivers/hwmon/Kconfig
> > > +++ b/drivers/hwmon/Kconfig
> > > @@ -1104,6 +1104,14 @@ config SENSORS_ISL29020
> > > Range values can be configured using sysfs.
> > > Lux data is accessible via sysfs.
> > >
> > > +config SENSORS_LIS331DL
> > > + tristate "STMicroeletronics LIS331DL three-axis digital accelerometer"
> > > + depends on I2C
> > > + help
> > > + If you say yes here you get support for the Accelerometer Devices
> > > + Device can be configured using sysfs.
> > > + x y Z data can be accessible via sysfs.
> > > +
> > > if ACPI
> > >
> > > comment "ACPI drivers"
> > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > > index 13d6832..ebeb2a2 100644
> > > --- a/drivers/hwmon/Makefile
> > > +++ b/drivers/hwmon/Makefile
> > > @@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.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_LIS331DL) += lis331dl.o
> > > obj-$(CONFIG_SENSORS_LM63) += lm63.o
> > > obj-$(CONFIG_SENSORS_LM70) += lm70.o
> > > obj-$(CONFIG_SENSORS_LM73) += lm73.o
> > > diff --git a/drivers/hwmon/lis331dl.c b/drivers/hwmon/lis331dl.c
> > > new file mode 100644
> > > index 0000000..34009eb
> > > --- /dev/null
> > > +++ b/drivers/hwmon/lis331dl.c
> > > @@ -0,0 +1,286 @@
> > > +/*
> > > + * lis331dl.c - ST LIS331DL Accelerometer Driver
> > > + *
> > > + * Copyright (C) 2009 Intel Corp
> > > + *
> > > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > + *
> > > + * 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; version 2 of the License.
> > > + *
> > > + * 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 <linux/module.h>
> > > +#include <linux/init.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/err.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/sysfs.h>
> > > +#include <linux/input-polldev.h>
> > > +
> > > +
> > > +#define POLL_INTERVAL 50
> > > +
> > > +struct lis331dl {
> > > + struct input_polled_dev *idev;
> > > + struct i2c_client *client;
> > > + struct mutex update_lock;
> > > +};
> > > +
> > > +static ssize_t data_rate_show(struct device *dev,
> > > + struct device_attribute *attr, char *buf)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + int val;
> > > + int speed = 100;
> > > +
> > > + val = i2c_smbus_read_byte_data(client, 0x20);
> > > + if (val & 0x80); /* 1= 400HZ 0= 100HZ */
> > > + speed = 400;
> > > + return sprintf(buf, "%d\n", speed);
> > > +}
> > > +
> > > +static ssize_t power_mode_show(struct device *dev,
> > > + struct device_attribute *attr, char *buf)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + int val;
> > > +
> > > + val = i2c_smbus_read_byte_data(client, 0x20) & 0x40;
> > > + if (val == 0x40)
> > > + val = 1;
> > > + return sprintf(buf, "%d\n", val);
> > > +}
> > > +
> > > +static int read_with_retries(struct i2c_client *client, int r)
> > > +{
> > > + int retries;
> > > + int ret;
> > > +
> > > + for (retries = 0; retries < 5; retries++) {
> > > + ret = i2c_smbus_read_byte_data(client, r);
> > > + if (ret != -ETIMEDOUT)
> > > + break;
> > > + msleep(10);
> > > + }
> > > + return ret;
> > > +}
> > > +
> > > +static void lis331dl_poll(struct input_polled_dev *idev)
> > > +{
> > > + struct input_dev *input = idev->input;
> > > + struct lis331dl *data = idev->private;
> > > + int x, y, z;
> > > + struct i2c_client *client = data->client;
> > > +
> > > + x = read_with_retries(client, 0x29);
> > > + y = read_with_retries(client, 0x2B);
> > > + z = read_with_retries(client, 0x2D);
> > > +
> > > + input_report_abs(input, ABS_X, x);
> > > + input_report_abs(input, ABS_Y, y);
> > > + input_report_abs(input, ABS_Z, z);
> > > + input_sync(input);
> > > +}
> > > +
> > > +static ssize_t data_rate_store(struct device *dev,
> > > + struct device_attribute *attr, const char *buf, size_t count)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + struct lis331dl *data = i2c_get_clientdata(client);
> > > + unsigned int ret_val;
> > > + unsigned long val;
> > > +
> > > + if (strict_strtoul(buf, 10, &val))
> > > + return -EINVAL;
> > > +
> > > + mutex_lock(&data->update_lock);
> > > +
> > > + ret_val = i2c_smbus_read_byte_data(client, 0x20);
> > > + ret_val &= 0x7F;
> > > +
> > > + switch (val) {
> > > + case 400:
> > > + ret_val |= 0x80;
> > > + case 100:
> > > + i2c_smbus_write_byte_data(client, 0x20, ret_val);
> > > + break;
> > > + default:
> > > + mutex_unlock(&data->update_lock);
> > > + return -EINVAL;
> > > + }
> > > + mutex_unlock(&data->update_lock);
> > > + return count;
> > > +}
> > > +
> > > +static ssize_t power_mode_store(struct device *dev,
> > > + struct device_attribute *attr, const char *buf, size_t count)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + struct lis331dl *data = i2c_get_clientdata(client);
> > > + unsigned int ret_val;
> > > + unsigned long val;
> > > +
> > > + if (strict_strtoul(buf, 10, &val))
> > > + return -EINVAL;
> > > +
> > > + mutex_lock(&data->update_lock);
> > > +
> > > + ret_val = i2c_smbus_read_byte_data(client, 0x20);
> > > + ret_val &= 0xBF;
> > > +
> > > + switch(val) {
> > > + case 1:
> > > + ret_val |= 0x40;
> > > + case 0:
> > > + i2c_smbus_write_byte_data(client, 0x20, ret_val);
> > > + break;
> > > + default:
> > > + mutex_unlock(&data->update_lock);
> > > + return -EINVAL;
> > > + }
> > > + mutex_unlock(&data->update_lock);
> > > + return count;
> > > +}
> > > +
> > > +static DEVICE_ATTR(data_rate, S_IRUGO | S_IWUSR,
> > > + data_rate_show, data_rate_store);
> > > +static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> > > + power_mode_show, power_mode_store);
> > > +
> > > +static struct attribute *mid_att_accelero[] = {
> > > + &dev_attr_data_rate.attr,
> > > + &dev_attr_power_state.attr,
> > > + NULL
> > > +};
> > > +
> > > +static struct attribute_group m_accelero_gr = {
> > > + .name = "lis331dl",
> > > + .attrs = mid_att_accelero
> > > +};
> > > +
> > > +static void accel_set_default_config(struct i2c_client *client)
> > > +{
> > > + i2c_smbus_write_byte_data(client, 0x20, 0x47);
> > > +}
> > > +
> > > +static int lis331dl_probe(struct i2c_client *client,
> > > + const struct i2c_device_id *id)
> > > +{
> > > + int res;
> > > + struct lis331dl *data;
> > > + struct input_polled_dev *idev;
> > > + struct input_dev *input;
> > > +
> > > + data = kzalloc(sizeof(struct lis331dl), GFP_KERNEL);
> > > + if (data == NULL) {
> > > + dev_err(&client->dev, "lis331dl: out of memory\n");
> > > + return -ENOMEM;
> > > + }
> > > + mutex_init(&data->update_lock);
> > > + i2c_set_clientdata(client, data);
> > > + data->client = client;
> > > +
> > > + res = sysfs_create_group(&client->dev.kobj, &m_accelero_gr);
> > > + if (res) {
> > > + dev_err(&client->dev, "lis331dl: sysfs group failed\n");
> > > + goto accelero_error1;
> > > + }
> > > + idev = input_allocate_polled_device();
> > > + if (!idev) {
> > > + res = -ENOMEM;
> > > + dev_err(&client->dev,
> > > + "lis331dl: unable to register input device\n");
> > > + goto accelero_error2;
> > > + }
> > > + idev->poll = lis331dl_poll;
> > > + idev->poll_interval = POLL_INTERVAL;
> > > +
> > > + idev->private = data;
> > > + data->idev = idev;
> > > +
> > > + input = idev->input;
> > > + input->name = "lis331dl";
> > > + input->phys = "lis331dl/input0";
> > > + input->id.bustype = BUS_I2C;
> > > + input->dev.parent = &client->dev;
> > > + input->evbit[0] = BIT_MASK(EV_ABS);
> > > + input_set_abs_params(input, ABS_X, 0, 255, 2, 2);
> > > + input_set_abs_params(input, ABS_Y, 0, 255, 2, 2);
> > > + input_set_abs_params(input, ABS_Z, 0, 255, 2, 2);
> > > +
> > > + res = input_register_polled_device(idev);
> > > + if (res == 0) {
> > > + dev_info(&client->dev,
> > > + "%s lis331dl: Accelerometer chip found",
> > > + client->name);
> > > + return 0;
> > > + }
> > > + input_free_polled_device(idev);
> > > +accelero_error2:
> > > + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
> > > +accelero_error1:
> > > + i2c_set_clientdata(client, NULL);
> > > + kfree(data);
> > > + return res;
> > > +}
> > > +
> > > +static int lis331dl_remove(struct i2c_client *client)
> > > +{
> > > + struct lis331dl *data = i2c_get_clientdata(client);
> > > +
> > > + input_unregister_polled_device(data->idev);
> > > + input_free_polled_device(data->idev);
> > > + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
> > > + kfree(data);
> > > + return 0;
> > > +}
> > > +
> > > +static struct i2c_device_id lis331dl_id[] = {
> > > + { "i2c_accel", 0 },
> > > + { }
> > > +};
> > > +
> > > +MODULE_DEVICE_TABLE(i2c, lis331dl_id);
> > > +
> > > +static struct i2c_driver lis331dl_driver = {
> > > + .driver = {
> > > + .name = "lis331dl",
> > > + },
> > > + .probe = lis331dl_probe,
> > > + .remove = lis331dl_remove,
> > > + .id_table = lis331dl_id,
> > > +};
> > > +
> > > +static int __init sensor_lis331dl_init(void)
> > > +{
> > > + int res;
> > > +
> > > + res = i2c_add_driver(&lis331dl_driver);
> > > + return res;
> > > +}
> > > +
> > > +static void __exit sensor_lis331dl_exit(void)
> > > +{
> > > + i2c_del_driver(&lis331dl_driver);
> > > +}
> > > +
> > > +module_init(sensor_lis331dl_init);
> > > +module_exit(sensor_lis331dl_exit);
> > > +
> > > +MODULE_AUTHOR("Kalhan Trisal <[email protected]>, Alan Cox");
> > > +MODULE_DESCRIPTION("STMicroelectronics LIS331DL Accelerometer Driver");
> > > +MODULE_LICENSE("GPL v2");
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at http://www.tux.org/lkml/
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-04-14 23:08:35

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2/4] isl29020: ambient light sensor

> Would it be possible to make the existing driver for the ISL29003
> support the ISL29020 as well?

I dug the manuals out for these to take a look - the answer is they are
quite different chips.

Alan

2010-04-14 23:10:05

by Alan

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

> I wouldn't want to be too picky, if this driver works fine with your
> hardware and it's tested, why not, but from looking at the spec of
> this chip (and at the code of this driver), it looks like it could be
> completely compatible with the lis3lv02d driver (with lis3lv02d_i2c).
> Same registers, including the WHO_AM_I register which returns 0x3B
> (which is a supported version).

I will take a look in detail. The IRQ bit is different and as you say
the retries can be added to it anyway.

>
> Have you tried and there were some specific incompatibilities?

I am picking this project up rather than being directly responsible for
its history - chances are nobody has.

Alan

2010-04-15 06:20:33

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 2/4] isl29020: ambient light sensor

On Wed, Apr 14, 2010 at 11:35:54PM +0100, Alan Cox wrote:
> > Would it be possible to make the existing driver for the ISL29003
> > support the ISL29020 as well?
>
> I dug the manuals out for these to take a look - the answer is they are
> quite different chips.

Hmm, sad :(

However, the driver should be applied to the ALS tree after all, unless
Jonathan plans to drop the whole thing, which I doubt.

Thanks,
Daniel

2010-04-15 09:47:46

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

On 15/04/10 01:05, Neil Brown wrote:
:
>
> The Openmoko Freerunner has as lis302D which is an SPI version of a very
> similar device, so making the one driver work on that was well would be nice
> if possible.
Definitely!

>
> The lis302d can detect threshold crossings and taps as well as simple
> orientation and I found it useful to include support for those in the driver
> as well.
> In particular:
> 1/ I allowed the 'data_rate' that was programmed to be any number (including
> fractions and 0) and the driver would report at that rate, using a timer
> for rates below 50Hz.
> When the app is only interested in long-term change, this can
> significantly reduce the amount of data that has to be handled by
> userspace.
Samu (from Nokia) has started to had such idea via the patch
lis3-setup-poll-interval-limits.patch which in now in the -mm tree. It
allows to specify the polling rate via the input interface
(/sys/devices/platform/lis3lv02d/input/input8/poll). But for now I don't
think the actual refresh rate is updated automatically, it would be a
nice addition.


>
> 2/ I allowed a 'threshold' to be set so that changes bigger than that get
> notified promptly even if that is faster than the requested data rate
It could be interesting too... but what interface are you using for the
userspace to specify this threshold? A sysfs file?

>
> 3/ I allowed taps to be detected and reported as BTN_X BTN_Y and BTN_Z
> input events.
This one should now be working with Samu's patch
lis3-interrupt-handlers-for-8bit-wakeup-and-click-events.patch also in
the -mm tree.

>
> Would it be sensible to include that support in this driver? The
> LIS331DL seem to have the same threshold and tap support.
So all in all, if you could have a look at what changes are necessary to
get the lis3lv02d driver work with your hardware, it would be great. (I
suspect it should be quite little.)

See you,
Eric

2010-04-15 10:13:07

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/4] isl29020: ambient light sensor

On 04/15/10 07:20, Daniel Mack wrote:
> On Wed, Apr 14, 2010 at 11:35:54PM +0100, Alan Cox wrote:
>>> Would it be possible to make the existing driver for the ISL29003
>>> support the ISL29020 as well?
>>
>> I dug the manuals out for these to take a look - the answer is they are
>> quite different chips.
>
> Hmm, sad :(
>
> However, the driver should be applied to the ALS tree after all, unless
> Jonathan plans to drop the whole thing, which I doubt.
Sadly ALS is dead. Linus made it pretty clear he wasn't going to pull it.

Hence currently either ALS drivers are going into misc (and can be moved
elsewhere later), or we are taking them into IIO (and hence staging)
where they fit fine and we can sort out a bridge to input to answer Linus'
issue with the original patch set.

In the thread following Alan's repost (having moved this driver to misc)
Greg just pointed out the sysfs interface needed documenting, and I've
suggested that we sort out the naming properly (in a way compliant with
hwmon and the new IIO abi. 90% of what the ALS subsystem contributed was
defining the ABI anyway!

Jonathan

2010-04-15 10:26:11

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

On 04/14/10 23:12, Éric Piel wrote:
> Op 14-04-10 14:52, Alan Cox schreef:
>> From: Kalhan Trisal <[email protected]>
>>
>> The acceleremeter driver reads the x,y,z coordinate registers and provides
>> the information to user through the input layer
>>
>> Conversion to input device, clean up and porting of retry fixes needed
>> for AAVA done by Alan Cox.
> Hello,
>
> I wouldn't want to be too picky, if this driver works fine with your
> hardware and it's tested, why not, but from looking at the spec of this
> chip (and at the code of this driver), it looks like it could be
> completely compatible with the lis3lv02d driver (with lis3lv02d_i2c).
> Same registers, including the WHO_AM_I register which returns 0x3B
> (which is a supported version).
>
> Have you tried and there were some specific incompatibilities? To me, it
> looks like the only thing not in the lis3lv02d is the read with multiple
> retries, and it could be easily added if necessary for your hardware.
>
> Cheers,
> Eric
Err. Anyone get a feeling of deja vu here?

http://lists.lm-sensors.org/pipermail/lm-sensors/2009-September/026706.html

When Kalhan originally posted this driver it was pointed out that it was compatible
with the existing one. A complete lack of communications lead to Kalhan (and someone
else, might have been Eric or Samu, can't recall) both implementing i2c support
in the driver. Can't find it right now but I'm fairly sure Kahlan reported that worked
fine for this chip as well?

So looks like a lack of communications here and that Alan has picked up an unnecessary
driver.

Jonathan
>
>>
>> Signed-off-by: Kalhan Trisal <[email protected]>
>> Signed-off-by: Alan Cox <[email protected]>
>> ---
>>
>> drivers/hwmon/Kconfig | 8 +
>> drivers/hwmon/Makefile | 1
>> drivers/hwmon/lis331dl.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 295 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/hwmon/lis331dl.c
>>
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 1fa2533..6868b9d 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1104,6 +1104,14 @@ config SENSORS_ISL29020
>> Range values can be configured using sysfs.
>> Lux data is accessible via sysfs.
>>
>> +config SENSORS_LIS331DL
>> + tristate "STMicroeletronics LIS331DL three-axis digital accelerometer"
>> + depends on I2C
>> + help
>> + If you say yes here you get support for the Accelerometer Devices
>> + Device can be configured using sysfs.
>> + x y Z data can be accessible via sysfs.
>> +
>> if ACPI
>>
>> comment "ACPI drivers"
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 13d6832..ebeb2a2 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.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_LIS331DL) += lis331dl.o
>> obj-$(CONFIG_SENSORS_LM63) += lm63.o
>> obj-$(CONFIG_SENSORS_LM70) += lm70.o
>> obj-$(CONFIG_SENSORS_LM73) += lm73.o
>> diff --git a/drivers/hwmon/lis331dl.c b/drivers/hwmon/lis331dl.c
>> new file mode 100644
>> index 0000000..34009eb
>> --- /dev/null
>> +++ b/drivers/hwmon/lis331dl.c
>> @@ -0,0 +1,286 @@
>> +/*
>> + * lis331dl.c - ST LIS331DL Accelerometer Driver
>> + *
>> + * Copyright (C) 2009 Intel Corp
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + *
>> + * 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; version 2 of the License.
>> + *
>> + * 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 <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/slab.h>
>> +#include <linux/i2c.h>
>> +#include <linux/err.h>
>> +#include <linux/delay.h>
>> +#include <linux/mutex.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/input-polldev.h>
>> +
>> +
>> +#define POLL_INTERVAL 50
>> +
>> +struct lis331dl {
>> + struct input_polled_dev *idev;
>> + struct i2c_client *client;
>> + struct mutex update_lock;
>> +};
>> +
>> +static ssize_t data_rate_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> + int val;
>> + int speed = 100;
>> +
>> + val = i2c_smbus_read_byte_data(client, 0x20);
>> + if (val & 0x80); /* 1= 400HZ 0= 100HZ */
>> + speed = 400;
>> + return sprintf(buf, "%d\n", speed);
>> +}
>> +
>> +static ssize_t power_mode_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> + int val;
>> +
>> + val = i2c_smbus_read_byte_data(client, 0x20) & 0x40;
>> + if (val == 0x40)
>> + val = 1;
>> + return sprintf(buf, "%d\n", val);
>> +}
>> +
>> +static int read_with_retries(struct i2c_client *client, int r)
>> +{
>> + int retries;
>> + int ret;
>> +
>> + for (retries = 0; retries < 5; retries++) {
>> + ret = i2c_smbus_read_byte_data(client, r);
>> + if (ret != -ETIMEDOUT)
>> + break;
>> + msleep(10);
>> + }
>> + return ret;
>> +}
>> +
>> +static void lis331dl_poll(struct input_polled_dev *idev)
>> +{
>> + struct input_dev *input = idev->input;
>> + struct lis331dl *data = idev->private;
>> + int x, y, z;
>> + struct i2c_client *client = data->client;
>> +
>> + x = read_with_retries(client, 0x29);
>> + y = read_with_retries(client, 0x2B);
>> + z = read_with_retries(client, 0x2D);
>> +
>> + input_report_abs(input, ABS_X, x);
>> + input_report_abs(input, ABS_Y, y);
>> + input_report_abs(input, ABS_Z, z);
>> + input_sync(input);
>> +}
>> +
>> +static ssize_t data_rate_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> + struct lis331dl *data = i2c_get_clientdata(client);
>> + unsigned int ret_val;
>> + unsigned long val;
>> +
>> + if (strict_strtoul(buf, 10, &val))
>> + return -EINVAL;
>> +
>> + mutex_lock(&data->update_lock);
>> +
>> + ret_val = i2c_smbus_read_byte_data(client, 0x20);
>> + ret_val &= 0x7F;
>> +
>> + switch (val) {
>> + case 400:
>> + ret_val |= 0x80;
>> + case 100:
>> + i2c_smbus_write_byte_data(client, 0x20, ret_val);
>> + break;
>> + default:
>> + mutex_unlock(&data->update_lock);
>> + return -EINVAL;
>> + }
>> + mutex_unlock(&data->update_lock);
>> + return count;
>> +}
>> +
>> +static ssize_t power_mode_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> + struct lis331dl *data = i2c_get_clientdata(client);
>> + unsigned int ret_val;
>> + unsigned long val;
>> +
>> + if (strict_strtoul(buf, 10, &val))
>> + return -EINVAL;
>> +
>> + mutex_lock(&data->update_lock);
>> +
>> + ret_val = i2c_smbus_read_byte_data(client, 0x20);
>> + ret_val &= 0xBF;
>> +
>> + switch(val) {
>> + case 1:
>> + ret_val |= 0x40;
>> + case 0:
>> + i2c_smbus_write_byte_data(client, 0x20, ret_val);
>> + break;
>> + default:
>> + mutex_unlock(&data->update_lock);
>> + return -EINVAL;
>> + }
>> + mutex_unlock(&data->update_lock);
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(data_rate, S_IRUGO | S_IWUSR,
>> + data_rate_show, data_rate_store);
>> +static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
>> + power_mode_show, power_mode_store);
>> +
>> +static struct attribute *mid_att_accelero[] = {
>> + &dev_attr_data_rate.attr,
>> + &dev_attr_power_state.attr,
>> + NULL
>> +};
>> +
>> +static struct attribute_group m_accelero_gr = {
>> + .name = "lis331dl",
>> + .attrs = mid_att_accelero
>> +};
>> +
>> +static void accel_set_default_config(struct i2c_client *client)
>> +{
>> + i2c_smbus_write_byte_data(client, 0x20, 0x47);
>> +}
>> +
>> +static int lis331dl_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + int res;
>> + struct lis331dl *data;
>> + struct input_polled_dev *idev;
>> + struct input_dev *input;
>> +
>> + data = kzalloc(sizeof(struct lis331dl), GFP_KERNEL);
>> + if (data == NULL) {
>> + dev_err(&client->dev, "lis331dl: out of memory\n");
>> + return -ENOMEM;
>> + }
>> + mutex_init(&data->update_lock);
>> + i2c_set_clientdata(client, data);
>> + data->client = client;
>> +
>> + res = sysfs_create_group(&client->dev.kobj, &m_accelero_gr);
>> + if (res) {
>> + dev_err(&client->dev, "lis331dl: sysfs group failed\n");
>> + goto accelero_error1;
>> + }
>> + idev = input_allocate_polled_device();
>> + if (!idev) {
>> + res = -ENOMEM;
>> + dev_err(&client->dev,
>> + "lis331dl: unable to register input device\n");
>> + goto accelero_error2;
>> + }
>> + idev->poll = lis331dl_poll;
>> + idev->poll_interval = POLL_INTERVAL;
>> +
>> + idev->private = data;
>> + data->idev = idev;
>> +
>> + input = idev->input;
>> + input->name = "lis331dl";
>> + input->phys = "lis331dl/input0";
>> + input->id.bustype = BUS_I2C;
>> + input->dev.parent = &client->dev;
>> + input->evbit[0] = BIT_MASK(EV_ABS);
>> + input_set_abs_params(input, ABS_X, 0, 255, 2, 2);
>> + input_set_abs_params(input, ABS_Y, 0, 255, 2, 2);
>> + input_set_abs_params(input, ABS_Z, 0, 255, 2, 2);
>> +
>> + res = input_register_polled_device(idev);
>> + if (res == 0) {
>> + dev_info(&client->dev,
>> + "%s lis331dl: Accelerometer chip found",
>> + client->name);
>> + return 0;
>> + }
>> + input_free_polled_device(idev);
>> +accelero_error2:
>> + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
>> +accelero_error1:
>> + i2c_set_clientdata(client, NULL);
>> + kfree(data);
>> + return res;
>> +}
>> +
>> +static int lis331dl_remove(struct i2c_client *client)
>> +{
>> + struct lis331dl *data = i2c_get_clientdata(client);
>> +
>> + input_unregister_polled_device(data->idev);
>> + input_free_polled_device(data->idev);
>> + sysfs_remove_group(&client->dev.kobj, &m_accelero_gr);
>> + kfree(data);
>> + return 0;
>> +}
>> +
>> +static struct i2c_device_id lis331dl_id[] = {
>> + { "i2c_accel", 0 },
>> + { }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, lis331dl_id);
>> +
>> +static struct i2c_driver lis331dl_driver = {
>> + .driver = {
>> + .name = "lis331dl",
>> + },
>> + .probe = lis331dl_probe,
>> + .remove = lis331dl_remove,
>> + .id_table = lis331dl_id,
>> +};
>> +
>> +static int __init sensor_lis331dl_init(void)
>> +{
>> + int res;
>> +
>> + res = i2c_add_driver(&lis331dl_driver);
>> + return res;
>> +}
>> +
>> +static void __exit sensor_lis331dl_exit(void)
>> +{
>> + i2c_del_driver(&lis331dl_driver);
>> +}
>> +
>> +module_init(sensor_lis331dl_init);
>> +module_exit(sensor_lis331dl_exit);
>> +
>> +MODULE_AUTHOR("Kalhan Trisal <[email protected]>, Alan Cox");
>> +MODULE_DESCRIPTION("STMicroelectronics LIS331DL Accelerometer Driver");
>> +MODULE_LICENSE("GPL v2");
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-04-15 10:29:33

by Jonathan Cameron

[permalink] [raw]
Subject: Re: isl29020: ALS driver as misc device

On 04/14/10 18:01, Greg KH wrote:
> On Wed, Apr 14, 2010 at 05:56:02PM +0100, Alan Cox wrote:
>> And this adds the isl29020 as a misc device per discussions
>>
>> isl29020: ambient light sensor
>>
>> From: Kalhan Trisal <[email protected]>
>>
>> The LS driver will read the latest Lux measurement based upon the
>> light brightness and will report the LUX output through sysfs interface.
>
> Please document this sysfs interface with an addition to the
> Documentatin/ABI/ directory.
As documenting this abi (which indeed should be done) is going to set
a precedence, perhaps this is a good time to discuss what this naming could
be.

In cleaning up the IIO abi Greg suggested that we match existing similar
abi's a closely as possible, which where possible makes a great deal of sense
(shared userspace code is possible and it makes everything a bit more predictable
for driver writers... particularly as I expect someone will sooner or later
make a combined hwmon and als chip).

The obvious similarity here is with hwmon.
So perhaps going with naming as

lux-> illuminance0_input (or I guess lux0_input would also work, I can change
the iio abi to match this as well).

It also occurs to me that we might want to associate the calibration with the
particular channel? There's sure to be a dual ALS chip along at some point.

Obviously the isl29020 would need updating as well. Everyone was happy to do
that when were writing the ALS subsystem, so I guess that won't have changed!

Jonathan

2010-04-15 10:35:24

by Alan

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

> Err. Anyone get a feeling of deja vu here?
>
> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-September/026706.html
>
> When Kalhan originally posted this driver it was pointed out that it
> was compatible with the existing one. A complete lack of
> communications lead to Kalhan (and someone else, might have been Eric
> or Samu, can't recall) both implementing i2c support in the driver.
> Can't find it right now but I'm fairly sure Kahlan reported that
> worked fine for this chip as well?
>
> So looks like a lack of communications here and that Alan has picked
> up an unnecessary driver.

Thanks it does indeed - except for the retry logic which has come from
another platform source and can be submitted in turn. Only other thing I
see odd is that the existing driver prints a scary error message when
there is no IRQ rather than just an info message.

Alan

2010-04-15 11:13:12

by Alan

[permalink] [raw]
Subject: Re: isl29020: ALS driver as misc device

> lux-> illuminance0_input (or I guess lux0_input would also work, I can change
> the iio abi to match this as well).
>
> It also occurs to me that we might want to associate the calibration with the
> particular channel? There's sure to be a dual ALS chip along at some point.
>
> Obviously the isl29020 would need updating as well. Everyone was happy to do
> that when were writing the ALS subsystem, so I guess that won't have changed!

I'm happy to make the 29020 comply, and I guess it wouldn't take long for
someone to run over the others. Just let me know what the sysfs interface
should look like.

2010-04-15 12:07:44

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

> Argument alignment

Makes it take more space

> Spacing

Mostly dropped

> Make chars static

Not sure I see the point (compiler generates good code anyway)

> Remove prefixes from dev_<level> logging

Good point.

2010-04-15 15:57:20

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/4] hmc6352: Add driver for the HMC6352 compass

On Thu, 2010-04-15 at 13:11 +0100, Alan Cox wrote:
> > Argument alignment
> Makes it take more space

Makes it look more like most kernel code.

> > Make chars static
> Not sure I see the point (compiler generates good code anyway)

Internal consistency. 2 use areas had it, 1 didn't.


2010-04-16 05:23:58

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 3/4] liss331d1: accelerometer driver

On Wed 2010-04-14 13:52:00, Alan Cox wrote:
> From: Kalhan Trisal <[email protected]>
>
> The acceleremeter driver reads the x,y,z coordinate registers and provides
> the information to user through the input layer
>
> Conversion to input device, clean up and porting of retry fixes needed
> for AAVA done by Alan Cox.

Out of interest, what is AAVA?
Pavel

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

2010-04-26 10:56:48

by Jonathan Cameron

[permalink] [raw]
Subject: Re: isl29020: ALS driver as misc device

On 04/15/10 12:17, Alan Cox wrote:
>> lux-> illuminance0_input (or I guess lux0_input would also work, I can change
>> the iio abi to match this as well).
>>
>> It also occurs to me that we might want to associate the calibration with the
>> particular channel? There's sure to be a dual ALS chip along at some point.
>>
>> Obviously the isl29020 would need updating as well. Everyone was happy to do
>> that when were writing the ALS subsystem, so I guess that won't have changed!
>
> I'm happy to make the 29020 comply, and I guess it wouldn't take long for
> someone to run over the others. Just let me know what the sysfs interface
> should look like.

Hi Alan,

I was hoping some others would jump in here with suggestions. Failing that, I'd
go with

illuminance0_input (in milli lux?)
(if this requires a float calculation, could go with the iio approach of having
illuminance0_raw and illuminance0_scale, where gain can be a float as it's just
a text string anyway).

illuminance0_range
illuminance0_range_available

For these range, I'd personally prefer to do it via scaling, but we can probably
be flexible here. Things will get complex if we start having ranges like 3...10 lux.

So illuminance0_calibscale (hence internal scale - obviously this will also effect
illuminance0_scale if you are going the 'raw' output and convert in userspace route).
Clearly, you would also need an illuminance0_calibscale_available interface to tell
you what discrete settings exist.

Jonathan