* [PATCH v2 0/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
@ 2014-08-01 3:02 Kuppuswamy Sathyanarayanan
2014-08-01 3:02 ` [PATCH v2 1/1] " Kuppuswamy Sathyanarayanan
2014-08-01 7:03 ` [PATCH v2 0/1] " Jonathan Cameron
0 siblings, 2 replies; 7+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2014-08-01 3:02 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, srinivas.pandruvada, sathyanarayanan.kuppuswamy
Please let me know your review comments.
PATCH v2 changes:
1. Combined ALS/PXS enable/disable functions into a single routine.
2. Removed extra debug messages.
3. Also addressed review comments from Peter Meerwald
Kuppuswamy Sathyanarayanan (1):
iio: jsa1212: Add JSA1212 proximity/ALS sensor
drivers/iio/light/Kconfig | 10 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/jsa1212.c | 490 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 501 insertions(+)
create mode 100644 drivers/iio/light/jsa1212.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
2014-08-01 3:02 [PATCH v2 0/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor Kuppuswamy Sathyanarayanan
@ 2014-08-01 3:02 ` Kuppuswamy Sathyanarayanan
2014-08-01 7:02 ` Jonathan Cameron
2014-08-07 14:06 ` Jonathan Cameron
2014-08-01 7:03 ` [PATCH v2 0/1] " Jonathan Cameron
1 sibling, 2 replies; 7+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2014-08-01 3:02 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, srinivas.pandruvada, sathyanarayanan.kuppuswamy
This patch adds a new driver for solteam opto JSA1212 proximity and
ambient light sensor.
Basic details of the chip can be found here.
http://www.solteamopto.com.tw/detail.php?ms=3&po_unit=2&pt_unit=29&p_unit=120
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/iio/light/Kconfig | 10 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/jsa1212.c | 490 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 501 insertions(+)
create mode 100644 drivers/iio/light/jsa1212.c
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index c89740d..07d0a76 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -87,6 +87,16 @@ config HID_SENSOR_PROX
To compile this driver as a module, choose M here: the
module will be called hid-sensor-prox.
+config JSA1212
+ tristate "JSA1212 ALS and proximity sensor driver"
+ depends on I2C
+ help
+ Say Y here if you want to build a IIO driver for JSA1212
+ proximity & ALS sensor device.
+
+ To compile this driver as a module, choose M here:
+ the module will be called jsa1212.
+
config SENSORS_LM3533
tristate "LM3533 ambient light sensor"
depends on MFD_LM3533
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 3eb36e5..ce85c86 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_CM36651) += cm36651.o
obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o
obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o
obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
+obj-$(CONFIG_JSA1212) += jsa1212.o
obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
obj-$(CONFIG_LTR501) += ltr501.o
obj-$(CONFIG_SENSORS_TSL2563) += tsl2563.o
diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
new file mode 100644
index 0000000..1597183
--- /dev/null
+++ b/drivers/iio/light/jsa1212.c
@@ -0,0 +1,490 @@
+/*
+ * JSA1212 Ambient Light & Proximity Sensor Driver
+ *
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+/* JSA1212 reg address */
+#define JSA1212_CONF_REG 0x01
+#define JSA1212_INT_REG 0x02
+#define JSA1212_PXS_LT_REG 0x03
+#define JSA1212_PXS_HT_REG 0x04
+#define JSA1212_ALS_TH1_REG 0x05
+#define JSA1212_ALS_TH2_REG 0x06
+#define JSA1212_ALS_TH3_REG 0x07
+#define JSA1212_PXS_DATA_REG 0x08
+#define JSA1212_ALS_DT1_REG 0x09
+#define JSA1212_ALS_DT2_REG 0x0A
+#define JSA1212_ALS_RNG_REG 0x0B
+#define JSA1212_MAX_REG 0x0C
+
+/* JSA1212 reg defaults */
+#define JSA1212_CONF_REG_DEF 0x58
+#define JSA1212_INT_REG_DEF 0x02
+#define JSA1212_PXS_LT_REG_DEF 0x00
+#define JSA1212_PXS_HT_REG_DEF 0xFF
+#define JSA1212_ALS_TH1_REG_DEF 0x00
+#define JSA1212_ALS_TH2_REG_DEF 0xF0
+#define JSA1212_ALS_TH3_REG_DEF 0xFF
+#define JSA1212_ALS_RNG_REG_DEF 0x00
+
+/* JSA1212 reg masks */
+#define JSA1212_CONF_MASK 0xFF
+#define JSA1212_INT_MASK 0xFF
+#define JSA1212_PXS_LT_MASK 0xFF
+#define JSA1212_PXS_HT_MASK 0xFF
+#define JSA1212_ALS_TH1_MASK 0xFF
+#define JSA1212_ALS_TH2_LT_MASK 0x0F
+#define JSA1212_ALS_TH2_HT_MASK 0xF0
+#define JSA1212_ALS_TH3_MASK 0xFF
+#define JSA1212_PXS_DATA_MASK 0xFF
+#define JSA1212_ALS_DATA_MASK 0xFFF
+#define JSA1212_ALS_DT1_MASK 0xFF
+#define JSA1212_ALS_DT2_MASK 0x0F
+#define JSA1212_ALS_RNG_MASK 0x07
+#define JSA1212_REG_MASK 0xFF
+
+/* JSA1212 CONF REG bits */
+#define JSA1212_CONF_PXS_MASK 0x80
+#define JSA1212_CONF_PXS_ENABLE 0x80
+#define JSA1212_CONF_PXS_DISABLE 0x00
+#define JSA1212_CONF_ALS_MASK 0x04
+#define JSA1212_CONF_ALS_ENABLE 0x04
+#define JSA1212_CONF_ALS_DISABLE 0x00
+
+/* JSA1212 INT REG bits */
+#define JSA1212_INT_CTRL_MASK 0x01
+#define JSA1212_INT_CTRL_EITHER 0x00
+#define JSA1212_INT_CTRL_BOTH 0x01
+#define JSA1212_INT_ALS_PRST_MASK 0x06
+#define JSA1212_INT_ALS_PRST_1CONV 0x00
+#define JSA1212_INT_ALS_PRST_4CONV 0x02
+#define JSA1212_INT_ALS_PRST_8CONV 0x04
+#define JSA1212_INT_ALS_PRST_16CONV 0x06
+#define JSA1212_INT_ALS_FLAG_MASK 0x08
+#define JSA1212_INT_ALS_FLAG_CLR 0x00
+#define JSA1212_INT_PXS_PRST_MASK 0x60
+#define JSA1212_INT_PXS_PRST_1CONV 0x00
+#define JSA1212_INT_PXS_PRST_4CONV 0x20
+#define JSA1212_INT_PXS_PRST_8CONV 0x40
+#define JSA1212_INT_PXS_PRST_16CONV 0x60
+#define JSA1212_INT_PXS_FLAG_MASK 0x80
+#define JSA1212_INT_PXS_FLAG_CLR 0x00
+
+/* JSA1212 ALS RNG REG bits */
+#define JSA1212_ALS_RNG_0_2048 0x00
+#define JSA1212_ALS_RNG_0_1024 0x01
+#define JSA1212_ALS_RNG_0_512 0x02
+#define JSA1212_ALS_RNG_0_256 0x03
+#define JSA1212_ALS_RNG_0_128 0x04
+
+/* JSA1212 INT threshold range */
+#define JSA1212_ALS_TH_MIN 0x0000
+#define JSA1212_ALS_TH_MAX 0x0FFF
+#define JSA1212_PXS_TH_MIN 0x00
+#define JSA1212_PXS_TH_MAX 0xFF
+
+#define JSA1212_ALS_DELAY_MS 0xC8
+#define JSA1212_PXS_DELAY_MS 0x64
+
+enum jsa1212_op_mode {
+ JSA1212_OPMODE_ALS_EN,
+ JSA1212_OPMODE_PXS_EN,
+};
+
+struct jsa1212_data {
+ struct i2c_client *client;
+ struct mutex lock;
+ u8 als_rng_idx;
+ unsigned long flags;
+};
+
+/* ALS range idx to val mapping */
+static const int jsa1212_als_range_val[] = {2048, 1024, 512, 256, 128,
+ 128, 128, 128};
+
+static int jsa1212_reg_update(struct jsa1212_data *data, u8 reg, u8 bit_mask,
+ u8 new_val)
+{
+ u8 old_cmd, new_cmd;
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, reg);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "%s: read reg (0X%02X) cmd failed\n",
+ __func__, reg);
+ return ret;
+ }
+
+ old_cmd = ret & JSA1212_REG_MASK;
+ new_cmd = old_cmd & ~bit_mask;
+ new_cmd |= new_val & bit_mask;
+
+ if (new_cmd != old_cmd) {
+ ret = i2c_smbus_write_byte_data(data->client, reg, new_cmd);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "%s: write reg (0X%02X) cmd (0X%02X) failed\n",
+ __func__, reg, new_cmd);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/* Enables or disables ALS function based on status */
+static int jsa1212_als_enable(struct jsa1212_data *data, u8 status)
+{
+ int ret;
+
+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
+ JSA1212_CONF_ALS_MASK,
+ status);
+ if (ret < 0)
+ return ret;
+
+ if (status)
+ set_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
+ else
+ clear_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
+
+ return ret;
+}
+
+/* Enables or disables PXS function based on status */
+static int jsa1212_pxs_enable(struct jsa1212_data *data, u8 status)
+{
+ int ret;
+
+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
+ JSA1212_CONF_PXS_MASK,
+ status);
+ if (ret < 0)
+ return ret;
+
+ if (status)
+ set_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
+ else
+ clear_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
+
+ return ret;
+}
+
+static int jsa1212_read_als_data(struct jsa1212_data *data,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
+ if (ret < 0)
+ return ret;
+
+ /* Delay for data output */
+ msleep(JSA1212_ALS_DELAY_MS);
+
+ /* Read 12 bit data */
+ ret = i2c_smbus_read_word_data(data->client, JSA1212_ALS_DT1_REG);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "als data read err\n");
+ return ret;
+ }
+
+ *val = ret & JSA1212_ALS_DATA_MASK;
+
+ ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_DISABLE);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int jsa1212_read_pxs_data(struct jsa1212_data *data,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
+ if (ret < 0)
+ return ret;
+
+ /* Delay for data output */
+ msleep(JSA1212_PXS_DELAY_MS);
+
+ /* Read out all data */
+ ret = i2c_smbus_read_byte_data(data->client, JSA1212_PXS_DATA_REG);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "pxs data read err\n");
+ return ret;
+ }
+
+ *val = ret & JSA1212_PXS_DATA_MASK;
+
+ ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_DISABLE);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int jsa1212_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ int ret;
+ struct jsa1212_data *data = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(&data->lock);
+ switch (chan->type) {
+ case IIO_LIGHT:
+ ret = jsa1212_read_als_data(data, val);
+ break;
+ case IIO_PROXIMITY:
+ ret = jsa1212_read_pxs_data(data, val);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ ret = ret < 0 ? ret : IIO_VAL_INT;
+ mutex_unlock(&data->lock);
+ break;
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_LIGHT:
+ *val = jsa1212_als_range_val[data->als_rng_idx];
+ *val2 = BIT(12); /* Max 12 bit value */
+ ret = IIO_VAL_FRACTIONAL;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static const struct iio_chan_spec jsa1212_channels[] = {
+ {
+ .type = IIO_LIGHT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ },
+ {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ }
+};
+
+static const struct iio_info jsa1212_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = &jsa1212_read_raw,
+};
+
+static int jsa1212_chip_init(struct jsa1212_data *data)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(data->client, JSA1212_CONF_REG,
+ JSA1212_CONF_REG_DEF);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(data->client, JSA1212_INT_REG,
+ JSA1212_INT_REG_DEF);
+ if (ret < 0)
+ return ret;
+
+ data->als_rng_idx = JSA1212_ALS_RNG_0_2048;
+ data->flags = 0x00;
+
+ return 0;
+}
+
+static int jsa1212_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct jsa1212_data *jsa1212_data;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+ return -ENODEV;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*jsa1212_data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ jsa1212_data = iio_priv(indio_dev);
+
+ i2c_set_clientdata(client, indio_dev);
+ jsa1212_data->client = client;
+
+ mutex_init(&jsa1212_data->lock);
+
+ ret = jsa1212_chip_init(jsa1212_data);
+ if (ret < 0)
+ return ret;
+
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->channels = jsa1212_channels;
+ indio_dev->num_channels = ARRAY_SIZE(jsa1212_channels);
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ indio_dev->info = &jsa1212_info;
+
+ ret = iio_device_register(indio_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "%s: register device failed\n", __func__);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int jsa1212_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct jsa1212_data *data = iio_priv(indio_dev);
+ int ret;
+
+ mutex_lock(&data->lock);
+
+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
+ JSA1212_CONF_ALS_MASK |
+ JSA1212_CONF_PXS_MASK,
+ JSA1212_CONF_ALS_DISABLE |
+ JSA1212_CONF_PXS_DISABLE);
+
+ if (ret < 0)
+ dev_err(&client->dev, "power off cmd failed\n");
+
+ mutex_unlock(&data->lock);
+
+ iio_device_unregister(indio_dev);
+
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int jsa1212_suspend(struct device *dev)
+{
+ int ret;
+ struct jsa1212_data *data;
+
+ data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+ mutex_lock(&data->lock);
+
+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
+ JSA1212_CONF_ALS_MASK |
+ JSA1212_CONF_PXS_MASK,
+ JSA1212_CONF_ALS_DISABLE |
+ JSA1212_CONF_PXS_DISABLE);
+
+ if (ret < 0)
+ dev_err(dev, "suspend cmd failed\n");
+
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int jsa1212_resume(struct device *dev)
+{
+ int ret = 0;
+ struct jsa1212_data *data;
+
+ data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+ mutex_lock(&data->lock);
+
+ if (test_bit(JSA1212_OPMODE_ALS_EN, &data->flags)) {
+ ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
+ if (ret < 0) {
+ dev_err(dev, "als resume failed\n");
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+ }
+
+ if (test_bit(JSA1212_OPMODE_PXS_EN, &data->flags)) {
+ ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
+ if (ret < 0)
+ dev_err(dev, "pxs resume failed\n");
+ }
+
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(jsa1212_pm_ops, jsa1212_suspend, jsa1212_resume);
+
+#define JSA1212_PM_OPS (&jsa1212_pm_ops)
+#else
+#define JSA1212_PM_OPS NULL
+#endif
+
+static const struct acpi_device_id jsa1212_acpi_match[] = {
+ {"JSA1212", 0},
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, jsa1212_acpi_match);
+
+static const struct i2c_device_id jsa1212_id[] = {
+ { "jsa1212", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, jsa1212_id);
+
+static struct i2c_driver jsa1212_driver = {
+ .driver = {
+ .name = "jsa1212",
+ .pm = JSA1212_PM_OPS,
+ .owner = THIS_MODULE,
+ .acpi_match_table = ACPI_PTR(jsa1212_acpi_match),
+ },
+ .probe = jsa1212_probe,
+ .remove = jsa1212_remove,
+ .id_table = jsa1212_id,
+};
+module_i2c_driver(jsa1212_driver);
+
+MODULE_AUTHOR("Sathya Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>");
+MODULE_DESCRIPTION("JSA1212 proximity/ambient light sensor driver");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
2014-08-01 3:02 ` [PATCH v2 1/1] " Kuppuswamy Sathyanarayanan
@ 2014-08-01 7:02 ` Jonathan Cameron
2014-08-07 14:06 ` Jonathan Cameron
1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2014-08-01 7:02 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan; +Cc: linux-iio, srinivas.pandruvada
On August 1, 2014 4:02:48 AM GMT+01:00, Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>This patch adds a new driver for solteam opto JSA1212 proximity and
>ambient light sensor.
>
>Basic details of the chip can be found here.
>
>http://www.solteamopto.com.tw/detail.php?ms=3&po_unit=2&pt_unit=29&p_unit=120
>
>Signed-off-by: Kuppuswamy Sathyanarayanan
><sathyanarayanan.kuppuswamy@linux.intel.com>
>Signed-off-by: Srinivas Pandruvada
><srinivas.pandruvada@linux.intel.com>
>---
A log of changes from previous versions should be here...
> drivers/iio/light/Kconfig | 10 +
> drivers/iio/light/Makefile | 1 +
>drivers/iio/light/jsa1212.c | 490
>+++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 501 insertions(+)
> create mode 100644 drivers/iio/light/jsa1212.c
>
>diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
>index c89740d..07d0a76 100644
>--- a/drivers/iio/light/Kconfig
>+++ b/drivers/iio/light/Kconfig
>@@ -87,6 +87,16 @@ config HID_SENSOR_PROX
> To compile this driver as a module, choose M here: the
> module will be called hid-sensor-prox.
>
>+config JSA1212
>+ tristate "JSA1212 ALS and proximity sensor driver"
>+ depends on I2C
>+ help
>+ Say Y here if you want to build a IIO driver for JSA1212
>+ proximity & ALS sensor device.
>+
>+ To compile this driver as a module, choose M here:
>+ the module will be called jsa1212.
>+
> config SENSORS_LM3533
> tristate "LM3533 ambient light sensor"
> depends on MFD_LM3533
>diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
>index 3eb36e5..ce85c86 100644
>--- a/drivers/iio/light/Makefile
>+++ b/drivers/iio/light/Makefile
>@@ -10,6 +10,7 @@ obj-$(CONFIG_CM36651) += cm36651.o
> obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o
> obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o
> obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
>+obj-$(CONFIG_JSA1212) += jsa1212.o
> obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
> obj-$(CONFIG_LTR501) += ltr501.o
> obj-$(CONFIG_SENSORS_TSL2563) += tsl2563.o
>diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
>new file mode 100644
>index 0000000..1597183
>--- /dev/null
>+++ b/drivers/iio/light/jsa1212.c
>@@ -0,0 +1,490 @@
>+/*
>+ * JSA1212 Ambient Light & Proximity Sensor Driver
>+ *
>+ * Copyright (c) 2014, Intel Corporation.
>+ *
>+ * This program is free software; you can redistribute it and/or
>modify it
>+ * under the terms and conditions of the GNU General Public License,
>+ * version 2, as published by the Free Software Foundation.
>+ *
>+ * This program is distributed in the hope 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.
>+ */
>+
>+#include <linux/kernel.h>
>+#include <linux/slab.h>
>+#include <linux/module.h>
>+#include <linux/moduleparam.h>
>+#include <linux/delay.h>
>+#include <linux/i2c.h>
>+#include <linux/interrupt.h>
>+#include <linux/irq.h>
>+#include <linux/mutex.h>
>+#include <linux/slab.h>
>+#include <linux/acpi.h>
>+#include <linux/gpio/consumer.h>
>+#include <linux/iio/iio.h>
>+#include <linux/iio/sysfs.h>
>+#include <linux/iio/events.h>
>+
>+/* JSA1212 reg address */
>+#define JSA1212_CONF_REG 0x01
>+#define JSA1212_INT_REG 0x02
>+#define JSA1212_PXS_LT_REG 0x03
>+#define JSA1212_PXS_HT_REG 0x04
>+#define JSA1212_ALS_TH1_REG 0x05
>+#define JSA1212_ALS_TH2_REG 0x06
>+#define JSA1212_ALS_TH3_REG 0x07
>+#define JSA1212_PXS_DATA_REG 0x08
>+#define JSA1212_ALS_DT1_REG 0x09
>+#define JSA1212_ALS_DT2_REG 0x0A
>+#define JSA1212_ALS_RNG_REG 0x0B
>+#define JSA1212_MAX_REG 0x0C
>+
>+/* JSA1212 reg defaults */
>+#define JSA1212_CONF_REG_DEF 0x58
>+#define JSA1212_INT_REG_DEF 0x02
>+#define JSA1212_PXS_LT_REG_DEF 0x00
>+#define JSA1212_PXS_HT_REG_DEF 0xFF
>+#define JSA1212_ALS_TH1_REG_DEF 0x00
>+#define JSA1212_ALS_TH2_REG_DEF 0xF0
>+#define JSA1212_ALS_TH3_REG_DEF 0xFF
>+#define JSA1212_ALS_RNG_REG_DEF 0x00
>+
>+/* JSA1212 reg masks */
>+#define JSA1212_CONF_MASK 0xFF
>+#define JSA1212_INT_MASK 0xFF
>+#define JSA1212_PXS_LT_MASK 0xFF
>+#define JSA1212_PXS_HT_MASK 0xFF
>+#define JSA1212_ALS_TH1_MASK 0xFF
>+#define JSA1212_ALS_TH2_LT_MASK 0x0F
>+#define JSA1212_ALS_TH2_HT_MASK 0xF0
>+#define JSA1212_ALS_TH3_MASK 0xFF
>+#define JSA1212_PXS_DATA_MASK 0xFF
>+#define JSA1212_ALS_DATA_MASK 0xFFF
>+#define JSA1212_ALS_DT1_MASK 0xFF
>+#define JSA1212_ALS_DT2_MASK 0x0F
>+#define JSA1212_ALS_RNG_MASK 0x07
>+#define JSA1212_REG_MASK 0xFF
>+
>+/* JSA1212 CONF REG bits */
>+#define JSA1212_CONF_PXS_MASK 0x80
>+#define JSA1212_CONF_PXS_ENABLE 0x80
>+#define JSA1212_CONF_PXS_DISABLE 0x00
>+#define JSA1212_CONF_ALS_MASK 0x04
>+#define JSA1212_CONF_ALS_ENABLE 0x04
>+#define JSA1212_CONF_ALS_DISABLE 0x00
>+
>+/* JSA1212 INT REG bits */
>+#define JSA1212_INT_CTRL_MASK 0x01
>+#define JSA1212_INT_CTRL_EITHER 0x00
>+#define JSA1212_INT_CTRL_BOTH 0x01
>+#define JSA1212_INT_ALS_PRST_MASK 0x06
>+#define JSA1212_INT_ALS_PRST_1CONV 0x00
>+#define JSA1212_INT_ALS_PRST_4CONV 0x02
>+#define JSA1212_INT_ALS_PRST_8CONV 0x04
>+#define JSA1212_INT_ALS_PRST_16CONV 0x06
>+#define JSA1212_INT_ALS_FLAG_MASK 0x08
>+#define JSA1212_INT_ALS_FLAG_CLR 0x00
>+#define JSA1212_INT_PXS_PRST_MASK 0x60
>+#define JSA1212_INT_PXS_PRST_1CONV 0x00
>+#define JSA1212_INT_PXS_PRST_4CONV 0x20
>+#define JSA1212_INT_PXS_PRST_8CONV 0x40
>+#define JSA1212_INT_PXS_PRST_16CONV 0x60
>+#define JSA1212_INT_PXS_FLAG_MASK 0x80
>+#define JSA1212_INT_PXS_FLAG_CLR 0x00
>+
>+/* JSA1212 ALS RNG REG bits */
>+#define JSA1212_ALS_RNG_0_2048 0x00
>+#define JSA1212_ALS_RNG_0_1024 0x01
>+#define JSA1212_ALS_RNG_0_512 0x02
>+#define JSA1212_ALS_RNG_0_256 0x03
>+#define JSA1212_ALS_RNG_0_128 0x04
>+
>+/* JSA1212 INT threshold range */
>+#define JSA1212_ALS_TH_MIN 0x0000
>+#define JSA1212_ALS_TH_MAX 0x0FFF
>+#define JSA1212_PXS_TH_MIN 0x00
>+#define JSA1212_PXS_TH_MAX 0xFF
>+
>+#define JSA1212_ALS_DELAY_MS 0xC8
>+#define JSA1212_PXS_DELAY_MS 0x64
>+
>+enum jsa1212_op_mode {
>+ JSA1212_OPMODE_ALS_EN,
>+ JSA1212_OPMODE_PXS_EN,
>+};
>+
>+struct jsa1212_data {
>+ struct i2c_client *client;
>+ struct mutex lock;
>+ u8 als_rng_idx;
>+ unsigned long flags;
>+};
>+
>+/* ALS range idx to val mapping */
>+static const int jsa1212_als_range_val[] = {2048, 1024, 512, 256, 128,
>+ 128, 128, 128};
>+
>+static int jsa1212_reg_update(struct jsa1212_data *data, u8 reg, u8
>bit_mask,
>+ u8 new_val)
>+{
>+ u8 old_cmd, new_cmd;
>+ int ret;
>+
>+ ret = i2c_smbus_read_byte_data(data->client, reg);
>+ if (ret < 0) {
>+ dev_err(&data->client->dev,
>+ "%s: read reg (0X%02X) cmd failed\n",
>+ __func__, reg);
>+ return ret;
>+ }
>+
>+ old_cmd = ret & JSA1212_REG_MASK;
>+ new_cmd = old_cmd & ~bit_mask;
>+ new_cmd |= new_val & bit_mask;
>+
>+ if (new_cmd != old_cmd) {
>+ ret = i2c_smbus_write_byte_data(data->client, reg, new_cmd);
>+ if (ret < 0) {
>+ dev_err(&data->client->dev,
>+ "%s: write reg (0X%02X) cmd (0X%02X) failed\n",
>+ __func__, reg, new_cmd);
>+ return ret;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
>+/* Enables or disables ALS function based on status */
>+static int jsa1212_als_enable(struct jsa1212_data *data, u8 status)
>+{
>+ int ret;
>+
>+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>+ JSA1212_CONF_ALS_MASK,
>+ status);
>+ if (ret < 0)
>+ return ret;
>+
>+ if (status)
>+ set_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
>+ else
>+ clear_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
>+
>+ return ret;
>+}
>+
>+/* Enables or disables PXS function based on status */
>+static int jsa1212_pxs_enable(struct jsa1212_data *data, u8 status)
>+{
>+ int ret;
>+
>+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>+ JSA1212_CONF_PXS_MASK,
>+ status);
>+ if (ret < 0)
>+ return ret;
>+
>+ if (status)
>+ set_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
>+ else
>+ clear_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
>+
>+ return ret;
>+}
>+
>+static int jsa1212_read_als_data(struct jsa1212_data *data,
>+ unsigned int *val)
>+{
>+ int ret;
>+
>+ ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
>+ if (ret < 0)
>+ return ret;
>+
>+ /* Delay for data output */
>+ msleep(JSA1212_ALS_DELAY_MS);
>+
>+ /* Read 12 bit data */
>+ ret = i2c_smbus_read_word_data(data->client, JSA1212_ALS_DT1_REG);
>+ if (ret < 0) {
>+ dev_err(&data->client->dev, "als data read err\n");
>+ return ret;
>+ }
>+
>+ *val = ret & JSA1212_ALS_DATA_MASK;
>+
>+ ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_DISABLE);
>+ if (ret < 0)
>+ return ret;
>+
>+ return 0;
>+}
>+
>+static int jsa1212_read_pxs_data(struct jsa1212_data *data,
>+ unsigned int *val)
>+{
>+ int ret;
>+
>+ ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
>+ if (ret < 0)
>+ return ret;
>+
>+ /* Delay for data output */
>+ msleep(JSA1212_PXS_DELAY_MS);
>+
>+ /* Read out all data */
>+ ret = i2c_smbus_read_byte_data(data->client, JSA1212_PXS_DATA_REG);
>+ if (ret < 0) {
>+ dev_err(&data->client->dev, "pxs data read err\n");
>+ return ret;
>+ }
>+
>+ *val = ret & JSA1212_PXS_DATA_MASK;
>+
>+ ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_DISABLE);
>+ if (ret < 0)
>+ return ret;
>+
>+ return 0;
>+}
>+
>+static int jsa1212_read_raw(struct iio_dev *indio_dev,
>+ struct iio_chan_spec const *chan,
>+ int *val, int *val2, long mask)
>+{
>+ int ret;
>+ struct jsa1212_data *data = iio_priv(indio_dev);
>+
>+ switch (mask) {
>+ case IIO_CHAN_INFO_RAW:
>+ mutex_lock(&data->lock);
>+ switch (chan->type) {
>+ case IIO_LIGHT:
>+ ret = jsa1212_read_als_data(data, val);
>+ break;
>+ case IIO_PROXIMITY:
>+ ret = jsa1212_read_pxs_data(data, val);
>+ break;
>+ default:
>+ ret = -EINVAL;
>+ break;
>+ }
>+ ret = ret < 0 ? ret : IIO_VAL_INT;
>+ mutex_unlock(&data->lock);
>+ break;
>+ case IIO_CHAN_INFO_SCALE:
>+ switch (chan->type) {
>+ case IIO_LIGHT:
>+ *val = jsa1212_als_range_val[data->als_rng_idx];
>+ *val2 = BIT(12); /* Max 12 bit value */
>+ ret = IIO_VAL_FRACTIONAL;
>+ break;
>+ default:
>+ ret = -EINVAL;
>+ break;
>+ }
>+ break;
>+ default:
>+ ret = -EINVAL;
>+ break;
>+ }
>+
>+ return ret;
>+}
>+
>+static const struct iio_chan_spec jsa1212_channels[] = {
>+ {
>+ .type = IIO_LIGHT,
>+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>+ BIT(IIO_CHAN_INFO_SCALE),
>+ },
>+ {
>+ .type = IIO_PROXIMITY,
>+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>+ }
>+};
>+
>+static const struct iio_info jsa1212_info = {
>+ .driver_module = THIS_MODULE,
>+ .read_raw = &jsa1212_read_raw,
>+};
>+
>+static int jsa1212_chip_init(struct jsa1212_data *data)
>+{
>+ int ret;
>+
>+ ret = i2c_smbus_write_byte_data(data->client, JSA1212_CONF_REG,
>+ JSA1212_CONF_REG_DEF);
>+ if (ret < 0)
>+ return ret;
>+
>+ ret = i2c_smbus_write_byte_data(data->client, JSA1212_INT_REG,
>+ JSA1212_INT_REG_DEF);
>+ if (ret < 0)
>+ return ret;
>+
>+ data->als_rng_idx = JSA1212_ALS_RNG_0_2048;
>+ data->flags = 0x00;
>+
>+ return 0;
>+}
>+
>+static int jsa1212_probe(struct i2c_client *client,
>+ const struct i2c_device_id *id)
>+{
>+ struct jsa1212_data *jsa1212_data;
>+ struct iio_dev *indio_dev;
>+ int ret;
>+
>+ if (!i2c_check_functionality(client->adapter,
>I2C_FUNC_SMBUS_BYTE_DATA))
>+ return -ENODEV;
>+
>+ indio_dev = devm_iio_device_alloc(&client->dev,
>sizeof(*jsa1212_data));
>+ if (!indio_dev)
>+ return -ENOMEM;
>+
>+ jsa1212_data = iio_priv(indio_dev);
>+
>+ i2c_set_clientdata(client, indio_dev);
>+ jsa1212_data->client = client;
>+
>+ mutex_init(&jsa1212_data->lock);
>+
>+ ret = jsa1212_chip_init(jsa1212_data);
>+ if (ret < 0)
>+ return ret;
>+
>+ indio_dev->dev.parent = &client->dev;
>+ indio_dev->channels = jsa1212_channels;
>+ indio_dev->num_channels = ARRAY_SIZE(jsa1212_channels);
>+ indio_dev->name = id->name;
>+ indio_dev->modes = INDIO_DIRECT_MODE;
>+
>+ indio_dev->info = &jsa1212_info;
>+
>+ ret = iio_device_register(indio_dev);
>+ if (ret < 0) {
>+ dev_err(&client->dev, "%s: register device failed\n", __func__);
>+ return ret;
>+ }
>+
>+ return 0;
>+}
>+
>+static int jsa1212_remove(struct i2c_client *client)
>+{
>+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
>+ struct jsa1212_data *data = iio_priv(indio_dev);
>+ int ret;
>+
>+ mutex_lock(&data->lock);
>+
>+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>+ JSA1212_CONF_ALS_MASK |
>+ JSA1212_CONF_PXS_MASK,
>+ JSA1212_CONF_ALS_DISABLE |
>+ JSA1212_CONF_PXS_DISABLE);
>+
>+ if (ret < 0)
>+ dev_err(&client->dev, "power off cmd failed\n");
>+
>+ mutex_unlock(&data->lock);
>+
>+ iio_device_unregister(indio_dev);
>+
>+ return ret;
>+}
>+
>+#ifdef CONFIG_PM_SLEEP
>+static int jsa1212_suspend(struct device *dev)
>+{
>+ int ret;
>+ struct jsa1212_data *data;
>+
>+ data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>+
>+ mutex_lock(&data->lock);
>+
>+ ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>+ JSA1212_CONF_ALS_MASK |
>+ JSA1212_CONF_PXS_MASK,
>+ JSA1212_CONF_ALS_DISABLE |
>+ JSA1212_CONF_PXS_DISABLE);
>+
>+ if (ret < 0)
>+ dev_err(dev, "suspend cmd failed\n");
>+
>+ mutex_unlock(&data->lock);
>+
>+ return ret;
>+}
>+
>+static int jsa1212_resume(struct device *dev)
>+{
>+ int ret = 0;
>+ struct jsa1212_data *data;
>+
>+ data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>+
>+ mutex_lock(&data->lock);
>+
>+ if (test_bit(JSA1212_OPMODE_ALS_EN, &data->flags)) {
>+ ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
>+ if (ret < 0) {
>+ dev_err(dev, "als resume failed\n");
>+ mutex_unlock(&data->lock);
>+ return ret;
>+ }
>+ }
>+
>+ if (test_bit(JSA1212_OPMODE_PXS_EN, &data->flags)) {
>+ ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
>+ if (ret < 0)
>+ dev_err(dev, "pxs resume failed\n");
>+ }
>+
>+ mutex_unlock(&data->lock);
>+
>+ return ret;
>+}
>+
>+static SIMPLE_DEV_PM_OPS(jsa1212_pm_ops, jsa1212_suspend,
>jsa1212_resume);
>+
>+#define JSA1212_PM_OPS (&jsa1212_pm_ops)
>+#else
>+#define JSA1212_PM_OPS NULL
>+#endif
>+
>+static const struct acpi_device_id jsa1212_acpi_match[] = {
>+ {"JSA1212", 0},
>+ { },
>+};
>+MODULE_DEVICE_TABLE(acpi, jsa1212_acpi_match);
>+
>+static const struct i2c_device_id jsa1212_id[] = {
>+ { "jsa1212", 0 },
>+ { }
>+};
>+MODULE_DEVICE_TABLE(i2c, jsa1212_id);
>+
>+static struct i2c_driver jsa1212_driver = {
>+ .driver = {
>+ .name = "jsa1212",
>+ .pm = JSA1212_PM_OPS,
>+ .owner = THIS_MODULE,
>+ .acpi_match_table = ACPI_PTR(jsa1212_acpi_match),
>+ },
>+ .probe = jsa1212_probe,
>+ .remove = jsa1212_remove,
>+ .id_table = jsa1212_id,
>+};
>+module_i2c_driver(jsa1212_driver);
>+
>+MODULE_AUTHOR("Sathya Kuppuswamy
><sathyanarayanan.kuppuswamy@linux.intel.com>");
>+MODULE_DESCRIPTION("JSA1212 proximity/ambient light sensor driver");
>+MODULE_LICENSE("GPL v2");
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
2014-08-01 3:02 [PATCH v2 0/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor Kuppuswamy Sathyanarayanan
2014-08-01 3:02 ` [PATCH v2 1/1] " Kuppuswamy Sathyanarayanan
@ 2014-08-01 7:03 ` Jonathan Cameron
1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2014-08-01 7:03 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan; +Cc: linux-iio, srinivas.pandruvada
On August 1, 2014 4:02:47 AM GMT+01:00, Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>Please let me know your review comments.
>
>PATCH v2 changes:
>
>1. Combined ALS/PXS enable/disable functions into a single routine.
>2. Removed extra debug messages.
>3. Also addressed review comments from Peter Meerwald
Ah pesky email client fed me emails in wrong order! Logging changes in a cover letter is fine.
Sorry for the false moan.
J
>
>Kuppuswamy Sathyanarayanan (1):
> iio: jsa1212: Add JSA1212 proximity/ALS sensor
>
> drivers/iio/light/Kconfig | 10 +
> drivers/iio/light/Makefile | 1 +
>drivers/iio/light/jsa1212.c | 490
>+++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 501 insertions(+)
> create mode 100644 drivers/iio/light/jsa1212.c
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
2014-08-01 3:02 ` [PATCH v2 1/1] " Kuppuswamy Sathyanarayanan
2014-08-01 7:02 ` Jonathan Cameron
@ 2014-08-07 14:06 ` Jonathan Cameron
2014-08-07 18:34 ` sathyanarayanan kuppuswamy
1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2014-08-07 14:06 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan; +Cc: linux-iio, srinivas.pandruvada
On 01/08/14 04:02, Kuppuswamy Sathyanarayanan wrote:
> This patch adds a new driver for solteam opto JSA1212 proximity and
> ambient light sensor.
>
> Basic details of the chip can be found here.
>
> http://www.solteamopto.com.tw/detail.php?ms=3&po_unit=2&pt_unit=29&p_unit=120
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Very nice. Some small comments inline.
The only element I really care about is the opaque use of a flags field
when a bitfield (or even booleans) would lead to clearer / simpler code.
Jonathan
> ---
> drivers/iio/light/Kconfig | 10 +
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/jsa1212.c | 490 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 501 insertions(+)
> create mode 100644 drivers/iio/light/jsa1212.c
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index c89740d..07d0a76 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -87,6 +87,16 @@ config HID_SENSOR_PROX
> To compile this driver as a module, choose M here: the
> module will be called hid-sensor-prox.
>
> +config JSA1212
> + tristate "JSA1212 ALS and proximity sensor driver"
> + depends on I2C
> + help
> + Say Y here if you want to build a IIO driver for JSA1212
> + proximity & ALS sensor device.
> +
> + To compile this driver as a module, choose M here:
> + the module will be called jsa1212.
> +
> config SENSORS_LM3533
> tristate "LM3533 ambient light sensor"
> depends on MFD_LM3533
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 3eb36e5..ce85c86 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_CM36651) += cm36651.o
> obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o
> obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o
> obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
> +obj-$(CONFIG_JSA1212) += jsa1212.o
> obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
> obj-$(CONFIG_LTR501) += ltr501.o
> obj-$(CONFIG_SENSORS_TSL2563) += tsl2563.o
> diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
> new file mode 100644
> index 0000000..1597183
> --- /dev/null
> +++ b/drivers/iio/light/jsa1212.c
> @@ -0,0 +1,490 @@
> +/*
> + * JSA1212 Ambient Light & Proximity Sensor Driver
> + *
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +
> +/* JSA1212 reg address */
> +#define JSA1212_CONF_REG 0x01
> +#define JSA1212_INT_REG 0x02
> +#define JSA1212_PXS_LT_REG 0x03
> +#define JSA1212_PXS_HT_REG 0x04
> +#define JSA1212_ALS_TH1_REG 0x05
> +#define JSA1212_ALS_TH2_REG 0x06
> +#define JSA1212_ALS_TH3_REG 0x07
> +#define JSA1212_PXS_DATA_REG 0x08
> +#define JSA1212_ALS_DT1_REG 0x09
> +#define JSA1212_ALS_DT2_REG 0x0A
> +#define JSA1212_ALS_RNG_REG 0x0B
> +#define JSA1212_MAX_REG 0x0C
> +
> +/* JSA1212 reg defaults */
> +#define JSA1212_CONF_REG_DEF 0x58
> +#define JSA1212_INT_REG_DEF 0x02
> +#define JSA1212_PXS_LT_REG_DEF 0x00
> +#define JSA1212_PXS_HT_REG_DEF 0xFF
> +#define JSA1212_ALS_TH1_REG_DEF 0x00
> +#define JSA1212_ALS_TH2_REG_DEF 0xF0
> +#define JSA1212_ALS_TH3_REG_DEF 0xFF
> +#define JSA1212_ALS_RNG_REG_DEF 0x00
> +
> +/* JSA1212 reg masks */
> +#define JSA1212_CONF_MASK 0xFF
> +#define JSA1212_INT_MASK 0xFF
> +#define JSA1212_PXS_LT_MASK 0xFF
> +#define JSA1212_PXS_HT_MASK 0xFF
> +#define JSA1212_ALS_TH1_MASK 0xFF
> +#define JSA1212_ALS_TH2_LT_MASK 0x0F
> +#define JSA1212_ALS_TH2_HT_MASK 0xF0
> +#define JSA1212_ALS_TH3_MASK 0xFF
> +#define JSA1212_PXS_DATA_MASK 0xFF
> +#define JSA1212_ALS_DATA_MASK 0xFFF
> +#define JSA1212_ALS_DT1_MASK 0xFF
> +#define JSA1212_ALS_DT2_MASK 0x0F
> +#define JSA1212_ALS_RNG_MASK 0x07
> +#define JSA1212_REG_MASK 0xFF
> +
> +/* JSA1212 CONF REG bits */
> +#define JSA1212_CONF_PXS_MASK 0x80
> +#define JSA1212_CONF_PXS_ENABLE 0x80
> +#define JSA1212_CONF_PXS_DISABLE 0x00
> +#define JSA1212_CONF_ALS_MASK 0x04
> +#define JSA1212_CONF_ALS_ENABLE 0x04
> +#define JSA1212_CONF_ALS_DISABLE 0x00
> +
> +/* JSA1212 INT REG bits */
> +#define JSA1212_INT_CTRL_MASK 0x01
> +#define JSA1212_INT_CTRL_EITHER 0x00
> +#define JSA1212_INT_CTRL_BOTH 0x01
> +#define JSA1212_INT_ALS_PRST_MASK 0x06
> +#define JSA1212_INT_ALS_PRST_1CONV 0x00
> +#define JSA1212_INT_ALS_PRST_4CONV 0x02
> +#define JSA1212_INT_ALS_PRST_8CONV 0x04
> +#define JSA1212_INT_ALS_PRST_16CONV 0x06
> +#define JSA1212_INT_ALS_FLAG_MASK 0x08
> +#define JSA1212_INT_ALS_FLAG_CLR 0x00
> +#define JSA1212_INT_PXS_PRST_MASK 0x60
> +#define JSA1212_INT_PXS_PRST_1CONV 0x00
> +#define JSA1212_INT_PXS_PRST_4CONV 0x20
> +#define JSA1212_INT_PXS_PRST_8CONV 0x40
> +#define JSA1212_INT_PXS_PRST_16CONV 0x60
> +#define JSA1212_INT_PXS_FLAG_MASK 0x80
> +#define JSA1212_INT_PXS_FLAG_CLR 0x00
> +
> +/* JSA1212 ALS RNG REG bits */
> +#define JSA1212_ALS_RNG_0_2048 0x00
> +#define JSA1212_ALS_RNG_0_1024 0x01
> +#define JSA1212_ALS_RNG_0_512 0x02
> +#define JSA1212_ALS_RNG_0_256 0x03
> +#define JSA1212_ALS_RNG_0_128 0x04
> +
> +/* JSA1212 INT threshold range */
> +#define JSA1212_ALS_TH_MIN 0x0000
> +#define JSA1212_ALS_TH_MAX 0x0FFF
> +#define JSA1212_PXS_TH_MIN 0x00
> +#define JSA1212_PXS_TH_MAX 0xFF
> +
> +#define JSA1212_ALS_DELAY_MS 0xC8
> +#define JSA1212_PXS_DELAY_MS 0x64
> +
> +enum jsa1212_op_mode {
> + JSA1212_OPMODE_ALS_EN,
> + JSA1212_OPMODE_PXS_EN,
> +};
> +
> +struct jsa1212_data {
> + struct i2c_client *client;
> + struct mutex lock;
> + u8 als_rng_idx;
I'd be tempted to use a bit field in place of the opaque flags.
That way it is obvious what this can contain.
> + unsigned long flags;
> +};
> +
> +/* ALS range idx to val mapping */
> +static const int jsa1212_als_range_val[] = {2048, 1024, 512, 256, 128,
> + 128, 128, 128};
> +
> +static int jsa1212_reg_update(struct jsa1212_data *data, u8 reg, u8 bit_mask,
> + u8 new_val)
> +{
> + u8 old_cmd, new_cmd;
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, reg);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "%s: read reg (0X%02X) cmd failed\n",
> + __func__, reg);
> + return ret;
> + }
> +
> + old_cmd = ret & JSA1212_REG_MASK;
> + new_cmd = old_cmd & ~bit_mask;
> + new_cmd |= new_val & bit_mask;
> +
> + if (new_cmd != old_cmd) {
> + ret = i2c_smbus_write_byte_data(data->client, reg, new_cmd);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "%s: write reg (0X%02X) cmd (0X%02X) failed\n",
> + __func__, reg, new_cmd);
> + return ret;
> + }
> + }
Perhaps it would be better to use regmap and avoid rerolling standard
functionality?
> +
> + return 0;
> +}
> +
> +/* Enables or disables ALS function based on status */
> +static int jsa1212_als_enable(struct jsa1212_data *data, u8 status)
> +{
> + int ret;
> +
> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
> + JSA1212_CONF_ALS_MASK,
> + status);
> + if (ret < 0)
> + return ret;
> +
> + if (status)
> + set_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
> + else
> + clear_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
> +
> + return ret;
> +}
> +
> +/* Enables or disables PXS function based on status */
> +static int jsa1212_pxs_enable(struct jsa1212_data *data, u8 status)
> +{
> + int ret;
> +
> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
> + JSA1212_CONF_PXS_MASK,
> + status);
> + if (ret < 0)
> + return ret;
> +
> + if (status)
> + set_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
> + else
> + clear_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
> +
> + return ret;
> +}
> +
> +static int jsa1212_read_als_data(struct jsa1212_data *data,
> + unsigned int *val)
> +{
> + int ret;
> +
> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + /* Delay for data output */
> + msleep(JSA1212_ALS_DELAY_MS);
> +
> + /* Read 12 bit data */
> + ret = i2c_smbus_read_word_data(data->client, JSA1212_ALS_DT1_REG);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "als data read err\n");
> + return ret;
> + }
> +
> + *val = ret & JSA1212_ALS_DATA_MASK;
> +
> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_DISABLE);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int jsa1212_read_pxs_data(struct jsa1212_data *data,
> + unsigned int *val)
> +{
> + int ret;
> +
> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + /* Delay for data output */
> + msleep(JSA1212_PXS_DELAY_MS);
> +
> + /* Read out all data */
> + ret = i2c_smbus_read_byte_data(data->client, JSA1212_PXS_DATA_REG);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "pxs data read err\n");
> + return ret;
> + }
> +
> + *val = ret & JSA1212_PXS_DATA_MASK;
> +
> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_DISABLE);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int jsa1212_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + int ret;
> + struct jsa1212_data *data = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&data->lock);
> + switch (chan->type) {
> + case IIO_LIGHT:
> + ret = jsa1212_read_als_data(data, val);
> + break;
> + case IIO_PROXIMITY:
> + ret = jsa1212_read_pxs_data(data, val);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
You could simplify this a tiny bit by having the read functions return
IIO_VAL_INT on success, but it's probably not worth doing..
> + ret = ret < 0 ? ret : IIO_VAL_INT;
> + mutex_unlock(&data->lock);
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_LIGHT:
> + *val = jsa1212_als_range_val[data->als_rng_idx];
> + *val2 = BIT(12); /* Max 12 bit value */
> + ret = IIO_VAL_FRACTIONAL;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct iio_chan_spec jsa1212_channels[] = {
> + {
> + .type = IIO_LIGHT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + },
> + {
> + .type = IIO_PROXIMITY,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + }
> +};
> +
> +static const struct iio_info jsa1212_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = &jsa1212_read_raw,
> +};
> +
> +static int jsa1212_chip_init(struct jsa1212_data *data)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(data->client, JSA1212_CONF_REG,
> + JSA1212_CONF_REG_DEF);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(data->client, JSA1212_INT_REG,
> + JSA1212_INT_REG_DEF);
> + if (ret < 0)
> + return ret;
> +
> + data->als_rng_idx = JSA1212_ALS_RNG_0_2048;
> + data->flags = 0x00;
This will already be true as it is created with a kzalloc call.
> +
> + return 0;
> +}
> +
> +static int jsa1212_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct jsa1212_data *jsa1212_data;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*jsa1212_data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + jsa1212_data = iio_priv(indio_dev);
> +
> + i2c_set_clientdata(client, indio_dev);
> + jsa1212_data->client = client;
> +
> + mutex_init(&jsa1212_data->lock);
> +
> + ret = jsa1212_chip_init(jsa1212_data);
> + if (ret < 0)
> + return ret;
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->channels = jsa1212_channels;
> + indio_dev->num_channels = ARRAY_SIZE(jsa1212_channels);
> + indio_dev->name = id->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + indio_dev->info = &jsa1212_info;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "%s: register device failed\n", __func__);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int jsa1212_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct jsa1212_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + mutex_lock(&data->lock);
> +
> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
> + JSA1212_CONF_ALS_MASK |
> + JSA1212_CONF_PXS_MASK,
> + JSA1212_CONF_ALS_DISABLE |
> + JSA1212_CONF_PXS_DISABLE);
> +
> + if (ret < 0)
> + dev_err(&client->dev, "power off cmd failed\n");
> +
> + mutex_unlock(&data->lock);
> +
> + iio_device_unregister(indio_dev);
> +
> + return ret;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int jsa1212_suspend(struct device *dev)
> +{
> + int ret;
> + struct jsa1212_data *data;
> +
> + data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> + mutex_lock(&data->lock);
> +
> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
> + JSA1212_CONF_ALS_MASK |
> + JSA1212_CONF_PXS_MASK,
> + JSA1212_CONF_ALS_DISABLE |
> + JSA1212_CONF_PXS_DISABLE);
> +
> + if (ret < 0)
> + dev_err(dev, "suspend cmd failed\n");
> +
> + mutex_unlock(&data->lock);
> +
> + return ret;
Given the remove and the suspend have ended up pretty similar,
perhaps a utility function would cut down on code repitition?
> +}
> +
> +static int jsa1212_resume(struct device *dev)
> +{
> + int ret = 0;
> + struct jsa1212_data *data;
> +
> + data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> + mutex_lock(&data->lock);
> +
> + if (test_bit(JSA1212_OPMODE_ALS_EN, &data->flags)) {
> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
> + if (ret < 0) {
> + dev_err(dev, "als resume failed\n");
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + }
> +
> + if (test_bit(JSA1212_OPMODE_PXS_EN, &data->flags)) {
> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
> + if (ret < 0)
> + dev_err(dev, "pxs resume failed\n");
> + }
> +
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(jsa1212_pm_ops, jsa1212_suspend, jsa1212_resume);
> +
> +#define JSA1212_PM_OPS (&jsa1212_pm_ops)
> +#else
> +#define JSA1212_PM_OPS NULL
> +#endif
> +
> +static const struct acpi_device_id jsa1212_acpi_match[] = {
> + {"JSA1212", 0},
> + { },
> +};
> +MODULE_DEVICE_TABLE(acpi, jsa1212_acpi_match);
> +
> +static const struct i2c_device_id jsa1212_id[] = {
> + { "jsa1212", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, jsa1212_id);
> +
> +static struct i2c_driver jsa1212_driver = {
> + .driver = {
> + .name = "jsa1212",
> + .pm = JSA1212_PM_OPS,
> + .owner = THIS_MODULE,
> + .acpi_match_table = ACPI_PTR(jsa1212_acpi_match),
> + },
> + .probe = jsa1212_probe,
> + .remove = jsa1212_remove,
> + .id_table = jsa1212_id,
> +};
> +module_i2c_driver(jsa1212_driver);
> +
> +MODULE_AUTHOR("Sathya Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>");
> +MODULE_DESCRIPTION("JSA1212 proximity/ambient light sensor driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
2014-08-07 14:06 ` Jonathan Cameron
@ 2014-08-07 18:34 ` sathyanarayanan kuppuswamy
2014-08-07 20:11 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: sathyanarayanan kuppuswamy @ 2014-08-07 18:34 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, srinivas.pandruvada
Hi Jonathan,
Thanks for your valuable comments. Please find my reply inline.
On 08/07/2014 07:06 AM, Jonathan Cameron wrote:
> On 01/08/14 04:02, Kuppuswamy Sathyanarayanan wrote:
>> This patch adds a new driver for solteam opto JSA1212 proximity and
>> ambient light sensor.
>>
>> Basic details of the chip can be found here.
>>
>> http://www.solteamopto.com.tw/detail.php?ms=3&po_unit=2&pt_unit=29&p_unit=120
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Very nice. Some small comments inline.
>
> The only element I really care about is the opaque use of a flags field
> when a bitfield (or even booleans) would lead to clearer / simpler code.
I agree. if its just just two modes I would go for a bool. But I am
planning to add event support for both proximity
and ALS modules. Once I do that, I would end up with four bool values
for four modes supported. That's why
I thought I could simplify things with flags.
Do you think I should use bool for now and when I add event support I
can change it if needed ?
>
> Jonathan
>
>> ---
>> drivers/iio/light/Kconfig | 10 +
>> drivers/iio/light/Makefile | 1 +
>> drivers/iio/light/jsa1212.c | 490 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 501 insertions(+)
>> create mode 100644 drivers/iio/light/jsa1212.c
>>
>> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
>> index c89740d..07d0a76 100644
>> --- a/drivers/iio/light/Kconfig
>> +++ b/drivers/iio/light/Kconfig
>> @@ -87,6 +87,16 @@ config HID_SENSOR_PROX
>> To compile this driver as a module, choose M here: the
>> module will be called hid-sensor-prox.
>>
>> +config JSA1212
>> + tristate "JSA1212 ALS and proximity sensor driver"
>> + depends on I2C
>> + help
>> + Say Y here if you want to build a IIO driver for JSA1212
>> + proximity & ALS sensor device.
>> +
>> + To compile this driver as a module, choose M here:
>> + the module will be called jsa1212.
>> +
>> config SENSORS_LM3533
>> tristate "LM3533 ambient light sensor"
>> depends on MFD_LM3533
>> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
>> index 3eb36e5..ce85c86 100644
>> --- a/drivers/iio/light/Makefile
>> +++ b/drivers/iio/light/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_CM36651) += cm36651.o
>> obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o
>> obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o
>> obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
>> +obj-$(CONFIG_JSA1212) += jsa1212.o
>> obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
>> obj-$(CONFIG_LTR501) += ltr501.o
>> obj-$(CONFIG_SENSORS_TSL2563) += tsl2563.o
>> diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
>> new file mode 100644
>> index 0000000..1597183
>> --- /dev/null
>> +++ b/drivers/iio/light/jsa1212.c
>> @@ -0,0 +1,490 @@
>> +/*
>> + * JSA1212 Ambient Light & Proximity Sensor Driver
>> + *
>> + * Copyright (c) 2014, Intel Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/delay.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/mutex.h>
>> +#include <linux/slab.h>
>> +#include <linux/acpi.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/events.h>
>> +
>> +/* JSA1212 reg address */
>> +#define JSA1212_CONF_REG 0x01
>> +#define JSA1212_INT_REG 0x02
>> +#define JSA1212_PXS_LT_REG 0x03
>> +#define JSA1212_PXS_HT_REG 0x04
>> +#define JSA1212_ALS_TH1_REG 0x05
>> +#define JSA1212_ALS_TH2_REG 0x06
>> +#define JSA1212_ALS_TH3_REG 0x07
>> +#define JSA1212_PXS_DATA_REG 0x08
>> +#define JSA1212_ALS_DT1_REG 0x09
>> +#define JSA1212_ALS_DT2_REG 0x0A
>> +#define JSA1212_ALS_RNG_REG 0x0B
>> +#define JSA1212_MAX_REG 0x0C
>> +
>> +/* JSA1212 reg defaults */
>> +#define JSA1212_CONF_REG_DEF 0x58
>> +#define JSA1212_INT_REG_DEF 0x02
>> +#define JSA1212_PXS_LT_REG_DEF 0x00
>> +#define JSA1212_PXS_HT_REG_DEF 0xFF
>> +#define JSA1212_ALS_TH1_REG_DEF 0x00
>> +#define JSA1212_ALS_TH2_REG_DEF 0xF0
>> +#define JSA1212_ALS_TH3_REG_DEF 0xFF
>> +#define JSA1212_ALS_RNG_REG_DEF 0x00
>> +
>> +/* JSA1212 reg masks */
>> +#define JSA1212_CONF_MASK 0xFF
>> +#define JSA1212_INT_MASK 0xFF
>> +#define JSA1212_PXS_LT_MASK 0xFF
>> +#define JSA1212_PXS_HT_MASK 0xFF
>> +#define JSA1212_ALS_TH1_MASK 0xFF
>> +#define JSA1212_ALS_TH2_LT_MASK 0x0F
>> +#define JSA1212_ALS_TH2_HT_MASK 0xF0
>> +#define JSA1212_ALS_TH3_MASK 0xFF
>> +#define JSA1212_PXS_DATA_MASK 0xFF
>> +#define JSA1212_ALS_DATA_MASK 0xFFF
>> +#define JSA1212_ALS_DT1_MASK 0xFF
>> +#define JSA1212_ALS_DT2_MASK 0x0F
>> +#define JSA1212_ALS_RNG_MASK 0x07
>> +#define JSA1212_REG_MASK 0xFF
>> +
>> +/* JSA1212 CONF REG bits */
>> +#define JSA1212_CONF_PXS_MASK 0x80
>> +#define JSA1212_CONF_PXS_ENABLE 0x80
>> +#define JSA1212_CONF_PXS_DISABLE 0x00
>> +#define JSA1212_CONF_ALS_MASK 0x04
>> +#define JSA1212_CONF_ALS_ENABLE 0x04
>> +#define JSA1212_CONF_ALS_DISABLE 0x00
>> +
>> +/* JSA1212 INT REG bits */
>> +#define JSA1212_INT_CTRL_MASK 0x01
>> +#define JSA1212_INT_CTRL_EITHER 0x00
>> +#define JSA1212_INT_CTRL_BOTH 0x01
>> +#define JSA1212_INT_ALS_PRST_MASK 0x06
>> +#define JSA1212_INT_ALS_PRST_1CONV 0x00
>> +#define JSA1212_INT_ALS_PRST_4CONV 0x02
>> +#define JSA1212_INT_ALS_PRST_8CONV 0x04
>> +#define JSA1212_INT_ALS_PRST_16CONV 0x06
>> +#define JSA1212_INT_ALS_FLAG_MASK 0x08
>> +#define JSA1212_INT_ALS_FLAG_CLR 0x00
>> +#define JSA1212_INT_PXS_PRST_MASK 0x60
>> +#define JSA1212_INT_PXS_PRST_1CONV 0x00
>> +#define JSA1212_INT_PXS_PRST_4CONV 0x20
>> +#define JSA1212_INT_PXS_PRST_8CONV 0x40
>> +#define JSA1212_INT_PXS_PRST_16CONV 0x60
>> +#define JSA1212_INT_PXS_FLAG_MASK 0x80
>> +#define JSA1212_INT_PXS_FLAG_CLR 0x00
>> +
>> +/* JSA1212 ALS RNG REG bits */
>> +#define JSA1212_ALS_RNG_0_2048 0x00
>> +#define JSA1212_ALS_RNG_0_1024 0x01
>> +#define JSA1212_ALS_RNG_0_512 0x02
>> +#define JSA1212_ALS_RNG_0_256 0x03
>> +#define JSA1212_ALS_RNG_0_128 0x04
>> +
>> +/* JSA1212 INT threshold range */
>> +#define JSA1212_ALS_TH_MIN 0x0000
>> +#define JSA1212_ALS_TH_MAX 0x0FFF
>> +#define JSA1212_PXS_TH_MIN 0x00
>> +#define JSA1212_PXS_TH_MAX 0xFF
>> +
>> +#define JSA1212_ALS_DELAY_MS 0xC8
>> +#define JSA1212_PXS_DELAY_MS 0x64
>> +
>> +enum jsa1212_op_mode {
>> + JSA1212_OPMODE_ALS_EN,
>> + JSA1212_OPMODE_PXS_EN,
>> +};
>> +
>> +struct jsa1212_data {
>> + struct i2c_client *client;
>> + struct mutex lock;
>> + u8 als_rng_idx;
> I'd be tempted to use a bit field in place of the opaque flags.
> That way it is obvious what this can contain.
>
>> + unsigned long flags;
>> +};
>> +
>> +/* ALS range idx to val mapping */
>> +static const int jsa1212_als_range_val[] = {2048, 1024, 512, 256, 128,
>> + 128, 128, 128};
>> +
>> +static int jsa1212_reg_update(struct jsa1212_data *data, u8 reg, u8 bit_mask,
>> + u8 new_val)
>> +{
>> + u8 old_cmd, new_cmd;
>> + int ret;
>> +
>> + ret = i2c_smbus_read_byte_data(data->client, reg);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev,
>> + "%s: read reg (0X%02X) cmd failed\n",
>> + __func__, reg);
>> + return ret;
>> + }
>> +
>> + old_cmd = ret & JSA1212_REG_MASK;
>> + new_cmd = old_cmd & ~bit_mask;
>> + new_cmd |= new_val & bit_mask;
>> +
>> + if (new_cmd != old_cmd) {
>> + ret = i2c_smbus_write_byte_data(data->client, reg, new_cmd);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev,
>> + "%s: write reg (0X%02X) cmd (0X%02X) failed\n",
>> + __func__, reg, new_cmd);
>> + return ret;
>> + }
>> + }
> Perhaps it would be better to use regmap and avoid rerolling standard
> functionality?
Using regmap would involve adding lot of initialization code. I thought
its more complicated than adding a single update function.
>> +
>> + return 0;
>> +}
>> +
>> +/* Enables or disables ALS function based on status */
>> +static int jsa1212_als_enable(struct jsa1212_data *data, u8 status)
>> +{
>> + int ret;
>> +
>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>> + JSA1212_CONF_ALS_MASK,
>> + status);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (status)
>> + set_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
>> + else
>> + clear_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
>> +
>> + return ret;
>> +}
>> +
>> +/* Enables or disables PXS function based on status */
>> +static int jsa1212_pxs_enable(struct jsa1212_data *data, u8 status)
>> +{
>> + int ret;
>> +
>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>> + JSA1212_CONF_PXS_MASK,
>> + status);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (status)
>> + set_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
>> + else
>> + clear_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
>> +
>> + return ret;
>> +}
>> +
>> +static int jsa1212_read_als_data(struct jsa1212_data *data,
>> + unsigned int *val)
>> +{
>> + int ret;
>> +
>> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Delay for data output */
>> + msleep(JSA1212_ALS_DELAY_MS);
>> +
>> + /* Read 12 bit data */
>> + ret = i2c_smbus_read_word_data(data->client, JSA1212_ALS_DT1_REG);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "als data read err\n");
>> + return ret;
>> + }
>> +
>> + *val = ret & JSA1212_ALS_DATA_MASK;
>> +
>> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_DISABLE);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static int jsa1212_read_pxs_data(struct jsa1212_data *data,
>> + unsigned int *val)
>> +{
>> + int ret;
>> +
>> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Delay for data output */
>> + msleep(JSA1212_PXS_DELAY_MS);
>> +
>> + /* Read out all data */
>> + ret = i2c_smbus_read_byte_data(data->client, JSA1212_PXS_DATA_REG);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "pxs data read err\n");
>> + return ret;
>> + }
>> +
>> + *val = ret & JSA1212_PXS_DATA_MASK;
>> +
>> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_DISABLE);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static int jsa1212_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int *val, int *val2, long mask)
>> +{
>> + int ret;
>> + struct jsa1212_data *data = iio_priv(indio_dev);
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + mutex_lock(&data->lock);
>> + switch (chan->type) {
>> + case IIO_LIGHT:
>> + ret = jsa1212_read_als_data(data, val);
>> + break;
>> + case IIO_PROXIMITY:
>> + ret = jsa1212_read_pxs_data(data, val);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
> You could simplify this a tiny bit by having the read functions return
> IIO_VAL_INT on success, but it's probably not worth doing..
>
>> + ret = ret < 0 ? ret : IIO_VAL_INT;
>> + mutex_unlock(&data->lock);
>> + break;
>> + case IIO_CHAN_INFO_SCALE:
>> + switch (chan->type) {
>> + case IIO_LIGHT:
>> + *val = jsa1212_als_range_val[data->als_rng_idx];
>> + *val2 = BIT(12); /* Max 12 bit value */
>> + ret = IIO_VAL_FRACTIONAL;
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static const struct iio_chan_spec jsa1212_channels[] = {
>> + {
>> + .type = IIO_LIGHT,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE),
>> + },
>> + {
>> + .type = IIO_PROXIMITY,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> + }
>> +};
>> +
>> +static const struct iio_info jsa1212_info = {
>> + .driver_module = THIS_MODULE,
>> + .read_raw = &jsa1212_read_raw,
>> +};
>> +
>> +static int jsa1212_chip_init(struct jsa1212_data *data)
>> +{
>> + int ret;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, JSA1212_CONF_REG,
>> + JSA1212_CONF_REG_DEF);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, JSA1212_INT_REG,
>> + JSA1212_INT_REG_DEF);
>> + if (ret < 0)
>> + return ret;
>> +
>> + data->als_rng_idx = JSA1212_ALS_RNG_0_2048;
>> + data->flags = 0x00;
> This will already be true as it is created with a kzalloc call.
Agree. I will remove it.
>> +
>> + return 0;
>> +}
>> +
>> +static int jsa1212_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct jsa1212_data *jsa1212_data;
>> + struct iio_dev *indio_dev;
>> + int ret;
>> +
>> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>> + return -ENODEV;
>> +
>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*jsa1212_data));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> +
>> + jsa1212_data = iio_priv(indio_dev);
>> +
>> + i2c_set_clientdata(client, indio_dev);
>> + jsa1212_data->client = client;
>> +
>> + mutex_init(&jsa1212_data->lock);
>> +
>> + ret = jsa1212_chip_init(jsa1212_data);
>> + if (ret < 0)
>> + return ret;
>> +
>> + indio_dev->dev.parent = &client->dev;
>> + indio_dev->channels = jsa1212_channels;
>> + indio_dev->num_channels = ARRAY_SIZE(jsa1212_channels);
>> + indio_dev->name = id->name;
>> + indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> + indio_dev->info = &jsa1212_info;
>> +
>> + ret = iio_device_register(indio_dev);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "%s: register device failed\n", __func__);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int jsa1212_remove(struct i2c_client *client)
>> +{
>> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> + struct jsa1212_data *data = iio_priv(indio_dev);
>> + int ret;
>> +
>> + mutex_lock(&data->lock);
>> +
>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>> + JSA1212_CONF_ALS_MASK |
>> + JSA1212_CONF_PXS_MASK,
>> + JSA1212_CONF_ALS_DISABLE |
>> + JSA1212_CONF_PXS_DISABLE);
>> +
>> + if (ret < 0)
>> + dev_err(&client->dev, "power off cmd failed\n");
>> +
>> + mutex_unlock(&data->lock);
>> +
>> + iio_device_unregister(indio_dev);
>> +
>> + return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int jsa1212_suspend(struct device *dev)
>> +{
>> + int ret;
>> + struct jsa1212_data *data;
>> +
>> + data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> + mutex_lock(&data->lock);
>> +
>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>> + JSA1212_CONF_ALS_MASK |
>> + JSA1212_CONF_PXS_MASK,
>> + JSA1212_CONF_ALS_DISABLE |
>> + JSA1212_CONF_PXS_DISABLE);
>> +
>> + if (ret < 0)
>> + dev_err(dev, "suspend cmd failed\n");
>> +
>> + mutex_unlock(&data->lock);
>> +
>> + return ret;
> Given the remove and the suspend have ended up pretty similar,
> perhaps a utility function would cut down on code repitition?
Makes sense. I will add a disable_all() helper function.
>> +}
>> +
>> +static int jsa1212_resume(struct device *dev)
>> +{
>> + int ret = 0;
>> + struct jsa1212_data *data;
>> +
>> + data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> + mutex_lock(&data->lock);
>> +
>> + if (test_bit(JSA1212_OPMODE_ALS_EN, &data->flags)) {
>> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
>> + if (ret < 0) {
>> + dev_err(dev, "als resume failed\n");
>> + mutex_unlock(&data->lock);
>> + return ret;
>> + }
>> + }
>> +
>> + if (test_bit(JSA1212_OPMODE_PXS_EN, &data->flags)) {
>> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
>> + if (ret < 0)
>> + dev_err(dev, "pxs resume failed\n");
>> + }
>> +
>> + mutex_unlock(&data->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(jsa1212_pm_ops, jsa1212_suspend, jsa1212_resume);
>> +
>> +#define JSA1212_PM_OPS (&jsa1212_pm_ops)
>> +#else
>> +#define JSA1212_PM_OPS NULL
>> +#endif
>> +
>> +static const struct acpi_device_id jsa1212_acpi_match[] = {
>> + {"JSA1212", 0},
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(acpi, jsa1212_acpi_match);
>> +
>> +static const struct i2c_device_id jsa1212_id[] = {
>> + { "jsa1212", 0 },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, jsa1212_id);
>> +
>> +static struct i2c_driver jsa1212_driver = {
>> + .driver = {
>> + .name = "jsa1212",
>> + .pm = JSA1212_PM_OPS,
>> + .owner = THIS_MODULE,
>> + .acpi_match_table = ACPI_PTR(jsa1212_acpi_match),
>> + },
>> + .probe = jsa1212_probe,
>> + .remove = jsa1212_remove,
>> + .id_table = jsa1212_id,
>> +};
>> +module_i2c_driver(jsa1212_driver);
>> +
>> +MODULE_AUTHOR("Sathya Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>");
>> +MODULE_DESCRIPTION("JSA1212 proximity/ambient light sensor driver");
>> +MODULE_LICENSE("GPL v2");
>>
--
Sathyanarayanan Kuppuswamy
Android kernel developer
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor
2014-08-07 18:34 ` sathyanarayanan kuppuswamy
@ 2014-08-07 20:11 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2014-08-07 20:11 UTC (permalink / raw)
To: sathyanarayanan.kuppuswamy; +Cc: linux-iio, srinivas.pandruvada
On 07/08/14 19:34, sathyanarayanan kuppuswamy wrote:
> Hi Jonathan,
>
> Thanks for your valuable comments. Please find my reply inline.
>
>
> On 08/07/2014 07:06 AM, Jonathan Cameron wrote:
>> On 01/08/14 04:02, Kuppuswamy Sathyanarayanan wrote:
>>> This patch adds a new driver for solteam opto JSA1212 proximity and
>>> ambient light sensor.
>>>
>>> Basic details of the chip can be found here.
>>>
>>> http://www.solteamopto.com.tw/detail.php?ms=3&po_unit=2&pt_unit=29&p_unit=120
>>>
>>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> Very nice. Some small comments inline.
>>
>> The only element I really care about is the opaque use of a flags field
>> when a bitfield (or even booleans) would lead to clearer / simpler code.
> I agree. if its just just two modes I would go for a bool. But I am planning to add event support for both proximity
> and ALS modules. Once I do that, I would end up with four bool values for four modes supported. That's why
> I thought I could simplify things with flags.
>
> Do you think I should use bool for now and when I add event support I can change it if needed ?
Even with 4 I'd still go with bools. Won't make that much difference and readability
is worth a few bytes in a one off structure.
>
>>
>> Jonathan
>>
>>> ---
>>> drivers/iio/light/Kconfig | 10 +
>>> drivers/iio/light/Makefile | 1 +
>>> drivers/iio/light/jsa1212.c | 490 +++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 501 insertions(+)
>>> create mode 100644 drivers/iio/light/jsa1212.c
>>>
>>> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
>>> index c89740d..07d0a76 100644
>>> --- a/drivers/iio/light/Kconfig
>>> +++ b/drivers/iio/light/Kconfig
>>> @@ -87,6 +87,16 @@ config HID_SENSOR_PROX
>>> To compile this driver as a module, choose M here: the
>>> module will be called hid-sensor-prox.
>>>
>>> +config JSA1212
>>> + tristate "JSA1212 ALS and proximity sensor driver"
>>> + depends on I2C
>>> + help
>>> + Say Y here if you want to build a IIO driver for JSA1212
>>> + proximity & ALS sensor device.
>>> +
>>> + To compile this driver as a module, choose M here:
>>> + the module will be called jsa1212.
>>> +
>>> config SENSORS_LM3533
>>> tristate "LM3533 ambient light sensor"
>>> depends on MFD_LM3533
>>> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
>>> index 3eb36e5..ce85c86 100644
>>> --- a/drivers/iio/light/Makefile
>>> +++ b/drivers/iio/light/Makefile
>>> @@ -10,6 +10,7 @@ obj-$(CONFIG_CM36651) += cm36651.o
>>> obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o
>>> obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o
>>> obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
>>> +obj-$(CONFIG_JSA1212) += jsa1212.o
>>> obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
>>> obj-$(CONFIG_LTR501) += ltr501.o
>>> obj-$(CONFIG_SENSORS_TSL2563) += tsl2563.o
>>> diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
>>> new file mode 100644
>>> index 0000000..1597183
>>> --- /dev/null
>>> +++ b/drivers/iio/light/jsa1212.c
>>> @@ -0,0 +1,490 @@
>>> +/*
>>> + * JSA1212 Ambient Light & Proximity Sensor Driver
>>> + *
>>> + * Copyright (c) 2014, Intel Corporation.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope 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.
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/module.h>
>>> +#include <linux/moduleparam.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/acpi.h>
>>> +#include <linux/gpio/consumer.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +#include <linux/iio/events.h>
>>> +
>>> +/* JSA1212 reg address */
>>> +#define JSA1212_CONF_REG 0x01
>>> +#define JSA1212_INT_REG 0x02
>>> +#define JSA1212_PXS_LT_REG 0x03
>>> +#define JSA1212_PXS_HT_REG 0x04
>>> +#define JSA1212_ALS_TH1_REG 0x05
>>> +#define JSA1212_ALS_TH2_REG 0x06
>>> +#define JSA1212_ALS_TH3_REG 0x07
>>> +#define JSA1212_PXS_DATA_REG 0x08
>>> +#define JSA1212_ALS_DT1_REG 0x09
>>> +#define JSA1212_ALS_DT2_REG 0x0A
>>> +#define JSA1212_ALS_RNG_REG 0x0B
>>> +#define JSA1212_MAX_REG 0x0C
>>> +
>>> +/* JSA1212 reg defaults */
>>> +#define JSA1212_CONF_REG_DEF 0x58
>>> +#define JSA1212_INT_REG_DEF 0x02
>>> +#define JSA1212_PXS_LT_REG_DEF 0x00
>>> +#define JSA1212_PXS_HT_REG_DEF 0xFF
>>> +#define JSA1212_ALS_TH1_REG_DEF 0x00
>>> +#define JSA1212_ALS_TH2_REG_DEF 0xF0
>>> +#define JSA1212_ALS_TH3_REG_DEF 0xFF
>>> +#define JSA1212_ALS_RNG_REG_DEF 0x00
>>> +
>>> +/* JSA1212 reg masks */
>>> +#define JSA1212_CONF_MASK 0xFF
>>> +#define JSA1212_INT_MASK 0xFF
>>> +#define JSA1212_PXS_LT_MASK 0xFF
>>> +#define JSA1212_PXS_HT_MASK 0xFF
>>> +#define JSA1212_ALS_TH1_MASK 0xFF
>>> +#define JSA1212_ALS_TH2_LT_MASK 0x0F
>>> +#define JSA1212_ALS_TH2_HT_MASK 0xF0
>>> +#define JSA1212_ALS_TH3_MASK 0xFF
>>> +#define JSA1212_PXS_DATA_MASK 0xFF
>>> +#define JSA1212_ALS_DATA_MASK 0xFFF
>>> +#define JSA1212_ALS_DT1_MASK 0xFF
>>> +#define JSA1212_ALS_DT2_MASK 0x0F
>>> +#define JSA1212_ALS_RNG_MASK 0x07
>>> +#define JSA1212_REG_MASK 0xFF
>>> +
>>> +/* JSA1212 CONF REG bits */
>>> +#define JSA1212_CONF_PXS_MASK 0x80
>>> +#define JSA1212_CONF_PXS_ENABLE 0x80
>>> +#define JSA1212_CONF_PXS_DISABLE 0x00
>>> +#define JSA1212_CONF_ALS_MASK 0x04
>>> +#define JSA1212_CONF_ALS_ENABLE 0x04
>>> +#define JSA1212_CONF_ALS_DISABLE 0x00
>>> +
>>> +/* JSA1212 INT REG bits */
>>> +#define JSA1212_INT_CTRL_MASK 0x01
>>> +#define JSA1212_INT_CTRL_EITHER 0x00
>>> +#define JSA1212_INT_CTRL_BOTH 0x01
>>> +#define JSA1212_INT_ALS_PRST_MASK 0x06
>>> +#define JSA1212_INT_ALS_PRST_1CONV 0x00
>>> +#define JSA1212_INT_ALS_PRST_4CONV 0x02
>>> +#define JSA1212_INT_ALS_PRST_8CONV 0x04
>>> +#define JSA1212_INT_ALS_PRST_16CONV 0x06
>>> +#define JSA1212_INT_ALS_FLAG_MASK 0x08
>>> +#define JSA1212_INT_ALS_FLAG_CLR 0x00
>>> +#define JSA1212_INT_PXS_PRST_MASK 0x60
>>> +#define JSA1212_INT_PXS_PRST_1CONV 0x00
>>> +#define JSA1212_INT_PXS_PRST_4CONV 0x20
>>> +#define JSA1212_INT_PXS_PRST_8CONV 0x40
>>> +#define JSA1212_INT_PXS_PRST_16CONV 0x60
>>> +#define JSA1212_INT_PXS_FLAG_MASK 0x80
>>> +#define JSA1212_INT_PXS_FLAG_CLR 0x00
>>> +
>>> +/* JSA1212 ALS RNG REG bits */
>>> +#define JSA1212_ALS_RNG_0_2048 0x00
>>> +#define JSA1212_ALS_RNG_0_1024 0x01
>>> +#define JSA1212_ALS_RNG_0_512 0x02
>>> +#define JSA1212_ALS_RNG_0_256 0x03
>>> +#define JSA1212_ALS_RNG_0_128 0x04
>>> +
>>> +/* JSA1212 INT threshold range */
>>> +#define JSA1212_ALS_TH_MIN 0x0000
>>> +#define JSA1212_ALS_TH_MAX 0x0FFF
>>> +#define JSA1212_PXS_TH_MIN 0x00
>>> +#define JSA1212_PXS_TH_MAX 0xFF
>>> +
>>> +#define JSA1212_ALS_DELAY_MS 0xC8
>>> +#define JSA1212_PXS_DELAY_MS 0x64
>>> +
>>> +enum jsa1212_op_mode {
>>> + JSA1212_OPMODE_ALS_EN,
>>> + JSA1212_OPMODE_PXS_EN,
>>> +};
>>> +
>>> +struct jsa1212_data {
>>> + struct i2c_client *client;
>>> + struct mutex lock;
>>> + u8 als_rng_idx;
>> I'd be tempted to use a bit field in place of the opaque flags.
>> That way it is obvious what this can contain.
>>
>>> + unsigned long flags;
>>> +};
>>> +
>>> +/* ALS range idx to val mapping */
>>> +static const int jsa1212_als_range_val[] = {2048, 1024, 512, 256, 128,
>>> + 128, 128, 128};
>>> +
>>> +static int jsa1212_reg_update(struct jsa1212_data *data, u8 reg, u8 bit_mask,
>>> + u8 new_val)
>>> +{
>>> + u8 old_cmd, new_cmd;
>>> + int ret;
>>> +
>>> + ret = i2c_smbus_read_byte_data(data->client, reg);
>>> + if (ret < 0) {
>>> + dev_err(&data->client->dev,
>>> + "%s: read reg (0X%02X) cmd failed\n",
>>> + __func__, reg);
>>> + return ret;
>>> + }
>>> +
>>> + old_cmd = ret & JSA1212_REG_MASK;
>>> + new_cmd = old_cmd & ~bit_mask;
>>> + new_cmd |= new_val & bit_mask;
>>> +
>>> + if (new_cmd != old_cmd) {
>>> + ret = i2c_smbus_write_byte_data(data->client, reg, new_cmd);
>>> + if (ret < 0) {
>>> + dev_err(&data->client->dev,
>>> + "%s: write reg (0X%02X) cmd (0X%02X) failed\n",
>>> + __func__, reg, new_cmd);
>>> + return ret;
>>> + }
>>> + }
>> Perhaps it would be better to use regmap and avoid rerolling standard
>> functionality?
> Using regmap would involve adding lot of initialization code. I thought its more complicated than adding a single update
> function.
Perhaps though it's not all that much intialization and you get
caching for free thus avoiding some of the reads in the update
calls. I don't really mind either way.
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* Enables or disables ALS function based on status */
>>> +static int jsa1212_als_enable(struct jsa1212_data *data, u8 status)
>>> +{
>>> + int ret;
>>> +
>>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>>> + JSA1212_CONF_ALS_MASK,
>>> + status);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (status)
>>> + set_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
>>> + else
>>> + clear_bit(JSA1212_OPMODE_ALS_EN, &data->flags);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/* Enables or disables PXS function based on status */
>>> +static int jsa1212_pxs_enable(struct jsa1212_data *data, u8 status)
>>> +{
>>> + int ret;
>>> +
>>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>>> + JSA1212_CONF_PXS_MASK,
>>> + status);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (status)
>>> + set_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
>>> + else
>>> + clear_bit(JSA1212_OPMODE_PXS_EN, &data->flags);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int jsa1212_read_als_data(struct jsa1212_data *data,
>>> + unsigned int *val)
>>> +{
>>> + int ret;
>>> +
>>> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Delay for data output */
>>> + msleep(JSA1212_ALS_DELAY_MS);
>>> +
>>> + /* Read 12 bit data */
>>> + ret = i2c_smbus_read_word_data(data->client, JSA1212_ALS_DT1_REG);
>>> + if (ret < 0) {
>>> + dev_err(&data->client->dev, "als data read err\n");
>>> + return ret;
>>> + }
>>> +
>>> + *val = ret & JSA1212_ALS_DATA_MASK;
>>> +
>>> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_DISABLE);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jsa1212_read_pxs_data(struct jsa1212_data *data,
>>> + unsigned int *val)
>>> +{
>>> + int ret;
>>> +
>>> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Delay for data output */
>>> + msleep(JSA1212_PXS_DELAY_MS);
>>> +
>>> + /* Read out all data */
>>> + ret = i2c_smbus_read_byte_data(data->client, JSA1212_PXS_DATA_REG);
>>> + if (ret < 0) {
>>> + dev_err(&data->client->dev, "pxs data read err\n");
>>> + return ret;
>>> + }
>>> +
>>> + *val = ret & JSA1212_PXS_DATA_MASK;
>>> +
>>> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_DISABLE);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jsa1212_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int *val, int *val2, long mask)
>>> +{
>>> + int ret;
>>> + struct jsa1212_data *data = iio_priv(indio_dev);
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + mutex_lock(&data->lock);
>>> + switch (chan->type) {
>>> + case IIO_LIGHT:
>>> + ret = jsa1212_read_als_data(data, val);
>>> + break;
>>> + case IIO_PROXIMITY:
>>> + ret = jsa1212_read_pxs_data(data, val);
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>> You could simplify this a tiny bit by having the read functions return
>> IIO_VAL_INT on success, but it's probably not worth doing..
>>
>>> + ret = ret < 0 ? ret : IIO_VAL_INT;
>>> + mutex_unlock(&data->lock);
>>> + break;
>>> + case IIO_CHAN_INFO_SCALE:
>>> + switch (chan->type) {
>>> + case IIO_LIGHT:
>>> + *val = jsa1212_als_range_val[data->als_rng_idx];
>>> + *val2 = BIT(12); /* Max 12 bit value */
>>> + ret = IIO_VAL_FRACTIONAL;
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static const struct iio_chan_spec jsa1212_channels[] = {
>>> + {
>>> + .type = IIO_LIGHT,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> + BIT(IIO_CHAN_INFO_SCALE),
>>> + },
>>> + {
>>> + .type = IIO_PROXIMITY,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>> + }
>>> +};
>>> +
>>> +static const struct iio_info jsa1212_info = {
>>> + .driver_module = THIS_MODULE,
>>> + .read_raw = &jsa1212_read_raw,
>>> +};
>>> +
>>> +static int jsa1212_chip_init(struct jsa1212_data *data)
>>> +{
>>> + int ret;
>>> +
>>> + ret = i2c_smbus_write_byte_data(data->client, JSA1212_CONF_REG,
>>> + JSA1212_CONF_REG_DEF);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = i2c_smbus_write_byte_data(data->client, JSA1212_INT_REG,
>>> + JSA1212_INT_REG_DEF);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + data->als_rng_idx = JSA1212_ALS_RNG_0_2048;
>>> + data->flags = 0x00;
>> This will already be true as it is created with a kzalloc call.
> Agree. I will remove it.
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jsa1212_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct jsa1212_data *jsa1212_data;
>>> + struct iio_dev *indio_dev;
>>> + int ret;
>>> +
>>> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>>> + return -ENODEV;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*jsa1212_data));
>>> + if (!indio_dev)
>>> + return -ENOMEM;
>>> +
>>> + jsa1212_data = iio_priv(indio_dev);
>>> +
>>> + i2c_set_clientdata(client, indio_dev);
>>> + jsa1212_data->client = client;
>>> +
>>> + mutex_init(&jsa1212_data->lock);
>>> +
>>> + ret = jsa1212_chip_init(jsa1212_data);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + indio_dev->dev.parent = &client->dev;
>>> + indio_dev->channels = jsa1212_channels;
>>> + indio_dev->num_channels = ARRAY_SIZE(jsa1212_channels);
>>> + indio_dev->name = id->name;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> + indio_dev->info = &jsa1212_info;
>>> +
>>> + ret = iio_device_register(indio_dev);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "%s: register device failed\n", __func__);
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jsa1212_remove(struct i2c_client *client)
>>> +{
>>> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
>>> + struct jsa1212_data *data = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + mutex_lock(&data->lock);
>>> +
>>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>>> + JSA1212_CONF_ALS_MASK |
>>> + JSA1212_CONF_PXS_MASK,
>>> + JSA1212_CONF_ALS_DISABLE |
>>> + JSA1212_CONF_PXS_DISABLE);
>>> +
>>> + if (ret < 0)
>>> + dev_err(&client->dev, "power off cmd failed\n");
>>> +
>>> + mutex_unlock(&data->lock);
>>> +
>>> + iio_device_unregister(indio_dev);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int jsa1212_suspend(struct device *dev)
>>> +{
>>> + int ret;
>>> + struct jsa1212_data *data;
>>> +
>>> + data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>> +
>>> + mutex_lock(&data->lock);
>>> +
>>> + ret = jsa1212_reg_update(data, JSA1212_CONF_REG,
>>> + JSA1212_CONF_ALS_MASK |
>>> + JSA1212_CONF_PXS_MASK,
>>> + JSA1212_CONF_ALS_DISABLE |
>>> + JSA1212_CONF_PXS_DISABLE);
>>> +
>>> + if (ret < 0)
>>> + dev_err(dev, "suspend cmd failed\n");
>>> +
>>> + mutex_unlock(&data->lock);
>>> +
>>> + return ret;
>> Given the remove and the suspend have ended up pretty similar,
>> perhaps a utility function would cut down on code repitition?
> Makes sense. I will add a disable_all() helper function.
>>> +}
>>> +
>>> +static int jsa1212_resume(struct device *dev)
>>> +{
>>> + int ret = 0;
>>> + struct jsa1212_data *data;
>>> +
>>> + data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>> +
>>> + mutex_lock(&data->lock);
>>> +
>>> + if (test_bit(JSA1212_OPMODE_ALS_EN, &data->flags)) {
>>> + ret = jsa1212_als_enable(data, JSA1212_CONF_ALS_ENABLE);
>>> + if (ret < 0) {
>>> + dev_err(dev, "als resume failed\n");
>>> + mutex_unlock(&data->lock);
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + if (test_bit(JSA1212_OPMODE_PXS_EN, &data->flags)) {
>>> + ret = jsa1212_pxs_enable(data, JSA1212_CONF_PXS_ENABLE);
>>> + if (ret < 0)
>>> + dev_err(dev, "pxs resume failed\n");
>>> + }
>>> +
>>> + mutex_unlock(&data->lock);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(jsa1212_pm_ops, jsa1212_suspend, jsa1212_resume);
>>> +
>>> +#define JSA1212_PM_OPS (&jsa1212_pm_ops)
>>> +#else
>>> +#define JSA1212_PM_OPS NULL
>>> +#endif
>>> +
>>> +static const struct acpi_device_id jsa1212_acpi_match[] = {
>>> + {"JSA1212", 0},
>>> + { },
>>> +};
>>> +MODULE_DEVICE_TABLE(acpi, jsa1212_acpi_match);
>>> +
>>> +static const struct i2c_device_id jsa1212_id[] = {
>>> + { "jsa1212", 0 },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, jsa1212_id);
>>> +
>>> +static struct i2c_driver jsa1212_driver = {
>>> + .driver = {
>>> + .name = "jsa1212",
>>> + .pm = JSA1212_PM_OPS,
>>> + .owner = THIS_MODULE,
>>> + .acpi_match_table = ACPI_PTR(jsa1212_acpi_match),
>>> + },
>>> + .probe = jsa1212_probe,
>>> + .remove = jsa1212_remove,
>>> + .id_table = jsa1212_id,
>>> +};
>>> +module_i2c_driver(jsa1212_driver);
>>> +
>>> +MODULE_AUTHOR("Sathya Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>");
>>> +MODULE_DESCRIPTION("JSA1212 proximity/ambient light sensor driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-08-07 20:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-01 3:02 [PATCH v2 0/1] iio: jsa1212: Add JSA1212 proximity/ALS sensor Kuppuswamy Sathyanarayanan
2014-08-01 3:02 ` [PATCH v2 1/1] " Kuppuswamy Sathyanarayanan
2014-08-01 7:02 ` Jonathan Cameron
2014-08-07 14:06 ` Jonathan Cameron
2014-08-07 18:34 ` sathyanarayanan kuppuswamy
2014-08-07 20:11 ` Jonathan Cameron
2014-08-01 7:03 ` [PATCH v2 0/1] " Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).