* [RFC v2 0/1] iio: light: add MAX30100 oximeter driver support @ 2015-11-18 9:19 Matt Ranostay 2015-11-18 9:19 ` [RFC v2 1/1] " Matt Ranostay 0 siblings, 1 reply; 7+ messages in thread From: Matt Ranostay @ 2015-11-18 9:19 UTC (permalink / raw) To: linux-iio, jic23; +Cc: Matt Ranostay Changes from v1: * Add temp channel * Reworked path to drivers/iio/health patch * Removed magic numbers to defines IMPORTANT NOTES: * Hasn't been tested on real hardware and won't be till second week of December due to author on holiday in Asia Matt Ranostay (1): iio: light: add MAX30100 oximeter driver support .../devicetree/bindings/iio/light/max30100.txt | 22 ++ drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/health/Kconfig | 21 + drivers/iio/health/Makefile | 7 + drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ 6 files changed, 475 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt create mode 100644 drivers/iio/health/Kconfig create mode 100644 drivers/iio/health/Makefile create mode 100644 drivers/iio/health/max30100.c -- 1.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC v2 1/1] iio: light: add MAX30100 oximeter driver support 2015-11-18 9:19 [RFC v2 0/1] iio: light: add MAX30100 oximeter driver support Matt Ranostay @ 2015-11-18 9:19 ` Matt Ranostay 2015-11-20 16:40 ` Peter Meerwald-Stadler 0 siblings, 1 reply; 7+ messages in thread From: Matt Ranostay @ 2015-11-18 9:19 UTC (permalink / raw) To: linux-iio, jic23; +Cc: Matt Ranostay MAX30100 is an heart rate and pulse oximeter sensor that works using two LEDS of different wavelengths, and detecting the light reflected back. This patchset adds support for both IR and RED LED channels which can be processed in userspace to determine heart rate and blood oxygen levels. Signed-off-by: Matt Ranostay <mranostay@gmail.com> --- .../devicetree/bindings/iio/light/max30100.txt | 22 ++ drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/health/Kconfig | 21 + drivers/iio/health/Makefile | 7 + drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ 6 files changed, 475 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt create mode 100644 drivers/iio/health/Kconfig create mode 100644 drivers/iio/health/Makefile create mode 100644 drivers/iio/health/max30100.c diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt new file mode 100644 index 0000000..435d6fc --- /dev/null +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt @@ -0,0 +1,22 @@ +* Maxim MAX30100 heart rate and pulse oximeter sensor + +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf + +Required properties: + + - compatible: must be "maxim,max30100" + - reg: the I2c address of the sensor + - interrupt-parent: should be the phandle for the interrupt controller + - interrupts : the sole interrupt generated by the device + + Refer to interrupt-controller/interrupts.txt for generic interrupt client + node bindings. + +Example: + +max30100@57 { + compatible = "maxim,max30100"; + reg = <0x57>; + interrupt-parent = <&gpio1>; + interrupts = <16 2>; +}; diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index 5dfb4f1..b846873 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" source "drivers/iio/dac/Kconfig" source "drivers/iio/frequency/Kconfig" source "drivers/iio/gyro/Kconfig" +source "drivers/iio/health/Kconfig" source "drivers/iio/humidity/Kconfig" source "drivers/iio/imu/Kconfig" source "drivers/iio/light/Kconfig" diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index dc73c1f..7371791 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -20,6 +20,7 @@ obj-y += common/ obj-y += dac/ obj-y += gyro/ obj-y += frequency/ +obj-y += health/ obj-y += humidity/ obj-y += imu/ obj-y += light/ diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig new file mode 100644 index 0000000..30d4ca0 --- /dev/null +++ b/drivers/iio/health/Kconfig @@ -0,0 +1,21 @@ +# +# Health sensors +# +# When adding new entries keep the list in alphabetical order + +menu "Health sensors" + +config MAX30100 + tristate "MAX30100 heart rate and pulse oximeter sensor" + depends on I2C + select REGMAP_I2C + select IIO_BUFFER + select IIO_KFIFO_BUF + help + Say Y here to build I2C interface support for the Maxim + MAX30100 heart rate, and pulse oximeter sensor. + + To compile this driver as a module, choose M here: the + module will be called max30100 + +endmenu diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile new file mode 100644 index 0000000..7c475d7 --- /dev/null +++ b/drivers/iio/health/Makefile @@ -0,0 +1,7 @@ +# +# Makefile for IIO Health sensors +# + +# When adding new entries keep the list in alphabetical order + +obj-$(CONFIG_MAX30100) += max30100.o diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c new file mode 100644 index 0000000..3ad6b31 --- /dev/null +++ b/drivers/iio/health/max30100.c @@ -0,0 +1,423 @@ +/* + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor + * + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/module.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/irq.h> +#include <linux/i2c.h> +#include <linux/mutex.h> +#include <linux/regmap.h> +#include <linux/iio/iio.h> +#include <linux/iio/buffer.h> +#include <linux/iio/kfifo_buf.h> + +#define MAX30100_REGMAP_NAME "max30100_regmap" +#define MAX30100_DRV_NAME "max30100" + +#define MAX30100_REG_INT_STATUS 0x00 +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) + +#define MAX30100_REG_INT_ENABLE 0x01 +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 + +#define MAX30100_REG_FIFO_DATA 0x05 + +#define MAX30100_REG_MODE_CONFIG 0x06 +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) + +#define MAX30100_REG_SPO2_CONFIG 0x07 +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) + +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 + +#define MAX30100_REG_LED_CONFIG 0x09 +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 + +#define MAX30100_REG_LED_CONFIG_24MA 0x07 +#define MAX30100_REG_LED_CONFIG_50MA 0x0f + +#define MAX30100_REG_TEMP_INTEGER 0x16 +#define MAX30100_REG_TEMP_FRACTION 0x17 + +struct max30100_data { + struct iio_dev *indio_dev; + struct mutex lock; + struct regmap *regmap; + + u8 buffer[4]; /* 4 8-bit channels */ +}; + +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) +{ + switch (reg) { + case MAX30100_REG_INT_STATUS: + case MAX30100_REG_MODE_CONFIG: + case MAX30100_REG_FIFO_DATA: + case MAX30100_REG_TEMP_INTEGER: + case MAX30100_REG_TEMP_FRACTION: + return true; + default: + return false; + } +} + +static const struct regmap_config max30100_regmap_config = { + .name = MAX30100_REGMAP_NAME, + + .reg_bits = 8, + .val_bits = 8, + + .max_register = MAX30100_REG_LED_CONFIG, + .cache_type = REGCACHE_FLAT, + + .volatile_reg = max30100_is_volatile_reg, +}; + +static const unsigned long max30100_scan_masks[] = {0x3, 0}; + +static const struct iio_chan_spec max30100_channels[] = { + { + .type = IIO_INTENSITY, + .channel2 = IIO_MOD_LIGHT_IR, + + .scan_index = 0, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_BE, + }, + }, + { + .type = IIO_INTENSITY, + .channel2 = IIO_MOD_LIGHT_RED, + + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_BE, + }, + }, + { + .type = IIO_TEMP, + .info_mask_separate = + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), + .scan_index = -1, + }, +}; + +static int max30100_set_powermode(struct max30100_data *data, bool state) +{ + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, + MAX30100_REG_MODE_CONFIG_PWR, + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); +} + +static int max30100_buffer_postenable(struct iio_dev *indio_dev) +{ + struct max30100_data *data = iio_priv(indio_dev); + + return max30100_set_powermode(data, true); +} + +static int max30100_buffer_predisable(struct iio_dev *indio_dev) +{ + struct max30100_data *data = iio_priv(indio_dev); + + return max30100_set_powermode(data, false); +} + +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { + .postenable = max30100_buffer_postenable, + .predisable = max30100_buffer_predisable, +}; + +static inline int max30100_fifo_is_empty(struct max30100_data *data) +{ + int ret, val; + + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); + if (ret) + return ret; + + return !!(val >> 4); +} + +static irqreturn_t max30100_interrupt_handler(int irq, void *private) +{ + struct iio_dev *indio_dev = private; + struct max30100_data *data = iio_priv(indio_dev); + int ret; + + mutex_lock(&data->lock); + + while (max30100_fifo_is_empty(data) != 0) { + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, + &data->buffer, 4); + + if (ret) + goto err_read; + + iio_push_to_buffers(data->indio_dev, data->buffer); + } + +err_read: + mutex_unlock(&data->lock); + + return IRQ_HANDLED; +} + +static int max30100_chip_init(struct max30100_data *data) +{ + int ret; + + /* RED IR LED = 24mA, IR LED = 50mA */ + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, + (MAX30100_REG_LED_CONFIG_24MA << + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | + MAX30100_REG_LED_CONFIG_50MA); + if (ret) + return ret; + + /* enable hi-res SPO2 readings with 100 samples a second */ + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | + MAX30100_REG_SPO2_CONFIG_100HZ | + MAX30100_REG_SPO2_CONFIG_1600US); + if (ret) + return ret; + + /* enable SPO2 + HR mode */ + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, + MAX30100_REG_MODE_CONFIG_MODE_MASK, + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); + if (ret) + return ret; + + /* enable SPO2 + HR interrupts */ + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, + MAX30100_REG_INT_ENABLE_MASK, + (MAX30100_REG_INT_ENABLE_HR_EN | + MAX30100_REG_INT_ENABLE_SPO2_EN) + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); +} + +static int max30100_read_temp(struct max30100_data *data, int *val) +{ + int ret; + int reg; + + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); + if (ret < 0) + return ret; + *val = reg << 4; + + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); + if (ret < 0) + return ret; + *val |= reg & 0xf; + + return 0; +} + +static int max30100_get_temp(struct max30100_data *data, int *val) +{ + int ret; + int tries = 3; + + /* start acquisition */ + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, + MAX30100_REG_MODE_CONFIG_TEMP_EN, + MAX30100_REG_MODE_CONFIG_TEMP_EN); + if (ret) + return ret; + + while (tries--) { + int reg; + + usleep_range(29000, 40000); + + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); + if (ret < 0) + break; + + /* bit reset to zero when reading is ready */ + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { + ret = max30100_read_temp(data, val); + break; + } + ret = -EIO; + } + + return ret; +} + +static int max30100_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct max30100_data *data = iio_priv(indio_dev); + int ret = -EINVAL; + + mutex_lock(&indio_dev->mlock); + + /* temperature reading can only be acquired while engine is running */ + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { + ret = -EAGAIN; + goto error_again; + } + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = max30100_get_temp(data, val); + if (!ret) + ret = IIO_VAL_INT; + break; + case IIO_CHAN_INFO_SCALE: + *val = 1; /* 0.0625 */ + *val2 = 16; + ret = IIO_VAL_FRACTIONAL; + break; + } + +error_again: + mutex_unlock(&indio_dev->mlock); + + return ret; +} + +static const struct iio_info max30100_info = { + .driver_module = THIS_MODULE, + .read_raw = max30100_read_raw, +}; + +static int max30100_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct max30100_data *data; + struct iio_buffer *buffer; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + buffer = devm_iio_kfifo_allocate(&client->dev); + if (!buffer) + return -ENOMEM; + + iio_device_attach_buffer(indio_dev, buffer); + + indio_dev->name = MAX30100_DRV_NAME; + indio_dev->channels = max30100_channels; + indio_dev->info = &max30100_info; + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); + indio_dev->available_scan_masks = max30100_scan_masks; + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); + indio_dev->setup_ops = &max30100_buffer_setup_ops; + + data = iio_priv(indio_dev); + data->indio_dev = indio_dev; + + mutex_init(&data->lock); + i2c_set_clientdata(client, indio_dev); + + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); + if (IS_ERR(data->regmap)) { + dev_err(&client->dev, "regmap initialization failed.\n"); + return PTR_ERR(data->regmap); + } + max30100_set_powermode(data, false); + + ret = max30100_chip_init(data); + if (ret) + goto error_out; + + if (client->irq <= 0) { + dev_err(&client->dev, "no valid irq defined\n"); + ret = -EINVAL; + goto error_out; + } + ret = devm_request_threaded_irq(&client->dev, client->irq, + NULL, max30100_interrupt_handler, + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + "max30100_irq", indio_dev); + if (ret) { + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); + goto error_out; + } + + return iio_device_register(indio_dev); +error_out: + + return ret; +} + +static int max30100_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(client); + struct max30100_data *data = iio_priv(indio_dev); + + iio_device_unregister(indio_dev); + max30100_set_powermode(data, false); + + return 0; +} + +static const struct i2c_device_id max30100_id[] = { + { "max30100", 0 }, + {} +}; +MODULE_DEVICE_TABLE(i2c, max30100_id); + +static const struct of_device_id max30100_dt_ids[] = { + { .compatible = "maxim,max30100" }, + { } +}; +MODULE_DEVICE_TABLE(of, max30100_dt_ids); + +static struct i2c_driver max30100_driver = { + .driver = { + .name = MAX30100_DRV_NAME, + .of_match_table = of_match_ptr(max30100_dt_ids), + }, + .probe = max30100_probe, + .remove = max30100_remove, + .id_table = max30100_id, +}; +module_i2c_driver(max30100_driver); + +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>"); +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); +MODULE_LICENSE("GPL"); -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/1] iio: light: add MAX30100 oximeter driver support 2015-11-18 9:19 ` [RFC v2 1/1] " Matt Ranostay @ 2015-11-20 16:40 ` Peter Meerwald-Stadler 2015-11-21 16:26 ` Jonathan Cameron 2015-12-02 18:36 ` Matt Ranostay 0 siblings, 2 replies; 7+ messages in thread From: Peter Meerwald-Stadler @ 2015-11-20 16:40 UTC (permalink / raw) To: Matt Ranostay; +Cc: linux-iio, jic23 On Wed, 18 Nov 2015, Matt Ranostay wrote: > MAX30100 is an heart rate and pulse oximeter sensor that works using > two LEDS of different wavelengths, and detecting the light reflected > back. > > This patchset adds support for both IR and RED LED channels which can red? this refers to a LED with visible red color? more nitpicking below > be processed in userspace to determine heart rate and blood oxygen > levels. > > Signed-off-by: Matt Ranostay <mranostay@gmail.com> > --- > .../devicetree/bindings/iio/light/max30100.txt | 22 ++ > drivers/iio/Kconfig | 1 + > drivers/iio/Makefile | 1 + > drivers/iio/health/Kconfig | 21 + > drivers/iio/health/Makefile | 7 + > drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ > 6 files changed, 475 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt > create mode 100644 drivers/iio/health/Kconfig > create mode 100644 drivers/iio/health/Makefile > create mode 100644 drivers/iio/health/max30100.c > > diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt > new file mode 100644 > index 0000000..435d6fc > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt > @@ -0,0 +1,22 @@ > +* Maxim MAX30100 heart rate and pulse oximeter sensor > + > +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf > + > +Required properties: > + > + - compatible: must be "maxim,max30100" > + - reg: the I2c address of the sensor I2C > + - interrupt-parent: should be the phandle for the interrupt controller > + - interrupts : the sole interrupt generated by the device no space before : for consistency > + > + Refer to interrupt-controller/interrupts.txt for generic interrupt client > + node bindings. > + > +Example: > + > +max30100@57 { > + compatible = "maxim,max30100"; > + reg = <0x57>; > + interrupt-parent = <&gpio1>; > + interrupts = <16 2>; > +}; > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > index 5dfb4f1..b846873 100644 > --- a/drivers/iio/Kconfig > +++ b/drivers/iio/Kconfig > @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" > source "drivers/iio/dac/Kconfig" > source "drivers/iio/frequency/Kconfig" > source "drivers/iio/gyro/Kconfig" > +source "drivers/iio/health/Kconfig" > source "drivers/iio/humidity/Kconfig" > source "drivers/iio/imu/Kconfig" > source "drivers/iio/light/Kconfig" > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > index dc73c1f..7371791 100644 > --- a/drivers/iio/Makefile > +++ b/drivers/iio/Makefile > @@ -20,6 +20,7 @@ obj-y += common/ > obj-y += dac/ > obj-y += gyro/ > obj-y += frequency/ > +obj-y += health/ > obj-y += humidity/ > obj-y += imu/ > obj-y += light/ > diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig > new file mode 100644 > index 0000000..30d4ca0 > --- /dev/null > +++ b/drivers/iio/health/Kconfig > @@ -0,0 +1,21 @@ > +# > +# Health sensors > +# > +# When adding new entries keep the list in alphabetical order > + > +menu "Health sensors" > + > +config MAX30100 > + tristate "MAX30100 heart rate and pulse oximeter sensor" > + depends on I2C > + select REGMAP_I2C > + select IIO_BUFFER > + select IIO_KFIFO_BUF > + help > + Say Y here to build I2C interface support for the Maxim > + MAX30100 heart rate, and pulse oximeter sensor. > + > + To compile this driver as a module, choose M here: the > + module will be called max30100 put a full stop (.) at the end of the sentence maybe :) > + > +endmenu > diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile > new file mode 100644 > index 0000000..7c475d7 > --- /dev/null > +++ b/drivers/iio/health/Makefile > @@ -0,0 +1,7 @@ > +# > +# Makefile for IIO Health sensors > +# > + > +# When adding new entries keep the list in alphabetical order > + > +obj-$(CONFIG_MAX30100) += max30100.o > diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c > new file mode 100644 > index 0000000..3ad6b31 > --- /dev/null > +++ b/drivers/iio/health/max30100.c > @@ -0,0 +1,423 @@ > +/* > + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor > + * > + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/irq.h> > +#include <linux/i2c.h> > +#include <linux/mutex.h> > +#include <linux/regmap.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/kfifo_buf.h> > + > +#define MAX30100_REGMAP_NAME "max30100_regmap" > +#define MAX30100_DRV_NAME "max30100" > + > +#define MAX30100_REG_INT_STATUS 0x00 > +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) > +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) > +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) > + > +#define MAX30100_REG_INT_ENABLE 0x01 > +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) > +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) > +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 > +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 > + > +#define MAX30100_REG_FIFO_DATA 0x05 > + > +#define MAX30100_REG_MODE_CONFIG 0x06 > +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) > +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) > +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 > +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) > +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) > + > +#define MAX30100_REG_SPO2_CONFIG 0x07 > +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) > + > +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) > +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 > + > +#define MAX30100_REG_LED_CONFIG 0x09 > +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 > + > +#define MAX30100_REG_LED_CONFIG_24MA 0x07 > +#define MAX30100_REG_LED_CONFIG_50MA 0x0f > + > +#define MAX30100_REG_TEMP_INTEGER 0x16 > +#define MAX30100_REG_TEMP_FRACTION 0x17 > + > +struct max30100_data { > + struct iio_dev *indio_dev; > + struct mutex lock; > + struct regmap *regmap; > + > + u8 buffer[4]; /* 4 8-bit channels */ > +}; > + > +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case MAX30100_REG_INT_STATUS: > + case MAX30100_REG_MODE_CONFIG: > + case MAX30100_REG_FIFO_DATA: > + case MAX30100_REG_TEMP_INTEGER: > + case MAX30100_REG_TEMP_FRACTION: > + return true; > + default: > + return false; > + } > +} > + > +static const struct regmap_config max30100_regmap_config = { > + .name = MAX30100_REGMAP_NAME, > + > + .reg_bits = 8, > + .val_bits = 8, > + > + .max_register = MAX30100_REG_LED_CONFIG, > + .cache_type = REGCACHE_FLAT, > + > + .volatile_reg = max30100_is_volatile_reg, > +}; > + > +static const unsigned long max30100_scan_masks[] = {0x3, 0}; > + > +static const struct iio_chan_spec max30100_channels[] = { > + { > + .type = IIO_INTENSITY, > + .channel2 = IIO_MOD_LIGHT_IR, > + > + .scan_index = 0, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_BE, > + }, > + }, > + { > + .type = IIO_INTENSITY, > + .channel2 = IIO_MOD_LIGHT_RED, > + > + .scan_index = 1, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, max30100_interrupt_handler() reads 4 bytes?! description of buffer in max30100_data: u8 buffer[4]; /* 4 8-bit channels */ this is confusing; maybe __be16 buffer[2]? > + .storagebits = 16, > + .endianness = IIO_BE, > + }, > + }, > + { > + .type = IIO_TEMP, > + .info_mask_separate = > + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = -1, > + }, > +}; > + > +static int max30100_set_powermode(struct max30100_data *data, bool state) > +{ > + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, > + MAX30100_REG_MODE_CONFIG_PWR, > + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); > +} > + > +static int max30100_buffer_postenable(struct iio_dev *indio_dev) > +{ > + struct max30100_data *data = iio_priv(indio_dev); > + > + return max30100_set_powermode(data, true); > +} > + > +static int max30100_buffer_predisable(struct iio_dev *indio_dev) > +{ > + struct max30100_data *data = iio_priv(indio_dev); > + > + return max30100_set_powermode(data, false); > +} > + > +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { > + .postenable = max30100_buffer_postenable, > + .predisable = max30100_buffer_predisable, > +}; > + > +static inline int max30100_fifo_is_empty(struct max30100_data *data) > +{ > + int ret, val; > + > + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); > + if (ret) > + return ret; > + > + return !!(val >> 4); > +} > + > +static irqreturn_t max30100_interrupt_handler(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct max30100_data *data = iio_priv(indio_dev); > + int ret; > + > + mutex_lock(&data->lock); > + > + while (max30100_fifo_is_empty(data) != 0) { > + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, > + &data->buffer, 4); > + > + if (ret) > + goto err_read; > + > + iio_push_to_buffers(data->indio_dev, data->buffer); > + } > + > +err_read: > + mutex_unlock(&data->lock); > + > + return IRQ_HANDLED; > +} > + > +static int max30100_chip_init(struct max30100_data *data) > +{ > + int ret; > + > + /* RED IR LED = 24mA, IR LED = 50mA */ > + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, > + (MAX30100_REG_LED_CONFIG_24MA << > + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | > + MAX30100_REG_LED_CONFIG_50MA); > + if (ret) > + return ret; > + > + /* enable hi-res SPO2 readings with 100 samples a second */ > + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, > + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | > + MAX30100_REG_SPO2_CONFIG_100HZ | > + MAX30100_REG_SPO2_CONFIG_1600US); > + if (ret) > + return ret; > + > + /* enable SPO2 + HR mode */ > + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, > + MAX30100_REG_MODE_CONFIG_MODE_MASK, > + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | > + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); > + if (ret) > + return ret; > + > + /* enable SPO2 + HR interrupts */ > + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, > + MAX30100_REG_INT_ENABLE_MASK, > + (MAX30100_REG_INT_ENABLE_HR_EN | > + MAX30100_REG_INT_ENABLE_SPO2_EN) > + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); > +} > + > +static int max30100_read_temp(struct max30100_data *data, int *val) > +{ > + int ret; > + int reg; > + > + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); > + if (ret < 0) > + return ret; > + *val = reg << 4; note that this is a signed value; I think negative temperatures are not processed correctly > + > + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); > + if (ret < 0) > + return ret; > + *val |= reg & 0xf; > + > + return 0; > +} > + > +static int max30100_get_temp(struct max30100_data *data, int *val) > +{ > + int ret; > + int tries = 3; > + > + /* start acquisition */ > + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, > + MAX30100_REG_MODE_CONFIG_TEMP_EN, > + MAX30100_REG_MODE_CONFIG_TEMP_EN); > + if (ret) > + return ret; > + > + while (tries--) { > + int reg; > + > + usleep_range(29000, 40000); > + > + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); > + if (ret < 0) > + break; > + > + /* bit reset to zero when reading is ready */ > + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { > + ret = max30100_read_temp(data, val); > + break; > + } > + ret = -EIO; > + } > + > + return ret; > +} > + > +static int max30100_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct max30100_data *data = iio_priv(indio_dev); > + int ret = -EINVAL; > + > + mutex_lock(&indio_dev->mlock); > + > + /* temperature reading can only be acquired while engine is running */ > + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { > + ret = -EAGAIN; > + goto error_again; > + } > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = max30100_get_temp(data, val); > + if (!ret) what if ret < 0? _get_temp() checks for ret < 0 INTENSITY channels cannot be raw_read? > + ret = IIO_VAL_INT; > + break; > + case IIO_CHAN_INFO_SCALE: > + *val = 1; /* 0.0625 */ > + *val2 = 16; > + ret = IIO_VAL_FRACTIONAL; > + break; > + } > + > +error_again: > + mutex_unlock(&indio_dev->mlock); > + > + return ret; > +} > + > +static const struct iio_info max30100_info = { > + .driver_module = THIS_MODULE, > + .read_raw = max30100_read_raw, > +}; > + > +static int max30100_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct max30100_data *data; > + struct iio_buffer *buffer; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + buffer = devm_iio_kfifo_allocate(&client->dev); > + if (!buffer) > + return -ENOMEM; > + > + iio_device_attach_buffer(indio_dev, buffer); > + > + indio_dev->name = MAX30100_DRV_NAME; > + indio_dev->channels = max30100_channels; > + indio_dev->info = &max30100_info; > + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); > + indio_dev->available_scan_masks = max30100_scan_masks; > + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); > + indio_dev->setup_ops = &max30100_buffer_setup_ops; > + > + data = iio_priv(indio_dev); > + data->indio_dev = indio_dev; > + > + mutex_init(&data->lock); > + i2c_set_clientdata(client, indio_dev); > + > + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); > + if (IS_ERR(data->regmap)) { > + dev_err(&client->dev, "regmap initialization failed.\n"); > + return PTR_ERR(data->regmap); > + } > + max30100_set_powermode(data, false); > + > + ret = max30100_chip_init(data); > + if (ret) > + goto error_out; > + > + if (client->irq <= 0) { > + dev_err(&client->dev, "no valid irq defined\n"); > + ret = -EINVAL; > + goto error_out; > + } > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + NULL, max30100_interrupt_handler, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + "max30100_irq", indio_dev); > + if (ret) { > + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); > + goto error_out; > + } > + > + return iio_device_register(indio_dev); newline here > +error_out: drop newline > + > + return ret; > +} > + > +static int max30100_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + struct max30100_data *data = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + max30100_set_powermode(data, false); > + > + return 0; > +} > + > +static const struct i2c_device_id max30100_id[] = { > + { "max30100", 0 }, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, max30100_id); > + > +static const struct of_device_id max30100_dt_ids[] = { > + { .compatible = "maxim,max30100" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, max30100_dt_ids); > + > +static struct i2c_driver max30100_driver = { > + .driver = { > + .name = MAX30100_DRV_NAME, > + .of_match_table = of_match_ptr(max30100_dt_ids), > + }, > + .probe = max30100_probe, > + .remove = max30100_remove, > + .id_table = max30100_id, > +}; > +module_i2c_driver(max30100_driver); > + > +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>"); > +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); > +MODULE_LICENSE("GPL"); > -- Peter Meerwald-Stadler +43-664-2444418 (mobile) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/1] iio: light: add MAX30100 oximeter driver support 2015-11-20 16:40 ` Peter Meerwald-Stadler @ 2015-11-21 16:26 ` Jonathan Cameron 2015-11-22 12:07 ` Matt Ranostay 2015-12-02 18:36 ` Matt Ranostay 1 sibling, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2015-11-21 16:26 UTC (permalink / raw) To: Peter Meerwald-Stadler, Matt Ranostay; +Cc: linux-iio On 20/11/15 16:40, Peter Meerwald-Stadler wrote: > On Wed, 18 Nov 2015, Matt Ranostay wrote: > >> MAX30100 is an heart rate and pulse oximeter sensor that works using >> two LEDS of different wavelengths, and detecting the light reflected >> back. >> >> This patchset adds support for both IR and RED LED channels which can > > red? this refers to a LED with visible red color? > more nitpicking below As I'm here, I'll answer that one. Yes. Absolutely, it's 700->800nm light which pretty much makes it mostly visible red. > >> be processed in userspace to determine heart rate and blood oxygen >> levels. >> >> Signed-off-by: Matt Ranostay <mranostay@gmail.com> Few additional comments from me. Coming along nicely. Guessing we want to hold this until you have real hardware tested with the final version anyway. Should be plenty of time. Jonathan >> --- >> .../devicetree/bindings/iio/light/max30100.txt | 22 ++ >> drivers/iio/Kconfig | 1 + >> drivers/iio/Makefile | 1 + >> drivers/iio/health/Kconfig | 21 + >> drivers/iio/health/Makefile | 7 + >> drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ >> 6 files changed, 475 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt >> create mode 100644 drivers/iio/health/Kconfig >> create mode 100644 drivers/iio/health/Makefile >> create mode 100644 drivers/iio/health/max30100.c >> >> diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt >> new file mode 100644 >> index 0000000..435d6fc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt >> @@ -0,0 +1,22 @@ >> +* Maxim MAX30100 heart rate and pulse oximeter sensor >> + >> +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf >> + >> +Required properties: >> + >> + - compatible: must be "maxim,max30100" >> + - reg: the I2c address of the sensor > > I2C > >> + - interrupt-parent: should be the phandle for the interrupt controller >> + - interrupts : the sole interrupt generated by the device > > no space before : for consistency > >> + >> + Refer to interrupt-controller/interrupts.txt for generic interrupt client >> + node bindings. >> + >> +Example: >> + >> +max30100@57 { >> + compatible = "maxim,max30100"; >> + reg = <0x57>; >> + interrupt-parent = <&gpio1>; >> + interrupts = <16 2>; >> +}; >> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >> index 5dfb4f1..b846873 100644 >> --- a/drivers/iio/Kconfig >> +++ b/drivers/iio/Kconfig >> @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" >> source "drivers/iio/dac/Kconfig" >> source "drivers/iio/frequency/Kconfig" >> source "drivers/iio/gyro/Kconfig" >> +source "drivers/iio/health/Kconfig" >> source "drivers/iio/humidity/Kconfig" >> source "drivers/iio/imu/Kconfig" >> source "drivers/iio/light/Kconfig" >> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >> index dc73c1f..7371791 100644 >> --- a/drivers/iio/Makefile >> +++ b/drivers/iio/Makefile >> @@ -20,6 +20,7 @@ obj-y += common/ >> obj-y += dac/ >> obj-y += gyro/ >> obj-y += frequency/ >> +obj-y += health/ >> obj-y += humidity/ >> obj-y += imu/ >> obj-y += light/ >> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig >> new file mode 100644 >> index 0000000..30d4ca0 >> --- /dev/null >> +++ b/drivers/iio/health/Kconfig >> @@ -0,0 +1,21 @@ >> +# >> +# Health sensors >> +# >> +# When adding new entries keep the list in alphabetical order >> + >> +menu "Health sensors" >> + >> +config MAX30100 >> + tristate "MAX30100 heart rate and pulse oximeter sensor" >> + depends on I2C >> + select REGMAP_I2C >> + select IIO_BUFFER >> + select IIO_KFIFO_BUF >> + help >> + Say Y here to build I2C interface support for the Maxim >> + MAX30100 heart rate, and pulse oximeter sensor. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called max30100 > > put a full stop (.) at the end of the sentence maybe :) > >> + >> +endmenu >> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile >> new file mode 100644 >> index 0000000..7c475d7 >> --- /dev/null >> +++ b/drivers/iio/health/Makefile >> @@ -0,0 +1,7 @@ >> +# >> +# Makefile for IIO Health sensors >> +# >> + >> +# When adding new entries keep the list in alphabetical order >> + >> +obj-$(CONFIG_MAX30100) += max30100.o >> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c >> new file mode 100644 >> index 0000000..3ad6b31 >> --- /dev/null >> +++ b/drivers/iio/health/max30100.c >> @@ -0,0 +1,423 @@ >> +/* >> + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor >> + * >> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/init.h> >> +#include <linux/interrupt.h> >> +#include <linux/delay.h> >> +#include <linux/err.h> >> +#include <linux/irq.h> >> +#include <linux/i2c.h> >> +#include <linux/mutex.h> >> +#include <linux/regmap.h> >> +#include <linux/iio/iio.h> >> +#include <linux/iio/buffer.h> >> +#include <linux/iio/kfifo_buf.h> >> + >> +#define MAX30100_REGMAP_NAME "max30100_regmap" >> +#define MAX30100_DRV_NAME "max30100" >> + >> +#define MAX30100_REG_INT_STATUS 0x00 >> +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) >> +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) >> +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) >> + >> +#define MAX30100_REG_INT_ENABLE 0x01 >> +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) >> +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) >> +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 >> +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 >> + >> +#define MAX30100_REG_FIFO_DATA 0x05 >> + >> +#define MAX30100_REG_MODE_CONFIG 0x06 >> +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) >> +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) >> +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 >> +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) >> +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) >> + >> +#define MAX30100_REG_SPO2_CONFIG 0x07 >> +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) >> + >> +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) >> +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 >> + >> +#define MAX30100_REG_LED_CONFIG 0x09 >> +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 >> + >> +#define MAX30100_REG_LED_CONFIG_24MA 0x07 >> +#define MAX30100_REG_LED_CONFIG_50MA 0x0f >> + >> +#define MAX30100_REG_TEMP_INTEGER 0x16 >> +#define MAX30100_REG_TEMP_FRACTION 0x17 >> + >> +struct max30100_data { >> + struct iio_dev *indio_dev; This is only used in one place and there is a perfectly good pointer to this in the interrupt anyway so drop it from here. (note this is a common one to spot, there is very rarely a good reason to have a pointer to the struct iio_dev in the private data. Everything should be the other way around!) >> + struct mutex lock; >> + struct regmap *regmap; >> + >> + u8 buffer[4]; /* 4 8-bit channels */ >> +}; >> + >> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) >> +{ >> + switch (reg) { >> + case MAX30100_REG_INT_STATUS: >> + case MAX30100_REG_MODE_CONFIG: >> + case MAX30100_REG_FIFO_DATA: >> + case MAX30100_REG_TEMP_INTEGER: >> + case MAX30100_REG_TEMP_FRACTION: >> + return true; >> + default: >> + return false; >> + } >> +} >> + >> +static const struct regmap_config max30100_regmap_config = { >> + .name = MAX30100_REGMAP_NAME, >> + >> + .reg_bits = 8, >> + .val_bits = 8, >> + >> + .max_register = MAX30100_REG_LED_CONFIG, >> + .cache_type = REGCACHE_FLAT, >> + >> + .volatile_reg = max30100_is_volatile_reg, >> +}; >> + >> +static const unsigned long max30100_scan_masks[] = {0x3, 0}; >> + >> +static const struct iio_chan_spec max30100_channels[] = { >> + { >> + .type = IIO_INTENSITY, >> + .channel2 = IIO_MOD_LIGHT_IR, >> + >> + .scan_index = 0, >> + .scan_type = { >> + .sign = 'u', >> + .realbits = 16, >> + .storagebits = 16, >> + .endianness = IIO_BE, >> + }, >> + }, >> + { >> + .type = IIO_INTENSITY, >> + .channel2 = IIO_MOD_LIGHT_RED, >> + >> + .scan_index = 1, >> + .scan_type = { >> + .sign = 'u', >> + .realbits = 16, > > max30100_interrupt_handler() reads 4 bytes?! > > description of buffer in max30100_data: > u8 buffer[4]; /* 4 8-bit channels */ > this is confusing; maybe __be16 buffer[2]? Agreed, __be16 version preferred. Makes it explicit what we have and fix the description (good spot Peter!) > >> + .storagebits = 16, >> + .endianness = IIO_BE, >> + }, >> + }, >> + { >> + .type = IIO_TEMP, >> + .info_mask_separate = >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >> + .scan_index = -1, >> + }, >> +}; >> + >> +static int max30100_set_powermode(struct max30100_data *data, bool state) >> +{ >> + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >> + MAX30100_REG_MODE_CONFIG_PWR, >> + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); >> +} >> + >> +static int max30100_buffer_postenable(struct iio_dev *indio_dev) >> +{ >> + struct max30100_data *data = iio_priv(indio_dev); >> + >> + return max30100_set_powermode(data, true); >> +} >> + >> +static int max30100_buffer_predisable(struct iio_dev *indio_dev) >> +{ >> + struct max30100_data *data = iio_priv(indio_dev); >> + >> + return max30100_set_powermode(data, false); >> +} >> + >> +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { >> + .postenable = max30100_buffer_postenable, >> + .predisable = max30100_buffer_predisable, >> +}; >> + >> +static inline int max30100_fifo_is_empty(struct max30100_data *data) >> +{ >> + int ret, val; >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); >> + if (ret) >> + return ret; >> + >> + return !!(val >> 4); You don't really need the !! as you only check it's != 0 later... I guess you could argue this makes it obvious it is a boolean though... >> +} >> + >> +static irqreturn_t max30100_interrupt_handler(int irq, void *private) >> +{ >> + struct iio_dev *indio_dev = private; >> + struct max30100_data *data = iio_priv(indio_dev); >> + int ret; >> + >> + mutex_lock(&data->lock); >> + >> + while (max30100_fifo_is_empty(data) != 0) { >> + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, >> + &data->buffer, 4); >> + >> + if (ret) >> + goto err_read; >> + use the local copy (ultimately private) instead of the version in data. >> + iio_push_to_buffers(data->indio_dev, data->buffer); >> + } >> + >> +err_read: >> + mutex_unlock(&data->lock); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int max30100_chip_init(struct max30100_data *data) >> +{ >> + int ret; >> + >> + /* RED IR LED = 24mA, IR LED = 50mA */ >> + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, >> + (MAX30100_REG_LED_CONFIG_24MA << >> + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | >> + MAX30100_REG_LED_CONFIG_50MA); >> + if (ret) >> + return ret; >> + >> + /* enable hi-res SPO2 readings with 100 samples a second */ >> + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, >> + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | >> + MAX30100_REG_SPO2_CONFIG_100HZ | >> + MAX30100_REG_SPO2_CONFIG_1600US); >> + if (ret) >> + return ret; >> + >> + /* enable SPO2 + HR mode */ >> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >> + MAX30100_REG_MODE_CONFIG_MODE_MASK, >> + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | >> + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); >> + if (ret) >> + return ret; >> + >> + /* enable SPO2 + HR interrupts */ >> + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, >> + MAX30100_REG_INT_ENABLE_MASK, >> + (MAX30100_REG_INT_ENABLE_HR_EN | >> + MAX30100_REG_INT_ENABLE_SPO2_EN) >> + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); >> +} >> + >> +static int max30100_read_temp(struct max30100_data *data, int *val) >> +{ >> + int ret; >> + int reg; >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); >> + if (ret < 0) >> + return ret; >> + *val = reg << 4; > > note that this is a signed value; I think negative temperatures are not > processed correctly Definitely looks like it's missing a sign extend to me as well! > >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); >> + if (ret < 0) >> + return ret; >> + *val |= reg & 0xf; >> + >> + return 0; >> +} >> + >> +static int max30100_get_temp(struct max30100_data *data, int *val) >> +{ >> + int ret; >> + int tries = 3; >> + >> + /* start acquisition */ >> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >> + MAX30100_REG_MODE_CONFIG_TEMP_EN, >> + MAX30100_REG_MODE_CONFIG_TEMP_EN); >> + if (ret) >> + return ret; >> + >> + while (tries--) { >> + int reg; Could use the interrupt that indicates temperature is ready... Why have you chosen not to? (might well be sensible but you want to have documentation somewhere here to explain why.) I guess it's because it is likely to be an occasional out of band reading and you don't want to complicate the interrupt handling? >> + >> + usleep_range(29000, 40000); >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); >> + if (ret < 0) >> + break; >> + >> + /* bit reset to zero when reading is ready */ >> + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { >> + ret = max30100_read_temp(data, val); >> + break; >> + } >> + ret = -EIO; >> + } >> + >> + return ret; >> +} >> + >> +static int max30100_read_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int *val, int *val2, long mask) >> +{ >> + struct max30100_data *data = iio_priv(indio_dev); >> + int ret = -EINVAL; >> + >> + mutex_lock(&indio_dev->mlock); >> + >> + /* temperature reading can only be acquired while engine is running */ >> + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { >> + ret = -EAGAIN; Possibly -EBUSY. EAGAIN tends to lead to usespace software immediately trying to read again whereas EBUSY tends to imply there is a 'reason' beyond just 'not now'. Not a particularly clear cut case however. Reorder this to be under the switch (mask) case IIO_CHAN_INFO_RAW though and the code flow will be simpler as you won't need to check we are trying a raw read twice. >> + goto error_again; >> + } >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_RAW: >> + ret = max30100_get_temp(data, val); >> + if (!ret) > > what if ret < 0? > _get_temp() checks for ret < 0 > > INTENSITY channels cannot be raw_read? It's not in the channel spec, so Matt's right here. However given this is 'unusual' it might act as a form of 'documentation' to explicitly verify we have a temperature channel here. (Peter, for why they have no _raw access see the previous version - basically there is no point in getting a single reading on this sort of device - it's the processing of the waveform that allows you to establish what the device is meant to be measuring. Makes sense to me). > >> + ret = IIO_VAL_INT; >> + break; >> + case IIO_CHAN_INFO_SCALE: >> + *val = 1; /* 0.0625 */ >> + *val2 = 16; >> + ret = IIO_VAL_FRACTIONAL; >> + break; >> + } >> + >> +error_again: >> + mutex_unlock(&indio_dev->mlock); >> + >> + return ret; >> +} >> + >> +static const struct iio_info max30100_info = { >> + .driver_module = THIS_MODULE, >> + .read_raw = max30100_read_raw, >> +}; >> + >> +static int max30100_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct max30100_data *data; >> + struct iio_buffer *buffer; >> + struct iio_dev *indio_dev; >> + int ret; >> + >> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >> + if (!indio_dev) >> + return -ENOMEM; >> + >> + buffer = devm_iio_kfifo_allocate(&client->dev); >> + if (!buffer) >> + return -ENOMEM; >> + >> + iio_device_attach_buffer(indio_dev, buffer); >> + >> + indio_dev->name = MAX30100_DRV_NAME; >> + indio_dev->channels = max30100_channels; >> + indio_dev->info = &max30100_info; >> + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); >> + indio_dev->available_scan_masks = max30100_scan_masks; >> + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); >> + indio_dev->setup_ops = &max30100_buffer_setup_ops; >> + >> + data = iio_priv(indio_dev); >> + data->indio_dev = indio_dev; >> + >> + mutex_init(&data->lock); >> + i2c_set_clientdata(client, indio_dev); >> + >> + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); >> + if (IS_ERR(data->regmap)) { >> + dev_err(&client->dev, "regmap initialization failed.\n"); >> + return PTR_ERR(data->regmap); >> + } >> + max30100_set_powermode(data, false); >> + >> + ret = max30100_chip_init(data); >> + if (ret) >> + goto error_out; >> + >> + if (client->irq <= 0) { >> + dev_err(&client->dev, "no valid irq defined\n"); >> + ret = -EINVAL; >> + goto error_out; >> + } >> + ret = devm_request_threaded_irq(&client->dev, client->irq, >> + NULL, max30100_interrupt_handler, >> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >> + "max30100_irq", indio_dev); >> + if (ret) { >> + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); >> + goto error_out; >> + } >> + >> + return iio_device_register(indio_dev); > > newline here > >> +error_out: > > drop newline > >> + >> + return ret; >> +} >> + >> +static int max30100_remove(struct i2c_client *client) >> +{ >> + struct iio_dev *indio_dev = i2c_get_clientdata(client); >> + struct max30100_data *data = iio_priv(indio_dev); >> + >> + iio_device_unregister(indio_dev); >> + max30100_set_powermode(data, false); >> + >> + return 0; >> +} >> + >> +static const struct i2c_device_id max30100_id[] = { >> + { "max30100", 0 }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(i2c, max30100_id); >> + >> +static const struct of_device_id max30100_dt_ids[] = { >> + { .compatible = "maxim,max30100" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, max30100_dt_ids); >> + >> +static struct i2c_driver max30100_driver = { >> + .driver = { >> + .name = MAX30100_DRV_NAME, >> + .of_match_table = of_match_ptr(max30100_dt_ids), >> + }, >> + .probe = max30100_probe, >> + .remove = max30100_remove, >> + .id_table = max30100_id, >> +}; >> +module_i2c_driver(max30100_driver); >> + >> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>"); >> +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); >> +MODULE_LICENSE("GPL"); >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/1] iio: light: add MAX30100 oximeter driver support 2015-11-21 16:26 ` Jonathan Cameron @ 2015-11-22 12:07 ` Matt Ranostay 2015-11-22 12:36 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Matt Ranostay @ 2015-11-22 12:07 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Peter Meerwald-Stadler, linux-iio@vger.kernel.org On Sat, Nov 21, 2015 at 11:26 PM, Jonathan Cameron <jic23@kernel.org> wrote: > On 20/11/15 16:40, Peter Meerwald-Stadler wrote: >> On Wed, 18 Nov 2015, Matt Ranostay wrote: >> >>> MAX30100 is an heart rate and pulse oximeter sensor that works using >>> two LEDS of different wavelengths, and detecting the light reflected >>> back. >>> >>> This patchset adds support for both IR and RED LED channels which can >> >> red? this refers to a LED with visible red color? >> more nitpicking below > As I'm here, I'll answer that one. Yes. Absolutely, it's 700->800nm light > which pretty much makes it mostly visible red. Yeah this device uses a "red" light (aka close enough to wavelength) mainly for the oxygen reading. >> >>> be processed in userspace to determine heart rate and blood oxygen >>> levels. >>> >>> Signed-off-by: Matt Ranostay <mranostay@gmail.com> > Few additional comments from me. Coming along nicely. > > Guessing we want to hold this until you have real hardware tested with > the final version anyway. Should be plenty of time. > > Jonathan >>> --- >>> .../devicetree/bindings/iio/light/max30100.txt | 22 ++ >>> drivers/iio/Kconfig | 1 + >>> drivers/iio/Makefile | 1 + >>> drivers/iio/health/Kconfig | 21 + >>> drivers/iio/health/Makefile | 7 + >>> drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ >>> 6 files changed, 475 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt >>> create mode 100644 drivers/iio/health/Kconfig >>> create mode 100644 drivers/iio/health/Makefile >>> create mode 100644 drivers/iio/health/max30100.c >>> >>> diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt >>> new file mode 100644 >>> index 0000000..435d6fc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt >>> @@ -0,0 +1,22 @@ >>> +* Maxim MAX30100 heart rate and pulse oximeter sensor >>> + >>> +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf >>> + >>> +Required properties: >>> + >>> + - compatible: must be "maxim,max30100" >>> + - reg: the I2c address of the sensor >> >> I2C >> >>> + - interrupt-parent: should be the phandle for the interrupt controller >>> + - interrupts : the sole interrupt generated by the device >> >> no space before : for consistency >> >>> + >>> + Refer to interrupt-controller/interrupts.txt for generic interrupt client >>> + node bindings. >>> + >>> +Example: >>> + >>> +max30100@57 { >>> + compatible = "maxim,max30100"; >>> + reg = <0x57>; >>> + interrupt-parent = <&gpio1>; >>> + interrupts = <16 2>; >>> +}; >>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >>> index 5dfb4f1..b846873 100644 >>> --- a/drivers/iio/Kconfig >>> +++ b/drivers/iio/Kconfig >>> @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" >>> source "drivers/iio/dac/Kconfig" >>> source "drivers/iio/frequency/Kconfig" >>> source "drivers/iio/gyro/Kconfig" >>> +source "drivers/iio/health/Kconfig" >>> source "drivers/iio/humidity/Kconfig" >>> source "drivers/iio/imu/Kconfig" >>> source "drivers/iio/light/Kconfig" >>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >>> index dc73c1f..7371791 100644 >>> --- a/drivers/iio/Makefile >>> +++ b/drivers/iio/Makefile >>> @@ -20,6 +20,7 @@ obj-y += common/ >>> obj-y += dac/ >>> obj-y += gyro/ >>> obj-y += frequency/ >>> +obj-y += health/ >>> obj-y += humidity/ >>> obj-y += imu/ >>> obj-y += light/ >>> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig >>> new file mode 100644 >>> index 0000000..30d4ca0 >>> --- /dev/null >>> +++ b/drivers/iio/health/Kconfig >>> @@ -0,0 +1,21 @@ >>> +# >>> +# Health sensors >>> +# >>> +# When adding new entries keep the list in alphabetical order >>> + >>> +menu "Health sensors" >>> + >>> +config MAX30100 >>> + tristate "MAX30100 heart rate and pulse oximeter sensor" >>> + depends on I2C >>> + select REGMAP_I2C >>> + select IIO_BUFFER >>> + select IIO_KFIFO_BUF >>> + help >>> + Say Y here to build I2C interface support for the Maxim >>> + MAX30100 heart rate, and pulse oximeter sensor. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called max30100 >> >> put a full stop (.) at the end of the sentence maybe :) >> >>> + >>> +endmenu >>> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile >>> new file mode 100644 >>> index 0000000..7c475d7 >>> --- /dev/null >>> +++ b/drivers/iio/health/Makefile >>> @@ -0,0 +1,7 @@ >>> +# >>> +# Makefile for IIO Health sensors >>> +# >>> + >>> +# When adding new entries keep the list in alphabetical order >>> + >>> +obj-$(CONFIG_MAX30100) += max30100.o >>> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c >>> new file mode 100644 >>> index 0000000..3ad6b31 >>> --- /dev/null >>> +++ b/drivers/iio/health/max30100.c >>> @@ -0,0 +1,423 @@ >>> +/* >>> + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor >>> + * >>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; either version 2 of the License, or >>> + * (at your option) any later version. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + */ >>> + >>> +#include <linux/module.h> >>> +#include <linux/init.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/delay.h> >>> +#include <linux/err.h> >>> +#include <linux/irq.h> >>> +#include <linux/i2c.h> >>> +#include <linux/mutex.h> >>> +#include <linux/regmap.h> >>> +#include <linux/iio/iio.h> >>> +#include <linux/iio/buffer.h> >>> +#include <linux/iio/kfifo_buf.h> >>> + >>> +#define MAX30100_REGMAP_NAME "max30100_regmap" >>> +#define MAX30100_DRV_NAME "max30100" >>> + >>> +#define MAX30100_REG_INT_STATUS 0x00 >>> +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) >>> +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) >>> +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) >>> + >>> +#define MAX30100_REG_INT_ENABLE 0x01 >>> +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) >>> +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) >>> +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 >>> +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 >>> + >>> +#define MAX30100_REG_FIFO_DATA 0x05 >>> + >>> +#define MAX30100_REG_MODE_CONFIG 0x06 >>> +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) >>> +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) >>> +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 >>> +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) >>> +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) >>> + >>> +#define MAX30100_REG_SPO2_CONFIG 0x07 >>> +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) >>> + >>> +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) >>> +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 >>> + >>> +#define MAX30100_REG_LED_CONFIG 0x09 >>> +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 >>> + >>> +#define MAX30100_REG_LED_CONFIG_24MA 0x07 >>> +#define MAX30100_REG_LED_CONFIG_50MA 0x0f >>> + >>> +#define MAX30100_REG_TEMP_INTEGER 0x16 >>> +#define MAX30100_REG_TEMP_FRACTION 0x17 >>> + >>> +struct max30100_data { >>> + struct iio_dev *indio_dev; > This is only used in one place and there is a perfectly good pointer to > this in the interrupt anyway so drop it from here. (note this is a > common one to spot, there is very rarely a good reason to have a pointer > to the struct iio_dev in the private data. Everything should be the other > way around!) Ah right :) > > >>> + struct mutex lock; >>> + struct regmap *regmap; >>> + >>> + u8 buffer[4]; /* 4 8-bit channels */ >>> +}; >>> + >>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) >>> +{ >>> + switch (reg) { >>> + case MAX30100_REG_INT_STATUS: >>> + case MAX30100_REG_MODE_CONFIG: >>> + case MAX30100_REG_FIFO_DATA: >>> + case MAX30100_REG_TEMP_INTEGER: >>> + case MAX30100_REG_TEMP_FRACTION: >>> + return true; >>> + default: >>> + return false; >>> + } >>> +} >>> + >>> +static const struct regmap_config max30100_regmap_config = { >>> + .name = MAX30100_REGMAP_NAME, >>> + >>> + .reg_bits = 8, >>> + .val_bits = 8, >>> + >>> + .max_register = MAX30100_REG_LED_CONFIG, >>> + .cache_type = REGCACHE_FLAT, >>> + >>> + .volatile_reg = max30100_is_volatile_reg, >>> +}; >>> + >>> +static const unsigned long max30100_scan_masks[] = {0x3, 0}; >>> + >>> +static const struct iio_chan_spec max30100_channels[] = { >>> + { >>> + .type = IIO_INTENSITY, >>> + .channel2 = IIO_MOD_LIGHT_IR, >>> + >>> + .scan_index = 0, >>> + .scan_type = { >>> + .sign = 'u', >>> + .realbits = 16, >>> + .storagebits = 16, >>> + .endianness = IIO_BE, >>> + }, >>> + }, >>> + { >>> + .type = IIO_INTENSITY, >>> + .channel2 = IIO_MOD_LIGHT_RED, >>> + >>> + .scan_index = 1, >>> + .scan_type = { >>> + .sign = 'u', >>> + .realbits = 16, >> >> max30100_interrupt_handler() reads 4 bytes?! >> >> description of buffer in max30100_data: >> u8 buffer[4]; /* 4 8-bit channels */ >> this is confusing; maybe __be16 buffer[2]? > Agreed, __be16 version preferred. Makes it explicit > what we have and fix the description (good spot Peter!) >> >>> + .storagebits = 16, >>> + .endianness = IIO_BE, >>> + }, >>> + }, >>> + { >>> + .type = IIO_TEMP, >>> + .info_mask_separate = >>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>> + .scan_index = -1, >>> + }, >>> +}; >>> + >>> +static int max30100_set_powermode(struct max30100_data *data, bool state) >>> +{ >>> + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>> + MAX30100_REG_MODE_CONFIG_PWR, >>> + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); >>> +} >>> + >>> +static int max30100_buffer_postenable(struct iio_dev *indio_dev) >>> +{ >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + >>> + return max30100_set_powermode(data, true); >>> +} >>> + >>> +static int max30100_buffer_predisable(struct iio_dev *indio_dev) >>> +{ >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + >>> + return max30100_set_powermode(data, false); >>> +} >>> + >>> +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { >>> + .postenable = max30100_buffer_postenable, >>> + .predisable = max30100_buffer_predisable, >>> +}; >>> + >>> +static inline int max30100_fifo_is_empty(struct max30100_data *data) >>> +{ >>> + int ret, val; >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); >>> + if (ret) >>> + return ret; >>> + >>> + return !!(val >> 4); > You don't really need the !! as you only check it's != 0 later... > I guess you could argue this makes it obvious it is a boolean though... >>> +} >>> + >>> +static irqreturn_t max30100_interrupt_handler(int irq, void *private) >>> +{ >>> + struct iio_dev *indio_dev = private; >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + int ret; >>> + >>> + mutex_lock(&data->lock); >>> + >>> + while (max30100_fifo_is_empty(data) != 0) { >>> + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, >>> + &data->buffer, 4); >>> + >>> + if (ret) >>> + goto err_read; >>> + > use the local copy (ultimately private) instead of the version in data. >>> + iio_push_to_buffers(data->indio_dev, data->buffer); >>> + } >>> + >>> +err_read: >>> + mutex_unlock(&data->lock); >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static int max30100_chip_init(struct max30100_data *data) >>> +{ >>> + int ret; >>> + >>> + /* RED IR LED = 24mA, IR LED = 50mA */ >>> + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, >>> + (MAX30100_REG_LED_CONFIG_24MA << >>> + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | >>> + MAX30100_REG_LED_CONFIG_50MA); >>> + if (ret) >>> + return ret; >>> + >>> + /* enable hi-res SPO2 readings with 100 samples a second */ >>> + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, >>> + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | >>> + MAX30100_REG_SPO2_CONFIG_100HZ | >>> + MAX30100_REG_SPO2_CONFIG_1600US); >>> + if (ret) >>> + return ret; >>> + >>> + /* enable SPO2 + HR mode */ >>> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>> + MAX30100_REG_MODE_CONFIG_MODE_MASK, >>> + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | >>> + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); >>> + if (ret) >>> + return ret; >>> + >>> + /* enable SPO2 + HR interrupts */ >>> + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, >>> + MAX30100_REG_INT_ENABLE_MASK, >>> + (MAX30100_REG_INT_ENABLE_HR_EN | >>> + MAX30100_REG_INT_ENABLE_SPO2_EN) >>> + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); >>> +} >>> + >>> +static int max30100_read_temp(struct max30100_data *data, int *val) >>> +{ >>> + int ret; >>> + int reg; >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); >>> + if (ret < 0) >>> + return ret; >>> + *val = reg << 4; >> >> note that this is a signed value; I think negative temperatures are not >> processed correctly > Definitely looks like it's missing a sign extend to me as well! D'oh! Yeah you are correct! >> >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); >>> + if (ret < 0) >>> + return ret; >>> + *val |= reg & 0xf; >>> + >>> + return 0; >>> +} >>> + >>> +static int max30100_get_temp(struct max30100_data *data, int *val) >>> +{ >>> + int ret; >>> + int tries = 3; >>> + >>> + /* start acquisition */ >>> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>> + MAX30100_REG_MODE_CONFIG_TEMP_EN, >>> + MAX30100_REG_MODE_CONFIG_TEMP_EN); >>> + if (ret) >>> + return ret; >>> + >>> + while (tries--) { >>> + int reg; > Could use the interrupt that indicates temperature is ready... > > Why have you chosen not to? (might well be sensible but you want > to have documentation somewhere here to explain why.) > > I guess it's because it is likely to be an occasional out of band reading > and you don't want to complicate the interrupt handling? Yeah I didn't want to complicate the interrupt handler. Pretty much the only reason I did it that way... Should I document that in a comment? >>> + >>> + usleep_range(29000, 40000); >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); >>> + if (ret < 0) >>> + break; >>> + >>> + /* bit reset to zero when reading is ready */ >>> + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { >>> + ret = max30100_read_temp(data, val); >>> + break; >>> + } >>> + ret = -EIO; >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +static int max30100_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + int ret = -EINVAL; >>> + >>> + mutex_lock(&indio_dev->mlock); >>> + >>> + /* temperature reading can only be acquired while engine is running */ >>> + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { >>> + ret = -EAGAIN; > Possibly -EBUSY. EAGAIN tends to lead to usespace software immediately > trying to read again whereas EBUSY tends to imply there is a 'reason' beyond > just 'not now'. > > Not a particularly clear cut case however. > > Reorder this to be under the switch (mask) case IIO_CHAN_INFO_RAW though > and the code flow will be simpler as you won't need to check we are trying > a raw read twice. Got it! > >>> + goto error_again; >>> + } >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + ret = max30100_get_temp(data, val); >>> + if (!ret) >> >> what if ret < 0? >> _get_temp() checks for ret < 0 >> >> INTENSITY channels cannot be raw_read? > It's not in the channel spec, so Matt's right here. However > given this is 'unusual' it might act as a form of 'documentation' > to explicitly verify we have a temperature channel here. > > (Peter, for why they have no _raw access see the previous version - > basically there is no point in getting a single reading on this sort > of device - it's the processing of the waveform that allows you to > establish what the device is meant to be measuring. Makes sense to > me). >> >>> + ret = IIO_VAL_INT; >>> + break; >>> + case IIO_CHAN_INFO_SCALE: >>> + *val = 1; /* 0.0625 */ >>> + *val2 = 16; >>> + ret = IIO_VAL_FRACTIONAL; >>> + break; >>> + } >>> + >>> +error_again: >>> + mutex_unlock(&indio_dev->mlock); >>> + >>> + return ret; >>> +} >>> + >>> +static const struct iio_info max30100_info = { >>> + .driver_module = THIS_MODULE, >>> + .read_raw = max30100_read_raw, >>> +}; >>> + >>> +static int max30100_probe(struct i2c_client *client, >>> + const struct i2c_device_id *id) >>> +{ >>> + struct max30100_data *data; >>> + struct iio_buffer *buffer; >>> + struct iio_dev *indio_dev; >>> + int ret; >>> + >>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + buffer = devm_iio_kfifo_allocate(&client->dev); >>> + if (!buffer) >>> + return -ENOMEM; >>> + >>> + iio_device_attach_buffer(indio_dev, buffer); >>> + >>> + indio_dev->name = MAX30100_DRV_NAME; >>> + indio_dev->channels = max30100_channels; >>> + indio_dev->info = &max30100_info; >>> + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); >>> + indio_dev->available_scan_masks = max30100_scan_masks; >>> + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); >>> + indio_dev->setup_ops = &max30100_buffer_setup_ops; >>> + >>> + data = iio_priv(indio_dev); >>> + data->indio_dev = indio_dev; >>> + >>> + mutex_init(&data->lock); >>> + i2c_set_clientdata(client, indio_dev); >>> + >>> + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); >>> + if (IS_ERR(data->regmap)) { >>> + dev_err(&client->dev, "regmap initialization failed.\n"); >>> + return PTR_ERR(data->regmap); >>> + } >>> + max30100_set_powermode(data, false); >>> + >>> + ret = max30100_chip_init(data); >>> + if (ret) >>> + goto error_out; >>> + >>> + if (client->irq <= 0) { >>> + dev_err(&client->dev, "no valid irq defined\n"); >>> + ret = -EINVAL; >>> + goto error_out; >>> + } >>> + ret = devm_request_threaded_irq(&client->dev, client->irq, >>> + NULL, max30100_interrupt_handler, >>> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >>> + "max30100_irq", indio_dev); >>> + if (ret) { >>> + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); >>> + goto error_out; >>> + } >>> + >>> + return iio_device_register(indio_dev); >> >> newline here >> >>> +error_out: >> >> drop newline >> >>> + >>> + return ret; >>> +} >>> + >>> +static int max30100_remove(struct i2c_client *client) >>> +{ >>> + struct iio_dev *indio_dev = i2c_get_clientdata(client); >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + >>> + iio_device_unregister(indio_dev); >>> + max30100_set_powermode(data, false); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct i2c_device_id max30100_id[] = { >>> + { "max30100", 0 }, >>> + {} >>> +}; >>> +MODULE_DEVICE_TABLE(i2c, max30100_id); >>> + >>> +static const struct of_device_id max30100_dt_ids[] = { >>> + { .compatible = "maxim,max30100" }, >>> + { } >>> +}; >>> +MODULE_DEVICE_TABLE(of, max30100_dt_ids); >>> + >>> +static struct i2c_driver max30100_driver = { >>> + .driver = { >>> + .name = MAX30100_DRV_NAME, >>> + .of_match_table = of_match_ptr(max30100_dt_ids), >>> + }, >>> + .probe = max30100_probe, >>> + .remove = max30100_remove, >>> + .id_table = max30100_id, >>> +}; >>> +module_i2c_driver(max30100_driver); >>> + >>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>"); >>> +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); >>> +MODULE_LICENSE("GPL"); >>> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/1] iio: light: add MAX30100 oximeter driver support 2015-11-22 12:07 ` Matt Ranostay @ 2015-11-22 12:36 ` Jonathan Cameron 0 siblings, 0 replies; 7+ messages in thread From: Jonathan Cameron @ 2015-11-22 12:36 UTC (permalink / raw) To: Matt Ranostay; +Cc: Peter Meerwald-Stadler, linux-iio@vger.kernel.org On 22/11/15 12:07, Matt Ranostay wrote: > On Sat, Nov 21, 2015 at 11:26 PM, Jonathan Cameron <jic23@kernel.org> wrote: >> On 20/11/15 16:40, Peter Meerwald-Stadler wrote: >>> On Wed, 18 Nov 2015, Matt Ranostay wrote: >>> >>>> MAX30100 is an heart rate and pulse oximeter sensor that works using >>>> two LEDS of different wavelengths, and detecting the light reflected >>>> back. >>>> >>>> This patchset adds support for both IR and RED LED channels which can >>> >>> red? this refers to a LED with visible red color? >>> more nitpicking below >> As I'm here, I'll answer that one. Yes. Absolutely, it's 700->800nm light >> which pretty much makes it mostly visible red. > > Yeah this device uses a "red" light (aka close enough to wavelength) > mainly for the oxygen reading. Basically all this email says is yes to documenting the not using interrupt for temp reading bit. J > >>> >>>> be processed in userspace to determine heart rate and blood oxygen >>>> levels. >>>> >>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com> >> Few additional comments from me. Coming along nicely. >> >> Guessing we want to hold this until you have real hardware tested with >> the final version anyway. Should be plenty of time. >> >> Jonathan >>>> --- >>>> .../devicetree/bindings/iio/light/max30100.txt | 22 ++ >>>> drivers/iio/Kconfig | 1 + >>>> drivers/iio/Makefile | 1 + >>>> drivers/iio/health/Kconfig | 21 + >>>> drivers/iio/health/Makefile | 7 + >>>> drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ >>>> 6 files changed, 475 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt >>>> create mode 100644 drivers/iio/health/Kconfig >>>> create mode 100644 drivers/iio/health/Makefile >>>> create mode 100644 drivers/iio/health/max30100.c >>>> >>>> diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt >>>> new file mode 100644 >>>> index 0000000..435d6fc >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt >>>> @@ -0,0 +1,22 @@ >>>> +* Maxim MAX30100 heart rate and pulse oximeter sensor >>>> + >>>> +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf >>>> + >>>> +Required properties: >>>> + >>>> + - compatible: must be "maxim,max30100" >>>> + - reg: the I2c address of the sensor >>> >>> I2C >>> >>>> + - interrupt-parent: should be the phandle for the interrupt controller >>>> + - interrupts : the sole interrupt generated by the device >>> >>> no space before : for consistency >>> >>>> + >>>> + Refer to interrupt-controller/interrupts.txt for generic interrupt client >>>> + node bindings. >>>> + >>>> +Example: >>>> + >>>> +max30100@57 { >>>> + compatible = "maxim,max30100"; >>>> + reg = <0x57>; >>>> + interrupt-parent = <&gpio1>; >>>> + interrupts = <16 2>; >>>> +}; >>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >>>> index 5dfb4f1..b846873 100644 >>>> --- a/drivers/iio/Kconfig >>>> +++ b/drivers/iio/Kconfig >>>> @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" >>>> source "drivers/iio/dac/Kconfig" >>>> source "drivers/iio/frequency/Kconfig" >>>> source "drivers/iio/gyro/Kconfig" >>>> +source "drivers/iio/health/Kconfig" >>>> source "drivers/iio/humidity/Kconfig" >>>> source "drivers/iio/imu/Kconfig" >>>> source "drivers/iio/light/Kconfig" >>>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >>>> index dc73c1f..7371791 100644 >>>> --- a/drivers/iio/Makefile >>>> +++ b/drivers/iio/Makefile >>>> @@ -20,6 +20,7 @@ obj-y += common/ >>>> obj-y += dac/ >>>> obj-y += gyro/ >>>> obj-y += frequency/ >>>> +obj-y += health/ >>>> obj-y += humidity/ >>>> obj-y += imu/ >>>> obj-y += light/ >>>> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig >>>> new file mode 100644 >>>> index 0000000..30d4ca0 >>>> --- /dev/null >>>> +++ b/drivers/iio/health/Kconfig >>>> @@ -0,0 +1,21 @@ >>>> +# >>>> +# Health sensors >>>> +# >>>> +# When adding new entries keep the list in alphabetical order >>>> + >>>> +menu "Health sensors" >>>> + >>>> +config MAX30100 >>>> + tristate "MAX30100 heart rate and pulse oximeter sensor" >>>> + depends on I2C >>>> + select REGMAP_I2C >>>> + select IIO_BUFFER >>>> + select IIO_KFIFO_BUF >>>> + help >>>> + Say Y here to build I2C interface support for the Maxim >>>> + MAX30100 heart rate, and pulse oximeter sensor. >>>> + >>>> + To compile this driver as a module, choose M here: the >>>> + module will be called max30100 >>> >>> put a full stop (.) at the end of the sentence maybe :) >>> >>>> + >>>> +endmenu >>>> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile >>>> new file mode 100644 >>>> index 0000000..7c475d7 >>>> --- /dev/null >>>> +++ b/drivers/iio/health/Makefile >>>> @@ -0,0 +1,7 @@ >>>> +# >>>> +# Makefile for IIO Health sensors >>>> +# >>>> + >>>> +# When adding new entries keep the list in alphabetical order >>>> + >>>> +obj-$(CONFIG_MAX30100) += max30100.o >>>> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c >>>> new file mode 100644 >>>> index 0000000..3ad6b31 >>>> --- /dev/null >>>> +++ b/drivers/iio/health/max30100.c >>>> @@ -0,0 +1,423 @@ >>>> +/* >>>> + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor >>>> + * >>>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com> >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify >>>> + * it under the terms of the GNU General Public License as published by >>>> + * the Free Software Foundation; either version 2 of the License, or >>>> + * (at your option) any later version. >>>> + * >>>> + * This program is distributed in the hope that it will be useful, >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> + * GNU General Public License for more details. >>>> + * >>>> + */ >>>> + >>>> +#include <linux/module.h> >>>> +#include <linux/init.h> >>>> +#include <linux/interrupt.h> >>>> +#include <linux/delay.h> >>>> +#include <linux/err.h> >>>> +#include <linux/irq.h> >>>> +#include <linux/i2c.h> >>>> +#include <linux/mutex.h> >>>> +#include <linux/regmap.h> >>>> +#include <linux/iio/iio.h> >>>> +#include <linux/iio/buffer.h> >>>> +#include <linux/iio/kfifo_buf.h> >>>> + >>>> +#define MAX30100_REGMAP_NAME "max30100_regmap" >>>> +#define MAX30100_DRV_NAME "max30100" >>>> + >>>> +#define MAX30100_REG_INT_STATUS 0x00 >>>> +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) >>>> +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) >>>> +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) >>>> + >>>> +#define MAX30100_REG_INT_ENABLE 0x01 >>>> +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) >>>> +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) >>>> +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 >>>> +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 >>>> + >>>> +#define MAX30100_REG_FIFO_DATA 0x05 >>>> + >>>> +#define MAX30100_REG_MODE_CONFIG 0x06 >>>> +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) >>>> +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) >>>> +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 >>>> +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) >>>> +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) >>>> + >>>> +#define MAX30100_REG_SPO2_CONFIG 0x07 >>>> +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) >>>> + >>>> +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) >>>> +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 >>>> + >>>> +#define MAX30100_REG_LED_CONFIG 0x09 >>>> +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 >>>> + >>>> +#define MAX30100_REG_LED_CONFIG_24MA 0x07 >>>> +#define MAX30100_REG_LED_CONFIG_50MA 0x0f >>>> + >>>> +#define MAX30100_REG_TEMP_INTEGER 0x16 >>>> +#define MAX30100_REG_TEMP_FRACTION 0x17 >>>> + >>>> +struct max30100_data { >>>> + struct iio_dev *indio_dev; >> This is only used in one place and there is a perfectly good pointer to >> this in the interrupt anyway so drop it from here. (note this is a >> common one to spot, there is very rarely a good reason to have a pointer >> to the struct iio_dev in the private data. Everything should be the other >> way around!) > > Ah right :) >> >> >>>> + struct mutex lock; >>>> + struct regmap *regmap; >>>> + >>>> + u8 buffer[4]; /* 4 8-bit channels */ >>>> +}; >>>> + >>>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) >>>> +{ >>>> + switch (reg) { >>>> + case MAX30100_REG_INT_STATUS: >>>> + case MAX30100_REG_MODE_CONFIG: >>>> + case MAX30100_REG_FIFO_DATA: >>>> + case MAX30100_REG_TEMP_INTEGER: >>>> + case MAX30100_REG_TEMP_FRACTION: >>>> + return true; >>>> + default: >>>> + return false; >>>> + } >>>> +} >>>> + >>>> +static const struct regmap_config max30100_regmap_config = { >>>> + .name = MAX30100_REGMAP_NAME, >>>> + >>>> + .reg_bits = 8, >>>> + .val_bits = 8, >>>> + >>>> + .max_register = MAX30100_REG_LED_CONFIG, >>>> + .cache_type = REGCACHE_FLAT, >>>> + >>>> + .volatile_reg = max30100_is_volatile_reg, >>>> +}; >>>> + >>>> +static const unsigned long max30100_scan_masks[] = {0x3, 0}; >>>> + >>>> +static const struct iio_chan_spec max30100_channels[] = { >>>> + { >>>> + .type = IIO_INTENSITY, >>>> + .channel2 = IIO_MOD_LIGHT_IR, >>>> + >>>> + .scan_index = 0, >>>> + .scan_type = { >>>> + .sign = 'u', >>>> + .realbits = 16, >>>> + .storagebits = 16, >>>> + .endianness = IIO_BE, >>>> + }, >>>> + }, >>>> + { >>>> + .type = IIO_INTENSITY, >>>> + .channel2 = IIO_MOD_LIGHT_RED, >>>> + >>>> + .scan_index = 1, >>>> + .scan_type = { >>>> + .sign = 'u', >>>> + .realbits = 16, >>> >>> max30100_interrupt_handler() reads 4 bytes?! >>> >>> description of buffer in max30100_data: >>> u8 buffer[4]; /* 4 8-bit channels */ >>> this is confusing; maybe __be16 buffer[2]? >> Agreed, __be16 version preferred. Makes it explicit >> what we have and fix the description (good spot Peter!) >>> >>>> + .storagebits = 16, >>>> + .endianness = IIO_BE, >>>> + }, >>>> + }, >>>> + { >>>> + .type = IIO_TEMP, >>>> + .info_mask_separate = >>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>>> + .scan_index = -1, >>>> + }, >>>> +}; >>>> + >>>> +static int max30100_set_powermode(struct max30100_data *data, bool state) >>>> +{ >>>> + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>>> + MAX30100_REG_MODE_CONFIG_PWR, >>>> + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); >>>> +} >>>> + >>>> +static int max30100_buffer_postenable(struct iio_dev *indio_dev) >>>> +{ >>>> + struct max30100_data *data = iio_priv(indio_dev); >>>> + >>>> + return max30100_set_powermode(data, true); >>>> +} >>>> + >>>> +static int max30100_buffer_predisable(struct iio_dev *indio_dev) >>>> +{ >>>> + struct max30100_data *data = iio_priv(indio_dev); >>>> + >>>> + return max30100_set_powermode(data, false); >>>> +} >>>> + >>>> +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { >>>> + .postenable = max30100_buffer_postenable, >>>> + .predisable = max30100_buffer_predisable, >>>> +}; >>>> + >>>> +static inline int max30100_fifo_is_empty(struct max30100_data *data) >>>> +{ >>>> + int ret, val; >>>> + >>>> + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + return !!(val >> 4); >> You don't really need the !! as you only check it's != 0 later... >> I guess you could argue this makes it obvious it is a boolean though... >>>> +} >>>> + >>>> +static irqreturn_t max30100_interrupt_handler(int irq, void *private) >>>> +{ >>>> + struct iio_dev *indio_dev = private; >>>> + struct max30100_data *data = iio_priv(indio_dev); >>>> + int ret; >>>> + >>>> + mutex_lock(&data->lock); >>>> + >>>> + while (max30100_fifo_is_empty(data) != 0) { >>>> + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, >>>> + &data->buffer, 4); >>>> + >>>> + if (ret) >>>> + goto err_read; >>>> + >> use the local copy (ultimately private) instead of the version in data. >>>> + iio_push_to_buffers(data->indio_dev, data->buffer); >>>> + } >>>> + >>>> +err_read: >>>> + mutex_unlock(&data->lock); >>>> + >>>> + return IRQ_HANDLED; >>>> +} >>>> + >>>> +static int max30100_chip_init(struct max30100_data *data) >>>> +{ >>>> + int ret; >>>> + >>>> + /* RED IR LED = 24mA, IR LED = 50mA */ >>>> + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, >>>> + (MAX30100_REG_LED_CONFIG_24MA << >>>> + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | >>>> + MAX30100_REG_LED_CONFIG_50MA); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* enable hi-res SPO2 readings with 100 samples a second */ >>>> + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, >>>> + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | >>>> + MAX30100_REG_SPO2_CONFIG_100HZ | >>>> + MAX30100_REG_SPO2_CONFIG_1600US); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* enable SPO2 + HR mode */ >>>> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>>> + MAX30100_REG_MODE_CONFIG_MODE_MASK, >>>> + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | >>>> + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* enable SPO2 + HR interrupts */ >>>> + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, >>>> + MAX30100_REG_INT_ENABLE_MASK, >>>> + (MAX30100_REG_INT_ENABLE_HR_EN | >>>> + MAX30100_REG_INT_ENABLE_SPO2_EN) >>>> + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); >>>> +} >>>> + >>>> +static int max30100_read_temp(struct max30100_data *data, int *val) >>>> +{ >>>> + int ret; >>>> + int reg; >>>> + >>>> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); >>>> + if (ret < 0) >>>> + return ret; >>>> + *val = reg << 4; >>> >>> note that this is a signed value; I think negative temperatures are not >>> processed correctly >> Definitely looks like it's missing a sign extend to me as well! > > D'oh! Yeah you are correct! > >>> >>>> + >>>> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); >>>> + if (ret < 0) >>>> + return ret; >>>> + *val |= reg & 0xf; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int max30100_get_temp(struct max30100_data *data, int *val) >>>> +{ >>>> + int ret; >>>> + int tries = 3; >>>> + >>>> + /* start acquisition */ >>>> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>>> + MAX30100_REG_MODE_CONFIG_TEMP_EN, >>>> + MAX30100_REG_MODE_CONFIG_TEMP_EN); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + while (tries--) { >>>> + int reg; >> Could use the interrupt that indicates temperature is ready... >> >> Why have you chosen not to? (might well be sensible but you want >> to have documentation somewhere here to explain why.) >> >> I guess it's because it is likely to be an occasional out of band reading >> and you don't want to complicate the interrupt handling? > > Yeah I didn't want to complicate the interrupt handler. Pretty much > the only reason I did it that way... > Should I document that in a comment? Yes. > >>>> + >>>> + usleep_range(29000, 40000); >>>> + >>>> + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); >>>> + if (ret < 0) >>>> + break; >>>> + >>>> + /* bit reset to zero when reading is ready */ >>>> + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { >>>> + ret = max30100_read_temp(data, val); >>>> + break; >>>> + } >>>> + ret = -EIO; >>>> + } >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int max30100_read_raw(struct iio_dev *indio_dev, >>>> + struct iio_chan_spec const *chan, >>>> + int *val, int *val2, long mask) >>>> +{ >>>> + struct max30100_data *data = iio_priv(indio_dev); >>>> + int ret = -EINVAL; >>>> + >>>> + mutex_lock(&indio_dev->mlock); >>>> + >>>> + /* temperature reading can only be acquired while engine is running */ >>>> + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { >>>> + ret = -EAGAIN; >> Possibly -EBUSY. EAGAIN tends to lead to usespace software immediately >> trying to read again whereas EBUSY tends to imply there is a 'reason' beyond >> just 'not now'. >> >> Not a particularly clear cut case however. >> >> Reorder this to be under the switch (mask) case IIO_CHAN_INFO_RAW though >> and the code flow will be simpler as you won't need to check we are trying >> a raw read twice. > > Got it! >> >>>> + goto error_again; >>>> + } >>>> + >>>> + switch (mask) { >>>> + case IIO_CHAN_INFO_RAW: >>>> + ret = max30100_get_temp(data, val); >>>> + if (!ret) >>> >>> what if ret < 0? >>> _get_temp() checks for ret < 0 >>> >>> INTENSITY channels cannot be raw_read? >> It's not in the channel spec, so Matt's right here. However >> given this is 'unusual' it might act as a form of 'documentation' >> to explicitly verify we have a temperature channel here. >> >> (Peter, for why they have no _raw access see the previous version - >> basically there is no point in getting a single reading on this sort >> of device - it's the processing of the waveform that allows you to >> establish what the device is meant to be measuring. Makes sense to >> me). >>> >>>> + ret = IIO_VAL_INT; >>>> + break; >>>> + case IIO_CHAN_INFO_SCALE: >>>> + *val = 1; /* 0.0625 */ >>>> + *val2 = 16; >>>> + ret = IIO_VAL_FRACTIONAL; >>>> + break; >>>> + } >>>> + >>>> +error_again: >>>> + mutex_unlock(&indio_dev->mlock); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static const struct iio_info max30100_info = { >>>> + .driver_module = THIS_MODULE, >>>> + .read_raw = max30100_read_raw, >>>> +}; >>>> + >>>> +static int max30100_probe(struct i2c_client *client, >>>> + const struct i2c_device_id *id) >>>> +{ >>>> + struct max30100_data *data; >>>> + struct iio_buffer *buffer; >>>> + struct iio_dev *indio_dev; >>>> + int ret; >>>> + >>>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >>>> + if (!indio_dev) >>>> + return -ENOMEM; >>>> + >>>> + buffer = devm_iio_kfifo_allocate(&client->dev); >>>> + if (!buffer) >>>> + return -ENOMEM; >>>> + >>>> + iio_device_attach_buffer(indio_dev, buffer); >>>> + >>>> + indio_dev->name = MAX30100_DRV_NAME; >>>> + indio_dev->channels = max30100_channels; >>>> + indio_dev->info = &max30100_info; >>>> + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); >>>> + indio_dev->available_scan_masks = max30100_scan_masks; >>>> + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); >>>> + indio_dev->setup_ops = &max30100_buffer_setup_ops; >>>> + >>>> + data = iio_priv(indio_dev); >>>> + data->indio_dev = indio_dev; >>>> + >>>> + mutex_init(&data->lock); >>>> + i2c_set_clientdata(client, indio_dev); >>>> + >>>> + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); >>>> + if (IS_ERR(data->regmap)) { >>>> + dev_err(&client->dev, "regmap initialization failed.\n"); >>>> + return PTR_ERR(data->regmap); >>>> + } >>>> + max30100_set_powermode(data, false); >>>> + >>>> + ret = max30100_chip_init(data); >>>> + if (ret) >>>> + goto error_out; >>>> + >>>> + if (client->irq <= 0) { >>>> + dev_err(&client->dev, "no valid irq defined\n"); >>>> + ret = -EINVAL; >>>> + goto error_out; >>>> + } >>>> + ret = devm_request_threaded_irq(&client->dev, client->irq, >>>> + NULL, max30100_interrupt_handler, >>>> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >>>> + "max30100_irq", indio_dev); >>>> + if (ret) { >>>> + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); >>>> + goto error_out; >>>> + } >>>> + >>>> + return iio_device_register(indio_dev); >>> >>> newline here >>> >>>> +error_out: >>> >>> drop newline >>> >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int max30100_remove(struct i2c_client *client) >>>> +{ >>>> + struct iio_dev *indio_dev = i2c_get_clientdata(client); >>>> + struct max30100_data *data = iio_priv(indio_dev); >>>> + >>>> + iio_device_unregister(indio_dev); >>>> + max30100_set_powermode(data, false); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static const struct i2c_device_id max30100_id[] = { >>>> + { "max30100", 0 }, >>>> + {} >>>> +}; >>>> +MODULE_DEVICE_TABLE(i2c, max30100_id); >>>> + >>>> +static const struct of_device_id max30100_dt_ids[] = { >>>> + { .compatible = "maxim,max30100" }, >>>> + { } >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, max30100_dt_ids); >>>> + >>>> +static struct i2c_driver max30100_driver = { >>>> + .driver = { >>>> + .name = MAX30100_DRV_NAME, >>>> + .of_match_table = of_match_ptr(max30100_dt_ids), >>>> + }, >>>> + .probe = max30100_probe, >>>> + .remove = max30100_remove, >>>> + .id_table = max30100_id, >>>> +}; >>>> +module_i2c_driver(max30100_driver); >>>> + >>>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>"); >>>> +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); >>>> +MODULE_LICENSE("GPL"); >>>> >>> >> > -- > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/1] iio: light: add MAX30100 oximeter driver support 2015-11-20 16:40 ` Peter Meerwald-Stadler 2015-11-21 16:26 ` Jonathan Cameron @ 2015-12-02 18:36 ` Matt Ranostay 1 sibling, 0 replies; 7+ messages in thread From: Matt Ranostay @ 2015-12-02 18:36 UTC (permalink / raw) To: Peter Meerwald-Stadler; +Cc: linux-iio@vger.kernel.org, Jonathan Cameron On Fri, Nov 20, 2015 at 8:40 AM, Peter Meerwald-Stadler <pmeerw@pmeerw.net> wrote: > On Wed, 18 Nov 2015, Matt Ranostay wrote: > >> MAX30100 is an heart rate and pulse oximeter sensor that works using >> two LEDS of different wavelengths, and detecting the light reflected >> back. >> >> This patchset adds support for both IR and RED LED channels which can > > red? this refers to a LED with visible red color? > more nitpicking below > >> be processed in userspace to determine heart rate and blood oxygen >> levels. >> >> Signed-off-by: Matt Ranostay <mranostay@gmail.com> >> --- >> .../devicetree/bindings/iio/light/max30100.txt | 22 ++ >> drivers/iio/Kconfig | 1 + >> drivers/iio/Makefile | 1 + >> drivers/iio/health/Kconfig | 21 + >> drivers/iio/health/Makefile | 7 + >> drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ >> 6 files changed, 475 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt >> create mode 100644 drivers/iio/health/Kconfig >> create mode 100644 drivers/iio/health/Makefile >> create mode 100644 drivers/iio/health/max30100.c >> >> diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt >> new file mode 100644 >> index 0000000..435d6fc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt >> @@ -0,0 +1,22 @@ >> +* Maxim MAX30100 heart rate and pulse oximeter sensor >> + >> +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf >> + >> +Required properties: >> + >> + - compatible: must be "maxim,max30100" >> + - reg: the I2c address of the sensor > > I2C > >> + - interrupt-parent: should be the phandle for the interrupt controller >> + - interrupts : the sole interrupt generated by the device > > no space before : for consistency > >> + >> + Refer to interrupt-controller/interrupts.txt for generic interrupt client >> + node bindings. >> + >> +Example: >> + >> +max30100@57 { >> + compatible = "maxim,max30100"; >> + reg = <0x57>; >> + interrupt-parent = <&gpio1>; >> + interrupts = <16 2>; >> +}; >> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >> index 5dfb4f1..b846873 100644 >> --- a/drivers/iio/Kconfig >> +++ b/drivers/iio/Kconfig >> @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" >> source "drivers/iio/dac/Kconfig" >> source "drivers/iio/frequency/Kconfig" >> source "drivers/iio/gyro/Kconfig" >> +source "drivers/iio/health/Kconfig" >> source "drivers/iio/humidity/Kconfig" >> source "drivers/iio/imu/Kconfig" >> source "drivers/iio/light/Kconfig" >> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >> index dc73c1f..7371791 100644 >> --- a/drivers/iio/Makefile >> +++ b/drivers/iio/Makefile >> @@ -20,6 +20,7 @@ obj-y += common/ >> obj-y += dac/ >> obj-y += gyro/ >> obj-y += frequency/ >> +obj-y += health/ >> obj-y += humidity/ >> obj-y += imu/ >> obj-y += light/ >> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig >> new file mode 100644 >> index 0000000..30d4ca0 >> --- /dev/null >> +++ b/drivers/iio/health/Kconfig >> @@ -0,0 +1,21 @@ >> +# >> +# Health sensors >> +# >> +# When adding new entries keep the list in alphabetical order >> + >> +menu "Health sensors" >> + >> +config MAX30100 >> + tristate "MAX30100 heart rate and pulse oximeter sensor" >> + depends on I2C >> + select REGMAP_I2C >> + select IIO_BUFFER >> + select IIO_KFIFO_BUF >> + help >> + Say Y here to build I2C interface support for the Maxim >> + MAX30100 heart rate, and pulse oximeter sensor. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called max30100 > > put a full stop (.) at the end of the sentence maybe :) > >> + >> +endmenu >> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile >> new file mode 100644 >> index 0000000..7c475d7 >> --- /dev/null >> +++ b/drivers/iio/health/Makefile >> @@ -0,0 +1,7 @@ >> +# >> +# Makefile for IIO Health sensors >> +# >> + >> +# When adding new entries keep the list in alphabetical order >> + >> +obj-$(CONFIG_MAX30100) += max30100.o >> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c >> new file mode 100644 >> index 0000000..3ad6b31 >> --- /dev/null >> +++ b/drivers/iio/health/max30100.c >> @@ -0,0 +1,423 @@ >> +/* >> + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor >> + * >> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/init.h> >> +#include <linux/interrupt.h> >> +#include <linux/delay.h> >> +#include <linux/err.h> >> +#include <linux/irq.h> >> +#include <linux/i2c.h> >> +#include <linux/mutex.h> >> +#include <linux/regmap.h> >> +#include <linux/iio/iio.h> >> +#include <linux/iio/buffer.h> >> +#include <linux/iio/kfifo_buf.h> >> + >> +#define MAX30100_REGMAP_NAME "max30100_regmap" >> +#define MAX30100_DRV_NAME "max30100" >> + >> +#define MAX30100_REG_INT_STATUS 0x00 >> +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) >> +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) >> +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) >> + >> +#define MAX30100_REG_INT_ENABLE 0x01 >> +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) >> +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) >> +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 >> +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 >> + >> +#define MAX30100_REG_FIFO_DATA 0x05 >> + >> +#define MAX30100_REG_MODE_CONFIG 0x06 >> +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) >> +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) >> +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 >> +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) >> +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) >> + >> +#define MAX30100_REG_SPO2_CONFIG 0x07 >> +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) >> + >> +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) >> +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 >> + >> +#define MAX30100_REG_LED_CONFIG 0x09 >> +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 >> + >> +#define MAX30100_REG_LED_CONFIG_24MA 0x07 >> +#define MAX30100_REG_LED_CONFIG_50MA 0x0f >> + >> +#define MAX30100_REG_TEMP_INTEGER 0x16 >> +#define MAX30100_REG_TEMP_FRACTION 0x17 >> + >> +struct max30100_data { >> + struct iio_dev *indio_dev; >> + struct mutex lock; >> + struct regmap *regmap; >> + >> + u8 buffer[4]; /* 4 8-bit channels */ >> +}; >> + >> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) >> +{ >> + switch (reg) { >> + case MAX30100_REG_INT_STATUS: >> + case MAX30100_REG_MODE_CONFIG: >> + case MAX30100_REG_FIFO_DATA: >> + case MAX30100_REG_TEMP_INTEGER: >> + case MAX30100_REG_TEMP_FRACTION: >> + return true; >> + default: >> + return false; >> + } >> +} >> + >> +static const struct regmap_config max30100_regmap_config = { >> + .name = MAX30100_REGMAP_NAME, >> + >> + .reg_bits = 8, >> + .val_bits = 8, >> + >> + .max_register = MAX30100_REG_LED_CONFIG, >> + .cache_type = REGCACHE_FLAT, >> + >> + .volatile_reg = max30100_is_volatile_reg, >> +}; >> + >> +static const unsigned long max30100_scan_masks[] = {0x3, 0}; >> + >> +static const struct iio_chan_spec max30100_channels[] = { >> + { >> + .type = IIO_INTENSITY, >> + .channel2 = IIO_MOD_LIGHT_IR, >> + >> + .scan_index = 0, >> + .scan_type = { >> + .sign = 'u', >> + .realbits = 16, >> + .storagebits = 16, >> + .endianness = IIO_BE, >> + }, >> + }, >> + { >> + .type = IIO_INTENSITY, >> + .channel2 = IIO_MOD_LIGHT_RED, >> + >> + .scan_index = 1, >> + .scan_type = { >> + .sign = 'u', >> + .realbits = 16, > > max30100_interrupt_handler() reads 4 bytes?! > > description of buffer in max30100_data: > u8 buffer[4]; /* 4 8-bit channels */ > this is confusing; maybe __be16 buffer[2]? > >> + .storagebits = 16, >> + .endianness = IIO_BE, >> + }, >> + }, >> + { >> + .type = IIO_TEMP, >> + .info_mask_separate = >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >> + .scan_index = -1, >> + }, >> +}; >> + >> +static int max30100_set_powermode(struct max30100_data *data, bool state) >> +{ >> + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >> + MAX30100_REG_MODE_CONFIG_PWR, >> + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); >> +} >> + >> +static int max30100_buffer_postenable(struct iio_dev *indio_dev) >> +{ >> + struct max30100_data *data = iio_priv(indio_dev); >> + >> + return max30100_set_powermode(data, true); >> +} >> + >> +static int max30100_buffer_predisable(struct iio_dev *indio_dev) >> +{ >> + struct max30100_data *data = iio_priv(indio_dev); >> + >> + return max30100_set_powermode(data, false); >> +} >> + >> +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { >> + .postenable = max30100_buffer_postenable, >> + .predisable = max30100_buffer_predisable, >> +}; >> + >> +static inline int max30100_fifo_is_empty(struct max30100_data *data) >> +{ >> + int ret, val; >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); >> + if (ret) >> + return ret; >> + >> + return !!(val >> 4); >> +} >> + >> +static irqreturn_t max30100_interrupt_handler(int irq, void *private) >> +{ >> + struct iio_dev *indio_dev = private; >> + struct max30100_data *data = iio_priv(indio_dev); >> + int ret; >> + >> + mutex_lock(&data->lock); >> + >> + while (max30100_fifo_is_empty(data) != 0) { >> + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, >> + &data->buffer, 4); >> + >> + if (ret) >> + goto err_read; >> + >> + iio_push_to_buffers(data->indio_dev, data->buffer); >> + } >> + >> +err_read: >> + mutex_unlock(&data->lock); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int max30100_chip_init(struct max30100_data *data) >> +{ >> + int ret; >> + >> + /* RED IR LED = 24mA, IR LED = 50mA */ >> + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, >> + (MAX30100_REG_LED_CONFIG_24MA << >> + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | >> + MAX30100_REG_LED_CONFIG_50MA); >> + if (ret) >> + return ret; >> + >> + /* enable hi-res SPO2 readings with 100 samples a second */ >> + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, >> + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | >> + MAX30100_REG_SPO2_CONFIG_100HZ | >> + MAX30100_REG_SPO2_CONFIG_1600US); >> + if (ret) >> + return ret; >> + >> + /* enable SPO2 + HR mode */ >> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >> + MAX30100_REG_MODE_CONFIG_MODE_MASK, >> + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | >> + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); >> + if (ret) >> + return ret; >> + >> + /* enable SPO2 + HR interrupts */ >> + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, >> + MAX30100_REG_INT_ENABLE_MASK, >> + (MAX30100_REG_INT_ENABLE_HR_EN | >> + MAX30100_REG_INT_ENABLE_SPO2_EN) >> + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); >> +} >> + >> +static int max30100_read_temp(struct max30100_data *data, int *val) >> +{ >> + int ret; >> + int reg; >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); >> + if (ret < 0) >> + return ret; >> + *val = reg << 4; > > note that this is a signed value; I think negative temperatures are not > processed correctly > >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); >> + if (ret < 0) >> + return ret; >> + *val |= reg & 0xf; >> + >> + return 0; >> +} >> + >> +static int max30100_get_temp(struct max30100_data *data, int *val) >> +{ >> + int ret; >> + int tries = 3; >> + >> + /* start acquisition */ >> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >> + MAX30100_REG_MODE_CONFIG_TEMP_EN, >> + MAX30100_REG_MODE_CONFIG_TEMP_EN); >> + if (ret) >> + return ret; >> + >> + while (tries--) { >> + int reg; >> + >> + usleep_range(29000, 40000); >> + >> + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); >> + if (ret < 0) >> + break; >> + >> + /* bit reset to zero when reading is ready */ >> + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { >> + ret = max30100_read_temp(data, val); >> + break; >> + } >> + ret = -EIO; >> + } >> + >> + return ret; >> +} >> + >> +static int max30100_read_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int *val, int *val2, long mask) >> +{ >> + struct max30100_data *data = iio_priv(indio_dev); >> + int ret = -EINVAL; >> + >> + mutex_lock(&indio_dev->mlock); >> + >> + /* temperature reading can only be acquired while engine is running */ >> + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { >> + ret = -EAGAIN; >> + goto error_again; >> + } >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_RAW: >> + ret = max30100_get_temp(data, val); >> + if (!ret) > > what if ret < 0? > _get_temp() checks for ret < 0 If ret is other than 0 (greater or lless) it will return the error code from *get_temp. Wouldn't this be the expected way to do this? > > INTENSITY channels cannot be raw_read? > >> + ret = IIO_VAL_INT; >> + break; >> + case IIO_CHAN_INFO_SCALE: >> + *val = 1; /* 0.0625 */ >> + *val2 = 16; >> + ret = IIO_VAL_FRACTIONAL; >> + break; >> + } >> + >> +error_again: >> + mutex_unlock(&indio_dev->mlock); >> + >> + return ret; >> +} >> + >> +static const struct iio_info max30100_info = { >> + .driver_module = THIS_MODULE, >> + .read_raw = max30100_read_raw, >> +}; >> + >> +static int max30100_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct max30100_data *data; >> + struct iio_buffer *buffer; >> + struct iio_dev *indio_dev; >> + int ret; >> + >> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >> + if (!indio_dev) >> + return -ENOMEM; >> + >> + buffer = devm_iio_kfifo_allocate(&client->dev); >> + if (!buffer) >> + return -ENOMEM; >> + >> + iio_device_attach_buffer(indio_dev, buffer); >> + >> + indio_dev->name = MAX30100_DRV_NAME; >> + indio_dev->channels = max30100_channels; >> + indio_dev->info = &max30100_info; >> + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); >> + indio_dev->available_scan_masks = max30100_scan_masks; >> + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); >> + indio_dev->setup_ops = &max30100_buffer_setup_ops; >> + >> + data = iio_priv(indio_dev); >> + data->indio_dev = indio_dev; >> + >> + mutex_init(&data->lock); >> + i2c_set_clientdata(client, indio_dev); >> + >> + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); >> + if (IS_ERR(data->regmap)) { >> + dev_err(&client->dev, "regmap initialization failed.\n"); >> + return PTR_ERR(data->regmap); >> + } >> + max30100_set_powermode(data, false); >> + >> + ret = max30100_chip_init(data); >> + if (ret) >> + goto error_out; >> + >> + if (client->irq <= 0) { >> + dev_err(&client->dev, "no valid irq defined\n"); >> + ret = -EINVAL; >> + goto error_out; >> + } >> + ret = devm_request_threaded_irq(&client->dev, client->irq, >> + NULL, max30100_interrupt_handler, >> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >> + "max30100_irq", indio_dev); >> + if (ret) { >> + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); >> + goto error_out; >> + } >> + >> + return iio_device_register(indio_dev); > > newline here > >> +error_out: > > drop newline > >> + >> + return ret; >> +} >> + >> +static int max30100_remove(struct i2c_client *client) >> +{ >> + struct iio_dev *indio_dev = i2c_get_clientdata(client); >> + struct max30100_data *data = iio_priv(indio_dev); >> + >> + iio_device_unregister(indio_dev); >> + max30100_set_powermode(data, false); >> + >> + return 0; >> +} >> + >> +static const struct i2c_device_id max30100_id[] = { >> + { "max30100", 0 }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(i2c, max30100_id); >> + >> +static const struct of_device_id max30100_dt_ids[] = { >> + { .compatible = "maxim,max30100" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, max30100_dt_ids); >> + >> +static struct i2c_driver max30100_driver = { >> + .driver = { >> + .name = MAX30100_DRV_NAME, >> + .of_match_table = of_match_ptr(max30100_dt_ids), >> + }, >> + .probe = max30100_probe, >> + .remove = max30100_remove, >> + .id_table = max30100_id, >> +}; >> +module_i2c_driver(max30100_driver); >> + >> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>"); >> +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); >> +MODULE_LICENSE("GPL"); >> > > -- > > Peter Meerwald-Stadler > +43-664-2444418 (mobile) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-02 18:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-18 9:19 [RFC v2 0/1] iio: light: add MAX30100 oximeter driver support Matt Ranostay 2015-11-18 9:19 ` [RFC v2 1/1] " Matt Ranostay 2015-11-20 16:40 ` Peter Meerwald-Stadler 2015-11-21 16:26 ` Jonathan Cameron 2015-11-22 12:07 ` Matt Ranostay 2015-11-22 12:36 ` Jonathan Cameron 2015-12-02 18:36 ` Matt Ranostay
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox