On Sun, 5 Nov 2017 23:48:13 +0100 (CET) Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx> wrote: > Hello Jonathan, > > > Another very nice driver. A couple of refactoring suggestions inline > > that I think would slightly (and it is slightly!) improve readability. > > I think we need some additional docs for the uv intensity channel as I can't > > find any documented units for that - so far they have been fairly randomly > > filtered and no docs have existed to map these to units. > > UV intensity is already documented (around line 1288) as > /sys/.../iio:deviceX/in_intensityY_uv_raw > however > /sys/.../iio:deviceX/in_intensity_raw and > /sys/.../iio:deviceX/in_intensity_uv_raw are missing, should these be > added for completeness?? Yes, if we have drivers using those elements they should be in the ABI docs. > > > As such I'm thinking w/m^2 would be a nicer base unit. If I'm missing > > a prior example of this then let me know - I haven't really looked :) > > intensity_uv is documented as unit-less currently; > not sure if we can suggest a unit after-the-fact; drivers may use _SCALE > to set a gain, and not care about the unit we can if we don't have current users specifying a scale ;) Which they shouldn't be if they are unit less. > > other chips such as the AS7262 are using micro W/cm2; > our channel modifiers (both, clear, uv, red, green blue) may not be > sufficient for spectral sensors (6 visible channels at 450, 500, 550, 570, > 600, 650 -- AS7262; NIR channels at 610, 680, 730, 760, 810, 860 -- > AS7263) Hmm. When we originally came up with the infrared terms I wondered if we would be better off with more explicit description of wavelength / frequency. Perhaps we need to add that as additional ABI? > > will post a v2 with your suggestions shortly, thanks for the review! > comments to your suggestions below... > > regards, p. > > > > --- > > > drivers/iio/light/Kconfig | 10 + > > > drivers/iio/light/Makefile | 1 + > > > drivers/iio/light/zopt2201.c | 563 +++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 574 insertions(+) > > > create mode 100644 drivers/iio/light/zopt2201.c > > > > > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > > > index 2356ed9285df..6a5835fab32e 100644 > > > --- a/drivers/iio/light/Kconfig > > > +++ b/drivers/iio/light/Kconfig > > > @@ -425,4 +425,14 @@ config VL6180 > > > To compile this driver as a module, choose M here: the > > > module will be called vl6180. > > > > > > +config ZOPT2201 > > > + tristate "ZOPT2201 ALS and UV B sensor" > > > + depends on I2C > > > + help > > > + Say Y here if you want to build a driver for the IDT > > > + ZOPT2201 ambient light and UV B sensor. > > > + > > > + To compile this driver as a module, choose M here: the > > > + module will be called zopt2201. > > > + > > > endmenu > > > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > > > index fa32fa459e2e..d99abdf6b746 100644 > > > --- a/drivers/iio/light/Makefile > > > +++ b/drivers/iio/light/Makefile > > > @@ -40,3 +40,4 @@ obj-$(CONFIG_US5182D) += us5182d.o > > > obj-$(CONFIG_VCNL4000) += vcnl4000.o > > > obj-$(CONFIG_VEML6070) += veml6070.o > > > obj-$(CONFIG_VL6180) += vl6180.o > > > +obj-$(CONFIG_ZOPT2201) += zopt2201.o > > > diff --git a/drivers/iio/light/zopt2201.c b/drivers/iio/light/zopt2201.c > > > new file mode 100644 > > > index 000000000000..07b299642ac3 > > > --- /dev/null > > > +++ b/drivers/iio/light/zopt2201.c > > > @@ -0,0 +1,563 @@ > > > +/* > > > + * zopt2201.c - Support for IDT ZOPT2201 ambient light and UV B sensor > > > + * > > > + * Copyright 2017 Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx> > > > + * > > > + * This file is subject to the terms and conditions of version 2 of > > > + * the GNU General Public License. See the file COPYING in the main > > > + * directory of this archive for more details. > > > + * > > > + * Datasheet: https://www.idt.com/document/dst/zopt2201-datasheet > > > + * 7-bit I2C slave addresses 0x53 (default) or 0x52 (programmed) > > > + * > > > + * TODO: interrupt support, ALS/UVB raw mode > > > + */ > > > + > > > +#include <linux/module.h> > > > +#include <linux/i2c.h> > > > +#include <linux/mutex.h> > > > +#include <linux/err.h> > > > +#include <linux/delay.h> > > > + > > > +#include <linux/iio/iio.h> > > > +#include <linux/iio/sysfs.h> > > > + > > > +#define ZOPT2201_DRV_NAME "zopt2201" > > > + > > > +/* Registers */ > > > +#define ZOPT2201_MAIN_CTRL 0x00 > > > +#define ZOPT2201_LS_MEAS_RATE 0x04 > > > +#define ZOPT2201_LS_GAIN 0x05 > > > +#define ZOPT2201_PART_ID 0x06 > > > +#define ZOPT2201_MAIN_STATUS 0x07 > > > +#define ZOPT2201_ALS_DATA 0x0d /* LSB first, 13 to 20 bits */ > > > +#define ZOPT2201_UVB_DATA 0x10 /* LSB first, 13 to 20 bits */ > > > +#define ZOPT2201_UV_COMP_DATA 0x13 /* LSB first, 13 to 20 bits */ > > > +#define ZOPT2201_COMP_DATA 0x16 /* LSB first, 13 to 20 bits */ > > > +#define ZOPT2201_INT_CFG 0x19 > > > +#define ZOPT2201_INT_PST 0x1a > > > + > > > +#define ZOPT2201_MAIN_CTRL_LS_MODE BIT(3) /* 0 .. ALS, 1 .. UV B */ > > > +#define ZOPT2201_MAIN_CTRL_LS_EN BIT(1) > > > + > > > +/* Values for ZOPT2201_LS_MEAS_RATE resolution / bit width */ > > > +#define ZOPT2201_MEAS_RES_20BIT 0 /* takes 400 ms */ > > > +#define ZOPT2201_MEAS_RES_19BIT 1 /* takes 200 ms */ > > > +#define ZOPT2201_MEAS_RES_18BIT 2 /* takes 100 ms, default */ > > > +#define ZOPT2201_MEAS_RES_17BIT 3 /* takes 50 ms */ > > > +#define ZOPT2201_MEAS_RES_16BIT 4 /* takes 25 ms */ > > > +#define ZOPT2201_MEAS_RES_13BIT 5 /* takes 3.125 ms */ > > > +#define ZOPT2201_MEAS_RES_SHIFT 4 > > > + > > > +/* Values for ZOPT2201_LS_MEAS_RATE measurement rate */ > > > +#define ZOPT2201_MEAS_FREQ_25MS 0 > > > +#define ZOPT2201_MEAS_FREQ_50MS 1 > > > +#define ZOPT2201_MEAS_FREQ_100MS 2 /* default */ > > > +#define ZOPT2201_MEAS_FREQ_200MS 3 > > > +#define ZOPT2201_MEAS_FREQ_500MS 4 > > > +#define ZOPT2201_MEAS_FREQ_1000MS 5 > > > +#define ZOPT2201_MEAS_FREQ_2000MS 6 > > > + > > > +/* Values for ZOPT2201_LS_GAIN */ > > > +#define ZOPT2201_LS_GAIN_1 0 > > > +#define ZOPT2201_LS_GAIN_3 1 > > > +#define ZOPT2201_LS_GAIN_6 2 > > > +#define ZOPT2201_LS_GAIN_9 3 > > > +#define ZOPT2201_LS_GAIN_18 4 > > > + > > > +/* Values for ZOPT2201_MAIN_STATUS */ > > > +#define ZOPT2201_MAIN_STATUS_POWERON BIT(5) > > > +#define ZOPT2201_MAIN_STATUS_INT BIT(4) > > > +#define ZOPT2201_MAIN_STATUS_DRDY BIT(3) > > > + > > > +#define ZOPT2201_PART_NUMBER 0xb2 > > > + > > > +struct zopt2201_data { > > > + struct i2c_client *client; > > > + struct mutex lock; > > > + u8 gain; > > > + u8 res; > > > + u8 rate; > > > +}; > > > + > > > +enum zopt2201_mode { > > > + ZOPT2201_MODE_ALS, > > > + ZOPT2201_MODE_UVB, > > > +}; > > > + > > > +static const struct { > > > + unsigned int gain; /* gain factor */ > > > + unsigned int scale; /* micro lux per count */ > > > +} zopt2201_gain_als[] = { > > > + { 1, 19200000 }, > > > + { 3, 6400000 }, > > > + { 6, 3200000 }, > > > + { 9, 2133333 }, > > > + { 18, 1066666 }, > > > +}; > > > + > > > +static const struct { > > > + unsigned int gain; /* gain factor */ > > > + unsigned int scale; /* micro W/cm2 per count */ > > > > I think docs need to be updated to cover having units for > > uv. Certainly good to assign units if we can to intensity > > channels. > > > > Hmm. micro W / cm2 so 0.001W/((0.01*0.01)m^2) > > micro W/cm2 so 0.000001W/cm2 so 0.01W/m^2 ? > > > If we are going to define a unit I'd prefer we ended up with > > w/m^2 and hence SI unit. > > I have changed the scale factor so that the driver now reports W/m^2; > the comment above ("micro W/cm2 per count") was incorrect, in fact the > values related to pico W/cm2 > > > > +} zopt2201_gain_uvb[] = { > > > + { 1, 46080000 }, > > > + { 3, 15360000 }, > > > + { 6, 7680000 }, > > > + { 9, 5120000 }, > > > + { 18, 2560000 }, > > > +}; > > > + > > > +static const struct { > > > + unsigned int bits; /* sensor resolution in bits */ > > > + unsigned long us; /* measurement time in micro seconds */ > > > +} zopt2201_resolution[] = { > > > + { 20, 400000 }, > > > + { 19, 200000 }, > > > + { 18, 100000 }, > > > + { 17, 50000 }, > > > + { 16, 25000 }, > > > + { 13, 3125 }, > > > +}; > > > + > > > +static const struct { > > > + unsigned int scale, uscale; /* scale factor as integer + micro */ > > > + u8 gain; /* gain register value */ > > > + u8 res; /* resolution register value */ > > > +} zopt2201_scale_als[] = { > > > + { 19, 200000, 0, 5 }, > > > + { 6, 400000, 1, 5 }, > > > + { 3, 200000, 2, 5 }, > > > + { 2, 400000, 0, 4 }, > > > + { 2, 133333, 3, 5 }, > > > + { 1, 200000, 0, 3 }, > > > + { 1, 66666, 4, 5 }, > > > + { 0, 800000, 1, 4 }, > > > + { 0, 600000, 0, 2 }, > > > + { 0, 400000, 2, 4 }, > > > + { 0, 300000, 0, 1 }, > > > + { 0, 266666, 3, 4 }, > > > + { 0, 200000, 2, 3 }, > > > + { 0, 150000, 0, 0 }, > > > + { 0, 133333, 4, 4 }, > > > + { 0, 100000, 2, 2 }, > > > + { 0, 66666, 4, 3 }, > > > + { 0, 50000, 2, 1 }, > > > + { 0, 33333, 4, 2 }, > > > + { 0, 25000, 2, 0 }, > > > + { 0, 16666, 4, 1 }, > > > + { 0, 8333, 4, 0 }, > > > +}; > > > + > > > +static const struct { > > > + unsigned int scale, uscale; /* scale factor as integer + micro */ > > > + u8 gain; /* gain register value */ > > > + u8 res; /* resolution register value */ > > > +} zopt2201_scale_uvb[] = { > > > + { 46, 80000, 0, 5 }, > > > + { 15, 360000, 1, 5 }, > > > + { 7, 680000, 2, 5 }, > > > + { 5, 760000, 0, 4 }, > > > + { 5, 120000, 3, 5 }, > > > + { 2, 880000, 0, 3 }, > > > + { 2, 560000, 4, 5 }, > > > + { 1, 920000, 1, 4 }, > > > + { 1, 440000, 0, 2 }, > > > + { 0, 960000, 2, 4 }, > > > + { 0, 720000, 0, 1 }, > > > + { 0, 640000, 3, 4 }, > > > + { 0, 480000, 2, 3 }, > > > + { 0, 360000, 0, 0 }, > > > + { 0, 320000, 4, 4 }, > > > + { 0, 240000, 2, 2 }, > > > + { 0, 160000, 4, 3 }, > > > + { 0, 120000, 2, 1 }, > > > + { 0, 80000, 4, 2 }, > > > + { 0, 60000, 2, 0 }, > > > + { 0, 40000, 4, 1 }, > > > + { 0, 20000, 4, 0 }, > > > +}; > > > + > > > +static int zopt2201_enable_mode(struct zopt2201_data *data, > > > + enum zopt2201_mode mode) > > > +{ > > > + u8 out = ZOPT2201_MAIN_CTRL_LS_EN; > > > + > > > + if (mode == ZOPT2201_MODE_UVB) > > > + out |= ZOPT2201_MAIN_CTRL_LS_MODE; > > > + > > > + return i2c_smbus_write_byte_data(data->client, ZOPT2201_MAIN_CTRL, out); > > > +} > > > + > > > +static int zopt2201_read(struct zopt2201_data *data, enum zopt2201_mode mode) > > > +{ > > > + struct i2c_client *client = data->client; > > > + int tries = 10; > > > + u8 buf[3]; > > > + int ret; > > > + > > > + mutex_lock(&data->lock); > > > + ret = zopt2201_enable_mode(data, mode); > > > + if (ret < 0) > > > + goto fail; > > > + > > > + while (tries--) { > > > + unsigned long t = zopt2201_resolution[data->res].us; > > > + > > > + if (t <= 20000) > > > + usleep_range(t, t + 1000); > > > + else > > > + msleep(t / 1000); > > > + ret = i2c_smbus_read_byte_data(client, ZOPT2201_MAIN_STATUS); > > > + if (ret < 0) > > > + goto fail; > > > + if (ret & ZOPT2201_MAIN_STATUS_DRDY) > > > + break; > > > + } > > > + > > > + if (tries < 0) { > > > + ret = -ETIMEDOUT; > > > + goto fail; > > > + } > > > + > > > + ret = i2c_smbus_read_i2c_block_data(client, mode == ZOPT2201_MODE_UVB ? > > > + ZOPT2201_UVB_DATA : ZOPT2201_ALS_DATA, sizeof(buf), buf); > > Having a ternary buried in there is a little hard to read. Please pull it > > out as a local variable assignment. > > OK, actually I've dropped the mode enum and will use the corresponding > register address directly (saves some lines) > > > > + if (ret < 0) > > > + goto fail; > > > + > > > + ret = i2c_smbus_write_byte_data(client, ZOPT2201_MAIN_CTRL, 0x00); > > > + if (ret < 0) > > > + goto fail; > > > + mutex_unlock(&data->lock); > > > + > > > + return (buf[2] << 16) | (buf[1] << 8) | buf[0]; > > > + > > > +fail: > > > + mutex_unlock(&data->lock); > > > + return ret; > > > +} > > > + > > > +static const struct iio_chan_spec zopt2201_channels[] = { > > > + { > > > + .type = IIO_LIGHT, > > > + .address = ZOPT2201_MODE_ALS, > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > > + BIT(IIO_CHAN_INFO_SCALE), > > > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME), > > > + }, > > > + { > > > + .type = IIO_INTENSITY, > > > + .modified = 1, > > > + .channel2 = IIO_MOD_LIGHT_UV, > > > + .address = ZOPT2201_MODE_UVB, > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > > + BIT(IIO_CHAN_INFO_SCALE), > > > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME), > > > + }, > > > + { > > > + .type = IIO_UVINDEX, > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > > > + }, > > > +}; > > > + > > > +static int zopt2201_read_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int *val, int *val2, long mask) > > > +{ > > > + struct zopt2201_data *data = iio_priv(indio_dev); > > > + u64 tmp; > > > + int ret; > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_RAW: > > > + ret = zopt2201_read(data, chan->address); > > > + if (ret < 0) > > > + return ret; > > > + *val = ret; > > > + return IIO_VAL_INT; > > > + case IIO_CHAN_INFO_PROCESSED: > > > + ret = zopt2201_read(data, ZOPT2201_MODE_UVB); > > > + if (ret < 0) > > > + return ret; > > > + *val = ret * 18 * > > > + (1 << (20 - zopt2201_resolution[data->res].bits)) / > > > + zopt2201_gain_uvb[data->gain].gain; > > > + return IIO_VAL_INT; > > > + case IIO_CHAN_INFO_SCALE: > > > + switch (chan->address) { > > > + case ZOPT2201_MODE_ALS: > > > + *val = zopt2201_gain_als[data->gain].scale; > > > > This is documented above as micro lux, but illuminance ABI is in > > lux. > > the scale factors above are given in micro lux > > the code below the switch statement basically divides by 1000000 (I could > have used IIO_VAL_FRACTIONAL, but that would use format string "%d.%09u" while > everything else here uses "%d.%06u") > > the resulting unit is Lux in fact > > > > + break; > > > + case ZOPT2201_MODE_UVB: > > > + *val = zopt2201_gain_uvb[data->gain].scale; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + *val2 = 1000000; > > > + *val2 *= (1 << (zopt2201_resolution[data->res].bits - 13)); > > > + tmp = div_s64(*val * 1000000ULL, *val2); > > > + *val = div_s64_rem(tmp, 1000000, val2); > > > + > > > + return IIO_VAL_INT_PLUS_MICRO; > > > + case IIO_CHAN_INFO_INT_TIME: > > > + *val = 0; > > > + *val2 = zopt2201_resolution[data->res].us; > > > + return IIO_VAL_INT_PLUS_MICRO; > > > + default: > > > + return -EINVAL; > > > + } > > > +} > > > + > > > +static int zopt2201_set_resolution(struct zopt2201_data *data, u8 res) > > > +{ > > > + int ret; > > > + > > > + ret = i2c_smbus_write_byte_data(data->client, ZOPT2201_LS_MEAS_RATE, > > > + (res << ZOPT2201_MEAS_RES_SHIFT) | > > > + data->rate); > > > + if (ret >= 0) > > > + data->res = res; > > > > Same as the case below - prefer the error case indented. Just that > > tiny bit nicer to read and means that when a reviewer scan's past it > > they won't stop to check what is going on... > > OK > > > > + > > > + return ret; > > > +} > > > + > > > +static int zopt2201_write_resolution(struct zopt2201_data *data, > > > + int val, int val2) > > > +{ > > > + int i, ret; > > > + > > > + if (val != 0) > > > + return -EINVAL; > > > + > > > + for (i = 0; i < ARRAY_SIZE(zopt2201_resolution); i++) > > > + if (val2 == zopt2201_resolution[i].us) { > > > + mutex_lock(&data->lock); > > > + ret = zopt2201_set_resolution(data, i); > > > + mutex_unlock(&data->lock); > > > + return ret; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static int zopt2201_set_gain(struct zopt2201_data *data, u8 gain) > > > +{ > > > + int ret; > > > + > > > + ret = i2c_smbus_write_byte_data(data->client, ZOPT2201_LS_GAIN, gain); > > > + if (ret >= 0) > > > + data->gain = gain; > > > > I'm a fuss pot about this, but I always like the indented one to be the > > error path. It adds a line of code, but it's easier to follow > > fair enough > > > if (ret < 0) > > return ret; > > > > data->gain = gain; > > > > return 0; > > > + > > > + return ret; > > > +} > > > + > > > +static int zopt2201_write_scale_als(struct zopt2201_data *data, > > > + int val, int val2) > > > +{ > > > + int i, ret; > > > + > > > + for (i = 0; i < ARRAY_SIZE(zopt2201_scale_als); i++) > > > + if (val == zopt2201_scale_als[i].scale && > > > + val2 == zopt2201_scale_als[i].uscale) { > > > + > > This is very deeply indented and it hurting readability. > > Perhaps a little utility function to handle the actual > > configuring might help? > > yes, code is a bit clearer > > > static int zopt2201_write_scale_als_by_idx(struct zopt2201_data *data, int index) > > { > > int ret; > > > > mutex_lock(&data->lock); > > ret = zopt2201_set_resolution(data, zopt2201_scale_als[i].res); > > if (ret < 0) > > goto unlock; > > > > ret = zopt2201_set_gain(data, zopt2201_scale_als[i].gain); > > > > unlock: > > mutex_unlock(&data->lock); > > return ret; > > } > > > > static int zopt2201_write_scale_als(struct zopt2201_data *data, > > int val, int val2) > > { > > int i, ret; > > > > for (i = 0; i < ARRAY_SIZE(zopt2201_scale_als); i++) > > if (val == zopt2201_scale_als[i].scale && > > val2 == zopt2201_scale_als[i].uscale) { > > return zopt2201_write_scale_als_by_idx(data, i); > > return -EINVAL; > > } > > > > Plus equivalent for the next one below. > > > > > + mutex_lock(&data->lock); > > > + ret = zopt2201_set_resolution(data, > > > + zopt2201_scale_als[i].res); > > > + if (ret < 0) > > > + goto fail; > > > + ret = zopt2201_set_gain(data, > > > + zopt2201_scale_als[i].gain); > > > + > > > +fail: > > > + mutex_unlock(&data->lock); > > > + return ret; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static int zopt2201_write_scale_uvb(struct zopt2201_data *data, > > > + int val, int val2) > > > +{ > > > + int i, ret; > > > + > > > + for (i = 0; i < ARRAY_SIZE(zopt2201_scale_uvb); i++) > > > + if (val == zopt2201_scale_uvb[i].scale && > > > + val2 == zopt2201_scale_uvb[i].uscale) { > > > + mutex_lock(&data->lock); > > > + ret = zopt2201_set_resolution(data, > > > + zopt2201_scale_uvb[i].res); > > > + if (ret < 0) > > > + goto fail; > > > + ret = zopt2201_set_gain(data, > > > + zopt2201_scale_uvb[i].gain); > > > + > > > +fail: > > > + mutex_unlock(&data->lock); > > > + return ret; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static int zopt2201_write_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int val, int val2, long mask) > > > +{ > > > + struct zopt2201_data *data = iio_priv(indio_dev); > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_INT_TIME: > > > + return zopt2201_write_resolution(data, val, val2); > > > + case IIO_CHAN_INFO_SCALE: > > > + switch (chan->address) { > > > + case ZOPT2201_MODE_ALS: > > > + return zopt2201_write_scale_als(data, val, val2); > > > + case ZOPT2201_MODE_UVB: > > > + return zopt2201_write_scale_uvb(data, val, val2); > > > + default: > > > + return -EINVAL; > > > + } > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static ssize_t zopt2201_show_int_time_available(struct device *dev, > > > + struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + size_t len = 0; > > > + int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(zopt2201_resolution); i++) > > > + len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06lu ", > > > + zopt2201_resolution[i].us); > > > + buf[len - 1] = '\n'; > > > + > > > + return len; > > > +} > > > + > > > +static IIO_DEV_ATTR_INT_TIME_AVAIL(zopt2201_show_int_time_available); > > > + > > > > One blank line is enough... > > Sure > > > > + > > > +static ssize_t zopt2201_show_als_scale_avail(struct device *dev, > > > + struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + ssize_t len = 0; > > > + int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(zopt2201_scale_als); i++) > > > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", > > > + zopt2201_scale_als[i].scale, > > > + zopt2201_scale_als[i].uscale); > > > + buf[len - 1] = '\n'; > > > + > > > + return len; > > > +} > > > + > > > +static ssize_t zopt2201_show_uvb_scale_avail(struct device *dev, > > > + struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + ssize_t len = 0; > > > + int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(zopt2201_scale_uvb); i++) > > > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", > > > + zopt2201_scale_uvb[i].scale, > > > + zopt2201_scale_uvb[i].uscale); > > > + buf[len - 1] = '\n'; > > > + > > > + return len; > > > +} > > > + > > > +static IIO_DEVICE_ATTR(in_illuminance_scale_available, 0444, > > > + zopt2201_show_als_scale_avail, NULL, 0); > > > +static IIO_DEVICE_ATTR(in_intensity_uv_scale_available, 0444, > > > + zopt2201_show_uvb_scale_avail, NULL, 0); > > > + > > > +static struct attribute *zopt2201_attributes[] = { > > > + &iio_dev_attr_integration_time_available.dev_attr.attr, > > > + &iio_dev_attr_in_illuminance_scale_available.dev_attr.attr, > > > + &iio_dev_attr_in_intensity_uv_scale_available.dev_attr.attr, > > > + NULL > > > +}; > > > + > > > +static const struct attribute_group zopt2201_attribute_group = { > > > + .attrs = zopt2201_attributes, > > > +}; > > > + > > > > Only 1 blank line. > > OK > > > > + > > > +static const struct iio_info zopt2201_info = { > > > + .read_raw = zopt2201_read_raw, > > > + .write_raw = zopt2201_write_raw, > > > + .attrs = &zopt2201_attribute_group, > > > +}; > > > + > > > +static int zopt2201_probe(struct i2c_client *client, > > > + const struct i2c_device_id *id) > > > +{ > > > + struct zopt2201_data *data; > > > + struct iio_dev *indio_dev; > > > + int ret; > > > + > > > + if (!i2c_check_functionality(client->adapter, > > > + I2C_FUNC_SMBUS_READ_I2C_BLOCK)) > > > + return -EOPNOTSUPP; > > > + > > > + ret = i2c_smbus_read_byte_data(client, ZOPT2201_PART_ID); > > > + if (ret < 0) > > > + return ret; > > > + if (ret != ZOPT2201_PART_NUMBER) > > > + return -ENODEV; > > > + > > > + 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 = &zopt2201_info; > > > + indio_dev->channels = zopt2201_channels; > > > + indio_dev->num_channels = ARRAY_SIZE(zopt2201_channels); > > > + indio_dev->name = ZOPT2201_DRV_NAME; > > > + indio_dev->modes = INDIO_DIRECT_MODE; > > > + > > > + data->rate = ZOPT2201_MEAS_FREQ_100MS; > > > + ret = zopt2201_set_resolution(data, ZOPT2201_MEAS_RES_18BIT); > > > + if (ret < 0) > > > + return ret; > > > + > > > + ret = zopt2201_set_gain(data, ZOPT2201_LS_GAIN_3); > > > + if (ret < 0) > > > + return ret; > > > + > > > + return devm_iio_device_register(&client->dev, indio_dev); > > > +} > > > + > > > +static const struct i2c_device_id zopt2201_id[] = { > > > + { "zopt2201", 0 }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(i2c, zopt2201_id); > > > > Hmm. I wonder if we should be adding device tree bindings for > > pretty much all drivers now. There is on going effort to > > drop the compatibility code that loads them based on the > > i2c device table. Anyhow, can be a follow up patch so > > not a problem right now. > > agree > > > > + > > > +static struct i2c_driver zopt2201_driver = { > > > + .driver = { > > > + .name = ZOPT2201_DRV_NAME, > > > + }, > > > + .probe = zopt2201_probe, > > > + .id_table = zopt2201_id, > > > +}; > > > + > > > +module_i2c_driver(zopt2201_driver); > > > + > > > +MODULE_AUTHOR("Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>"); > > > +MODULE_DESCRIPTION("IDT ZOPT2201 ambient light and UV B sensor driver"); > > > +MODULE_LICENSE("GPL"); > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
