On Wed, Feb 28, 2018 at 2:15 AM, Pierre Bourdon <delroth@xxxxxxxxxx> wrote: > Ambient light sensor that supports visible light and IR measurements and > configurable gain/integration time. > > This is written as an additional driver instead of being added into the > existing bh1750 / bh1780 drivers. The bh1730 chip is significantly > different from either of these two: > > * bh1750 is not fully smbus compatible and only partially supports > continuous reading mode since not all of its supported chips have this > feature. > > * bh1780 does gain adjustment in hardware and exposes lux values > directly, as opposed to bh1730 which provides two raw channels (IR + > visible light) and requires manual gain adjustment and lux computation > in the driver. > FWIW, Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> Some minors below. You might address them. > Changed in v2: > * Split off DT documentation change into a separate commit. > * Use i2c's probe_new. > > Changed in v3: > * Moved from 64 bit to 32 bit arithmetic where possible. > * Changed IIO channel configuration for consistency with other drivers. > * Refactored mathematical computations for readability. > * Removed unnecessary CONFIG_OF checks. > > Signed-off-by: Pierre Bourdon <delroth@xxxxxxxxxx> > --- > drivers/iio/light/Kconfig | 10 + > drivers/iio/light/Makefile | 1 + > drivers/iio/light/bh1730.c | 434 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 445 insertions(+) > create mode 100644 drivers/iio/light/bh1730.c > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 93fd421b10d7..286a7f78762b 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -63,6 +63,16 @@ config APDS9960 > To compile this driver as a module, choose M here: the > module will be called apds9960 > > +config BH1730 > + tristate "ROHM BH1730 ambient light sensor" > + depends on I2C > + help > + Say Y here to build support for the ROHM BH1730FVC ambient > + light sensor. > + > + To compile this driver as a module, choose M here: the module will > + be called bh1730. > + > config BH1750 > tristate "ROHM BH1750 ambient light sensor" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index f714067a7816..eb9010a10536 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_ADJD_S311) += adjd_s311.o > obj-$(CONFIG_AL3320A) += al3320a.o > obj-$(CONFIG_APDS9300) += apds9300.o > obj-$(CONFIG_APDS9960) += apds9960.o > +obj-$(CONFIG_BH1730) += bh1730.o > obj-$(CONFIG_BH1750) += bh1750.o > obj-$(CONFIG_BH1780) += bh1780.o > obj-$(CONFIG_CM32181) += cm32181.o > diff --git a/drivers/iio/light/bh1730.c b/drivers/iio/light/bh1730.c > new file mode 100644 > index 000000000000..4a23fbd6ff84 > --- /dev/null > +++ b/drivers/iio/light/bh1730.c > @@ -0,0 +1,434 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ROHM BH1730 ambient light sensor driver > + * > + * Copyright (c) 2018 Google, Inc. > + * Author: Pierre Bourdon <delroth@xxxxxxxxxx> > + * > + * Based on a previous non-iio BH1730FVC driver: > + * Copyright (C) 2012 Samsung Electronics. All rights reserved. > + * Author: Won Huh <won.huh@xxxxxxxxxxx> > + * > + * Data sheets: > + * http://www.rohm.com/web/global/datasheet/BH1730FVC/bh1730fvc-e > + */ > + > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/time.h> > + > +#define BH1730_CMD_BIT BIT(7) > + > +#define BH1730_REG_CONTROL 0x00 > +#define BH1730_REG_TIMING 0x01 > +#define BH1730_REG_INTERRUPT 0x02 > +#define BH1730_REG_THLLOW 0x03 > +#define BH1730_REG_THLHIGH 0x04 > +#define BH1730_REG_THHLOW 0x05 > +#define BH1730_REG_THHHIGH 0x06 > +#define BH1730_REG_GAIN 0x07 > +#define BH1730_REG_ID 0x12 > +#define BH1730_REG_DATA0LOW 0x14 > +#define BH1730_REG_DATA0HIGH 0x15 > +#define BH1730_REG_DATA1LOW 0x16 > +#define BH1730_REG_DATA1HIGH 0x17 > + > +#define BH1730_CONTROL_POWER_ON BIT(0) > +#define BH1730_CONTROL_MEASURE BIT(1) > + > +#define BH1730_INTERNAL_CLOCK_NS 2800 > + > +#define BH1730_DEFAULT_INTEG_MS 150 > + > +enum bh1730_gain { > + BH1730_GAIN_1X = 0, > + BH1730_GAIN_2X, > + BH1730_GAIN_64X, > + BH1730_GAIN_128X, > +}; > +#define BH1730_MAX_GAIN_MULTIPLIER 128 > + > +struct bh1730_data { > + struct i2c_client *client; > + enum bh1730_gain gain; > + int itime; > +}; > +#define BH1730_MAX_GAIN_MULTIPLIER 128 > + > +static int bh1730_read_word(struct bh1730_data *bh1730, u8 reg) > +{ > + int ret = i2c_smbus_read_word_data(bh1730->client, > + BH1730_CMD_BIT | reg); > + if (ret < 0) > + dev_err(&bh1730->client->dev, > + "i2c read failed error %d, register %01x\n", > + ret, reg); > + > + return ret; > +} > + > +static int bh1730_write(struct bh1730_data *bh1730, u8 reg, u8 val) > +{ > + int ret = i2c_smbus_write_byte_data(bh1730->client, > + BH1730_CMD_BIT | reg, > + val); > + if (ret < 0) > + dev_err(&bh1730->client->dev, > + "i2c write failed error %d, register %01x\n", > + ret, reg); > + > + return ret; > +} > + > +static int gain_setting_to_multiplier(enum bh1730_gain gain) > +{ > + switch (gain) { > + case BH1730_GAIN_1X: > + return 1; > + case BH1730_GAIN_2X: > + return 2; > + case BH1730_GAIN_64X: > + return 64; > + case BH1730_GAIN_128X: > + return 128; > + default: > + return -EINVAL; > + } > +} > + > +static int bh1730_gain_multiplier(struct bh1730_data *bh1730) > +{ > + int multiplier = gain_setting_to_multiplier(bh1730->gain); > + > + if (multiplier < 0) { > + dev_warn(&bh1730->client->dev, > + "invalid gain multiplier settings: %d\n", > + bh1730->gain); > + bh1730->gain = BH1730_GAIN_1X; > + multiplier = 1; > + } > + > + return multiplier; > +} > + > +static int bh1730_itime_us(struct bh1730_data *bh1730) > +{ > + return (BH1730_INTERNAL_CLOCK_NS * 964 * (256 - bh1730->itime)) > + / NSEC_PER_USEC; Better to leave / on the above line Or /* At worst case mul will be 688296000, so, there is no 32-bit overflow */ ...() { int mul = BH1730_INTERNAL_CLOCK_NS * 964 * (256 - bh1730->itime); return mul / NSEC_PER_USEC; } > +} > + > +static int bh1730_set_gain(struct bh1730_data *bh1730, enum bh1730_gain gain) > +{ > + int ret = bh1730_write(bh1730, BH1730_REG_GAIN, gain); > + > + if (ret < 0) > + return ret; > + > + bh1730->gain = gain; > + > + return 0; > +} > + > +static int bh1730_set_integration_time_ms(struct bh1730_data *bh1730, > + int time_ms) > +{ > + int ret, time_ns, itime; > + > + /* Prefilter obviously invalid time_ms values that would overflow. */ > + if (time_ms <= 0 || time_ms > 1000) > + goto out_of_range; > + > + time_ns = time_ms * NSEC_PER_MSEC; > + itime = 256 - DIV_ROUND_CLOSEST(time_ns, > + BH1730_INTERNAL_CLOCK_NS * 964); > + > + /* ITIME == 0 is reserved for manual integration mode. */ > + if (itime <= 0 || itime > 255) Just side note: Suprisingly how many in_range() implementations we have in kernel... > + goto out_of_range; > + > + ret = bh1730_write(bh1730, BH1730_REG_TIMING, itime); > + if (ret < 0) > + return ret; > + > + bh1730->itime = itime; > + > + return 0; > + > +out_of_range: > + dev_warn(&bh1730->client->dev, "integration time out of range: %dms\n", > + time_ms); > + > + return -ERANGE; > +} > + > +static void bh1730_wait_for_next_measurement(struct bh1730_data *bh1730) > +{ > + udelay(bh1730_itime_us(bh1730) + > + DIV_ROUND_UP(BH1730_INTERNAL_CLOCK_NS * 714, NSEC_PER_USEC)); int step = DIV_ROUND_UP(BH1730_INTERNAL_CLOCK_NS * 714, NSEC_PER_USEC); udelay(... + step); ? > +} > + > +static int bh1730_adjust_gain(struct bh1730_data *bh1730) > +{ > + int visible, ir, highest, gain, ret, i; int visible, ir, highest, gain; unsigned int i; int ret; > + > + visible = bh1730_read_word(bh1730, BH1730_REG_DATA0LOW); > + if (visible < 0) > + return visible; > + > + ir = bh1730_read_word(bh1730, BH1730_REG_DATA1LOW); > + if (ir < 0) > + return ir; > + > + highest = max(visible, ir); > + > + /* > + * If the read value is being clamped, assume the worst and go to the > + * lowest possible gain. The alternative is doing multiple > + * recalibrations, which would be slower and have the same effect. > + */ > + if (highest == USHRT_MAX) > + gain = 1; > + else > + gain = bh1730_gain_multiplier(bh1730); > + > + highest = (highest * BH1730_MAX_GAIN_MULTIPLIER) / gain; > + > + /* > + * Find the lowest gain multiplier which puts the measured values > + * above 1024. This threshold is chosen to match the gap between 2X > + * multiplier and 64X (next available) while keeping some margin. > + */ > + for (i = BH1730_GAIN_1X; i < BH1730_GAIN_128X; ++i) { > + int adj = highest * gain_setting_to_multiplier(i) / > + BH1730_MAX_GAIN_MULTIPLIER; I would rather int adj; adj = ...; if (adj ...) break; > + > + if (adj >= 1024) > + break; > + } > + > + if (i != bh1730->gain) { > + ret = bh1730_set_gain(bh1730, i); > + if (ret < 0) > + return ret; > + > + bh1730_wait_for_next_measurement(bh1730); > + } > + > + return 0; > +} > + > +static int bh1730_get_millilux(struct bh1730_data *bh1730) > +{ > + int visible, ir, visible_coef, ir_coef; > + u64 millilux; > + > + visible = bh1730_read_word(bh1730, BH1730_REG_DATA0LOW); > + if (visible < 0) > + return visible; > + > + ir = bh1730_read_word(bh1730, BH1730_REG_DATA1LOW); > + if (ir < 0) > + return ir; > + > + if (ir * 1000 / visible < 500) { > + visible_coef = 5002; > + ir_coef = 7502; > + } else if (ir * 1000 / visible < 754) { > + visible_coef = 2250; > + ir_coef = 2000; > + } else if (ir * 1000 / visible < 1029) { > + visible_coef = 1999; > + ir_coef = 1667; > + } else if (ir * 1000 / visible < 1373) { > + visible_coef = 885; > + ir_coef = 583; > + } else if (ir * 1000 / visible < 1879) { > + visible_coef = 309; > + ir_coef = 165; > + } else { > + return 0; > + } > + > + millilux = 103ULL * (visible_coef * visible - ir_coef * ir); > + millilux *= USEC_PER_MSEC; Can it be one line? > + do_div(millilux, bh1730_itime_us(bh1730)); > + do_div(millilux, bh1730_gain_multiplier(bh1730)); > + > + /* > + * Overflow here can only happen in extreme conditions: > + * - Completely saturated visible light sensor and no measured IR. > + * - Integration time < 16ms (driver currently defaults to 150ms). > + */ > + if (millilux > INT_MAX) > + return -ERANGE; > + > + return (int)millilux; > +} > + > +static int bh1730_power_on(struct bh1730_data *bh1730) > +{ > + return bh1730_write(bh1730, BH1730_REG_CONTROL, > + BH1730_CONTROL_POWER_ON | BH1730_CONTROL_MEASURE); > +} > + > +static int bh1730_set_defaults(struct bh1730_data *bh1730) > +{ > + int ret; > + > + ret = bh1730_set_gain(bh1730, BH1730_GAIN_1X); > + if (ret < 0) > + return ret; > + > + ret = bh1730_set_integration_time_ms(bh1730, BH1730_DEFAULT_INTEG_MS); > + if (ret < 0) > + return ret; > + > + bh1730_wait_for_next_measurement(bh1730); > + > + return 0; > +} > + > +static int bh1730_power_off(struct bh1730_data *bh1730) > +{ > + return bh1730_write(bh1730, BH1730_REG_CONTROL, 0); > +} > + > +static int bh1730_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct bh1730_data *bh1730 = iio_priv(indio_dev); > + int data_reg, ret; > + > + ret = bh1730_adjust_gain(bh1730); > + if (ret < 0) > + return ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + ret = bh1730_get_millilux(bh1730); > + if (ret < 0) > + return ret; > + *val = ret / 1000; > + *val2 = (ret % 1000) * 1000; > + return IIO_VAL_INT_PLUS_MICRO; > + case IIO_CHAN_INFO_RAW: > + switch (chan->channel2) { > + case IIO_MOD_LIGHT_CLEAR: > + data_reg = BH1730_REG_DATA0LOW; > + break; > + case IIO_MOD_LIGHT_IR: > + data_reg = BH1730_REG_DATA1LOW; > + break; > + default: > + return -EINVAL; > + } > + ret = bh1730_read_word(bh1730, data_reg); > + if (ret < 0) > + return ret; > + ret = ret * 1000 / bh1730_gain_multiplier(bh1730); > + *val = ret / 1000; > + *val2 = (ret % 1000) * 1000; > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_info bh1730_info = { > + .read_raw = bh1730_read_raw, > +}; > + > +static const struct iio_chan_spec bh1730_channels[] = { > + { > + .type = IIO_LIGHT, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + }, > + { > + .type = IIO_INTENSITY, > + .modified = 1, > + .channel2 = IIO_MOD_LIGHT_CLEAR, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + }, > + { > + .type = IIO_INTENSITY, > + .modified = 1, > + .channel2 = IIO_MOD_LIGHT_IR, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + }, > +}; > + > +static int bh1730_probe(struct i2c_client *client) > +{ > + struct bh1730_data *bh1730; > + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); > + struct iio_dev *indio_dev; > + int ret; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE)) > + return -EIO; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*bh1730)); > + if (!indio_dev) > + return -ENOMEM; > + > + bh1730 = iio_priv(indio_dev); > + bh1730->client = client; > + i2c_set_clientdata(client, indio_dev); > + > + ret = bh1730_power_on(bh1730); > + if (ret < 0) > + return ret; > + > + ret = bh1730_set_defaults(bh1730); > + if (ret < 0) > + return ret; > + > + indio_dev->dev.parent = &client->dev; > + indio_dev->info = &bh1730_info; > + indio_dev->name = "bh1730"; > + indio_dev->channels = bh1730_channels; > + indio_dev->num_channels = ARRAY_SIZE(bh1730_channels); > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + ret = iio_device_register(indio_dev); > + if (ret) > + goto out_power_off; > + return 0; > + > +out_power_off: > + bh1730_power_off(bh1730); > + return ret; > +} > + > +static int bh1730_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + struct bh1730_data *bh1730 = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + return bh1730_power_off(bh1730); > +} > + > +static const struct of_device_id of_bh1730_match[] = { > + { .compatible = "rohm,bh1730fvc" }, > + {}, No need to have a comma. > +}; > +MODULE_DEVICE_TABLE(of, of_bh1730_match); > + > +static struct i2c_driver bh1730_driver = { > + .probe_new = bh1730_probe, > + .remove = bh1730_remove, > + .driver = { > + .name = "bh1730", > + .of_match_table = of_bh1730_match, > + }, > +}; > +module_i2c_driver(bh1730_driver); > + > +MODULE_AUTHOR("Pierre Bourdon <delroth@xxxxxxxxxx>"); > +MODULE_DESCRIPTION("ROHM BH1730FVC driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.16.2.395.g2e18187dfd-goog > -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
