* [PATCH v3 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor
@ 2015-08-14 9:29 Adriana Reus
2015-08-14 9:29 ` [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d " Adriana Reus
2015-08-14 9:29 ` [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor Adriana Reus
0 siblings, 2 replies; 10+ messages in thread
From: Adriana Reus @ 2015-08-14 9:29 UTC (permalink / raw)
To: jic23, pmeerw, daniel.baluta
Cc: linux-iio, linux-kernel, devicetree, Adriana Reus
This series adds basic support for this als and proximity sensor and devicetree docs.
Resending the series because I forgot to cc devicetree. Sorry for the follow-up.
Adriana Reus (2):
iio: light: Add support for UPISEMI uS5182d als and proximity sensor
devicetree: Add documentation for UPISEMI us5182d ALS and Proximity
sensor
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
.../devicetree/bindings/iio/light/us5182d.txt | 24 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/iio/light/Kconfig | 10 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/us5182d.c | 484 +++++++++++++++++++++
6 files changed, 521 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/light/us5182d.txt
create mode 100644 drivers/iio/light/us5182d.c
--
1.9.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d als and proximity sensor 2015-08-14 9:29 [PATCH v3 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor Adriana Reus @ 2015-08-14 9:29 ` Adriana Reus 2015-08-15 14:27 ` Jonathan Cameron 2015-08-14 9:29 ` [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor Adriana Reus 1 sibling, 1 reply; 10+ messages in thread From: Adriana Reus @ 2015-08-14 9:29 UTC (permalink / raw) To: jic23, pmeerw, daniel.baluta Cc: linux-iio, linux-kernel, devicetree, Adriana Reus Add support for UPISEMI us5182d als and proximity sensor. Supports raw readings. Data sheet for this device can be found here: http://www.upi-semi.com/temp/uS5182D-DS-P0103-temp.pdf Signed-off-by: Adriana Reus <adriana.reus@intel.com> --- Changes since v2: - Adressed Peter's comments (with the exception of setting the ths_vals array as const - if I do that the compiler will eventually whine because it gets passed as an argument to device_property_read). - Tried to break down the compound register values into more intuitive definitions. - Eliminated the read helper functions. drivers/iio/light/Kconfig | 10 + drivers/iio/light/Makefile | 1 + drivers/iio/light/us5182d.c | 484 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 495 insertions(+) create mode 100644 drivers/iio/light/us5182d.c diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index 7ed859a..0442f01 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -287,6 +287,16 @@ config TSL4531 To compile this driver as a module, choose M here: the module will be called tsl4531. +config US5182D + tristate "UPISEMI light and proximity sensor" + depends on I2C + help + If you say yes here you get support for the UPISEMI US5182D + ambient light and proximity sensor. + + This driver can also be built as a module. If so, the module + will be called us5182d. + config VCNL4000 tristate "VCNL4000 combined ALS and proximity sensor" depends on I2C diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index 91c74c0..528cc8f 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -27,4 +27,5 @@ obj-$(CONFIG_STK3310) += stk3310.o obj-$(CONFIG_TCS3414) += tcs3414.o obj-$(CONFIG_TCS3472) += tcs3472.o obj-$(CONFIG_TSL4531) += tsl4531.o +obj-$(CONFIG_US5182D) += us5182d.o obj-$(CONFIG_VCNL4000) += vcnl4000.o diff --git a/drivers/iio/light/us5182d.c b/drivers/iio/light/us5182d.c new file mode 100644 index 0000000..9ba99f5 --- /dev/null +++ b/drivers/iio/light/us5182d.c @@ -0,0 +1,484 @@ +/* + * Copyright (c) 2015 Intel Corporation + * + * Driver for UPISEMI us5182d Proximity and Ambient Light Sensor. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope 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. + * + * To do: Interrupt support. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/acpi.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/mutex.h> + +#define US5182D_REG_CFG0 0x00 +#define US5182D_REG_CFG1 0x01 +#define US5182D_REG_CFG2 0x02 +#define US5182D_REG_CFG3 0x03 +#define US5182D_REG_CFG4 0x10 + +/* + * Registers for tuning the auto dark current cancelling feature. + * DARK_TH(reg 0x27,0x28) - threshold (counts) for auto dark cancelling. + * when ALS > DARK_TH --> ALS_Code = ALS - Upper(0x2A) * Dark + * when ALS < DARK_TH --> ALS_Code = ALS - Lower(0x29) * Dark + */ +#define US5182D_REG_UDARK_TH 0x27 +#define US5182D_REG_DARK_AUTO_EN 0x2b +#define US5182D_REG_AUTO_LDARK_GAIN 0x29 +#define US5182D_REG_AUTO_HDARK_GAIN 0x2a + +#define US5182D_ONESHOT_EN BIT(6) +#define US5182D_SHUTDOWN_EN BIT(7) +#define US5182D_OPMODE_ALS 0x01 +#define US5182D_OPMODE_PX 0x02 +#define US5182D_OPMODE_SHIFT 4 +#define US5182D_WORD_ENABLE BIT(0) +#define US5182D_ALS_RES16 BIT(4) +#define US5182D_PX_RES16 BIT(4) +#define US5182D_PXGAIN_DEFAULT BIT(2) +#define US5182D_LED_CURRENT100 (BIT(4) | BIT(5)) + +#define US5182D_REG_CFG0_DEFAULT \ + (US5182D_SHUTDOWN_EN | US5182D_WORD_ENABLE) +#define US5182D_REG_CFG1_DEFAULT US5182D_ALS_RES16 +#define US5182D_REG_CFG2_DEFAULT \ + (US5182D_PX_RES16 | US5182D_PXGAIN_DEFAULT) +#define US5182D_REG_CFG3_DEFAULT US5182D_LED_CURRENT100 +#define US5182D_REG_CFG4_DEFAULT 0x00 + +#define US5182D_REG_DARK_AUTO_EN_DEFAULT 0x80 +#define US5182D_REG_AUTO_LDARK_GAIN_DEFAULT 0x16 +#define US5182D_REG_AUTO_HDARK_GAIN_DEFAULT 0x00 + +#define US5182D_REG_ADL 0x0c +#define US5182D_REG_PDL 0x0e + +#define US5182D_REG_MODE_STORE 0x21 +#define US5182D_STORE_MODE 0x01 + +#define US5182D_REG_CHIPID 0xb2 + +#define US5182D_OPMODE_MASK GENMASK(5, 4) +#define US5182D_AGAIN_MASK 0x07 +#define US5182D_RESET_CHIP 0x01 + +#define US5182D_CHIPID 0x26 +#define US5182D_DRV_NAME "us5182d" + +#define US5182D_GA_RESOLUTION 1000 + +#define US5182D_READ_BYTE 1 +#define US5182D_READ_WORD 2 +#define US5182D_OPSTORE_SLEEP_TIME 20 /* ms */ + +/* available ranges: [12354, 7065, 3998, 2202, 1285, 498, 256, 138] lux */ +static const int us5182d_scales[] = {188500, 107800, 61000, 33600, 19600, 7600, + 3900, 2100}; + +/* + * experimental th's that work with US5182D sensor on evaluation board + * roughly between 12-32 lux + */ +static u16 us5182d_dark_ths_vals[] = {170, 200, 512, 512, 800, 2000, 4000, + 8000}; + +enum mode { + US5182D_ALS_PX, + US5182D_ALS_ONLY, + US5182D_PX_ONLY +}; + +struct us5182d_data { + struct i2c_client *client; + /* protect opmode */ + struct mutex lock; + + /* Glass attenuation factor */ + u32 ga; + + /* Dark gain tuning */ + u8 lower_dark_gain; + u8 upper_dark_gain; + u16 *us5182d_dark_ths; + + u8 opmode; +}; + +static IIO_CONST_ATTR(in_illuminance_scale_available, + "0.0021 0.0039 0.0076 0.0196 0.0336 0.061 0.1078 0.1885"); + +static struct attribute *us5182d_attrs[] = { + &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, + NULL +}; + +static const struct attribute_group us5182d_attr_group = { + .attrs = us5182d_attrs, +}; + +static const struct { + u8 reg; + u8 val; +} us5182d_regvals[] = { + {US5182D_REG_CFG0, US5182D_REG_CFG0_DEFAULT}, + {US5182D_REG_CFG1, US5182D_REG_CFG1_DEFAULT}, + {US5182D_REG_CFG2, US5182D_REG_CFG2_DEFAULT}, + {US5182D_REG_CFG3, US5182D_REG_CFG3_DEFAULT}, + {US5182D_REG_MODE_STORE, US5182D_STORE_MODE}, + {US5182D_REG_CFG4, US5182D_REG_CFG4_DEFAULT}, +}; + +static const struct iio_chan_spec us5182d_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 int us5182d_get_als(struct us5182d_data *data) +{ + int ret; + unsigned long result; + + ret = i2c_smbus_read_word_data(data->client, + US5182D_REG_ADL); + if (ret < 0) + return ret; + + result = ret * data->ga / US5182D_GA_RESOLUTION; + if (result > 0xffff) + result = 0xffff; + + return result; +} + +static int us5182d_set_opmode(struct us5182d_data *data, u8 mode) +{ + int ret; + + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG0); + if (ret < 0) + return ret; + + ret = ret | US5182D_ONESHOT_EN; + + /* update mode */ + ret = ret & ~US5182D_OPMODE_MASK; + ret = ret | (mode << US5182D_OPMODE_SHIFT); + + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG0, ret); + if (ret < 0) + return ret; + + if (mode == data->opmode) + return 0; + + data->opmode = mode; + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_MODE_STORE, + US5182D_STORE_MODE); + if (ret < 0) + return ret; + msleep(US5182D_OPSTORE_SLEEP_TIME); + + return 0; +} + +static int us5182d_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct us5182d_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + switch (chan->type) { + case IIO_LIGHT: + mutex_lock(&data->lock); + ret = us5182d_set_opmode(data, US5182D_OPMODE_ALS); + if (ret < 0) + goto out_err; + + ret = us5182d_get_als(data); + if (ret < 0) + goto out_err; + mutex_unlock(&data->lock); + *val = ret; + return IIO_VAL_INT; + case IIO_PROXIMITY: + mutex_lock(&data->lock); + ret = us5182d_set_opmode(data, US5182D_OPMODE_PX); + if (ret < 0) + goto out_err; + + ret = i2c_smbus_read_word_data(data->client, + US5182D_REG_PDL); + if (ret < 0) + goto out_err; + mutex_unlock(&data->lock); + *val = ret; + return IIO_VAL_INT; + default: + return -EINVAL; + } + + case IIO_CHAN_INFO_SCALE: + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); + if (ret < 0) + return ret; + *val = 0; + ret = (ret & US5182D_AGAIN_MASK); + *val2 = us5182d_scales[ret]; + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } + + return -EINVAL; +out_err: + mutex_unlock(&data->lock); + return ret; +} + +static int us5182d_update_dark_th(struct us5182d_data *data, int index) +{ + __be16 dark_th = cpu_to_be16(data->us5182d_dark_ths[index]); + u8 *bytes = (u8 *)&dark_th; + int ret; + + /* Registers Dark_Th (0x27 0x28) don't work in word mode accessing */ + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH, + bytes[0]); + if (ret < 0) + return ret; + + return i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH + 1, + bytes[1]); +} + +static int us5182d_apply_scale(struct us5182d_data *data, int index) +{ + int ret; + + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); + if (ret < 0) + return ret; + + ret = ret & (~US5182D_AGAIN_MASK); + ret |= index; + + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG1, ret); + if (ret < 0) + return ret; + + return us5182d_update_dark_th(data, index); +} + +static int us5182d_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int val, + int val2, long mask) +{ + struct us5182d_data *data = iio_priv(indio_dev); + int ret, i; + + switch (mask) { + case IIO_CHAN_INFO_SCALE: + if (val != 0) + return -EINVAL; + for (i = 0; i < ARRAY_SIZE(us5182d_scales); i++) + if (val2 == us5182d_scales[i]) { + mutex_lock(&data->lock); + ret = us5182d_apply_scale(data, i); + mutex_unlock(&data->lock); + return ret; + } + break; + default: + return -EINVAL; + } + + return -EINVAL; +} + +static const struct iio_info us5182d_info = { + .driver_module = THIS_MODULE, + .read_raw = us5182d_read_raw, + .write_raw = us5182d_write_raw, + .attrs = &us5182d_attr_group, +}; + +static int us5182d_reset(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + + return i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG3, + US5182D_RESET_CHIP); +} + +static int us5182d_init(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + int i, ret; + + ret = us5182d_reset(indio_dev); + if (ret < 0) + return ret; + + data->opmode = 0; + for (i = 0; i < ARRAY_SIZE(us5182d_regvals); i++) { + ret = i2c_smbus_write_byte_data(data->client, + us5182d_regvals[i].reg, + us5182d_regvals[i].val); + if (ret < 0) + return ret; + } + + return 0; +} + +static void us5182d_get_platform_data(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + + if (device_property_read_u32(&data->client->dev, "upisemi,glass-coef", + &data->ga)) + data->ga = US5182D_GA_RESOLUTION; + if (device_property_read_u16_array(&data->client->dev, + "upisemi,dark-ths", + data->us5182d_dark_ths, + ARRAY_SIZE(us5182d_dark_ths_vals))) + data->us5182d_dark_ths = us5182d_dark_ths_vals; + if (device_property_read_u8(&data->client->dev, + "upisemi,upper-dark-gain", + &data->upper_dark_gain)) + data->upper_dark_gain = US5182D_REG_AUTO_HDARK_GAIN_DEFAULT; + if (device_property_read_u8(&data->client->dev, + "upisemi,lower-dark-gain", + &data->lower_dark_gain)) + data->lower_dark_gain = US5182D_REG_AUTO_LDARK_GAIN_DEFAULT; +} + +static int us5182d_dark_gain_config(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + u8 index = US5182D_REG_CFG1_DEFAULT & US5182D_AGAIN_MASK; + int ret; + + ret = us5182d_update_dark_th(data, index); + if (ret < 0) + return ret; + + ret = i2c_smbus_write_byte_data(data->client, + US5182D_REG_AUTO_LDARK_GAIN, + data->lower_dark_gain); + if (ret < 0) + return ret; + + ret = i2c_smbus_write_byte_data(data->client, + US5182D_REG_AUTO_HDARK_GAIN, + data->upper_dark_gain); + if (ret < 0) + return ret; + + return i2c_smbus_write_byte_data(data->client, US5182D_REG_DARK_AUTO_EN, + US5182D_REG_DARK_AUTO_EN_DEFAULT); +} + +static int us5182d_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct us5182d_data *data; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + i2c_set_clientdata(client, indio_dev); + data->client = client; + + mutex_init(&data->lock); + + indio_dev->dev.parent = &client->dev; + indio_dev->info = &us5182d_info; + indio_dev->name = US5182D_DRV_NAME; + indio_dev->channels = us5182d_channels; + indio_dev->num_channels = ARRAY_SIZE(us5182d_channels); + indio_dev->modes = INDIO_DIRECT_MODE; + + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CHIPID); + if (ret != US5182D_CHIPID) { + dev_err(&data->client->dev, + "Failed to detect US5182 light chip\n"); + return (ret < 0) ? ret : -ENODEV; + } + + us5182d_get_platform_data(indio_dev); + ret = us5182d_init(indio_dev); + if (ret < 0) + return ret; + + ret = us5182d_dark_gain_config(indio_dev); + if (ret < 0) + return ret; + + return iio_device_register(indio_dev); +} + +static int us5182d_remove(struct i2c_client *client) +{ + iio_device_unregister(i2c_get_clientdata(client)); + return i2c_smbus_write_byte_data(client, US5182D_REG_CFG0, + US5182D_REG_CFG0_DEFAULT); +} + +static const struct acpi_device_id us5182d_acpi_match[] = { + { "USD5182", 0}, + {} +}; + +MODULE_DEVICE_TABLE(acpi, us5182d_acpi_match); + +static const struct i2c_device_id us5182d_id[] = { + {"usd5182", 0}, + {} +}; + +MODULE_DEVICE_TABLE(i2c, us5182d_id); + +static struct i2c_driver us5182d_driver = { + .driver = { + .name = US5182D_DRV_NAME, + .acpi_match_table = ACPI_PTR(us5182d_acpi_match), + }, + .probe = us5182d_probe, + .remove = us5182d_remove, + .id_table = us5182d_id, + +}; +module_i2c_driver(us5182d_driver); + +MODULE_AUTHOR("Adriana Reus <adriana.reus@intel.com>"); +MODULE_DESCRIPTION("Driver for us5182d Proximity and Light Sensor"); +MODULE_LICENSE("GPL v2"); -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d als and proximity sensor 2015-08-14 9:29 ` [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d " Adriana Reus @ 2015-08-15 14:27 ` Jonathan Cameron 2015-08-18 15:28 ` Adriana Reus 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2015-08-15 14:27 UTC (permalink / raw) To: Adriana Reus, pmeerw, daniel.baluta; +Cc: linux-iio, linux-kernel, devicetree On 14/08/15 10:29, Adriana Reus wrote: > Add support for UPISEMI us5182d als and proximity sensor. > Supports raw readings. > Data sheet for this device can be found here: > http://www.upi-semi.com/temp/uS5182D-DS-P0103-temp.pdf > > Signed-off-by: Adriana Reus <adriana.reus@intel.com> Mostly looking pretty good. I've partly been (slightly) pickier than normal here because we are stalled for a few weeks anyway by the fact the merge window for IIO is now probably done (about a week before Linus opens his) and also we have device tree docs that will need comments or to sit for quite a while on the mailing list. Thanks, Jonathan > --- > Changes since v2: > - Adressed Peter's comments (with the exception of setting > the ths_vals array as const - if I do that the compiler will > eventually whine because it gets passed as an argument to device_property_read). That should be a giveaway that you want to have a copy rather than one instance of this. Stick a copy in your data structure with defaults if no others are supplied. Note we might have two different instances of this part on one device with different thresholds... > - Tried to break down the compound register values into more intuitive definitions. Much improved . > - Eliminated the read helper functions. > > drivers/iio/light/Kconfig | 10 + > drivers/iio/light/Makefile | 1 + > drivers/iio/light/us5182d.c | 484 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 495 insertions(+) > create mode 100644 drivers/iio/light/us5182d.c > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 7ed859a..0442f01 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -287,6 +287,16 @@ config TSL4531 > To compile this driver as a module, choose M here: the > module will be called tsl4531. > > +config US5182D > + tristate "UPISEMI light and proximity sensor" > + depends on I2C > + help > + If you say yes here you get support for the UPISEMI US5182D > + ambient light and proximity sensor. > + > + This driver can also be built as a module. If so, the module > + will be called us5182d. > + > config VCNL4000 > tristate "VCNL4000 combined ALS and proximity sensor" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index 91c74c0..528cc8f 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -27,4 +27,5 @@ obj-$(CONFIG_STK3310) += stk3310.o > obj-$(CONFIG_TCS3414) += tcs3414.o > obj-$(CONFIG_TCS3472) += tcs3472.o > obj-$(CONFIG_TSL4531) += tsl4531.o > +obj-$(CONFIG_US5182D) += us5182d.o > obj-$(CONFIG_VCNL4000) += vcnl4000.o > diff --git a/drivers/iio/light/us5182d.c b/drivers/iio/light/us5182d.c > new file mode 100644 > index 0000000..9ba99f5 > --- /dev/null > +++ b/drivers/iio/light/us5182d.c > @@ -0,0 +1,484 @@ > +/* > + * Copyright (c) 2015 Intel Corporation > + * > + * Driver for UPISEMI us5182d Proximity and Ambient Light Sensor. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + * > + * This program is distributed in the hope 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. > + * > + * To do: Interrupt support. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/acpi.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/mutex.h> > + > +#define US5182D_REG_CFG0 0x00 > +#define US5182D_REG_CFG1 0x01 > +#define US5182D_REG_CFG2 0x02 > +#define US5182D_REG_CFG3 0x03 > +#define US5182D_REG_CFG4 0x10 > + > +/* > + * Registers for tuning the auto dark current cancelling feature. > + * DARK_TH(reg 0x27,0x28) - threshold (counts) for auto dark cancelling. > + * when ALS > DARK_TH --> ALS_Code = ALS - Upper(0x2A) * Dark > + * when ALS < DARK_TH --> ALS_Code = ALS - Lower(0x29) * Dark > + */ > +#define US5182D_REG_UDARK_TH 0x27 > +#define US5182D_REG_DARK_AUTO_EN 0x2b > +#define US5182D_REG_AUTO_LDARK_GAIN 0x29 > +#define US5182D_REG_AUTO_HDARK_GAIN 0x2a > + I'd be tempted to name these to make it clear which reg they are in. US5182D_CFG0_ONESHOT_EN etc. Hartmut did a nice tidy up of another driver the other day. He pulled the register address and elements down into one place and used a it of indenting to make it easy to read. For example. #define US5182D_REG_CFG0 0x00 #define US5182D_CFG0_ONESHOT_EN BIT(6) #define US5182D_SHUTDOWN_EN BIT(7) etc. Just a suggestion! I don't really care as long as it is clear what the register map is! > +#define US5182D_ONESHOT_EN BIT(6) > +#define US5182D_SHUTDOWN_EN BIT(7) > +#define US5182D_OPMODE_ALS 0x01 > +#define US5182D_OPMODE_PX 0x02 > +#define US5182D_OPMODE_SHIFT 4 > +#define US5182D_WORD_ENABLE BIT(0) > +#define US5182D_ALS_RES16 BIT(4) > +#define US5182D_PX_RES16 BIT(4) > +#define US5182D_PXGAIN_DEFAULT BIT(2) > +#define US5182D_LED_CURRENT100 (BIT(4) | BIT(5)) > + > +#define US5182D_REG_CFG0_DEFAULT \ > + (US5182D_SHUTDOWN_EN | US5182D_WORD_ENABLE) I'm missing something here, but SHUTDOWN_EN is never disabled? I'd have assumed that meant the device was always off! Guessing some magic is going on that might want a comment here. (just read datasheet section on this so I now understand what you how this works, but please add a brief explanation, perhaps where you set the oneshot enable). Also more I'd not bother having these defined like this up here. Doesn't add anything over using them directly in the array below. > +#define US5182D_REG_CFG1_DEFAULT US5182D_ALS_RES16 > +#define US5182D_REG_CFG2_DEFAULT \ > + (US5182D_PX_RES16 | US5182D_PXGAIN_DEFAULT) > +#define US5182D_REG_CFG3_DEFAULT US5182D_LED_CURRENT100 > +#define US5182D_REG_CFG4_DEFAULT 0x00 > + > +#define US5182D_REG_DARK_AUTO_EN_DEFAULT 0x80 > +#define US5182D_REG_AUTO_LDARK_GAIN_DEFAULT 0x16 > +#define US5182D_REG_AUTO_HDARK_GAIN_DEFAULT 0x00 > + > +#define US5182D_REG_ADL 0x0c > +#define US5182D_REG_PDL 0x0e > + > +#define US5182D_REG_MODE_STORE 0x21 > +#define US5182D_STORE_MODE 0x01 > + > +#define US5182D_REG_CHIPID 0xb2 > + > +#define US5182D_OPMODE_MASK GENMASK(5, 4) > +#define US5182D_AGAIN_MASK 0x07 > +#define US5182D_RESET_CHIP 0x01 > + > +#define US5182D_CHIPID 0x26 > +#define US5182D_DRV_NAME "us5182d" > + > +#define US5182D_GA_RESOLUTION 1000 > + > +#define US5182D_READ_BYTE 1 > +#define US5182D_READ_WORD 2 > +#define US5182D_OPSTORE_SLEEP_TIME 20 /* ms */ > + > +/* available ranges: [12354, 7065, 3998, 2202, 1285, 498, 256, 138] lux */ > +static const int us5182d_scales[] = {188500, 107800, 61000, 33600, 19600, 7600, > + 3900, 2100}; > + > +/* > + * experimental th's that work with US5182D sensor on evaluation board > + * roughly between 12-32 lux Experimental (or I'll get a trivial batch within a day or so 'fixing' it ;) Also long hand 'th' in the comment (thresholds persumably?). > + */ > +static u16 us5182d_dark_ths_vals[] = {170, 200, 512, 512, 800, 2000, 4000, > + 8000}; > + > +enum mode { > + US5182D_ALS_PX, > + US5182D_ALS_ONLY, > + US5182D_PX_ONLY > +}; > + > +struct us5182d_data { > + struct i2c_client *client; > + /* protect opmode */ Why not name it to indicate such? > + struct mutex lock; > + > + /* Glass attenuation factor */ > + u32 ga; > + > + /* Dark gain tuning */ > + u8 lower_dark_gain; > + u8 upper_dark_gain; > + u16 *us5182d_dark_ths; > + > + u8 opmode; > +}; > + > +static IIO_CONST_ATTR(in_illuminance_scale_available, > + "0.0021 0.0039 0.0076 0.0196 0.0336 0.061 0.1078 0.1885"); > + > +static struct attribute *us5182d_attrs[] = { > + &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, > + NULL > +}; > + > +static const struct attribute_group us5182d_attr_group = { > + .attrs = us5182d_attrs, > +}; > + > +static const struct { > + u8 reg; > + u8 val; > +} us5182d_regvals[] = { > + {US5182D_REG_CFG0, US5182D_REG_CFG0_DEFAULT}, > + {US5182D_REG_CFG1, US5182D_REG_CFG1_DEFAULT}, > + {US5182D_REG_CFG2, US5182D_REG_CFG2_DEFAULT}, > + {US5182D_REG_CFG3, US5182D_REG_CFG3_DEFAULT}, > + {US5182D_REG_MODE_STORE, US5182D_STORE_MODE}, > + {US5182D_REG_CFG4, US5182D_REG_CFG4_DEFAULT}, > +}; > + > +static const struct iio_chan_spec us5182d_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 int us5182d_get_als(struct us5182d_data *data) > +{ > + int ret; > + unsigned long result; > + > + ret = i2c_smbus_read_word_data(data->client, > + US5182D_REG_ADL); > + if (ret < 0) > + return ret; > + > + result = ret * data->ga / US5182D_GA_RESOLUTION; > + if (result > 0xffff) > + result = 0xffff; > + > + return result; > +} > + > +static int us5182d_set_opmode(struct us5182d_data *data, u8 mode) > +{ > + int ret; > + > + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG0); > + if (ret < 0) > + return ret; > + > + ret = ret | US5182D_ONESHOT_EN; > + > + /* update mode */ > + ret = ret & ~US5182D_OPMODE_MASK; > + ret = ret | (mode << US5182D_OPMODE_SHIFT); > + > + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG0, ret); > + if (ret < 0) > + return ret; > + > + if (mode == data->opmode) > + return 0; > + > + data->opmode = mode; > + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_MODE_STORE, > + US5182D_STORE_MODE); This is a lock / commit register? If so what is happening here is a little bit quirky and a brief comment to explain would be good. > + if (ret < 0) > + return ret; blank line here would make it a touch more readable (don't be afraid to add white space in moderation!) > + msleep(US5182D_OPSTORE_SLEEP_TIME); > + > + return 0; > +} > + > +static int us5182d_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + struct us5182d_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + switch (chan->type) { > + case IIO_LIGHT: > + mutex_lock(&data->lock); > + ret = us5182d_set_opmode(data, US5182D_OPMODE_ALS); > + if (ret < 0) > + goto out_err; > + > + ret = us5182d_get_als(data); > + if (ret < 0) > + goto out_err; > + mutex_unlock(&data->lock); > + *val = ret; > + return IIO_VAL_INT; > + case IIO_PROXIMITY: > + mutex_lock(&data->lock); > + ret = us5182d_set_opmode(data, US5182D_OPMODE_PX); > + if (ret < 0) > + goto out_err; > + > + ret = i2c_smbus_read_word_data(data->client, > + US5182D_REG_PDL); > + if (ret < 0) > + goto out_err; > + mutex_unlock(&data->lock); > + *val = ret; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + > + case IIO_CHAN_INFO_SCALE: > + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); > + if (ret < 0) > + return ret; I'd add a blank line here to aid readability (nitpick) > + *val = 0; > + ret = (ret & US5182D_AGAIN_MASK); > + *val2 = us5182d_scales[ret]; Maybe roll the two lines above into one? Blank line here also nice for readability and in equivalent places. > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + return -EINVAL; > + } > + > + return -EINVAL; > +out_err: > + mutex_unlock(&data->lock); > + return ret; > +} > + > +static int us5182d_update_dark_th(struct us5182d_data *data, int index) > +{ > + __be16 dark_th = cpu_to_be16(data->us5182d_dark_ths[index]); > + u8 *bytes = (u8 *)&dark_th; Cast it at use site rather than here. > + int ret; > + > + /* Registers Dark_Th (0x27 0x28) don't work in word mode accessing */ > + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH, > + bytes[0]); > + if (ret < 0) > + return ret; > + > + return i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH + 1, > + bytes[1]); > +} > + > +static int us5182d_apply_scale(struct us5182d_data *data, int index) > +{ > + int ret; > + > + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); > + if (ret < 0) > + return ret; > + > + ret = ret & (~US5182D_AGAIN_MASK); > + ret |= index; > + > + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG1, ret); > + if (ret < 0) > + return ret; > + > + return us5182d_update_dark_th(data, index); > +} > + > +static int us5182d_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int val, > + int val2, long mask) > +{ > + struct us5182d_data *data = iio_priv(indio_dev); > + int ret, i; > + > + switch (mask) { > + case IIO_CHAN_INFO_SCALE: > + if (val != 0) > + return -EINVAL; > + for (i = 0; i < ARRAY_SIZE(us5182d_scales); i++) > + if (val2 == us5182d_scales[i]) { > + mutex_lock(&data->lock); > + ret = us5182d_apply_scale(data, i); I'd be tempted to move the lock into apply_scale... Otherwise, you should add kernel-doc to that function to specify the lock should be held. (I think given the calls out to update_dark_th in there, I'd be tempted to just document the need for both those funcs to be called with the lock held). > + mutex_unlock(&data->lock); > + return ret; > + } > + break; > + default: > + return -EINVAL; > + } > + > + return -EINVAL; > +} > + > +static const struct iio_info us5182d_info = { > + .driver_module = THIS_MODULE, > + .read_raw = us5182d_read_raw, > + .write_raw = us5182d_write_raw, > + .attrs = &us5182d_attr_group, > +}; > + > +static int us5182d_reset(struct iio_dev *indio_dev) > +{ > + struct us5182d_data *data = iio_priv(indio_dev); > + > + return i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG3, > + US5182D_RESET_CHIP); > +} > + > +static int us5182d_init(struct iio_dev *indio_dev) > +{ > + struct us5182d_data *data = iio_priv(indio_dev); > + int i, ret; > + > + ret = us5182d_reset(indio_dev); > + if (ret < 0) > + return ret; > + > + data->opmode = 0; > + for (i = 0; i < ARRAY_SIZE(us5182d_regvals); i++) { > + ret = i2c_smbus_write_byte_data(data->client, > + us5182d_regvals[i].reg, > + us5182d_regvals[i].val); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +static void us5182d_get_platform_data(struct iio_dev *indio_dev) > +{ > + struct us5182d_data *data = iio_priv(indio_dev); > + > + if (device_property_read_u32(&data->client->dev, "upisemi,glass-coef", > + &data->ga)) > + data->ga = US5182D_GA_RESOLUTION; > + if (device_property_read_u16_array(&data->client->dev, > + "upisemi,dark-ths", > + data->us5182d_dark_ths, > + ARRAY_SIZE(us5182d_dark_ths_vals))) > + data->us5182d_dark_ths = us5182d_dark_ths_vals; > + if (device_property_read_u8(&data->client->dev, > + "upisemi,upper-dark-gain", > + &data->upper_dark_gain)) > + data->upper_dark_gain = US5182D_REG_AUTO_HDARK_GAIN_DEFAULT; > + if (device_property_read_u8(&data->client->dev, > + "upisemi,lower-dark-gain", > + &data->lower_dark_gain)) > + data->lower_dark_gain = US5182D_REG_AUTO_LDARK_GAIN_DEFAULT; > +} > + > +static int us5182d_dark_gain_config(struct iio_dev *indio_dev) > +{ > + struct us5182d_data *data = iio_priv(indio_dev); > + u8 index = US5182D_REG_CFG1_DEFAULT & US5182D_AGAIN_MASK; > + int ret; > + > + ret = us5182d_update_dark_th(data, index); > + if (ret < 0) > + return ret; > + > + ret = i2c_smbus_write_byte_data(data->client, > + US5182D_REG_AUTO_LDARK_GAIN, > + data->lower_dark_gain); > + if (ret < 0) > + return ret; > + > + ret = i2c_smbus_write_byte_data(data->client, > + US5182D_REG_AUTO_HDARK_GAIN, > + data->upper_dark_gain); > + if (ret < 0) > + return ret; > + > + return i2c_smbus_write_byte_data(data->client, US5182D_REG_DARK_AUTO_EN, > + US5182D_REG_DARK_AUTO_EN_DEFAULT); > +} > + > +static int us5182d_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct us5182d_data *data; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + i2c_set_clientdata(client, indio_dev); > + data->client = client; > + > + mutex_init(&data->lock); > + > + indio_dev->dev.parent = &client->dev; > + indio_dev->info = &us5182d_info; > + indio_dev->name = US5182D_DRV_NAME; > + indio_dev->channels = us5182d_channels; > + indio_dev->num_channels = ARRAY_SIZE(us5182d_channels); > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CHIPID); > + if (ret != US5182D_CHIPID) { > + dev_err(&data->client->dev, > + "Failed to detect US5182 light chip\n"); > + return (ret < 0) ? ret : -ENODEV; > + } > + > + us5182d_get_platform_data(indio_dev); > + ret = us5182d_init(indio_dev); > + if (ret < 0) > + return ret; > + > + ret = us5182d_dark_gain_config(indio_dev); > + if (ret < 0) > + return ret; > + > + return iio_device_register(indio_dev); > +} > + > +static int us5182d_remove(struct i2c_client *client) > +{ > + iio_device_unregister(i2c_get_clientdata(client)); > + return i2c_smbus_write_byte_data(client, US5182D_REG_CFG0, > + US5182D_REG_CFG0_DEFAULT); Nitpick :) I'd be tempted here (as we are shutting down and it doesn't matter anyway) to use just the SHUTDOWN_EN value as then it's immediately obvious what you are doing here. > +} > + > +static const struct acpi_device_id us5182d_acpi_match[] = { > + { "USD5182", 0}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(acpi, us5182d_acpi_match); > + > +static const struct i2c_device_id us5182d_id[] = { > + {"usd5182", 0}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(i2c, us5182d_id); > + > +static struct i2c_driver us5182d_driver = { > + .driver = { > + .name = US5182D_DRV_NAME, > + .acpi_match_table = ACPI_PTR(us5182d_acpi_match), > + }, > + .probe = us5182d_probe, > + .remove = us5182d_remove, > + .id_table = us5182d_id, > + > +}; > +module_i2c_driver(us5182d_driver); > + > +MODULE_AUTHOR("Adriana Reus <adriana.reus@intel.com>"); > +MODULE_DESCRIPTION("Driver for us5182d Proximity and Light Sensor"); > +MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d als and proximity sensor 2015-08-15 14:27 ` Jonathan Cameron @ 2015-08-18 15:28 ` Adriana Reus 2015-08-18 17:33 ` Jonathan Cameron 0 siblings, 1 reply; 10+ messages in thread From: Adriana Reus @ 2015-08-18 15:28 UTC (permalink / raw) To: Jonathan Cameron, pmeerw, daniel.baluta Cc: linux-iio, linux-kernel, devicetree Thanks for the review, added some comments inline, next patch set coming soon. Adriana On 15.08.2015 17:27, Jonathan Cameron wrote: > On 14/08/15 10:29, Adriana Reus wrote: >> Add support for UPISEMI us5182d als and proximity sensor. >> Supports raw readings. >> Data sheet for this device can be found here: >> http://www.upi-semi.com/temp/uS5182D-DS-P0103-temp.pdf >> >> Signed-off-by: Adriana Reus <adriana.reus@intel.com> > Mostly looking pretty good. I've partly been (slightly) pickier than normal > here because we are stalled for a few weeks anyway by the fact the merge > window for IIO is now probably done (about a week before Linus opens his) > and also we have device tree docs that will need comments or to sit for > quite a while on the mailing list. > > Thanks, > > Jonathan >> --- >> Changes since v2: >> - Adressed Peter's comments (with the exception of setting >> the ths_vals array as const - if I do that the compiler will >> eventually whine because it gets passed as an argument to device_property_read). > That should be a giveaway that you want to have a copy rather than one > instance of this. Stick a copy in your data structure with defaults > if no others are supplied. > > Note we might have two different instances of this part on one device > with different thresholds... > > >> - Tried to break down the compound register values into more intuitive definitions. > Much improved . >> - Eliminated the read helper functions. >> >> drivers/iio/light/Kconfig | 10 + >> drivers/iio/light/Makefile | 1 + >> drivers/iio/light/us5182d.c | 484 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 495 insertions(+) >> create mode 100644 drivers/iio/light/us5182d.c >> >> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig >> index 7ed859a..0442f01 100644 >> --- a/drivers/iio/light/Kconfig >> +++ b/drivers/iio/light/Kconfig >> @@ -287,6 +287,16 @@ config TSL4531 >> To compile this driver as a module, choose M here: the >> module will be called tsl4531. >> >> +config US5182D >> + tristate "UPISEMI light and proximity sensor" >> + depends on I2C >> + help >> + If you say yes here you get support for the UPISEMI US5182D >> + ambient light and proximity sensor. >> + >> + This driver can also be built as a module. If so, the module >> + will be called us5182d. >> + >> config VCNL4000 >> tristate "VCNL4000 combined ALS and proximity sensor" >> depends on I2C >> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile >> index 91c74c0..528cc8f 100644 >> --- a/drivers/iio/light/Makefile >> +++ b/drivers/iio/light/Makefile >> @@ -27,4 +27,5 @@ obj-$(CONFIG_STK3310) += stk3310.o >> obj-$(CONFIG_TCS3414) += tcs3414.o >> obj-$(CONFIG_TCS3472) += tcs3472.o >> obj-$(CONFIG_TSL4531) += tsl4531.o >> +obj-$(CONFIG_US5182D) += us5182d.o >> obj-$(CONFIG_VCNL4000) += vcnl4000.o >> diff --git a/drivers/iio/light/us5182d.c b/drivers/iio/light/us5182d.c >> new file mode 100644 >> index 0000000..9ba99f5 >> --- /dev/null >> +++ b/drivers/iio/light/us5182d.c >> @@ -0,0 +1,484 @@ >> +/* >> + * Copyright (c) 2015 Intel Corporation >> + * >> + * Driver for UPISEMI us5182d Proximity and Ambient Light Sensor. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published by >> + * the Free Software Foundation. >> + * >> + * This program is distributed in the hope 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. >> + * >> + * To do: Interrupt support. >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/acpi.h> >> +#include <linux/delay.h> >> +#include <linux/i2c.h> >> +#include <linux/iio/iio.h> >> +#include <linux/iio/sysfs.h> >> +#include <linux/mutex.h> >> + >> +#define US5182D_REG_CFG0 0x00 >> +#define US5182D_REG_CFG1 0x01 >> +#define US5182D_REG_CFG2 0x02 >> +#define US5182D_REG_CFG3 0x03 >> +#define US5182D_REG_CFG4 0x10 >> + >> +/* >> + * Registers for tuning the auto dark current cancelling feature. >> + * DARK_TH(reg 0x27,0x28) - threshold (counts) for auto dark cancelling. >> + * when ALS > DARK_TH --> ALS_Code = ALS - Upper(0x2A) * Dark >> + * when ALS < DARK_TH --> ALS_Code = ALS - Lower(0x29) * Dark >> + */ >> +#define US5182D_REG_UDARK_TH 0x27 >> +#define US5182D_REG_DARK_AUTO_EN 0x2b >> +#define US5182D_REG_AUTO_LDARK_GAIN 0x29 >> +#define US5182D_REG_AUTO_HDARK_GAIN 0x2a >> + > I'd be tempted to name these to make it clear which reg > they are in. > US5182D_CFG0_ONESHOT_EN etc. > Hartmut did a nice tidy up of another driver the other day. > He pulled the register address and elements down into one > place and used a it of indenting to make it easy to read. > > For example. > > #define US5182D_REG_CFG0 0x00 > #define US5182D_CFG0_ONESHOT_EN BIT(6) > #define US5182D_SHUTDOWN_EN BIT(7) > > etc. Just a suggestion! I don't really care as long as it > is clear what the register map is! > >> +#define US5182D_ONESHOT_EN BIT(6) >> +#define US5182D_SHUTDOWN_EN BIT(7) >> +#define US5182D_OPMODE_ALS 0x01 >> +#define US5182D_OPMODE_PX 0x02 >> +#define US5182D_OPMODE_SHIFT 4 >> +#define US5182D_WORD_ENABLE BIT(0) >> +#define US5182D_ALS_RES16 BIT(4) >> +#define US5182D_PX_RES16 BIT(4) >> +#define US5182D_PXGAIN_DEFAULT BIT(2) >> +#define US5182D_LED_CURRENT100 (BIT(4) | BIT(5)) >> + >> +#define US5182D_REG_CFG0_DEFAULT \ >> + (US5182D_SHUTDOWN_EN | US5182D_WORD_ENABLE) > I'm missing something here, but SHUTDOWN_EN is never disabled? > I'd have assumed that meant the device was always off! In oneshot mode the chip gets powered off automatically after taking the required measurement. I'll add a comment. > > Guessing some magic is going on that might want a comment here. > (just read datasheet section on this so I now understand what you > how this works, but please add a brief explanation, perhaps > where you set the oneshot enable). > > Also more I'd not bother having these defined like this > up here. Doesn't add anything over using them directly > in the array below. > >> +#define US5182D_REG_CFG1_DEFAULT US5182D_ALS_RES16 >> +#define US5182D_REG_CFG2_DEFAULT \ >> + (US5182D_PX_RES16 | US5182D_PXGAIN_DEFAULT) >> +#define US5182D_REG_CFG3_DEFAULT US5182D_LED_CURRENT100 >> +#define US5182D_REG_CFG4_DEFAULT 0x00 >> + >> +#define US5182D_REG_DARK_AUTO_EN_DEFAULT 0x80 >> +#define US5182D_REG_AUTO_LDARK_GAIN_DEFAULT 0x16 >> +#define US5182D_REG_AUTO_HDARK_GAIN_DEFAULT 0x00 >> + >> +#define US5182D_REG_ADL 0x0c >> +#define US5182D_REG_PDL 0x0e >> + >> +#define US5182D_REG_MODE_STORE 0x21 >> +#define US5182D_STORE_MODE 0x01 >> + >> +#define US5182D_REG_CHIPID 0xb2 >> + >> +#define US5182D_OPMODE_MASK GENMASK(5, 4) >> +#define US5182D_AGAIN_MASK 0x07 >> +#define US5182D_RESET_CHIP 0x01 >> + >> +#define US5182D_CHIPID 0x26 >> +#define US5182D_DRV_NAME "us5182d" >> + >> +#define US5182D_GA_RESOLUTION 1000 >> + >> +#define US5182D_READ_BYTE 1 >> +#define US5182D_READ_WORD 2 >> +#define US5182D_OPSTORE_SLEEP_TIME 20 /* ms */ >> + >> +/* available ranges: [12354, 7065, 3998, 2202, 1285, 498, 256, 138] lux */ >> +static const int us5182d_scales[] = {188500, 107800, 61000, 33600, 19600, 7600, >> + 3900, 2100}; >> + >> +/* >> + * experimental th's that work with US5182D sensor on evaluation board >> + * roughly between 12-32 lux > Experimental (or I'll get a trivial batch within a day or so 'fixing' it ;) > Also long hand 'th' in the comment (thresholds persumably?). ok >> + */ >> +static u16 us5182d_dark_ths_vals[] = {170, 200, 512, 512, 800, 2000, 4000, >> + 8000}; >> + >> +enum mode { >> + US5182D_ALS_PX, >> + US5182D_ALS_ONLY, >> + US5182D_PX_ONLY >> +}; >> + >> +struct us5182d_data { >> + struct i2c_client *client; >> + /* protect opmode */ > Why not name it to indicate such? >> + struct mutex lock; >> + >> + /* Glass attenuation factor */ >> + u32 ga; >> + >> + /* Dark gain tuning */ >> + u8 lower_dark_gain; >> + u8 upper_dark_gain; >> + u16 *us5182d_dark_ths; >> + >> + u8 opmode; >> +}; >> + >> +static IIO_CONST_ATTR(in_illuminance_scale_available, >> + "0.0021 0.0039 0.0076 0.0196 0.0336 0.061 0.1078 0.1885"); >> + >> +static struct attribute *us5182d_attrs[] = { >> + &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, >> + NULL >> +}; >> + >> +static const struct attribute_group us5182d_attr_group = { >> + .attrs = us5182d_attrs, >> +}; >> + >> +static const struct { >> + u8 reg; >> + u8 val; >> +} us5182d_regvals[] = { >> + {US5182D_REG_CFG0, US5182D_REG_CFG0_DEFAULT}, >> + {US5182D_REG_CFG1, US5182D_REG_CFG1_DEFAULT}, >> + {US5182D_REG_CFG2, US5182D_REG_CFG2_DEFAULT}, >> + {US5182D_REG_CFG3, US5182D_REG_CFG3_DEFAULT}, >> + {US5182D_REG_MODE_STORE, US5182D_STORE_MODE}, >> + {US5182D_REG_CFG4, US5182D_REG_CFG4_DEFAULT}, >> +}; >> + >> +static const struct iio_chan_spec us5182d_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 int us5182d_get_als(struct us5182d_data *data) >> +{ >> + int ret; >> + unsigned long result; >> + >> + ret = i2c_smbus_read_word_data(data->client, >> + US5182D_REG_ADL); >> + if (ret < 0) >> + return ret; >> + >> + result = ret * data->ga / US5182D_GA_RESOLUTION; >> + if (result > 0xffff) >> + result = 0xffff; >> + >> + return result; >> +} >> + >> +static int us5182d_set_opmode(struct us5182d_data *data, u8 mode) >> +{ >> + int ret; >> + >> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG0); >> + if (ret < 0) >> + return ret; >> + >> + ret = ret | US5182D_ONESHOT_EN; >> + >> + /* update mode */ >> + ret = ret & ~US5182D_OPMODE_MASK; >> + ret = ret | (mode << US5182D_OPMODE_SHIFT); >> + >> + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG0, ret); >> + if (ret < 0) >> + return ret; >> + >> + if (mode == data->opmode) >> + return 0; >> + >> + data->opmode = mode; >> + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_MODE_STORE, >> + US5182D_STORE_MODE); > This is a lock / commit register? If so what is happening here is a little > bit quirky and a brief comment to explain would be good. I'm sorry, I am not familiar with that terminology, so i'm not sure if that qualifies, the data sheet simply instructs that after a new opmode is set - we should additionally write 1 in the mode storage register. I'll add a comment nonetheless. > >> + if (ret < 0) >> + return ret; > blank line here would make it a touch more readable (don't be afraid > to add white space in moderation!) >> + msleep(US5182D_OPSTORE_SLEEP_TIME); >> + >> + return 0; >> +} >> + >> +static int us5182d_read_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, int *val, >> + int *val2, long mask) >> +{ >> + struct us5182d_data *data = iio_priv(indio_dev); >> + int ret; >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_RAW: >> + switch (chan->type) { >> + case IIO_LIGHT: >> + mutex_lock(&data->lock); >> + ret = us5182d_set_opmode(data, US5182D_OPMODE_ALS); >> + if (ret < 0) >> + goto out_err; >> + >> + ret = us5182d_get_als(data); >> + if (ret < 0) >> + goto out_err; >> + mutex_unlock(&data->lock); >> + *val = ret; >> + return IIO_VAL_INT; >> + case IIO_PROXIMITY: >> + mutex_lock(&data->lock); >> + ret = us5182d_set_opmode(data, US5182D_OPMODE_PX); >> + if (ret < 0) >> + goto out_err; >> + >> + ret = i2c_smbus_read_word_data(data->client, >> + US5182D_REG_PDL); >> + if (ret < 0) >> + goto out_err; >> + mutex_unlock(&data->lock); >> + *val = ret; >> + return IIO_VAL_INT; >> + default: >> + return -EINVAL; >> + } >> + >> + case IIO_CHAN_INFO_SCALE: >> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); >> + if (ret < 0) >> + return ret; > I'd add a blank line here to aid readability (nitpick) >> + *val = 0; >> + ret = (ret & US5182D_AGAIN_MASK); >> + *val2 = us5182d_scales[ret]; > Maybe roll the two lines above into one? > Blank line here also nice for readability and in equivalent places. >> + return IIO_VAL_INT_PLUS_MICRO; >> + default: >> + return -EINVAL; >> + } >> + >> + return -EINVAL; >> +out_err: >> + mutex_unlock(&data->lock); >> + return ret; >> +} >> + >> +static int us5182d_update_dark_th(struct us5182d_data *data, int index) >> +{ >> + __be16 dark_th = cpu_to_be16(data->us5182d_dark_ths[index]); >> + u8 *bytes = (u8 *)&dark_th; > Cast it at use site rather than here. > >> + int ret; >> + >> + /* Registers Dark_Th (0x27 0x28) don't work in word mode accessing */ >> + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH, >> + bytes[0]); >> + if (ret < 0) >> + return ret; >> + >> + return i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH + 1, >> + bytes[1]); >> +} >> + >> +static int us5182d_apply_scale(struct us5182d_data *data, int index) >> +{ >> + int ret; >> + >> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); >> + if (ret < 0) >> + return ret; >> + >> + ret = ret & (~US5182D_AGAIN_MASK); >> + ret |= index; >> + >> + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG1, ret); >> + if (ret < 0) >> + return ret; >> + >> + return us5182d_update_dark_th(data, index); >> +} >> + >> +static int us5182d_write_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, int val, >> + int val2, long mask) >> +{ >> + struct us5182d_data *data = iio_priv(indio_dev); >> + int ret, i; >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_SCALE: >> + if (val != 0) >> + return -EINVAL; >> + for (i = 0; i < ARRAY_SIZE(us5182d_scales); i++) >> + if (val2 == us5182d_scales[i]) { >> + mutex_lock(&data->lock); >> + ret = us5182d_apply_scale(data, i); > > I'd be tempted to move the lock into apply_scale... Otherwise, you > should add kernel-doc to that function to specify the lock should be held. > (I think given the calls out to update_dark_th in there, I'd be tempted > to just document the need for both those funcs to be called with the > lock held). >> + mutex_unlock(&data->lock); >> + return ret; >> + } >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return -EINVAL; >> +} >> + >> +static const struct iio_info us5182d_info = { >> + .driver_module = THIS_MODULE, >> + .read_raw = us5182d_read_raw, >> + .write_raw = us5182d_write_raw, >> + .attrs = &us5182d_attr_group, >> +}; >> + >> +static int us5182d_reset(struct iio_dev *indio_dev) >> +{ >> + struct us5182d_data *data = iio_priv(indio_dev); >> + >> + return i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG3, >> + US5182D_RESET_CHIP); >> +} >> + >> +static int us5182d_init(struct iio_dev *indio_dev) >> +{ >> + struct us5182d_data *data = iio_priv(indio_dev); >> + int i, ret; >> + >> + ret = us5182d_reset(indio_dev); >> + if (ret < 0) >> + return ret; >> + >> + data->opmode = 0; >> + for (i = 0; i < ARRAY_SIZE(us5182d_regvals); i++) { >> + ret = i2c_smbus_write_byte_data(data->client, >> + us5182d_regvals[i].reg, >> + us5182d_regvals[i].val); >> + if (ret < 0) >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static void us5182d_get_platform_data(struct iio_dev *indio_dev) >> +{ >> + struct us5182d_data *data = iio_priv(indio_dev); >> + >> + if (device_property_read_u32(&data->client->dev, "upisemi,glass-coef", >> + &data->ga)) >> + data->ga = US5182D_GA_RESOLUTION; >> + if (device_property_read_u16_array(&data->client->dev, >> + "upisemi,dark-ths", >> + data->us5182d_dark_ths, >> + ARRAY_SIZE(us5182d_dark_ths_vals))) >> + data->us5182d_dark_ths = us5182d_dark_ths_vals; >> + if (device_property_read_u8(&data->client->dev, >> + "upisemi,upper-dark-gain", >> + &data->upper_dark_gain)) >> + data->upper_dark_gain = US5182D_REG_AUTO_HDARK_GAIN_DEFAULT; >> + if (device_property_read_u8(&data->client->dev, >> + "upisemi,lower-dark-gain", >> + &data->lower_dark_gain)) >> + data->lower_dark_gain = US5182D_REG_AUTO_LDARK_GAIN_DEFAULT; >> +} >> + >> +static int us5182d_dark_gain_config(struct iio_dev *indio_dev) >> +{ >> + struct us5182d_data *data = iio_priv(indio_dev); >> + u8 index = US5182D_REG_CFG1_DEFAULT & US5182D_AGAIN_MASK; >> + int ret; >> + >> + ret = us5182d_update_dark_th(data, index); >> + if (ret < 0) >> + return ret; >> + >> + ret = i2c_smbus_write_byte_data(data->client, >> + US5182D_REG_AUTO_LDARK_GAIN, >> + data->lower_dark_gain); >> + if (ret < 0) >> + return ret; >> + >> + ret = i2c_smbus_write_byte_data(data->client, >> + US5182D_REG_AUTO_HDARK_GAIN, >> + data->upper_dark_gain); >> + if (ret < 0) >> + return ret; >> + >> + return i2c_smbus_write_byte_data(data->client, US5182D_REG_DARK_AUTO_EN, >> + US5182D_REG_DARK_AUTO_EN_DEFAULT); >> +} >> + >> +static int us5182d_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct us5182d_data *data; >> + struct iio_dev *indio_dev; >> + int ret; >> + >> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >> + if (!indio_dev) >> + return -ENOMEM; >> + >> + data = iio_priv(indio_dev); >> + i2c_set_clientdata(client, indio_dev); >> + data->client = client; >> + >> + mutex_init(&data->lock); >> + >> + indio_dev->dev.parent = &client->dev; >> + indio_dev->info = &us5182d_info; >> + indio_dev->name = US5182D_DRV_NAME; >> + indio_dev->channels = us5182d_channels; >> + indio_dev->num_channels = ARRAY_SIZE(us5182d_channels); >> + indio_dev->modes = INDIO_DIRECT_MODE; >> + >> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CHIPID); >> + if (ret != US5182D_CHIPID) { >> + dev_err(&data->client->dev, >> + "Failed to detect US5182 light chip\n"); >> + return (ret < 0) ? ret : -ENODEV; >> + } >> + >> + us5182d_get_platform_data(indio_dev); >> + ret = us5182d_init(indio_dev); >> + if (ret < 0) >> + return ret; >> + >> + ret = us5182d_dark_gain_config(indio_dev); >> + if (ret < 0) >> + return ret; >> + >> + return iio_device_register(indio_dev); >> +} >> + >> +static int us5182d_remove(struct i2c_client *client) >> +{ >> + iio_device_unregister(i2c_get_clientdata(client)); >> + return i2c_smbus_write_byte_data(client, US5182D_REG_CFG0, >> + US5182D_REG_CFG0_DEFAULT); > Nitpick :) > I'd be tempted here (as we are shutting down and it doesn't matter anyway) > to use just the SHUTDOWN_EN value as then it's immediately obvious > what you are doing here. would look nicer > >> +} >> + >> +static const struct acpi_device_id us5182d_acpi_match[] = { >> + { "USD5182", 0}, >> + {} >> +}; >> + >> +MODULE_DEVICE_TABLE(acpi, us5182d_acpi_match); >> + >> +static const struct i2c_device_id us5182d_id[] = { >> + {"usd5182", 0}, >> + {} >> +}; >> + >> +MODULE_DEVICE_TABLE(i2c, us5182d_id); >> + >> +static struct i2c_driver us5182d_driver = { >> + .driver = { >> + .name = US5182D_DRV_NAME, >> + .acpi_match_table = ACPI_PTR(us5182d_acpi_match), >> + }, >> + .probe = us5182d_probe, >> + .remove = us5182d_remove, >> + .id_table = us5182d_id, >> + >> +}; >> +module_i2c_driver(us5182d_driver); >> + >> +MODULE_AUTHOR("Adriana Reus <adriana.reus@intel.com>"); >> +MODULE_DESCRIPTION("Driver for us5182d Proximity and Light Sensor"); >> +MODULE_LICENSE("GPL v2"); >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d als and proximity sensor 2015-08-18 15:28 ` Adriana Reus @ 2015-08-18 17:33 ` Jonathan Cameron 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2015-08-18 17:33 UTC (permalink / raw) To: Adriana Reus, Jonathan Cameron, pmeerw, daniel.baluta Cc: linux-iio, linux-kernel, devicetree On 18 August 2015 16:28:20 BST, Adriana Reus <adriana.reus@intel.com> wrote: >Thanks for the review, added some comments inline, next patch set >coming >soon. > >Adriana > >On 15.08.2015 17:27, Jonathan Cameron wrote: >> On 14/08/15 10:29, Adriana Reus wrote: >>> Add support for UPISEMI us5182d als and proximity sensor. >>> Supports raw readings. >>> Data sheet for this device can be found here: >>> http://www.upi-semi.com/temp/uS5182D-DS-P0103-temp.pdf >>> >>> Signed-off-by: Adriana Reus <adriana.reus@intel.com> >> Mostly looking pretty good. I've partly been (slightly) pickier than >normal >> here because we are stalled for a few weeks anyway by the fact the >merge >> window for IIO is now probably done (about a week before Linus opens >his) >> and also we have device tree docs that will need comments or to sit >for >> quite a while on the mailing list. >> >> Thanks, >> >> Jonathan >>> --- >>> Changes since v2: >>> - Adressed Peter's comments (with the exception of setting >>> the ths_vals array as const - if I do that the compiler will >>> eventually whine because it gets passed as an argument to >device_property_read). >> That should be a giveaway that you want to have a copy rather than >one >> instance of this. Stick a copy in your data structure with defaults >> if no others are supplied. >> >> Note we might have two different instances of this part on one device >> with different thresholds... >> >> >>> - Tried to break down the compound register values into more >intuitive definitions. >> Much improved . >>> - Eliminated the read helper functions. >>> >>> drivers/iio/light/Kconfig | 10 + >>> drivers/iio/light/Makefile | 1 + >>> drivers/iio/light/us5182d.c | 484 >++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 495 insertions(+) >>> create mode 100644 drivers/iio/light/us5182d.c >>> >>> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig >>> index 7ed859a..0442f01 100644 >>> --- a/drivers/iio/light/Kconfig >>> +++ b/drivers/iio/light/Kconfig >>> @@ -287,6 +287,16 @@ config TSL4531 >>> To compile this driver as a module, choose M here: the >>> module will be called tsl4531. >>> >>> +config US5182D >>> + tristate "UPISEMI light and proximity sensor" >>> + depends on I2C >>> + help >>> + If you say yes here you get support for the UPISEMI US5182D >>> + ambient light and proximity sensor. >>> + >>> + This driver can also be built as a module. If so, the module >>> + will be called us5182d. >>> + >>> config VCNL4000 >>> tristate "VCNL4000 combined ALS and proximity sensor" >>> depends on I2C >>> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile >>> index 91c74c0..528cc8f 100644 >>> --- a/drivers/iio/light/Makefile >>> +++ b/drivers/iio/light/Makefile >>> @@ -27,4 +27,5 @@ obj-$(CONFIG_STK3310) += stk3310.o >>> obj-$(CONFIG_TCS3414) += tcs3414.o >>> obj-$(CONFIG_TCS3472) += tcs3472.o >>> obj-$(CONFIG_TSL4531) += tsl4531.o >>> +obj-$(CONFIG_US5182D) += us5182d.o >>> obj-$(CONFIG_VCNL4000) += vcnl4000.o >>> diff --git a/drivers/iio/light/us5182d.c >b/drivers/iio/light/us5182d.c >>> new file mode 100644 >>> index 0000000..9ba99f5 >>> --- /dev/null >>> +++ b/drivers/iio/light/us5182d.c >>> @@ -0,0 +1,484 @@ >>> +/* >>> + * Copyright (c) 2015 Intel Corporation >>> + * >>> + * Driver for UPISEMI us5182d Proximity and Ambient Light Sensor. >>> + * >>> + * This program is free software; you can redistribute it and/or >modify it >>> + * under the terms of the GNU General Public License version 2 as >published by >>> + * the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope 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. >>> + * >>> + * To do: Interrupt support. >>> + */ >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/acpi.h> >>> +#include <linux/delay.h> >>> +#include <linux/i2c.h> >>> +#include <linux/iio/iio.h> >>> +#include <linux/iio/sysfs.h> >>> +#include <linux/mutex.h> >>> + >>> +#define US5182D_REG_CFG0 0x00 >>> +#define US5182D_REG_CFG1 0x01 >>> +#define US5182D_REG_CFG2 0x02 >>> +#define US5182D_REG_CFG3 0x03 >>> +#define US5182D_REG_CFG4 0x10 >>> + >>> +/* >>> + * Registers for tuning the auto dark current cancelling feature. >>> + * DARK_TH(reg 0x27,0x28) - threshold (counts) for auto dark >cancelling. >>> + * when ALS > DARK_TH --> ALS_Code = ALS - Upper(0x2A) * Dark >>> + * when ALS < DARK_TH --> ALS_Code = ALS - Lower(0x29) * Dark >>> + */ >>> +#define US5182D_REG_UDARK_TH 0x27 >>> +#define US5182D_REG_DARK_AUTO_EN 0x2b >>> +#define US5182D_REG_AUTO_LDARK_GAIN 0x29 >>> +#define US5182D_REG_AUTO_HDARK_GAIN 0x2a >>> + >> I'd be tempted to name these to make it clear which reg >> they are in. >> US5182D_CFG0_ONESHOT_EN etc. >> Hartmut did a nice tidy up of another driver the other day. >> He pulled the register address and elements down into one >> place and used a it of indenting to make it easy to read. >> >> For example. >> >> #define US5182D_REG_CFG0 0x00 >> #define US5182D_CFG0_ONESHOT_EN BIT(6) >> #define US5182D_SHUTDOWN_EN BIT(7) >> >> etc. Just a suggestion! I don't really care as long as it >> is clear what the register map is! > >> >>> +#define US5182D_ONESHOT_EN BIT(6) >>> +#define US5182D_SHUTDOWN_EN BIT(7) >>> +#define US5182D_OPMODE_ALS 0x01 >>> +#define US5182D_OPMODE_PX 0x02 >>> +#define US5182D_OPMODE_SHIFT 4 >>> +#define US5182D_WORD_ENABLE BIT(0) >>> +#define US5182D_ALS_RES16 BIT(4) >>> +#define US5182D_PX_RES16 BIT(4) >>> +#define US5182D_PXGAIN_DEFAULT BIT(2) >>> +#define US5182D_LED_CURRENT100 (BIT(4) | BIT(5)) >>> + >>> +#define US5182D_REG_CFG0_DEFAULT \ >>> + (US5182D_SHUTDOWN_EN | US5182D_WORD_ENABLE) >> I'm missing something here, but SHUTDOWN_EN is never disabled? >> I'd have assumed that meant the device was always off! > >In oneshot mode the chip gets powered off automatically after taking >the >required measurement. I'll add a comment. >> >> Guessing some magic is going on that might want a comment here. >> (just read datasheet section on this so I now understand what you >> how this works, but please add a brief explanation, perhaps >> where you set the oneshot enable). >> >> Also more I'd not bother having these defined like this >> up here. Doesn't add anything over using them directly >> in the array below. >> >>> +#define US5182D_REG_CFG1_DEFAULT US5182D_ALS_RES16 >>> +#define US5182D_REG_CFG2_DEFAULT \ >>> + (US5182D_PX_RES16 | US5182D_PXGAIN_DEFAULT) >>> +#define US5182D_REG_CFG3_DEFAULT US5182D_LED_CURRENT100 >>> +#define US5182D_REG_CFG4_DEFAULT 0x00 >>> + >>> +#define US5182D_REG_DARK_AUTO_EN_DEFAULT 0x80 >>> +#define US5182D_REG_AUTO_LDARK_GAIN_DEFAULT 0x16 >>> +#define US5182D_REG_AUTO_HDARK_GAIN_DEFAULT 0x00 >>> + >>> +#define US5182D_REG_ADL 0x0c >>> +#define US5182D_REG_PDL 0x0e >>> + >>> +#define US5182D_REG_MODE_STORE 0x21 >>> +#define US5182D_STORE_MODE 0x01 >>> + >>> +#define US5182D_REG_CHIPID 0xb2 >>> + >>> +#define US5182D_OPMODE_MASK GENMASK(5, 4) >>> +#define US5182D_AGAIN_MASK 0x07 >>> +#define US5182D_RESET_CHIP 0x01 >>> + >>> +#define US5182D_CHIPID 0x26 >>> +#define US5182D_DRV_NAME "us5182d" >>> + >>> +#define US5182D_GA_RESOLUTION 1000 >>> + >>> +#define US5182D_READ_BYTE 1 >>> +#define US5182D_READ_WORD 2 >>> +#define US5182D_OPSTORE_SLEEP_TIME 20 /* ms */ >>> + >>> +/* available ranges: [12354, 7065, 3998, 2202, 1285, 498, 256, 138] >lux */ >>> +static const int us5182d_scales[] = {188500, 107800, 61000, 33600, >19600, 7600, >>> + 3900, 2100}; >>> + >>> +/* >>> + * experimental th's that work with US5182D sensor on evaluation >board >>> + * roughly between 12-32 lux >> Experimental (or I'll get a trivial batch within a day or so 'fixing' >it ;) >> Also long hand 'th' in the comment (thresholds persumably?). >ok >>> + */ >>> +static u16 us5182d_dark_ths_vals[] = {170, 200, 512, 512, 800, >2000, 4000, >>> + 8000}; >>> + >>> +enum mode { >>> + US5182D_ALS_PX, >>> + US5182D_ALS_ONLY, >>> + US5182D_PX_ONLY >>> +}; >>> + >>> +struct us5182d_data { >>> + struct i2c_client *client; >>> + /* protect opmode */ >> Why not name it to indicate such? >>> + struct mutex lock; >>> + >>> + /* Glass attenuation factor */ >>> + u32 ga; >>> + >>> + /* Dark gain tuning */ >>> + u8 lower_dark_gain; >>> + u8 upper_dark_gain; >>> + u16 *us5182d_dark_ths; >>> + >>> + u8 opmode; >>> +}; >>> + >>> +static IIO_CONST_ATTR(in_illuminance_scale_available, >>> + "0.0021 0.0039 0.0076 0.0196 0.0336 0.061 0.1078 0.1885"); >>> + >>> +static struct attribute *us5182d_attrs[] = { >>> + &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, >>> + NULL >>> +}; >>> + >>> +static const struct attribute_group us5182d_attr_group = { >>> + .attrs = us5182d_attrs, >>> +}; >>> + >>> +static const struct { >>> + u8 reg; >>> + u8 val; >>> +} us5182d_regvals[] = { >>> + {US5182D_REG_CFG0, US5182D_REG_CFG0_DEFAULT}, >>> + {US5182D_REG_CFG1, US5182D_REG_CFG1_DEFAULT}, >>> + {US5182D_REG_CFG2, US5182D_REG_CFG2_DEFAULT}, >>> + {US5182D_REG_CFG3, US5182D_REG_CFG3_DEFAULT}, >>> + {US5182D_REG_MODE_STORE, US5182D_STORE_MODE}, >>> + {US5182D_REG_CFG4, US5182D_REG_CFG4_DEFAULT}, >>> +}; >>> + >>> +static const struct iio_chan_spec us5182d_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 int us5182d_get_als(struct us5182d_data *data) >>> +{ >>> + int ret; >>> + unsigned long result; >>> + >>> + ret = i2c_smbus_read_word_data(data->client, >>> + US5182D_REG_ADL); >>> + if (ret < 0) >>> + return ret; >>> + >>> + result = ret * data->ga / US5182D_GA_RESOLUTION; >>> + if (result > 0xffff) >>> + result = 0xffff; >>> + >>> + return result; >>> +} >>> + >>> +static int us5182d_set_opmode(struct us5182d_data *data, u8 mode) >>> +{ >>> + int ret; >>> + >>> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG0); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = ret | US5182D_ONESHOT_EN; >>> + >>> + /* update mode */ >>> + ret = ret & ~US5182D_OPMODE_MASK; >>> + ret = ret | (mode << US5182D_OPMODE_SHIFT); >>> + >>> + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG0, >ret); >>> + if (ret < 0) >>> + return ret; >>> + >>> + if (mode == data->opmode) >>> + return 0; >>> + >>> + data->opmode = mode; >>> + ret = i2c_smbus_write_byte_data(data->client, >US5182D_REG_MODE_STORE, >>> + US5182D_STORE_MODE); >> This is a lock / commit register? If so what is happening here is a >little >> bit quirky and a brief comment to explain would be good. >I'm sorry, I am not familiar with that terminology, so i'm not sure if >that qualifies, the data sheet simply instructs that after a new opmode I probably made the terms up :) Anyhow it is whatever I meant! > >is set - we should additionally write 1 in the mode storage register. >I'll add a comment nonetheless. >> >>> + if (ret < 0) >>> + return ret; >> blank line here would make it a touch more readable (don't be afraid >> to add white space in moderation!) >>> + msleep(US5182D_OPSTORE_SLEEP_TIME); >>> + >>> + return 0; >>> +} >>> + >>> +static int us5182d_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, int *val, >>> + int *val2, long mask) >>> +{ >>> + struct us5182d_data *data = iio_priv(indio_dev); >>> + int ret; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + switch (chan->type) { >>> + case IIO_LIGHT: >>> + mutex_lock(&data->lock); >>> + ret = us5182d_set_opmode(data, US5182D_OPMODE_ALS); >>> + if (ret < 0) >>> + goto out_err; >>> + >>> + ret = us5182d_get_als(data); >>> + if (ret < 0) >>> + goto out_err; >>> + mutex_unlock(&data->lock); >>> + *val = ret; >>> + return IIO_VAL_INT; >>> + case IIO_PROXIMITY: >>> + mutex_lock(&data->lock); >>> + ret = us5182d_set_opmode(data, US5182D_OPMODE_PX); >>> + if (ret < 0) >>> + goto out_err; >>> + >>> + ret = i2c_smbus_read_word_data(data->client, >>> + US5182D_REG_PDL); >>> + if (ret < 0) >>> + goto out_err; >>> + mutex_unlock(&data->lock); >>> + *val = ret; >>> + return IIO_VAL_INT; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + case IIO_CHAN_INFO_SCALE: >>> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); >>> + if (ret < 0) >>> + return ret; >> I'd add a blank line here to aid readability (nitpick) >>> + *val = 0; >>> + ret = (ret & US5182D_AGAIN_MASK); >>> + *val2 = us5182d_scales[ret]; >> Maybe roll the two lines above into one? >> Blank line here also nice for readability and in equivalent places. >>> + return IIO_VAL_INT_PLUS_MICRO; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + return -EINVAL; >>> +out_err: >>> + mutex_unlock(&data->lock); >>> + return ret; >>> +} >>> + >>> +static int us5182d_update_dark_th(struct us5182d_data *data, int >index) >>> +{ >>> + __be16 dark_th = cpu_to_be16(data->us5182d_dark_ths[index]); >>> + u8 *bytes = (u8 *)&dark_th; >> Cast it at use site rather than here. >> >>> + int ret; >>> + >>> + /* Registers Dark_Th (0x27 0x28) don't work in word mode accessing >*/ >>> + ret = i2c_smbus_write_byte_data(data->client, >US5182D_REG_UDARK_TH, >>> + bytes[0]); >>> + if (ret < 0) >>> + return ret; >>> + >>> + return i2c_smbus_write_byte_data(data->client, >US5182D_REG_UDARK_TH + 1, >>> + bytes[1]); >>> +} >>> + >>> +static int us5182d_apply_scale(struct us5182d_data *data, int >index) >>> +{ >>> + int ret; >>> + >>> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = ret & (~US5182D_AGAIN_MASK); >>> + ret |= index; >>> + >>> + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG1, >ret); >>> + if (ret < 0) >>> + return ret; >>> + >>> + return us5182d_update_dark_th(data, index); >>> +} >>> + >>> +static int us5182d_write_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, int val, >>> + int val2, long mask) >>> +{ >>> + struct us5182d_data *data = iio_priv(indio_dev); >>> + int ret, i; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_SCALE: >>> + if (val != 0) >>> + return -EINVAL; >>> + for (i = 0; i < ARRAY_SIZE(us5182d_scales); i++) >>> + if (val2 == us5182d_scales[i]) { >>> + mutex_lock(&data->lock); >>> + ret = us5182d_apply_scale(data, i); >> >> I'd be tempted to move the lock into apply_scale... Otherwise, you >> should add kernel-doc to that function to specify the lock should be >held. >> (I think given the calls out to update_dark_th in there, I'd be >tempted >> to just document the need for both those funcs to be called with the >> lock held). >>> + mutex_unlock(&data->lock); >>> + return ret; >>> + } >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + return -EINVAL; >>> +} >>> + >>> +static const struct iio_info us5182d_info = { >>> + .driver_module = THIS_MODULE, >>> + .read_raw = us5182d_read_raw, >>> + .write_raw = us5182d_write_raw, >>> + .attrs = &us5182d_attr_group, >>> +}; >>> + >>> +static int us5182d_reset(struct iio_dev *indio_dev) >>> +{ >>> + struct us5182d_data *data = iio_priv(indio_dev); >>> + >>> + return i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG3, >>> + US5182D_RESET_CHIP); >>> +} >>> + >>> +static int us5182d_init(struct iio_dev *indio_dev) >>> +{ >>> + struct us5182d_data *data = iio_priv(indio_dev); >>> + int i, ret; >>> + >>> + ret = us5182d_reset(indio_dev); >>> + if (ret < 0) >>> + return ret; >>> + >>> + data->opmode = 0; >>> + for (i = 0; i < ARRAY_SIZE(us5182d_regvals); i++) { >>> + ret = i2c_smbus_write_byte_data(data->client, >>> + us5182d_regvals[i].reg, >>> + us5182d_regvals[i].val); >>> + if (ret < 0) >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static void us5182d_get_platform_data(struct iio_dev *indio_dev) >>> +{ >>> + struct us5182d_data *data = iio_priv(indio_dev); >>> + >>> + if (device_property_read_u32(&data->client->dev, >"upisemi,glass-coef", >>> + &data->ga)) >>> + data->ga = US5182D_GA_RESOLUTION; >>> + if (device_property_read_u16_array(&data->client->dev, >>> + "upisemi,dark-ths", >>> + data->us5182d_dark_ths, >>> + ARRAY_SIZE(us5182d_dark_ths_vals))) >>> + data->us5182d_dark_ths = us5182d_dark_ths_vals; >>> + if (device_property_read_u8(&data->client->dev, >>> + "upisemi,upper-dark-gain", >>> + &data->upper_dark_gain)) >>> + data->upper_dark_gain = US5182D_REG_AUTO_HDARK_GAIN_DEFAULT; >>> + if (device_property_read_u8(&data->client->dev, >>> + "upisemi,lower-dark-gain", >>> + &data->lower_dark_gain)) >>> + data->lower_dark_gain = US5182D_REG_AUTO_LDARK_GAIN_DEFAULT; >>> +} >>> + >>> +static int us5182d_dark_gain_config(struct iio_dev *indio_dev) >>> +{ >>> + struct us5182d_data *data = iio_priv(indio_dev); >>> + u8 index = US5182D_REG_CFG1_DEFAULT & US5182D_AGAIN_MASK; >>> + int ret; >>> + >>> + ret = us5182d_update_dark_th(data, index); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = i2c_smbus_write_byte_data(data->client, >>> + US5182D_REG_AUTO_LDARK_GAIN, >>> + data->lower_dark_gain); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = i2c_smbus_write_byte_data(data->client, >>> + US5182D_REG_AUTO_HDARK_GAIN, >>> + data->upper_dark_gain); >>> + if (ret < 0) >>> + return ret; >>> + >>> + return i2c_smbus_write_byte_data(data->client, >US5182D_REG_DARK_AUTO_EN, >>> + US5182D_REG_DARK_AUTO_EN_DEFAULT); >>> +} >>> + >>> +static int us5182d_probe(struct i2c_client *client, >>> + const struct i2c_device_id *id) >>> +{ >>> + struct us5182d_data *data; >>> + struct iio_dev *indio_dev; >>> + int ret; >>> + >>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + data = iio_priv(indio_dev); >>> + i2c_set_clientdata(client, indio_dev); >>> + data->client = client; >>> + >>> + mutex_init(&data->lock); >>> + >>> + indio_dev->dev.parent = &client->dev; >>> + indio_dev->info = &us5182d_info; >>> + indio_dev->name = US5182D_DRV_NAME; >>> + indio_dev->channels = us5182d_channels; >>> + indio_dev->num_channels = ARRAY_SIZE(us5182d_channels); >>> + indio_dev->modes = INDIO_DIRECT_MODE; >>> + >>> + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CHIPID); >>> + if (ret != US5182D_CHIPID) { >>> + dev_err(&data->client->dev, >>> + "Failed to detect US5182 light chip\n"); >>> + return (ret < 0) ? ret : -ENODEV; >>> + } >>> + >>> + us5182d_get_platform_data(indio_dev); >>> + ret = us5182d_init(indio_dev); >>> + if (ret < 0) >>> + return ret; >>> + >>> + ret = us5182d_dark_gain_config(indio_dev); >>> + if (ret < 0) >>> + return ret; >>> + >>> + return iio_device_register(indio_dev); >>> +} >>> + >>> +static int us5182d_remove(struct i2c_client *client) >>> +{ >>> + iio_device_unregister(i2c_get_clientdata(client)); >>> + return i2c_smbus_write_byte_data(client, US5182D_REG_CFG0, >>> + US5182D_REG_CFG0_DEFAULT); >> Nitpick :) >> I'd be tempted here (as we are shutting down and it doesn't matter >anyway) >> to use just the SHUTDOWN_EN value as then it's immediately obvious >> what you are doing here. >would look nicer >> >>> +} >>> + >>> +static const struct acpi_device_id us5182d_acpi_match[] = { >>> + { "USD5182", 0}, >>> + {} >>> +}; >>> + >>> +MODULE_DEVICE_TABLE(acpi, us5182d_acpi_match); >>> + >>> +static const struct i2c_device_id us5182d_id[] = { >>> + {"usd5182", 0}, >>> + {} >>> +}; >>> + >>> +MODULE_DEVICE_TABLE(i2c, us5182d_id); >>> + >>> +static struct i2c_driver us5182d_driver = { >>> + .driver = { >>> + .name = US5182D_DRV_NAME, >>> + .acpi_match_table = ACPI_PTR(us5182d_acpi_match), >>> + }, >>> + .probe = us5182d_probe, >>> + .remove = us5182d_remove, >>> + .id_table = us5182d_id, >>> + >>> +}; >>> +module_i2c_driver(us5182d_driver); >>> + >>> +MODULE_AUTHOR("Adriana Reus <adriana.reus@intel.com>"); >>> +MODULE_DESCRIPTION("Driver for us5182d Proximity and Light >Sensor"); >>> +MODULE_LICENSE("GPL v2"); >>> >> >-- >To unsubscribe from this list: send the line "unsubscribe linux-iio" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor 2015-08-14 9:29 [PATCH v3 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor Adriana Reus 2015-08-14 9:29 ` [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d " Adriana Reus @ 2015-08-14 9:29 ` Adriana Reus 2015-08-15 14:31 ` Jonathan Cameron 1 sibling, 1 reply; 10+ messages in thread From: Adriana Reus @ 2015-08-14 9:29 UTC (permalink / raw) To: jic23, pmeerw, daniel.baluta Cc: linux-iio, linux-kernel, devicetree, Adriana Reus Added entries in trivial-devices and i2c/vendor-prefixes for the us5182d als and proximity sensor. Also added a documentation file for this sensor's properties. Signed-off-by: Adriana Reus <adriana.reus@intel.com> --- No changes - resending because I forgot to cc devicetree. .../devicetree/bindings/i2c/trivial-devices.txt | 1 + .../devicetree/bindings/iio/light/us5182d.txt | 24 ++++++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.txt | 1 + 3 files changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/us5182d.txt diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index 00f8652..96d3b9c 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -99,4 +99,5 @@ ti,tsc2003 I2C Touch-Screen Controller ti,tmp102 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface ti,tmp103 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface ti,tmp275 Digital Temperature Sensor +upisemi,usd5182 Als and Proximity Sensor winbond,wpct301 i2c trusted platform module (TPM) diff --git a/Documentation/devicetree/bindings/iio/light/us5182d.txt b/Documentation/devicetree/bindings/iio/light/us5182d.txt new file mode 100644 index 0000000..9ac3336 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/light/us5182d.txt @@ -0,0 +1,24 @@ +* UPISEMI us5182d I2C ALS and Proximity sensor + +Required properties: +- compatible: must be "upisemi,usd5182" +- reg: the I2C address of the device + +Optional properties: +- upisemi,glass-coef: glass attenuation factor +- upisemi,dark-ths: array of thresholds corresponding to every scale +- upisemi,upper-dark-gain: tuning factor applied when light > th +- upisemi,lower-dark-gain: tuning factor applied when light < th + +Example: + + usd5182@39 { + compatible = "upisemi,usd5182"; + reg = <0x39>; + upisemi,glass-coef = < 1000 >; + upisemi,dark-ths = /bits/ 16 <170 200 512 512 800 2000 4000 8000>; + upisemi,upper-dark-gain = /bits/ 8 <0x00>; + upisemi,lower-dark-gain = /bits/ 8 <0x16>; + }; + + diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index d444757..5b40bab 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -211,6 +211,7 @@ toshiba Toshiba Corporation toumaz Toumaz tplink TP-LINK Technologies Co., Ltd. truly Truly Semiconductors Limited +upisemi uPI Semiconductor Corp. usi Universal Scientific Industrial Co., Ltd. v3 V3 Semiconductor variscite Variscite Ltd. -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor 2015-08-14 9:29 ` [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor Adriana Reus @ 2015-08-15 14:31 ` Jonathan Cameron 2015-08-18 15:13 ` Adriana Reus 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2015-08-15 14:31 UTC (permalink / raw) To: Adriana Reus, pmeerw, daniel.baluta; +Cc: linux-iio, linux-kernel, devicetree On 14/08/15 10:29, Adriana Reus wrote: > Added entries in trivial-devices and i2c/vendor-prefixes for > the us5182d als and proximity sensor. Also added a documentation file for > this sensor's properties. > > Signed-off-by: Adriana Reus <adriana.reus@intel.com> It's not a trivial device if it has it's own docs. So don't add it to that list (the point is to not have separate docs for devices that don't really have any device tree data other than where they are. Few more bits inline. > --- > No changes - resending because I forgot to cc devicetree. > .../devicetree/bindings/i2c/trivial-devices.txt | 1 + > .../devicetree/bindings/iio/light/us5182d.txt | 24 ++++++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.txt | 1 + > 3 files changed, 26 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/light/us5182d.txt > > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > index 00f8652..96d3b9c 100644 > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > @@ -99,4 +99,5 @@ ti,tsc2003 I2C Touch-Screen Controller > ti,tmp102 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface > ti,tmp103 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface > ti,tmp275 Digital Temperature Sensor > +upisemi,usd5182 Als and Proximity Sensor > winbond,wpct301 i2c trusted platform module (TPM) > diff --git a/Documentation/devicetree/bindings/iio/light/us5182d.txt b/Documentation/devicetree/bindings/iio/light/us5182d.txt > new file mode 100644 > index 0000000..9ac3336 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/light/us5182d.txt > @@ -0,0 +1,24 @@ > +* UPISEMI us5182d I2C ALS and Proximity sensor > + > +Required properties: > +- compatible: must be "upisemi,usd5182" > +- reg: the I2C address of the device > + > +Optional properties: > +- upisemi,glass-coef: glass attenuation factor > +- upisemi,dark-ths: array of thresholds corresponding to every scale That needs more detail. I've read the driver and I am not sure what exactly you mean! Why should a scale have a threshold? > +- upisemi,upper-dark-gain: tuning factor applied when light > th > +- upisemi,lower-dark-gain: tuning factor applied when light < th > + > +Example: > + > + usd5182@39 { > + compatible = "upisemi,usd5182"; > + reg = <0x39>; > + upisemi,glass-coef = < 1000 >; > + upisemi,dark-ths = /bits/ 16 <170 200 512 512 800 2000 4000 8000>; > + upisemi,upper-dark-gain = /bits/ 8 <0x00>; > + upisemi,lower-dark-gain = /bits/ 8 <0x16>; Not sure why these are in hex.. Or why we care if they are 8 bits. If there is a limit on the possible values, perhaps mention it in the docs above. > + }; > + One blank line at the end is neough. > + > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt > index d444757..5b40bab 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.txt > +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt > @@ -211,6 +211,7 @@ toshiba Toshiba Corporation > toumaz Toumaz > tplink TP-LINK Technologies Co., Ltd. > truly Truly Semiconductors Limited > +upisemi uPI Semiconductor Corp. > usi Universal Scientific Industrial Co., Ltd. > v3 V3 Semiconductor > variscite Variscite Ltd. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor 2015-08-15 14:31 ` Jonathan Cameron @ 2015-08-18 15:13 ` Adriana Reus 2015-08-18 17:36 ` Jonathan Cameron 0 siblings, 1 reply; 10+ messages in thread From: Adriana Reus @ 2015-08-18 15:13 UTC (permalink / raw) To: Jonathan Cameron, pmeerw, daniel.baluta Cc: linux-iio, linux-kernel, devicetree Thank you Jonathan, I'll add a new patch set soon, added some comments inline also. Adriana On 15.08.2015 17:31, Jonathan Cameron wrote: > On 14/08/15 10:29, Adriana Reus wrote: >> Added entries in trivial-devices and i2c/vendor-prefixes for >> the us5182d als and proximity sensor. Also added a documentation file for >> this sensor's properties. >> >> Signed-off-by: Adriana Reus <adriana.reus@intel.com> > It's not a trivial device if it has it's own docs. So don't add it to that list > (the point is to not have separate docs for devices that don't really have > any device tree data other than where they are. right, I'll add a new path set soon. > > Few more bits inline. >> --- >> No changes - resending because I forgot to cc devicetree. >> .../devicetree/bindings/i2c/trivial-devices.txt | 1 + >> .../devicetree/bindings/iio/light/us5182d.txt | 24 ++++++++++++++++++++++ >> .../devicetree/bindings/vendor-prefixes.txt | 1 + >> 3 files changed, 26 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/light/us5182d.txt >> >> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt >> index 00f8652..96d3b9c 100644 >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt >> @@ -99,4 +99,5 @@ ti,tsc2003 I2C Touch-Screen Controller >> ti,tmp102 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface >> ti,tmp103 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface >> ti,tmp275 Digital Temperature Sensor >> +upisemi,usd5182 Als and Proximity Sensor >> winbond,wpct301 i2c trusted platform module (TPM) >> diff --git a/Documentation/devicetree/bindings/iio/light/us5182d.txt b/Documentation/devicetree/bindings/iio/light/us5182d.txt >> new file mode 100644 >> index 0000000..9ac3336 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/light/us5182d.txt >> @@ -0,0 +1,24 @@ >> +* UPISEMI us5182d I2C ALS and Proximity sensor >> + >> +Required properties: >> +- compatible: must be "upisemi,usd5182" >> +- reg: the I2C address of the device >> + >> +Optional properties: >> +- upisemi,glass-coef: glass attenuation factor >> +- upisemi,dark-ths: array of thresholds corresponding to every scale > That needs more detail. I've read the driver and I am not sure what exactly > you mean! Why should a scale have a threshold? I'll try to be more specific. These are values representing adc counts, that's why there are different values corresponding to every scale. >> +- upisemi,upper-dark-gain: tuning factor applied when light > th >> +- upisemi,lower-dark-gain: tuning factor applied when light < th >> + >> +Example: >> + >> + usd5182@39 { >> + compatible = "upisemi,usd5182"; >> + reg = <0x39>; >> + upisemi,glass-coef = < 1000 >; >> + upisemi,dark-ths = /bits/ 16 <170 200 512 512 800 2000 4000 8000>; >> + upisemi,upper-dark-gain = /bits/ 8 <0x00>; >> + upisemi,lower-dark-gain = /bits/ 8 <0x16>; > Not sure why these are in hex.. Or why we care if they are 8 bits. If there is a limit > on the possible values, perhaps mention it in the docs above. I should have been (much) more specific here: that represents a float number with 4 integer bits and 4 fractional bits (Q4.4), so I find hex more intuitive since it's split in half, let me know if you think otherwise. I'll add a more complete description. As for the "/bits/ x" it's because currently in the driver I use the read_property_u8 or *_u16 functions, these require that the dts entry be as I wrote it in the example, and since it's an example it should be functional, so I feel that is ok as it is. > > >> + }; >> + > One blank line at the end is neough. >> + >> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt >> index d444757..5b40bab 100644 >> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt >> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt >> @@ -211,6 +211,7 @@ toshiba Toshiba Corporation >> toumaz Toumaz >> tplink TP-LINK Technologies Co., Ltd. >> truly Truly Semiconductors Limited >> +upisemi uPI Semiconductor Corp. >> usi Universal Scientific Industrial Co., Ltd. >> v3 V3 Semiconductor >> variscite Variscite Ltd. >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor 2015-08-18 15:13 ` Adriana Reus @ 2015-08-18 17:36 ` Jonathan Cameron 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2015-08-18 17:36 UTC (permalink / raw) To: Adriana Reus, Jonathan Cameron, pmeerw, daniel.baluta Cc: linux-iio, linux-kernel, devicetree On 18 August 2015 16:13:02 BST, Adriana Reus <adriana.reus@intel.com> wrote: > >Thank you Jonathan, I'll add a new patch set soon, added some comments >inline also. > >Adriana >On 15.08.2015 17:31, Jonathan Cameron wrote: >> On 14/08/15 10:29, Adriana Reus wrote: >>> Added entries in trivial-devices and i2c/vendor-prefixes for >>> the us5182d als and proximity sensor. Also added a documentation >file for >>> this sensor's properties. >>> >>> Signed-off-by: Adriana Reus <adriana.reus@intel.com> >> It's not a trivial device if it has it's own docs. So don't add it >to that list >> (the point is to not have separate docs for devices that don't really >have >> any device tree data other than where they are. >right, I'll add a new path set soon. >> >> Few more bits inline. >>> --- >>> No changes - resending because I forgot to cc devicetree. >>> .../devicetree/bindings/i2c/trivial-devices.txt | 1 + >>> .../devicetree/bindings/iio/light/us5182d.txt | 24 >++++++++++++++++++++++ >>> .../devicetree/bindings/vendor-prefixes.txt | 1 + >>> 3 files changed, 26 insertions(+) >>> create mode 100644 >Documentation/devicetree/bindings/iio/light/us5182d.txt >>> >>> diff --git >a/Documentation/devicetree/bindings/i2c/trivial-devices.txt >b/Documentation/devicetree/bindings/i2c/trivial-devices.txt >>> index 00f8652..96d3b9c 100644 >>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt >>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt >>> @@ -99,4 +99,5 @@ ti,tsc2003 I2C Touch-Screen Controller >>> ti,tmp102 Low Power Digital Temperature Sensor with SMBUS/Two >Wire Serial Interface >>> ti,tmp103 Low Power Digital Temperature Sensor with SMBUS/Two >Wire Serial Interface >>> ti,tmp275 Digital Temperature Sensor >>> +upisemi,usd5182 Als and Proximity Sensor >>> winbond,wpct301 i2c trusted platform module (TPM) >>> diff --git a/Documentation/devicetree/bindings/iio/light/us5182d.txt >b/Documentation/devicetree/bindings/iio/light/us5182d.txt >>> new file mode 100644 >>> index 0000000..9ac3336 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/light/us5182d.txt >>> @@ -0,0 +1,24 @@ >>> +* UPISEMI us5182d I2C ALS and Proximity sensor >>> + >>> +Required properties: >>> +- compatible: must be "upisemi,usd5182" >>> +- reg: the I2C address of the device >>> + >>> +Optional properties: >>> +- upisemi,glass-coef: glass attenuation factor >>> +- upisemi,dark-ths: array of thresholds corresponding to every >scale >> That needs more detail. I've read the driver and I am not sure what >exactly >> you mean! Why should a scale have a threshold? >I'll try to be more specific. These are values representing adc counts, > >that's why there are different values corresponding to every scale. >>> +- upisemi,upper-dark-gain: tuning factor applied when light > th >>> +- upisemi,lower-dark-gain: tuning factor applied when light < th >>> + >>> +Example: >>> + >>> + usd5182@39 { >>> + compatible = "upisemi,usd5182"; >>> + reg = <0x39>; >>> + upisemi,glass-coef = < 1000 >; >>> + upisemi,dark-ths = /bits/ 16 <170 200 512 512 800 >2000 4000 8000>; >>> + upisemi,upper-dark-gain = /bits/ 8 <0x00>; >>> + upisemi,lower-dark-gain = /bits/ 8 <0x16>; >> Not sure why these are in hex.. Or why we care if they are 8 bits. >If there is a limit >> on the possible values, perhaps mention it in the docs above. >I should have been (much) more specific here: that represents a float >number with 4 integer bits and 4 fractional bits (Q4.4), so I find hex >more intuitive since it's split in half, let me know if you think >otherwise. One for the device tree guys to answer rather than me. My gut feeling would be a representation that made this obvious but I have no idea what normal choice would be. >I'll add a more complete description. As for the "/bits/ x" >it's because currently in the driver I use the read_property_u8 or >*_u16 >functions, these require that the dts entry be as I wrote it in the >example, and since it's an example it should be functional, so I feel >that is ok as it is. >> >> >>> + }; >>> + >> One blank line at the end is neough. >>> + >>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt >b/Documentation/devicetree/bindings/vendor-prefixes.txt >>> index d444757..5b40bab 100644 >>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt >>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt >>> @@ -211,6 +211,7 @@ toshiba Toshiba Corporation >>> toumaz Toumaz >>> tplink TP-LINK Technologies Co., Ltd. >>> truly Truly Semiconductors Limited >>> +upisemi uPI Semiconductor Corp. >>> usi Universal Scientific Industrial Co., Ltd. >>> v3 V3 Semiconductor >>> variscite Variscite Ltd. >>> >> >-- >To unsubscribe from this list: send the line "unsubscribe linux-iio" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor
@ 2015-08-14 9:17 Adriana Reus
2015-08-14 9:17 ` [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d " Adriana Reus
0 siblings, 1 reply; 10+ messages in thread
From: Adriana Reus @ 2015-08-14 9:17 UTC (permalink / raw)
To: jic23, pmeerw, daniel.baluta; +Cc: linux-iio, linux-kernel, Adriana Reus
This series adds basic support for this als and proximity sensor and devicetree docs.
Adriana Reus (2):
iio: light: Add support for UPISEMI uS5182d als and proximity sensor
devicetree: Add documentation for UPISEMI us5182d ALS and Proximity
sensor
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
.../devicetree/bindings/iio/light/us5182d.txt | 24 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/iio/light/Kconfig | 10 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/us5182d.c | 484 +++++++++++++++++++++
6 files changed, 521 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/light/us5182d.txt
create mode 100644 drivers/iio/light/us5182d.c
--
1.9.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d als and proximity sensor 2015-08-14 9:17 [PATCH 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor Adriana Reus @ 2015-08-14 9:17 ` Adriana Reus 0 siblings, 0 replies; 10+ messages in thread From: Adriana Reus @ 2015-08-14 9:17 UTC (permalink / raw) To: jic23, pmeerw, daniel.baluta; +Cc: linux-iio, linux-kernel, Adriana Reus Add support for UPISEMI us5182d als and proximity sensor. Supports raw readings. Data sheet for this device can be found here: http://www.upi-semi.com/temp/uS5182D-DS-P0103-temp.pdf Signed-off-by: Adriana Reus <adriana.reus@intel.com> --- Changes since v2: - Adressed Peter's comments (with the exception of setting the ths_vals array as const - if I do that the compiler will eventually whine because it gets passed as an argument to device_property_read). - Tried to break down the compound register values into more intuitive definitions. - Eliminated the read helper functions. drivers/iio/light/Kconfig | 10 + drivers/iio/light/Makefile | 1 + drivers/iio/light/us5182d.c | 484 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 495 insertions(+) create mode 100644 drivers/iio/light/us5182d.c diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index 7ed859a..0442f01 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -287,6 +287,16 @@ config TSL4531 To compile this driver as a module, choose M here: the module will be called tsl4531. +config US5182D + tristate "UPISEMI light and proximity sensor" + depends on I2C + help + If you say yes here you get support for the UPISEMI US5182D + ambient light and proximity sensor. + + This driver can also be built as a module. If so, the module + will be called us5182d. + config VCNL4000 tristate "VCNL4000 combined ALS and proximity sensor" depends on I2C diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index 91c74c0..528cc8f 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -27,4 +27,5 @@ obj-$(CONFIG_STK3310) += stk3310.o obj-$(CONFIG_TCS3414) += tcs3414.o obj-$(CONFIG_TCS3472) += tcs3472.o obj-$(CONFIG_TSL4531) += tsl4531.o +obj-$(CONFIG_US5182D) += us5182d.o obj-$(CONFIG_VCNL4000) += vcnl4000.o diff --git a/drivers/iio/light/us5182d.c b/drivers/iio/light/us5182d.c new file mode 100644 index 0000000..9ba99f5 --- /dev/null +++ b/drivers/iio/light/us5182d.c @@ -0,0 +1,484 @@ +/* + * Copyright (c) 2015 Intel Corporation + * + * Driver for UPISEMI us5182d Proximity and Ambient Light Sensor. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope 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. + * + * To do: Interrupt support. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/acpi.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/mutex.h> + +#define US5182D_REG_CFG0 0x00 +#define US5182D_REG_CFG1 0x01 +#define US5182D_REG_CFG2 0x02 +#define US5182D_REG_CFG3 0x03 +#define US5182D_REG_CFG4 0x10 + +/* + * Registers for tuning the auto dark current cancelling feature. + * DARK_TH(reg 0x27,0x28) - threshold (counts) for auto dark cancelling. + * when ALS > DARK_TH --> ALS_Code = ALS - Upper(0x2A) * Dark + * when ALS < DARK_TH --> ALS_Code = ALS - Lower(0x29) * Dark + */ +#define US5182D_REG_UDARK_TH 0x27 +#define US5182D_REG_DARK_AUTO_EN 0x2b +#define US5182D_REG_AUTO_LDARK_GAIN 0x29 +#define US5182D_REG_AUTO_HDARK_GAIN 0x2a + +#define US5182D_ONESHOT_EN BIT(6) +#define US5182D_SHUTDOWN_EN BIT(7) +#define US5182D_OPMODE_ALS 0x01 +#define US5182D_OPMODE_PX 0x02 +#define US5182D_OPMODE_SHIFT 4 +#define US5182D_WORD_ENABLE BIT(0) +#define US5182D_ALS_RES16 BIT(4) +#define US5182D_PX_RES16 BIT(4) +#define US5182D_PXGAIN_DEFAULT BIT(2) +#define US5182D_LED_CURRENT100 (BIT(4) | BIT(5)) + +#define US5182D_REG_CFG0_DEFAULT \ + (US5182D_SHUTDOWN_EN | US5182D_WORD_ENABLE) +#define US5182D_REG_CFG1_DEFAULT US5182D_ALS_RES16 +#define US5182D_REG_CFG2_DEFAULT \ + (US5182D_PX_RES16 | US5182D_PXGAIN_DEFAULT) +#define US5182D_REG_CFG3_DEFAULT US5182D_LED_CURRENT100 +#define US5182D_REG_CFG4_DEFAULT 0x00 + +#define US5182D_REG_DARK_AUTO_EN_DEFAULT 0x80 +#define US5182D_REG_AUTO_LDARK_GAIN_DEFAULT 0x16 +#define US5182D_REG_AUTO_HDARK_GAIN_DEFAULT 0x00 + +#define US5182D_REG_ADL 0x0c +#define US5182D_REG_PDL 0x0e + +#define US5182D_REG_MODE_STORE 0x21 +#define US5182D_STORE_MODE 0x01 + +#define US5182D_REG_CHIPID 0xb2 + +#define US5182D_OPMODE_MASK GENMASK(5, 4) +#define US5182D_AGAIN_MASK 0x07 +#define US5182D_RESET_CHIP 0x01 + +#define US5182D_CHIPID 0x26 +#define US5182D_DRV_NAME "us5182d" + +#define US5182D_GA_RESOLUTION 1000 + +#define US5182D_READ_BYTE 1 +#define US5182D_READ_WORD 2 +#define US5182D_OPSTORE_SLEEP_TIME 20 /* ms */ + +/* available ranges: [12354, 7065, 3998, 2202, 1285, 498, 256, 138] lux */ +static const int us5182d_scales[] = {188500, 107800, 61000, 33600, 19600, 7600, + 3900, 2100}; + +/* + * experimental th's that work with US5182D sensor on evaluation board + * roughly between 12-32 lux + */ +static u16 us5182d_dark_ths_vals[] = {170, 200, 512, 512, 800, 2000, 4000, + 8000}; + +enum mode { + US5182D_ALS_PX, + US5182D_ALS_ONLY, + US5182D_PX_ONLY +}; + +struct us5182d_data { + struct i2c_client *client; + /* protect opmode */ + struct mutex lock; + + /* Glass attenuation factor */ + u32 ga; + + /* Dark gain tuning */ + u8 lower_dark_gain; + u8 upper_dark_gain; + u16 *us5182d_dark_ths; + + u8 opmode; +}; + +static IIO_CONST_ATTR(in_illuminance_scale_available, + "0.0021 0.0039 0.0076 0.0196 0.0336 0.061 0.1078 0.1885"); + +static struct attribute *us5182d_attrs[] = { + &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, + NULL +}; + +static const struct attribute_group us5182d_attr_group = { + .attrs = us5182d_attrs, +}; + +static const struct { + u8 reg; + u8 val; +} us5182d_regvals[] = { + {US5182D_REG_CFG0, US5182D_REG_CFG0_DEFAULT}, + {US5182D_REG_CFG1, US5182D_REG_CFG1_DEFAULT}, + {US5182D_REG_CFG2, US5182D_REG_CFG2_DEFAULT}, + {US5182D_REG_CFG3, US5182D_REG_CFG3_DEFAULT}, + {US5182D_REG_MODE_STORE, US5182D_STORE_MODE}, + {US5182D_REG_CFG4, US5182D_REG_CFG4_DEFAULT}, +}; + +static const struct iio_chan_spec us5182d_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 int us5182d_get_als(struct us5182d_data *data) +{ + int ret; + unsigned long result; + + ret = i2c_smbus_read_word_data(data->client, + US5182D_REG_ADL); + if (ret < 0) + return ret; + + result = ret * data->ga / US5182D_GA_RESOLUTION; + if (result > 0xffff) + result = 0xffff; + + return result; +} + +static int us5182d_set_opmode(struct us5182d_data *data, u8 mode) +{ + int ret; + + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG0); + if (ret < 0) + return ret; + + ret = ret | US5182D_ONESHOT_EN; + + /* update mode */ + ret = ret & ~US5182D_OPMODE_MASK; + ret = ret | (mode << US5182D_OPMODE_SHIFT); + + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG0, ret); + if (ret < 0) + return ret; + + if (mode == data->opmode) + return 0; + + data->opmode = mode; + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_MODE_STORE, + US5182D_STORE_MODE); + if (ret < 0) + return ret; + msleep(US5182D_OPSTORE_SLEEP_TIME); + + return 0; +} + +static int us5182d_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct us5182d_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + switch (chan->type) { + case IIO_LIGHT: + mutex_lock(&data->lock); + ret = us5182d_set_opmode(data, US5182D_OPMODE_ALS); + if (ret < 0) + goto out_err; + + ret = us5182d_get_als(data); + if (ret < 0) + goto out_err; + mutex_unlock(&data->lock); + *val = ret; + return IIO_VAL_INT; + case IIO_PROXIMITY: + mutex_lock(&data->lock); + ret = us5182d_set_opmode(data, US5182D_OPMODE_PX); + if (ret < 0) + goto out_err; + + ret = i2c_smbus_read_word_data(data->client, + US5182D_REG_PDL); + if (ret < 0) + goto out_err; + mutex_unlock(&data->lock); + *val = ret; + return IIO_VAL_INT; + default: + return -EINVAL; + } + + case IIO_CHAN_INFO_SCALE: + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); + if (ret < 0) + return ret; + *val = 0; + ret = (ret & US5182D_AGAIN_MASK); + *val2 = us5182d_scales[ret]; + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } + + return -EINVAL; +out_err: + mutex_unlock(&data->lock); + return ret; +} + +static int us5182d_update_dark_th(struct us5182d_data *data, int index) +{ + __be16 dark_th = cpu_to_be16(data->us5182d_dark_ths[index]); + u8 *bytes = (u8 *)&dark_th; + int ret; + + /* Registers Dark_Th (0x27 0x28) don't work in word mode accessing */ + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH, + bytes[0]); + if (ret < 0) + return ret; + + return i2c_smbus_write_byte_data(data->client, US5182D_REG_UDARK_TH + 1, + bytes[1]); +} + +static int us5182d_apply_scale(struct us5182d_data *data, int index) +{ + int ret; + + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CFG1); + if (ret < 0) + return ret; + + ret = ret & (~US5182D_AGAIN_MASK); + ret |= index; + + ret = i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG1, ret); + if (ret < 0) + return ret; + + return us5182d_update_dark_th(data, index); +} + +static int us5182d_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int val, + int val2, long mask) +{ + struct us5182d_data *data = iio_priv(indio_dev); + int ret, i; + + switch (mask) { + case IIO_CHAN_INFO_SCALE: + if (val != 0) + return -EINVAL; + for (i = 0; i < ARRAY_SIZE(us5182d_scales); i++) + if (val2 == us5182d_scales[i]) { + mutex_lock(&data->lock); + ret = us5182d_apply_scale(data, i); + mutex_unlock(&data->lock); + return ret; + } + break; + default: + return -EINVAL; + } + + return -EINVAL; +} + +static const struct iio_info us5182d_info = { + .driver_module = THIS_MODULE, + .read_raw = us5182d_read_raw, + .write_raw = us5182d_write_raw, + .attrs = &us5182d_attr_group, +}; + +static int us5182d_reset(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + + return i2c_smbus_write_byte_data(data->client, US5182D_REG_CFG3, + US5182D_RESET_CHIP); +} + +static int us5182d_init(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + int i, ret; + + ret = us5182d_reset(indio_dev); + if (ret < 0) + return ret; + + data->opmode = 0; + for (i = 0; i < ARRAY_SIZE(us5182d_regvals); i++) { + ret = i2c_smbus_write_byte_data(data->client, + us5182d_regvals[i].reg, + us5182d_regvals[i].val); + if (ret < 0) + return ret; + } + + return 0; +} + +static void us5182d_get_platform_data(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + + if (device_property_read_u32(&data->client->dev, "upisemi,glass-coef", + &data->ga)) + data->ga = US5182D_GA_RESOLUTION; + if (device_property_read_u16_array(&data->client->dev, + "upisemi,dark-ths", + data->us5182d_dark_ths, + ARRAY_SIZE(us5182d_dark_ths_vals))) + data->us5182d_dark_ths = us5182d_dark_ths_vals; + if (device_property_read_u8(&data->client->dev, + "upisemi,upper-dark-gain", + &data->upper_dark_gain)) + data->upper_dark_gain = US5182D_REG_AUTO_HDARK_GAIN_DEFAULT; + if (device_property_read_u8(&data->client->dev, + "upisemi,lower-dark-gain", + &data->lower_dark_gain)) + data->lower_dark_gain = US5182D_REG_AUTO_LDARK_GAIN_DEFAULT; +} + +static int us5182d_dark_gain_config(struct iio_dev *indio_dev) +{ + struct us5182d_data *data = iio_priv(indio_dev); + u8 index = US5182D_REG_CFG1_DEFAULT & US5182D_AGAIN_MASK; + int ret; + + ret = us5182d_update_dark_th(data, index); + if (ret < 0) + return ret; + + ret = i2c_smbus_write_byte_data(data->client, + US5182D_REG_AUTO_LDARK_GAIN, + data->lower_dark_gain); + if (ret < 0) + return ret; + + ret = i2c_smbus_write_byte_data(data->client, + US5182D_REG_AUTO_HDARK_GAIN, + data->upper_dark_gain); + if (ret < 0) + return ret; + + return i2c_smbus_write_byte_data(data->client, US5182D_REG_DARK_AUTO_EN, + US5182D_REG_DARK_AUTO_EN_DEFAULT); +} + +static int us5182d_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct us5182d_data *data; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + i2c_set_clientdata(client, indio_dev); + data->client = client; + + mutex_init(&data->lock); + + indio_dev->dev.parent = &client->dev; + indio_dev->info = &us5182d_info; + indio_dev->name = US5182D_DRV_NAME; + indio_dev->channels = us5182d_channels; + indio_dev->num_channels = ARRAY_SIZE(us5182d_channels); + indio_dev->modes = INDIO_DIRECT_MODE; + + ret = i2c_smbus_read_byte_data(data->client, US5182D_REG_CHIPID); + if (ret != US5182D_CHIPID) { + dev_err(&data->client->dev, + "Failed to detect US5182 light chip\n"); + return (ret < 0) ? ret : -ENODEV; + } + + us5182d_get_platform_data(indio_dev); + ret = us5182d_init(indio_dev); + if (ret < 0) + return ret; + + ret = us5182d_dark_gain_config(indio_dev); + if (ret < 0) + return ret; + + return iio_device_register(indio_dev); +} + +static int us5182d_remove(struct i2c_client *client) +{ + iio_device_unregister(i2c_get_clientdata(client)); + return i2c_smbus_write_byte_data(client, US5182D_REG_CFG0, + US5182D_REG_CFG0_DEFAULT); +} + +static const struct acpi_device_id us5182d_acpi_match[] = { + { "USD5182", 0}, + {} +}; + +MODULE_DEVICE_TABLE(acpi, us5182d_acpi_match); + +static const struct i2c_device_id us5182d_id[] = { + {"usd5182", 0}, + {} +}; + +MODULE_DEVICE_TABLE(i2c, us5182d_id); + +static struct i2c_driver us5182d_driver = { + .driver = { + .name = US5182D_DRV_NAME, + .acpi_match_table = ACPI_PTR(us5182d_acpi_match), + }, + .probe = us5182d_probe, + .remove = us5182d_remove, + .id_table = us5182d_id, + +}; +module_i2c_driver(us5182d_driver); + +MODULE_AUTHOR("Adriana Reus <adriana.reus@intel.com>"); +MODULE_DESCRIPTION("Driver for us5182d Proximity and Light Sensor"); +MODULE_LICENSE("GPL v2"); -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-08-18 17:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-14 9:29 [PATCH v3 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor Adriana Reus 2015-08-14 9:29 ` [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d " Adriana Reus 2015-08-15 14:27 ` Jonathan Cameron 2015-08-18 15:28 ` Adriana Reus 2015-08-18 17:33 ` Jonathan Cameron 2015-08-14 9:29 ` [PATCH v3 2/2] devicetree: Add documentation for UPISEMI us5182d ALS and Proximity sensor Adriana Reus 2015-08-15 14:31 ` Jonathan Cameron 2015-08-18 15:13 ` Adriana Reus 2015-08-18 17:36 ` Jonathan Cameron -- strict thread matches above, loose matches on Subject: below -- 2015-08-14 9:17 [PATCH 0/2] Add support and documentation for UPISEMI us5182d als and proximity sensor Adriana Reus 2015-08-14 9:17 ` [PATCH v3 1/2] iio: light: Add support for UPISEMI uS5182d " Adriana Reus
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).