On 11/04/16 19:52, Crestez Dan Leonard wrote: > This just adds support for reporting illuminance with default settings. > > Important default registers are written on probe because the device > otherwise lacks a reset function. > > Signed-off-by: Crestez Dan Leonard <leonard.crestez@xxxxxxxxx> cc'ing Mark Brown for regcache flat discussion.. Mark I don't suppose you have any docs anywhere I can point people at on this. It comes round rather to frequently for my taste and I can't immediately find any references in the tree or code to explain what REGCACHE_FLAT is for... I've cc'd your previous email on the subject, but it's not really documentation. > --- > drivers/iio/light/Kconfig | 11 ++ > drivers/iio/light/Makefile | 1 + > drivers/iio/light/max44000.c | 336 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 348 insertions(+) > create mode 100644 drivers/iio/light/max44000.c > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index cfd3df8..b4ab65b 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -223,6 +223,17 @@ config LTR501 > This driver can also be built as a module. If so, the module > will be called ltr501. > > +config MAX44000 > + tristate "MAX44000 Ambient and Infrared Proximity Sensor" > + depends on I2C > + select REGMAP_I2C > + help > + Say Y here if you want to build support for Maxim Integrated's > + MAX44000 ambient and infrared proximity sensor device. > + > + To compile this driver as a module, choose M here: > + the module will be called max44000. > + > config OPT3001 > tristate "Texas Instruments OPT3001 Light Sensor" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index b2c3105..c77f27f 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -20,6 +20,7 @@ obj-$(CONFIG_ISL29125) += isl29125.o > obj-$(CONFIG_JSA1212) += jsa1212.o > obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o > obj-$(CONFIG_LTR501) += ltr501.o > +obj-$(CONFIG_MAX44000) += max44000.o > obj-$(CONFIG_OPT3001) += opt3001.o > obj-$(CONFIG_PA12203001) += pa12203001.o > obj-$(CONFIG_RPR0521) += rpr0521.o > diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c > new file mode 100644 > index 0000000..707cc24 > --- /dev/null > +++ b/drivers/iio/light/max44000.c > @@ -0,0 +1,336 @@ > +/* > + * MAX44000 Ambient and Infrared Proximity Sensor > + * > + * Copyright (c) 2016, Intel Corporation. > + * > + * 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. > + * > + * Data sheet: https://datasheets.maximintegrated.com/en/ds/MAX44000.pdf > + * > + * 7-bit I2C slave address 0x4a > + */ > + > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/util_macros.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/acpi.h> > + > +#define MAX44000_DRV_NAME "max44000" > + > +/* Registers in datasheet order */ > +#define MAX44000_REG_STATUS 0x00 > +#define MAX44000_REG_CFG_MAIN 0x01 > +#define MAX44000_REG_CFG_RX 0x02 > +#define MAX44000_REG_CFG_TX 0x03 > +#define MAX44000_REG_ALS_DATA_HI 0x04 > +#define MAX44000_REG_ALS_DATA_LO 0x05 > +#define MAX44000_REG_PRX_DATA 0x16 > +#define MAX44000_REG_ALS_UPTHR_HI 0x06 > +#define MAX44000_REG_ALS_UPTHR_LO 0x07 > +#define MAX44000_REG_ALS_LOTHR_HI 0x08 > +#define MAX44000_REG_ALS_LOTHR_LO 0x09 > +#define MAX44000_REG_PST 0x0a > +#define MAX44000_REG_PRX_IND 0x0b > +#define MAX44000_REG_PRX_THR 0x0c > +#define MAX44000_REG_TRIM_GAIN_GREEN 0x0f > +#define MAX44000_REG_TRIM_GAIN_IR 0x10 > + > +/* REG_CFG bits */ > +#define MAX44000_CFG_ALSINTE 0x01 > +#define MAX44000_CFG_PRXINTE 0x02 > +#define MAX44000_CFG_MASK 0x1c > +#define MAX44000_CFG_MODE_SHUTDOWN 0x00 > +#define MAX44000_CFG_MODE_ALS_GIR 0x04 > +#define MAX44000_CFG_MODE_ALS_G 0x08 > +#define MAX44000_CFG_MODE_ALS_IR 0x0c > +#define MAX44000_CFG_MODE_ALS_PRX 0x10 > +#define MAX44000_CFG_MODE_PRX 0x14 > +#define MAX44000_CFG_TRIM 0x20 > + > +/* > + * Upper 4 bits are not documented but start as 1 on powerup > + * Setting them to 0 causes proximity to misbehave so set them to 1 > + */ > +#define MAX44000_REG_CFG_RX_DEFAULT 0xf0 > + > +#define MAX44000_ALSDATA_OVERFLOW 0x4000 > + > +#define MAX44000_REGMASK_READABLE ( \ Whilst an improvement I'd still prefer the nice clean switch statement in the function. Let the compiler do the magic for you. > + (1 << MAX44000_REG_STATUS) | \ > + (1 << MAX44000_REG_CFG_MAIN) | \ > + (1 << MAX44000_REG_CFG_RX) | \ > + (1 << MAX44000_REG_CFG_TX) | \ > + (1 << MAX44000_REG_ALS_DATA_HI) | \ > + (1 << MAX44000_REG_ALS_DATA_LO) | \ > + (1 << MAX44000_REG_PRX_DATA) | \ > + (1 << MAX44000_REG_ALS_UPTHR_HI) | \ > + (1 << MAX44000_REG_ALS_UPTHR_LO) | \ > + (1 << MAX44000_REG_ALS_LOTHR_HI) | \ > + (1 << MAX44000_REG_ALS_LOTHR_LO) | \ > + (1 << MAX44000_REG_PST) | \ > + (1 << MAX44000_REG_PRX_IND) | \ > + (1 << MAX44000_REG_PRX_THR) | \ > + (1 << MAX44000_REG_TRIM_GAIN_GREEN) | \ > + (1 << MAX44000_REG_TRIM_GAIN_IR)) > + > +#define MAX44000_REGMASK_WRITEABLE ( \ > + (1 << MAX44000_REG_CFG_MAIN) | \ > + (1 << MAX44000_REG_CFG_RX) | \ > + (1 << MAX44000_REG_CFG_TX) | \ > + (1 << MAX44000_REG_ALS_UPTHR_HI) | \ > + (1 << MAX44000_REG_ALS_UPTHR_LO) | \ > + (1 << MAX44000_REG_ALS_LOTHR_HI) | \ > + (1 << MAX44000_REG_ALS_LOTHR_LO) | \ > + (1 << MAX44000_REG_PST) | \ > + (1 << MAX44000_REG_PRX_IND) | \ > + (1 << MAX44000_REG_PRX_THR) | \ > + (1 << MAX44000_REG_TRIM_GAIN_GREEN) | \ > + (1 << MAX44000_REG_TRIM_GAIN_IR)) > + > +#define MAX44000_REGMASK_VOLATILE ( \ > + (1 << MAX44000_REG_STATUS) | \ > + (1 << MAX44000_REG_ALS_DATA_HI) | \ > + (1 << MAX44000_REG_ALS_DATA_LO) | \ > + (1 << MAX44000_REG_PRX_DATA)) > + > +struct max44000_data { > + struct mutex lock; > + struct regmap *regmap; > +}; > + > +/* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */ > +#define MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2 5 > + > +static const struct iio_chan_spec max44000_channels[] = { > + { > + .type = IIO_LIGHT, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > + }, > +}; > + > +static int max44000_read_alsval(struct max44000_data *data) > +{ > + u16 regval; > + int ret; > + > + ret = regmap_bulk_read(data->regmap, MAX44000_REG_ALS_DATA_HI, > + ®val, sizeof(regval)); > + if (ret < 0) > + return ret; > + > + regval = be16_to_cpu(regval); > + > + /* > + * Overflow is explained on datasheet page 17. > + * > + * It's a warning that either the G or IR channel has become saturated > + * and that the value in the register is likely incorrect. > + * > + * The recommendation is to change the scale (ALSPGA). > + * The driver just returns the max representable value. > + */ Good comment. Thanks for adding it. > + if (regval & MAX44000_ALSDATA_OVERFLOW) > + return 0x3FFF; > + > + return regval; > +} > + > +static int max44000_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct max44000_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 = max44000_read_alsval(data); > + mutex_unlock(&data->lock); > + if (ret < 0) > + return ret; > + *val = ret; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > + > + case IIO_CHAN_INFO_SCALE: > + switch (chan->type) { > + case IIO_LIGHT: > + *val = 1; On this one I'm not sure the define adds anything given it really is a numerical value.. I'd just put it inline here. > + *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2; > + return IIO_VAL_FRACTIONAL_LOG2; > + > + default: > + return -EINVAL; > + } > + > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_info max44000_info = { > + .driver_module = THIS_MODULE, > + .read_raw = max44000_read_raw, > +}; > + > +static bool max44000_readable_reg(struct device *dev, unsigned int reg) > +{ > + return (1 << reg) & MAX44000_REGMASK_READABLE; > +} > + > +static bool max44000_writeable_reg(struct device *dev, unsigned int reg) > +{ > + return (1 << reg) & MAX44000_REGMASK_WRITEABLE; > +} > + > +static bool max44000_volatile_reg(struct device *dev, unsigned int reg) > +{ > + return (1 << reg) & MAX44000_REGMASK_VOLATILE; > +} > + > +static bool max44000_precious_reg(struct device *dev, unsigned int reg) > +{ > + return reg == MAX44000_REG_STATUS; > +} > + > +/* Datasheet pages 9-10: */ > +static const struct reg_default max44000_reg_defaults[] = { > + { MAX44000_REG_CFG_MAIN, 0x24 }, This must be broken down into defines showing what the various bits are doing. > + { MAX44000_REG_CFG_RX, MAX44000_REG_CFG_RX_DEFAULT }, > + { MAX44000_REG_CFG_TX, 0x00 }, > + { MAX44000_REG_ALS_UPTHR_HI, 0x00 }, > + { MAX44000_REG_ALS_UPTHR_LO, 0x00 }, > + { MAX44000_REG_ALS_LOTHR_HI, 0x00 }, > + { MAX44000_REG_ALS_LOTHR_LO, 0x00 }, > + { MAX44000_REG_PST, 0x00 }, > + { MAX44000_REG_PRX_IND, 0x00 }, > + { MAX44000_REG_PRX_THR, 0x00 }, > + { MAX44000_REG_TRIM_GAIN_GREEN, 0x80 }, > + { MAX44000_REG_TRIM_GAIN_IR, 0x80 }, > +}; > + > +static const struct regmap_config max44000_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + > + .max_register = MAX44000_REG_PRX_DATA, > + .readable_reg = max44000_readable_reg, > + .writeable_reg = max44000_writeable_reg, > + .volatile_reg = max44000_volatile_reg, > + .precious_reg = max44000_precious_reg, > + > + .use_single_rw = 1, > + .cache_type = REGCACHE_FLAT, Please read: http://www.spinics.net/lists/linux-iio/msg22996.html You definitely want REGCACHE_RBTREE which behaves quite differently wrt to what happens on startup of the cache. > + If we aren't assuming particular values, what is the purpose of providing defaults? They are probably wrong.. > + .reg_defaults = max44000_reg_defaults, > + .num_reg_defaults = ARRAY_SIZE(max44000_reg_defaults), > +}; > + > +static int max44000_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct max44000_data *data; > + struct iio_dev *indio_dev; > + int ret, reg; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + data = iio_priv(indio_dev); > + data->regmap = devm_regmap_init_i2c(client, &max44000_regmap_config); > + if (IS_ERR(data->regmap)) { > + dev_err(&client->dev, "regmap_init failed!\n"); > + return PTR_ERR(data->regmap); > + } > + > + i2c_set_clientdata(client, indio_dev); > + mutex_init(&data->lock); > + indio_dev->dev.parent = &client->dev; > + indio_dev->info = &max44000_info; > + indio_dev->name = MAX44000_DRV_NAME; > + indio_dev->channels = max44000_channels; > + indio_dev->num_channels = ARRAY_SIZE(max44000_channels); > + > + /* > + * The device doesn't have a reset function so we just clear some > + * important bits at probe time to ensure sane operation. > + * > + * Since we don't support interrupts/events the threshold values are > + * not important. We also don't touch trim values. > + */ > + > + /* Reset ALS scaling bits */ > + ret = regmap_write(data->regmap, MAX44000_REG_CFG_RX, MAX44000_REG_CFG_RX_DEFAULT); > + if (ret < 0) { > + dev_err(&client->dev, "failed to write default CFG_RX: %d\n", ret); > + return ret; > + } > + > + /* Reset CFG bits to ALS-only mode and no interrupts */ > + reg = MAX44000_CFG_TRIM | MAX44000_CFG_MODE_ALS_GIR; > + ret = regmap_write(data->regmap, MAX44000_REG_CFG_MAIN, reg); > + if (ret < 0) { > + dev_err(&client->dev, "failed to write init config: %d\n", ret); > + return ret; > + } > + I prefer this careful code making it clear what you are doing on setting the defaults to the previous wholesale version. > + /* Read status at least once to clear any stale interrupt bits. */ > + ret = regmap_read(data->regmap, MAX44000_REG_STATUS, ®); > + if (ret < 0) { > + dev_err(&client->dev, "failed to read init status: %d\n", ret); > + return ret; > + } > + > + return iio_device_register(indio_dev); > +} > + > +static int max44000_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + > + iio_device_unregister(indio_dev); blank line here ideally (nitpick on the day :) > + return 0; > +} > + > +static const struct i2c_device_id max44000_id[] = { > + {"max44000", 0}, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max44000_id); > + > +#ifdef CONFIG_ACPI > +static const struct acpi_device_id max44000_acpi_match[] = { > + {"MAX44000", 0}, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, max44000_acpi_match); > +#endif > + > +static struct i2c_driver max44000_driver = { > + .driver = { > + .name = MAX44000_DRV_NAME, > + .acpi_match_table = ACPI_PTR(max44000_acpi_match), > + }, > + .probe = max44000_probe, > + .remove = max44000_remove, > + .id_table = max44000_id, > +}; > + > +module_i2c_driver(max44000_driver); > + > +MODULE_AUTHOR("Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>"); > +MODULE_DESCRIPTION("MAX44000 Ambient and Infrared Proximity Sensor"); > +MODULE_LICENSE("GPL v2"); > -- 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
