* Re: [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor
[not found] ` <1453742941-12086-5-git-send-email-afd@ti.com>
@ 2016-01-25 20:21 ` Peter Meerwald-Stadler
2016-01-25 20:36 ` Andrew F. Davis
2016-01-30 15:10 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Peter Meerwald-Stadler @ 2016-01-25 20:21 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Dan Murphy,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
linux-iio, linux-api, devicetree, linux-kernel
> Add driver for the TI AFE4403 heart rate monitor and pulse oximeter.
> This device detects reflected LED light fluctuations and presents an ADC
> value to the user space for further signal processing.
some comments below
> Data sheet located here:
> http://www.ti.com/product/AFE4403/datasheet
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
> drivers/iio/health/Kconfig | 12 +
> drivers/iio/health/Makefile | 1 +
> drivers/iio/health/afe4403.c | 698 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 711 insertions(+)
> create mode 100644 drivers/iio/health/afe4403.c
>
> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig
> index 632a14b..f0c1977 100644
> --- a/drivers/iio/health/Kconfig
> +++ b/drivers/iio/health/Kconfig
> @@ -7,6 +7,18 @@ menu "Health Sensors"
>
> menu "Heart Rate Monitors"
>
> +config AFE4403
> + tristate "TI AFE4403 Heart Rate Monitor"
> + depends on SPI_MASTER
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes to choose the Texas Instruments AFE4403
> + heart rate monitor and low-cost pulse oximeter.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called afe4403.
> +
> config AFE4404
> tristate "TI AFE4404 heart rate and pulse oximeter sensor"
> depends on I2C
> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile
> index b37c0d5..9955a2a 100644
> --- a/drivers/iio/health/Makefile
> +++ b/drivers/iio/health/Makefile
> @@ -4,5 +4,6 @@
>
> # When adding new entries keep the list in alphabetical order
>
> +obj-$(CONFIG_AFE4403) += afe4403.o
> obj-$(CONFIG_AFE4404) += afe4404.o
> obj-$(CONFIG_MAX30100) += max30100.o
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> new file mode 100644
> index 0000000..2c8565f
> --- /dev/null
> +++ b/drivers/iio/health/afe4403.c
> @@ -0,0 +1,698 @@
> +/*
> + * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#include "afe440x.h"
> +
> +#define AFE4403_DRIVER_NAME "afe4403"
> +
> +/* AFE4403 Registers */
> +#define AFE4403_TIAGAIN 0x20
> +#define AFE4403_TIA_AMB_GAIN 0x21
> +
> +/* AFE4403 GAIN register fields */
> +#define AFE4403_TIAGAIN_RES_MASK GENMASK(2, 0)
> +#define AFE4403_TIAGAIN_RES_SHIFT 0
> +#define AFE4403_TIAGAIN_CAP_MASK GENMASK(7, 3)
> +#define AFE4403_TIAGAIN_CAP_SHIFT 3
> +
> +/* AFE4403 LEDCNTRL register fields */
> +#define AFE440X_LEDCNTRL_LED1_MASK GENMASK(15, 8)
if these defines are generic for AFE440X, they should go to the header
file; if not, they should be named AFE4403
> +#define AFE440X_LEDCNTRL_LED1_SHIFT 8
> +#define AFE440X_LEDCNTRL_LED2_MASK GENMASK(7, 0)
> +#define AFE440X_LEDCNTRL_LED2_SHIFT 0
> +#define AFE440X_LEDCNTRL_LED_RANGE_MASK GENMASK(17, 16)
> +#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT 16
> +
> +/* AFE4403 CONTROL2 register fields */
> +#define AFE440X_CONTROL2_PWR_DWN_TX BIT(2)
> +#define AFE440X_CONTROL2_EN_SLOW_DIAG BIT(8)
> +#define AFE440X_CONTROL2_DIAG_OUT_TRI BIT(10)
> +#define AFE440X_CONTROL2_TX_BRDG_MOD BIT(11)
> +#define AFE440X_CONTROL2_TX_REF_MASK GENMASK(18, 17)
> +#define AFE440X_CONTROL2_TX_REF_SHIFT 17
> +
> +/* AFE4404 NULL fields */
> +#define NULL_MASK 0
> +#define NULL_SHIFT 0
needed? prefix?
> +
> +/* AFE4403 LEDCNTRL values */
> +#define AFE440X_LEDCNTRL_RANGE_TX_HALF 0x1
> +#define AFE440X_LEDCNTRL_RANGE_TX_FULL 0x2
> +#define AFE440X_LEDCNTRL_RANGE_TX_OFF 0x3
> +
> +/* AFE4403 CONTROL2 values */
> +#define AFE440X_CONTROL2_TX_REF_025 0x0
> +#define AFE440X_CONTROL2_TX_REF_050 0x1
> +#define AFE440X_CONTROL2_TX_REF_100 0x2
> +#define AFE440X_CONTROL2_TX_REF_075 0x3
> +
> +/* AFE4403 CONTROL3 values */
> +#define AFE440X_CONTROL3_CLK_DIV_2 0x0
AFE440X (then move to .h) or AFE4403?
> +#define AFE440X_CONTROL3_CLK_DIV_4 0x2
> +#define AFE440X_CONTROL3_CLK_DIV_6 0x3
> +#define AFE440X_CONTROL3_CLK_DIV_8 0x4
> +#define AFE440X_CONTROL3_CLK_DIV_12 0x5
> +#define AFE440X_CONTROL3_CLK_DIV_1 0x7
> +
> +/* AFE4403 TIAGAIN_CAP values */
> +#define AFE4403_TIAGAIN_CAP_5_P 0x0
> +#define AFE4403_TIAGAIN_CAP_10_P 0x1
> +#define AFE4403_TIAGAIN_CAP_20_P 0x2
> +#define AFE4403_TIAGAIN_CAP_30_P 0x3
> +#define AFE4403_TIAGAIN_CAP_55_P 0x8
> +#define AFE4403_TIAGAIN_CAP_155_P 0x10
> +
> +/* AFE4403 TIAGAIN_RES values */
> +#define AFE4403_TIAGAIN_RES_500_K 0x0
> +#define AFE4403_TIAGAIN_RES_250_K 0x1
> +#define AFE4403_TIAGAIN_RES_100_K 0x2
> +#define AFE4403_TIAGAIN_RES_50_K 0x3
> +#define AFE4403_TIAGAIN_RES_25_K 0x4
> +#define AFE4403_TIAGAIN_RES_10_K 0x5
> +#define AFE4403_TIAGAIN_RES_1_M 0x6
> +#define AFE4403_TIAGAIN_RES_NONE 0x7
> +
> +/**
> + * struct afe4403_data
> + * @dev - Device structure
> + * @spi - SPI device handle
> + * @regmap - Register map of the device
> + * @regulator - Pointer to the regulator for the IC
> + * @trig - IIO trigger for this device
> + * @irq - ADC_RDY line interrupt number
> + */
> +struct afe4403_data {
> + struct device *dev;
> + struct spi_device *spi;
> + struct regmap *regmap;
> + struct regulator *regulator;
> + struct iio_trigger *trig;
> + int irq;
> +};
> +
> +enum afe4403_chan_id {
> + LED1,
> + ALED1,
> + LED2,
> + ALED2,
> + LED1_ALED1,
> + LED2_ALED2,
> + ILED1,
> + ILED2,
> +};
> +
> +static const struct afe440x_reg_info afe4403_reg_info[] = {
> + AFE440X_REG_INFO(LED1, AFE440X_LED1VAL, 0, NULL),
> + AFE440X_REG_INFO(ALED1, AFE440X_ALED1VAL, 0, NULL),
> + AFE440X_REG_INFO(LED2, AFE440X_LED2VAL, 0, NULL),
> + AFE440X_REG_INFO(ALED2, AFE440X_ALED2VAL, 0, NULL),
> + AFE440X_REG_INFO(LED1_ALED1, AFE440X_LED1_ALED1VAL, 0, NULL),
> + AFE440X_REG_INFO(LED2_ALED2, AFE440X_LED2_ALED2VAL, 0, NULL),
> + AFE440X_REG_INFO(ILED1, AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
> + AFE440X_REG_INFO(ILED2, AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
> +};
> +
> +static const struct iio_chan_spec afe4403_channels[] = {
> + /* ADC values */
> + AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
> + AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
> + AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> + /* LED current */
> + AFE440X_CURRENT_CHAN(ILED1, "led1"),
> + AFE440X_CURRENT_CHAN(ILED2, "led2"),
> +};
> +
> +static const struct afe440x_val_table afe4403_res_table[] = {
> + { 500000 }, { 250000 }, { 100000 }, { 50000 },
> + { 25000 }, { 10000 }, { 1000000 }, { 0 },
> +};
> +AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table);
> +
> +static const struct afe440x_val_table afe4403_cap_table[] = {
> + { 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 },
> + { 0, 30000 }, { 0, 35000 }, { 0, 45000 }, { 0, 50000 },
> + { 0, 55000 }, { 0, 60000 }, { 0, 70000 }, { 0, 75000 },
> + { 0, 80000 }, { 0, 85000 }, { 0, 95000 }, { 0, 100000 },
> + { 0, 155000 }, { 0, 160000 }, { 0, 170000 }, { 0, 175000 },
> + { 0, 180000 }, { 0, 185000 }, { 0, 195000 }, { 0, 200000 },
> + { 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 },
> + { 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 },
> +};
> +AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table);
> +
> +static ssize_t afe440x_show_register(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
common code with afe4404?
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> + unsigned int reg_val, type;
> + int vals[2];
> + int ret, val_len;
> +
> + ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val);
> + if (ret)
> + return ret;
> +
> + reg_val &= afe440x_attr->mask;
> + reg_val >>= afe440x_attr->shift;
> +
> + switch (afe440x_attr->type) {
> + case SIMPLE:
> + type = IIO_VAL_INT;
> + val_len = 1;
> + vals[0] = reg_val;
> + break;
> + case RESISTANCE:
> + case CAPACITANCE:
> + type = IIO_VAL_INT_PLUS_MICRO;
> + val_len = 2;
> + if (reg_val < afe440x_attr->table_size) {
> + vals[0] = afe440x_attr->val_table[reg_val].integer;
> + vals[1] = afe440x_attr->val_table[reg_val].fract;
> + break;
> + }
> + return -EINVAL;
> + default:
> + return -EINVAL;
> + }
> +
> + return iio_format_value(buf, type, val_len, vals);
> +}
> +
> +static ssize_t afe440x_store_register(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> + int val, integer, fract, ret;
> +
> + ret = iio_str_to_fixpoint(buf, 100000, &integer, &fract);
> + if (ret)
> + return ret;
> +
> + switch (afe440x_attr->type) {
> + case SIMPLE:
> + val = integer;
> + break;
> + case RESISTANCE:
> + case CAPACITANCE:
> + for (val = 0; val < afe440x_attr->table_size; val++)
> + if (afe440x_attr->val_table[val].integer == integer &&
> + afe440x_attr->val_table[val].fract == fract)
> + break;
> + if (val == afe440x_attr->table_size)
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
> + afe440x_attr->mask,
> + (val << afe440x_attr->shift));
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
> +
> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
> +
> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
> +
> +static struct attribute *afe440x_attributes[] = {
> + &afe440x_attr_tia_separate_en.dev_attr.attr,
> + &afe440x_attr_tia_resistance1.dev_attr.attr,
> + &afe440x_attr_tia_capacitance1.dev_attr.attr,
> + &afe440x_attr_tia_resistance2.dev_attr.attr,
> + &afe440x_attr_tia_capacitance2.dev_attr.attr,
> + &dev_attr_tia_resistance_available.attr,
> + &dev_attr_tia_capacitance_available.attr,
> + NULL
> +};
> +
> +static const struct attribute_group afe440x_attribute_group = {
> + .attrs = afe440x_attributes
> +};
> +
> +static int afe4403_read(struct afe4403_data *afe, unsigned int reg, u32 *val)
> +{
> + u8 tx[4] ____cacheline_aligned = {AFE440X_CONTROL0, 0x0, 0x0,
> + AFE440X_CONTROL0_READ};
> + u8 rx[3];
> + int ret;
> +
> + /* Enable reading from the device */
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto out;
> +
> + ret = spi_write_then_read(afe->spi, ®, 1, rx, 3);
> + if (ret)
> + goto out;
> +
> + *val = (rx[0] << 16) |
> + (rx[1] << 8) |
> + (rx[2]);
> +
> + /* Disable reading from the device */
> + tx[3] = AFE440X_CONTROL0_WRITE;
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto out;
> +
> +out:
> +
> + return ret;
> +}
> +
> +static int afe4403_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
> + int ret;
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = afe4403_read(afe, reg_info.reg, val);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OFFSET:
> + ret = regmap_read(afe->regmap, reg_info.offreg,
> + val);
> + if (ret)
> + return ret;
> + *val &= reg_info.mask;
> + *val >>= reg_info.shift;
> + return IIO_VAL_INT;
> + }
> + break;
> + case IIO_CURRENT:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = regmap_read(afe->regmap, reg_info.reg, val);
> + if (ret)
> + return ret;
> + *val &= reg_info.mask;
> + *val >>= reg_info.shift;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = 0;
> + *val2 = 800000;
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int afe4403_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + switch (mask) {
> + case IIO_CHAN_INFO_OFFSET:
> + return regmap_update_bits(afe->regmap,
> + reg_info.offreg,
> + reg_info.mask,
> + (val << reg_info.shift));
> + }
> + break;
> + case IIO_CURRENT:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return regmap_update_bits(afe->regmap,
> + reg_info.reg,
> + reg_info.mask,
> + (val << reg_info.shift));
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_info afe4403_iio_info = {
> + .attrs = &afe440x_attribute_group,
> + .read_raw = afe4403_read_raw,
> + .write_raw = afe4403_write_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static irqreturn_t afe4403_trigger_handler(int irq, void *private)
> +{
> + struct iio_poll_func *pf = private;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret, bit, i = 0;
> + s32 buffer[8];
> + u8 tx[4] ____cacheline_aligned = {AFE440X_CONTROL0, 0x0, 0x0,
> + AFE440X_CONTROL0_READ};
> + u8 rx[3];
> +
> + /* Enable reading from the device */
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto err;
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + ret = spi_write_then_read(afe->spi,
> + &afe4403_reg_info[bit].reg, 1,
> + rx, 3);
> + if (ret)
> + goto err;
> +
> + buffer[i++] = (rx[0] << 16) |
> + (rx[1] << 8) |
> + (rx[2]);
> + }
> +
> + /* Disable reading from the device */
> + tx[3] = AFE440X_CONTROL0_WRITE;
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto err;
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buffer, pf->timestamp);
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct iio_trigger_ops afe4403_trigger_ops = {
> + .owner = THIS_MODULE,
> +};
> +
> +#define AFE4403_TIMING_PAIRS \
> + { AFE440X_LED2STC, 0x000050 }, \
> + { AFE440X_LED2ENDC, 0x0003e7 }, \
> + { AFE440X_LED1LEDSTC, 0x0007d0 }, \
> + { AFE440X_LED1LEDENDC, 0x000bb7 }, \
> + { AFE440X_ALED2STC, 0x000438 }, \
> + { AFE440X_ALED2ENDC, 0x0007cf }, \
> + { AFE440X_LED1STC, 0x000820 }, \
> + { AFE440X_LED1ENDC, 0x000bb7 }, \
> + { AFE440X_LED2LEDSTC, 0x000000 }, \
> + { AFE440X_LED2LEDENDC, 0x0003e7 }, \
> + { AFE440X_ALED1STC, 0x000c08 }, \
> + { AFE440X_ALED1ENDC, 0x000f9f }, \
> + { AFE440X_LED2CONVST, 0x0003ef }, \
> + { AFE440X_LED2CONVEND, 0x0007cf }, \
> + { AFE440X_ALED2CONVST, 0x0007d7 }, \
> + { AFE440X_ALED2CONVEND, 0x000bb7 }, \
> + { AFE440X_LED1CONVST, 0x000bbf }, \
> + { AFE440X_LED1CONVEND, 0x009c3f }, \
> + { AFE440X_ALED1CONVST, 0x000fa7 }, \
> + { AFE440X_ALED1CONVEND, 0x001387 }, \
> + { AFE440X_ADCRSTSTCT0, 0x0003e8 }, \
> + { AFE440X_ADCRSTENDCT0, 0x0003eb }, \
> + { AFE440X_ADCRSTSTCT1, 0x0007d0 }, \
> + { AFE440X_ADCRSTENDCT1, 0x0007d3 }, \
> + { AFE440X_ADCRSTSTCT2, 0x000bb8 }, \
> + { AFE440X_ADCRSTENDCT2, 0x000bbb }, \
> + { AFE440X_ADCRSTSTCT3, 0x000fa0 }, \
> + { AFE440X_ADCRSTENDCT3, 0x000fa3 }, \
> + { AFE440X_PRPCOUNT, 0x009c3f }, \
> + { AFE440X_PDNCYCLESTC, 0x001518 }, \
> + { AFE440X_PDNCYCLEENDC, 0x00991f }
> +
> +static const struct reg_sequence afe4403_reg_sequences[] = {
> + AFE4403_TIMING_PAIRS,
> + { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN | 0x000007},
> + { AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES_1_M },
> + { AFE440X_LEDCNTRL, (0x14 << AFE440X_LEDCNTRL_LED1_SHIFT) |
> + (0x14 << AFE440X_LEDCNTRL_LED2_SHIFT) },
> + { AFE440X_CONTROL2, AFE440X_CONTROL2_TX_REF_050 <<
> + AFE440X_CONTROL2_TX_REF_SHIFT },
> +};
> +
> +static const struct regmap_range afe4403_yes_ranges[] = {
> + regmap_reg_range(AFE440X_LED2VAL, AFE440X_LED1_ALED1VAL),
> +};
> +
> +static const struct regmap_access_table afe4403_volatile_table = {
> + .yes_ranges = afe4403_yes_ranges,
> + .n_yes_ranges = ARRAY_SIZE(afe4403_yes_ranges),
> +};
> +
> +static const struct regmap_config afe4403_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 24,
> +
> + .max_register = AFE440X_PDNCYCLEENDC,
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_table = &afe4403_volatile_table,
> +};
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id afe4403_of_match[] = {
> + { .compatible = "ti,afe4403", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, afe4403_of_match);
> +#endif
> +
> +static int afe4403_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_update_bits(afe->regmap, AFE440X_CONTROL2,
> + AFE440X_CONTROL2_PDN_AFE,
> + AFE440X_CONTROL2_PDN_AFE);
> + if (ret)
> + return ret;
> +
> + ret = regulator_disable(afe->regulator);
> + if (ret) {
> + dev_err(dev, "Unable to disable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int afe4403_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regulator_enable(afe->regulator);
> + if (ret) {
> + dev_err(dev, "Unable to enable regulator\n");
> + return ret;
> + }
> +
> + ret = regmap_update_bits(afe->regmap, AFE440X_CONTROL2,
> + AFE440X_CONTROL2_PDN_AFE, 0);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(afe4403_pm_ops, afe4403_suspend, afe4403_resume);
> +
> +static int afe4403_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct afe4403_data *afe;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + afe = iio_priv(indio_dev);
> + spi_set_drvdata(spi, indio_dev);
> +
> + afe->dev = &spi->dev;
> + afe->spi = spi;
> + afe->irq = spi->irq;
> +
> + afe->regmap = devm_regmap_init_spi(spi, &afe4403_regmap_config);
> + if (IS_ERR(afe->regmap)) {
> + dev_err(afe->dev, "Unable to allocate register map\n");
> + return PTR_ERR(afe->regmap);
> + }
> +
> + afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
> + if (IS_ERR(afe->regulator)) {
> + dev_err(afe->dev, "Unable to get regulator\n");
> + return PTR_ERR(afe->regulator);
> + }
> + ret = regulator_enable(afe->regulator);
> + if (ret) {
> + dev_err(afe->dev, "Unable to enable regulator\n");
> + return ret;
> + }
> +
> + ret = regmap_write(afe->regmap, AFE440X_CONTROL0,
> + AFE440X_CONTROL0_SW_RESET);
> + if (ret) {
> + dev_err(afe->dev, "Unable to reset device\n");
> + return ret;
> + }
> +
> + ret = regmap_multi_reg_write(afe->regmap, afe4403_reg_sequences,
> + ARRAY_SIZE(afe4403_reg_sequences));
> + if (ret) {
> + dev_err(afe->dev, "Unable to set register defaults\n");
> + return ret;
> + }
> +
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->dev.parent = afe->dev;
> + indio_dev->channels = afe4403_channels;
> + indio_dev->num_channels = ARRAY_SIZE(afe4403_channels);
> + indio_dev->name = AFE4403_DRIVER_NAME;
> + indio_dev->info = &afe4403_iio_info;
> +
> + if (afe->irq > 0) {
> + afe->trig = devm_iio_trigger_alloc(afe->dev,
> + "%s-dev%d",
> + indio_dev->name,
> + indio_dev->id);
> + if (!afe->trig) {
> + dev_err(afe->dev, "Unable to allocate IIO trigger\n");
> + return -ENOMEM;
> + }
> +
> + iio_trigger_set_drvdata(afe->trig, indio_dev);
> +
> + afe->trig->ops = &afe4403_trigger_ops;
> + afe->trig->dev.parent = afe->dev;
> +
> + ret = iio_trigger_register(afe->trig);
> + if (ret) {
> + dev_err(afe->dev, "Unable to register IIO trigger\n");
> + return ret;
> + }
> +
> + ret = devm_request_threaded_irq(afe->dev, afe->irq,
> + iio_trigger_generic_data_rdy_poll,
> + NULL, IRQF_ONESHOT,
> + AFE4403_DRIVER_NAME,
> + afe->trig);
> + if (ret) {
> + dev_err(afe->dev, "Unable to request IRQ\n");
> + return ret;
> + }
> + }
> +
> + ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
> + afe4403_trigger_handler, NULL);
> + if (ret) {
> + dev_err(afe->dev, "Unable to setup buffer\n");
> + return ret;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(afe->dev, "Unable to register IIO device\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int afe4403_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + iio_device_unregister(indio_dev);
> +
> + iio_trigger_unregister(afe->trig);
> +
> + ret = regulator_disable(afe->regulator);
> + if (ret) {
> + dev_err(afe->dev, "Unable to disable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct spi_device_id afe4403_ids[] = {
> + { "afe4403", 0 },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, afe4403_ids);
> +
> +static struct spi_driver afe4403_spi_driver = {
> + .driver = {
> + .name = AFE4403_DRIVER_NAME,
> + .of_match_table = of_match_ptr(afe4403_of_match),
> + .pm = &afe4403_pm_ops,
> + },
> + .probe = afe4403_probe,
> + .remove = afe4403_remove,
> + .id_table = afe4403_ids,
> +};
> +module_spi_driver(afe4403_spi_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_DESCRIPTION("TI AFE4403 Heart Rate and Pulse Oximeter");
> +MODULE_LICENSE("GPL v2");
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor
2016-01-25 20:21 ` [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor Peter Meerwald-Stadler
@ 2016-01-25 20:36 ` Andrew F. Davis
0 siblings, 0 replies; 6+ messages in thread
From: Andrew F. Davis @ 2016-01-25 20:36 UTC (permalink / raw)
To: Peter Meerwald-Stadler
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Dan Murphy,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
linux-iio, linux-api, devicetree, linux-kernel
On 01/25/2016 02:21 PM, Peter Meerwald-Stadler wrote:
>
>> Add driver for the TI AFE4403 heart rate monitor and pulse oximeter.
>> This device detects reflected LED light fluctuations and presents an ADC
>> value to the user space for further signal processing.
>
> some comments below
>
>> Data sheet located here:
>> http://www.ti.com/product/AFE4403/datasheet
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> ---
>> drivers/iio/health/Kconfig | 12 +
>> drivers/iio/health/Makefile | 1 +
>> drivers/iio/health/afe4403.c | 698 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 711 insertions(+)
>> create mode 100644 drivers/iio/health/afe4403.c
>>
>> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig
>> index 632a14b..f0c1977 100644
>> --- a/drivers/iio/health/Kconfig
>> +++ b/drivers/iio/health/Kconfig
>> @@ -7,6 +7,18 @@ menu "Health Sensors"
>>
>> menu "Heart Rate Monitors"
>>
>> +config AFE4403
>> + tristate "TI AFE4403 Heart Rate Monitor"
>> + depends on SPI_MASTER
>> + select IIO_BUFFER
>> + select IIO_TRIGGERED_BUFFER
>> + help
>> + Say yes to choose the Texas Instruments AFE4403
>> + heart rate monitor and low-cost pulse oximeter.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called afe4403.
>> +
>> config AFE4404
>> tristate "TI AFE4404 heart rate and pulse oximeter sensor"
>> depends on I2C
>> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile
>> index b37c0d5..9955a2a 100644
>> --- a/drivers/iio/health/Makefile
>> +++ b/drivers/iio/health/Makefile
>> @@ -4,5 +4,6 @@
>>
>> # When adding new entries keep the list in alphabetical order
>>
>> +obj-$(CONFIG_AFE4403) += afe4403.o
>> obj-$(CONFIG_AFE4404) += afe4404.o
>> obj-$(CONFIG_MAX30100) += max30100.o
>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>> new file mode 100644
>> index 0000000..2c8565f
>> --- /dev/null
>> +++ b/drivers/iio/health/afe4403.c
>> @@ -0,0 +1,698 @@
>> +/*
>> + * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters
>> + *
>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
>> + * Andrew F. Davis <afd@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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/device.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +
>> +#include "afe440x.h"
>> +
>> +#define AFE4403_DRIVER_NAME "afe4403"
>> +
>> +/* AFE4403 Registers */
>> +#define AFE4403_TIAGAIN 0x20
>> +#define AFE4403_TIA_AMB_GAIN 0x21
>> +
>> +/* AFE4403 GAIN register fields */
>> +#define AFE4403_TIAGAIN_RES_MASK GENMASK(2, 0)
>> +#define AFE4403_TIAGAIN_RES_SHIFT 0
>> +#define AFE4403_TIAGAIN_CAP_MASK GENMASK(7, 3)
>> +#define AFE4403_TIAGAIN_CAP_SHIFT 3
>> +
>> +/* AFE4403 LEDCNTRL register fields */
>> +#define AFE440X_LEDCNTRL_LED1_MASK GENMASK(15, 8)
>
> if these defines are generic for AFE440X, they should go to the header
> file; if not, they should be named AFE4403
>
The register "AFE440X_LEDCNTRL" is common, the field "LED1" in that
register is not, only common field definitions are in the common
header.
>> +#define AFE440X_LEDCNTRL_LED1_SHIFT 8
>> +#define AFE440X_LEDCNTRL_LED2_MASK GENMASK(7, 0)
>> +#define AFE440X_LEDCNTRL_LED2_SHIFT 0
>> +#define AFE440X_LEDCNTRL_LED_RANGE_MASK GENMASK(17, 16)
>> +#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT 16
>> +
>> +/* AFE4403 CONTROL2 register fields */
>> +#define AFE440X_CONTROL2_PWR_DWN_TX BIT(2)
>> +#define AFE440X_CONTROL2_EN_SLOW_DIAG BIT(8)
>> +#define AFE440X_CONTROL2_DIAG_OUT_TRI BIT(10)
>> +#define AFE440X_CONTROL2_TX_BRDG_MOD BIT(11)
>> +#define AFE440X_CONTROL2_TX_REF_MASK GENMASK(18, 17)
>> +#define AFE440X_CONTROL2_TX_REF_SHIFT 17
>> +
>> +/* AFE4404 NULL fields */
>> +#define NULL_MASK 0
>> +#define NULL_SHIFT 0
>
> needed? prefix?
>
Same as other patch.
>> +
>> +/* AFE4403 LEDCNTRL values */
>> +#define AFE440X_LEDCNTRL_RANGE_TX_HALF 0x1
>> +#define AFE440X_LEDCNTRL_RANGE_TX_FULL 0x2
>> +#define AFE440X_LEDCNTRL_RANGE_TX_OFF 0x3
>> +
>> +/* AFE4403 CONTROL2 values */
>> +#define AFE440X_CONTROL2_TX_REF_025 0x0
>> +#define AFE440X_CONTROL2_TX_REF_050 0x1
>> +#define AFE440X_CONTROL2_TX_REF_100 0x2
>> +#define AFE440X_CONTROL2_TX_REF_075 0x3
>> +
>> +/* AFE4403 CONTROL3 values */
>> +#define AFE440X_CONTROL3_CLK_DIV_2 0x0
>
> AFE440X (then move to .h) or AFE4403?
>
Same as above.
>> +#define AFE440X_CONTROL3_CLK_DIV_4 0x2
>> +#define AFE440X_CONTROL3_CLK_DIV_6 0x3
>> +#define AFE440X_CONTROL3_CLK_DIV_8 0x4
>> +#define AFE440X_CONTROL3_CLK_DIV_12 0x5
>> +#define AFE440X_CONTROL3_CLK_DIV_1 0x7
>> +
>> +/* AFE4403 TIAGAIN_CAP values */
>> +#define AFE4403_TIAGAIN_CAP_5_P 0x0
>> +#define AFE4403_TIAGAIN_CAP_10_P 0x1
>> +#define AFE4403_TIAGAIN_CAP_20_P 0x2
>> +#define AFE4403_TIAGAIN_CAP_30_P 0x3
>> +#define AFE4403_TIAGAIN_CAP_55_P 0x8
>> +#define AFE4403_TIAGAIN_CAP_155_P 0x10
>> +
>> +/* AFE4403 TIAGAIN_RES values */
>> +#define AFE4403_TIAGAIN_RES_500_K 0x0
>> +#define AFE4403_TIAGAIN_RES_250_K 0x1
>> +#define AFE4403_TIAGAIN_RES_100_K 0x2
>> +#define AFE4403_TIAGAIN_RES_50_K 0x3
>> +#define AFE4403_TIAGAIN_RES_25_K 0x4
>> +#define AFE4403_TIAGAIN_RES_10_K 0x5
>> +#define AFE4403_TIAGAIN_RES_1_M 0x6
>> +#define AFE4403_TIAGAIN_RES_NONE 0x7
>> +
>> +/**
>> + * struct afe4403_data
>> + * @dev - Device structure
>> + * @spi - SPI device handle
>> + * @regmap - Register map of the device
>> + * @regulator - Pointer to the regulator for the IC
>> + * @trig - IIO trigger for this device
>> + * @irq - ADC_RDY line interrupt number
>> + */
>> +struct afe4403_data {
>> + struct device *dev;
>> + struct spi_device *spi;
>> + struct regmap *regmap;
>> + struct regulator *regulator;
>> + struct iio_trigger *trig;
>> + int irq;
>> +};
>> +
>> +enum afe4403_chan_id {
>> + LED1,
>> + ALED1,
>> + LED2,
>> + ALED2,
>> + LED1_ALED1,
>> + LED2_ALED2,
>> + ILED1,
>> + ILED2,
>> +};
>> +
>> +static const struct afe440x_reg_info afe4403_reg_info[] = {
>> + AFE440X_REG_INFO(LED1, AFE440X_LED1VAL, 0, NULL),
>> + AFE440X_REG_INFO(ALED1, AFE440X_ALED1VAL, 0, NULL),
>> + AFE440X_REG_INFO(LED2, AFE440X_LED2VAL, 0, NULL),
>> + AFE440X_REG_INFO(ALED2, AFE440X_ALED2VAL, 0, NULL),
>> + AFE440X_REG_INFO(LED1_ALED1, AFE440X_LED1_ALED1VAL, 0, NULL),
>> + AFE440X_REG_INFO(LED2_ALED2, AFE440X_LED2_ALED2VAL, 0, NULL),
>> + AFE440X_REG_INFO(ILED1, AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
>> + AFE440X_REG_INFO(ILED2, AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
>> +};
>> +
>> +static const struct iio_chan_spec afe4403_channels[] = {
>> + /* ADC values */
>> + AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
>> + AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
>> + AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
>> + AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
>> + AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
>> + AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
>> + /* LED current */
>> + AFE440X_CURRENT_CHAN(ILED1, "led1"),
>> + AFE440X_CURRENT_CHAN(ILED2, "led2"),
>> +};
>> +
>> +static const struct afe440x_val_table afe4403_res_table[] = {
>> + { 500000 }, { 250000 }, { 100000 }, { 50000 },
>> + { 25000 }, { 10000 }, { 1000000 }, { 0 },
>> +};
>> +AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table);
>> +
>> +static const struct afe440x_val_table afe4403_cap_table[] = {
>> + { 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 },
>> + { 0, 30000 }, { 0, 35000 }, { 0, 45000 }, { 0, 50000 },
>> + { 0, 55000 }, { 0, 60000 }, { 0, 70000 }, { 0, 75000 },
>> + { 0, 80000 }, { 0, 85000 }, { 0, 95000 }, { 0, 100000 },
>> + { 0, 155000 }, { 0, 160000 }, { 0, 170000 }, { 0, 175000 },
>> + { 0, 180000 }, { 0, 185000 }, { 0, 195000 }, { 0, 200000 },
>> + { 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 },
>> + { 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 },
>> +};
>> +AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table);
>> +
>> +static ssize_t afe440x_show_register(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>
> common code with afe4404?
>
Yes, all functions/structs/variables that are identical are given
the "afe440x" name, this way they can maybe be moved somewhere
common at some point and not require any renaming in the calling
functions.
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/4] iio: health: Add driver for the TI AFE4404 heart monitor
[not found] ` <1453742941-12086-3-git-send-email-afd@ti.com>
@ 2016-01-30 14:59 ` Jonathan Cameron
2016-02-02 15:50 ` Andrew F. Davis
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2016-01-30 14:59 UTC (permalink / raw)
To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Dan Murphy, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
Cc: linux-iio, linux-api, devicetree, linux-kernel
On 25/01/16 17:28, Andrew F. Davis wrote:
> Add driver for the TI AFE4404 heart rate monitor and pulse oximeter.
> This device detects reflected LED light fluctuations and presents an ADC
> value to the user space for further signal processing.
>
> Datasheet: http://www.ti.com/product/AFE4404/datasheet
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Hi Andrew,
This changed rather more than I was expecting, but fair enough.
A few bits are missing from remove. Few other little bits inline,
though mostly those are about readability rather than what the code
is doing.
I was a little suprised to see no control over whether the trigger is
enabled or not. I'm trying to get my head around what the result of this will
be. I guess we'll have interrupts pinging away merilly doing nothing until
the buffer is attached then the demux will get set up to actually do
something with the data.
hmm. Might be fine, if I ususual. Normally devices have some means of
masking the interrupt.
Jonathan
> ---
> .../ABI/testing/sysfs-bus-iio-health-afe440x | 54 ++
> drivers/iio/health/Kconfig | 19 +-
> drivers/iio/health/Makefile | 1 +
> drivers/iio/health/afe4404.c | 665 +++++++++++++++++++++
> drivers/iio/health/afe440x.h | 193 ++++++
> 5 files changed, 931 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> create mode 100644 drivers/iio/health/afe4404.c
> create mode 100644 drivers/iio/health/afe440x.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> new file mode 100644
> index 0000000..3740f25
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> @@ -0,0 +1,54 @@
> +What: /sys/bus/iio/devices/iio:deviceX/tia_resistanceY
> + /sys/bus/iio/devices/iio:deviceX/tia_capacitanceY
> +Date: December 2015
> +KernelVersion:
> +Contact: Andrew F. Davis <afd@ti.com>
> +Description:
> + Get and set the resistance and the capacitance settings for the
> + Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
> + Rf2 and Cf2 values.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/tia_separate_en
> +Date: December 2015
> +KernelVersion:
> +Contact: Andrew F. Davis <afd@ti.com>
> +Description:
> + Enable or disable separate settings for the TransImpedance
> + Amplifier above, when disabled both values are set by the
> + first channel.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
> + /sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
> +Date: December 2015
> +KernelVersion:
> +Contact: Andrew F. Davis <afd@ti.com>
> +Description:
> + Get measured values from the ADC for these stages. Y is the
> + specific LED number. The values are expressed in 24-bit twos
> + complement.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
> +Date: December 2015
> +KernelVersion:
> +Contact: Andrew F. Davis <afd@ti.com>
> +Description:
> + Get differential values from the ADC for these stages. Y is the
> + specific LED number. The values are expressed in 24-bit twos
> + complement for the specified LEDs.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
> + /sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
> +Date: December 2015
> +KernelVersion:
> +Contact: Andrew F. Davis <afd@ti.com>
> +Description:
> + Get and set the offset cancellation DAC setting for these
> + stages. The values are expressed in 5-bit sign-magnitude.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
> +Date: December 2015
> +KernelVersion:
> +Contact: Andrew F. Davis <afd@ti.com>
> +Description:
> + Get and set the LED current for the specified LED. Y is the
> + specific LED number.
> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig
> index a647679..632a14b 100644
> --- a/drivers/iio/health/Kconfig
> +++ b/drivers/iio/health/Kconfig
> @@ -3,7 +3,22 @@
> #
> # When adding new entries keep the list in alphabetical order
>
> -menu "Health sensors"
> +menu "Health Sensors"
> +
> +menu "Heart Rate Monitors"
> +
> +config AFE4404
> + tristate "TI AFE4404 heart rate and pulse oximeter sensor"
> + depends on I2C
> + select REGMAP_I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes to choose the Texas Instruments AFE4404
> + heart rate monitor and low-cost pulse oximeter.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called afe4404.
>
> config MAX30100
> tristate "MAX30100 heart rate and pulse oximeter sensor"
> @@ -19,3 +34,5 @@ config MAX30100
> module will be called max30100.
>
> endmenu
> +
> +endmenu
> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile
> index 7c475d7..b37c0d5 100644
> --- a/drivers/iio/health/Makefile
> +++ b/drivers/iio/health/Makefile
> @@ -4,4 +4,5 @@
>
> # When adding new entries keep the list in alphabetical order
>
> +obj-$(CONFIG_AFE4404) += afe4404.o
> obj-$(CONFIG_MAX30100) += max30100.o
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> new file mode 100644
> index 0000000..58f3e35
> --- /dev/null
> +++ b/drivers/iio/health/afe4404.c
> @@ -0,0 +1,665 @@
> +/*
> + * AFE4404 Heart Rate Monitors and Low-Cost Pulse Oximeters
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#include "afe440x.h"
> +
> +#define AFE4404_DRIVER_NAME "afe4404"
> +
> +/* AFE4404 registers */
> +#define AFE4404_TIA_GAIN_SEP 0x20
> +#define AFE4404_TIA_GAIN 0x21
> +#define AFE4404_PROG_TG_STC 0x34
> +#define AFE4404_PROG_TG_ENDC 0x35
> +#define AFE4404_LED3LEDSTC 0x36
> +#define AFE4404_LED3LEDENDC 0x37
> +#define AFE4404_CLKDIV_PRF 0x39
> +#define AFE4404_OFFDAC 0x3a
> +#define AFE4404_DEC 0x3d
> +#define AFE4404_AVG_LED2_ALED2VAL 0x3f
> +#define AFE4404_AVG_LED1_ALED1VAL 0x40
> +
> +/* AFE4404 GAIN register fields */
> +#define AFE4404_TIA_GAIN_RES_MASK GENMASK(2, 0)
> +#define AFE4404_TIA_GAIN_RES_SHIFT 0
> +#define AFE4404_TIA_GAIN_CAP_MASK GENMASK(5, 3)
> +#define AFE4404_TIA_GAIN_CAP_SHIFT 3
> +
> +/* AFE4404 LEDCNTRL register fields */
> +#define AFE4404_LEDCNTRL_ILED1_MASK GENMASK(5, 0)
> +#define AFE4404_LEDCNTRL_ILED1_SHIFT 0
> +#define AFE4404_LEDCNTRL_ILED2_MASK GENMASK(11, 6)
> +#define AFE4404_LEDCNTRL_ILED2_SHIFT 6
> +#define AFE4404_LEDCNTRL_ILED3_MASK GENMASK(17, 12)
> +#define AFE4404_LEDCNTRL_ILED3_SHIFT 12
> +
> +/* AFE4404 CONTROL2 register fields */
> +#define AFE440X_CONTROL2_ILED_2X_MASK BIT(17)
> +#define AFE440X_CONTROL2_ILED_2X_SHIFT 17
> +
> +/* AFE4404 CONTROL3 register fields */
> +#define AFE440X_CONTROL3_OSC_ENABLE BIT(9)
> +
> +/* AFE4404 OFFDAC register current fields */
> +#define AFE4404_OFFDAC_CURR_LED1_MASK GENMASK(9, 5)
> +#define AFE4404_OFFDAC_CURR_LED1_SHIFT 5
> +#define AFE4404_OFFDAC_CURR_LED2_MASK GENMASK(19, 15)
> +#define AFE4404_OFFDAC_CURR_LED2_SHIFT 15
> +#define AFE4404_OFFDAC_CURR_LED3_MASK GENMASK(4, 0)
> +#define AFE4404_OFFDAC_CURR_LED3_SHIFT 0
> +#define AFE4404_OFFDAC_CURR_ALED1_MASK GENMASK(14, 10)
> +#define AFE4404_OFFDAC_CURR_ALED1_SHIFT 10
> +#define AFE4404_OFFDAC_CURR_ALED2_MASK GENMASK(4, 0)
> +#define AFE4404_OFFDAC_CURR_ALED2_SHIFT 0
> +
> +/* AFE4404 NULL fields */
> +#define NULL_MASK 0
> +#define NULL_SHIFT 0
> +
> +/* AFE4404 TIA_GAIN_CAP values */
> +#define AFE4404_TIA_GAIN_CAP_5_P 0x0
> +#define AFE4404_TIA_GAIN_CAP_2_5_P 0x1
> +#define AFE4404_TIA_GAIN_CAP_10_P 0x2
> +#define AFE4404_TIA_GAIN_CAP_7_5_P 0x3
> +#define AFE4404_TIA_GAIN_CAP_20_P 0x4
> +#define AFE4404_TIA_GAIN_CAP_17_5_P 0x5
> +#define AFE4404_TIA_GAIN_CAP_25_P 0x6
> +#define AFE4404_TIA_GAIN_CAP_22_5_P 0x7
> +
> +/* AFE4404 TIA_GAIN_RES values */
> +#define AFE4404_TIA_GAIN_RES_500_K 0x0
> +#define AFE4404_TIA_GAIN_RES_250_K 0x1
> +#define AFE4404_TIA_GAIN_RES_100_K 0x2
> +#define AFE4404_TIA_GAIN_RES_50_K 0x3
> +#define AFE4404_TIA_GAIN_RES_25_K 0x4
> +#define AFE4404_TIA_GAIN_RES_10_K 0x5
> +#define AFE4404_TIA_GAIN_RES_1_M 0x6
> +#define AFE4404_TIA_GAIN_RES_2_M 0x7
> +
> +/**
> + * struct afe4404_data
> + * @dev - Device structure
> + * @regmap - Register map of the device
> + * @regulator - Pointer to the regulator for the IC
> + * @trig - IIO trigger for this device
> + * @irq - ADC_RDY line interrupt number
> + */
> +struct afe4404_data {
> + struct device *dev;
> + struct regmap *regmap;
> + struct regulator *regulator;
> + struct iio_trigger *trig;
> + int irq;
> +};
> +
> +enum afe4404_chan_id {
> + LED1,
> + ALED1,
> + LED2,
> + ALED2,
> + LED3,
> + LED1_ALED1,
> + LED2_ALED2,
> + ILED1,
> + ILED2,
> + ILED3,
> +};
> +
> +static const struct afe440x_reg_info afe4404_reg_info[] = {
As noted below, I'd find
[LED1] = AFE440X_REG_INFO(AFE440_LED1VAL...
more readable.
> + AFE440X_REG_INFO(LED1, AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
> + AFE440X_REG_INFO(ALED1, AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
> + AFE440X_REG_INFO(LED2, AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
> + AFE440X_REG_INFO(ALED2, AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
> + AFE440X_REG_INFO(LED3, AFE440X_ALED2VAL, 0, NULL),
> + AFE440X_REG_INFO(LED1_ALED1, AFE440X_LED1_ALED1VAL, 0, NULL),
> + AFE440X_REG_INFO(LED2_ALED2, AFE440X_LED2_ALED2VAL, 0, NULL),
> + AFE440X_REG_INFO(ILED1, AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
> + AFE440X_REG_INFO(ILED2, AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
> + AFE440X_REG_INFO(ILED3, AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
> +};
> +
> +static const struct iio_chan_spec afe4404_channels[] = {
> + /* ADC values */
> + AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
> + AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> + AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
> + AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> + AFE440X_INTENSITY_CHAN(LED3, "led3", BIT(IIO_CHAN_INFO_OFFSET)),
> + AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> + /* LED current */
> + AFE440X_CURRENT_CHAN(ILED1, "led1"),
> + AFE440X_CURRENT_CHAN(ILED2, "led2"),
> + AFE440X_CURRENT_CHAN(ILED3, "led3"),
> +};
> +
> +static const struct afe440x_val_table afe4404_res_table[] = {
> + { .integer = 500000, .fract = 0 },
> + { .integer = 250000, .fract = 0 },
> + { .integer = 100000, .fract = 0 },
> + { .integer = 50000, .fract = 0 },
> + { .integer = 25000, .fract = 0 },
> + { .integer = 10000, .fract = 0 },
> + { .integer = 1000000, .fract = 0 },
> + { .integer = 2000000, .fract = 0 },
> +};
> +AFE440X_TABLE_ATTR(tia_resistance_available, afe4404_res_table);
> +
> +static const struct afe440x_val_table afe4404_cap_table[] = {
> + { .integer = 0, .fract = 5000 },
> + { .integer = 0, .fract = 2500 },
> + { .integer = 0, .fract = 10000 },
> + { .integer = 0, .fract = 7500 },
> + { .integer = 0, .fract = 20000 },
> + { .integer = 0, .fract = 17500 },
> + { .integer = 0, .fract = 25000 },
> + { .integer = 0, .fract = 22500 },
> +};
> +AFE440X_TABLE_ATTR(tia_capacitance_available, afe4404_cap_table);
> +
> +static ssize_t afe440x_show_register(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> + unsigned int reg_val, type;
> + int vals[2];
> + int ret, val_len;
> +
> + ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val);
> + if (ret)
> + return ret;
> +
> + reg_val &= afe440x_attr->mask;
> + reg_val >>= afe440x_attr->shift;
> +
> + switch (afe440x_attr->type) {
> + case SIMPLE:
> + type = IIO_VAL_INT;
> + val_len = 1;
> + vals[0] = reg_val;
> + break;
> + case RESISTANCE:
> + case CAPACITANCE:
> + type = IIO_VAL_INT_PLUS_MICRO;
> + val_len = 2;
> + if (reg_val < afe440x_attr->table_size) {
> + vals[0] = afe440x_attr->val_table[reg_val].integer;
> + vals[1] = afe440x_attr->val_table[reg_val].fract;
> + break;
> + }
> + return -EINVAL;
> + default:
> + return -EINVAL;
> + }
> +
> + return iio_format_value(buf, type, val_len, vals);
> +}
> +
> +static ssize_t afe440x_store_register(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> + int val, integer, fract, ret;
> +
> + ret = iio_str_to_fixpoint(buf, 100000, &integer, &fract);
> + if (ret)
> + return ret;
> +
> + switch (afe440x_attr->type) {
> + case SIMPLE:
> + val = integer;
> + break;
> + case RESISTANCE:
> + case CAPACITANCE:
> + for (val = 0; val < afe440x_attr->table_size; val++)
> + if (afe440x_attr->val_table[val].integer == integer &&
> + afe440x_attr->val_table[val].fract == fract)
> + break;
> + if (val == afe440x_attr->table_size)
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
> + afe440x_attr->mask,
> + (val << afe440x_attr->shift));
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
> +
> +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
> +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
> +
> +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
> +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
> +
> +static struct attribute *afe440x_attributes[] = {
> + &afe440x_attr_tia_separate_en.dev_attr.attr,
> + &afe440x_attr_tia_resistance1.dev_attr.attr,
> + &afe440x_attr_tia_capacitance1.dev_attr.attr,
> + &afe440x_attr_tia_resistance2.dev_attr.attr,
> + &afe440x_attr_tia_capacitance2.dev_attr.attr,
> + &dev_attr_tia_resistance_available.attr,
> + &dev_attr_tia_capacitance_available.attr,
> + NULL
> +};
> +
> +static const struct attribute_group afe440x_attribute_group = {
> + .attrs = afe440x_attributes
> +};
> +
> +static int afe4404_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
> + int ret;
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = regmap_read(afe->regmap, reg_info.reg, val);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OFFSET:
> + ret = regmap_read(afe->regmap, reg_info.offreg,
> + val);
> + if (ret)
> + return ret;
> + *val &= reg_info.mask;
> + *val >>= reg_info.shift;
> + return IIO_VAL_INT;
> + }
> + break;
> + case IIO_CURRENT:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = regmap_read(afe->regmap, reg_info.reg, val);
> + if (ret)
> + return ret;
> + *val &= reg_info.mask;
> + *val >>= reg_info.shift;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = 0;
> + *val2 = 800000;
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int afe4404_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + switch (mask) {
> + case IIO_CHAN_INFO_OFFSET:
> + return regmap_update_bits(afe->regmap,
> + reg_info.offreg,
> + reg_info.mask,
> + (val << reg_info.shift));
> + }
> + break;
> + case IIO_CURRENT:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return regmap_update_bits(afe->regmap,
> + reg_info.reg,
> + reg_info.mask,
> + (val << reg_info.shift));
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_info afe4404_iio_info = {
> + .attrs = &afe440x_attribute_group,
> + .read_raw = afe4404_read_raw,
> + .write_raw = afe4404_write_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static irqreturn_t afe4404_trigger_handler(int irq, void *private)
> +{
> + struct iio_poll_func *pf = private;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + int ret, bit, i = 0;
> + s32 buffer[10];
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg,
> + &buffer[i++]);
> + if (ret)
> + goto err;
> + }
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buffer, pf->timestamp);
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct iio_trigger_ops afe4404_trigger_ops = {
> + .owner = THIS_MODULE,
> +};
Hmm, no state control which is unusual.
> +
> +/* Default timings from data-sheet */
> +#define AFE4404_TIMING_PAIRS \
> + { AFE440X_PRPCOUNT, 39999 }, \
> + { AFE440X_LED2LEDSTC, 0 }, \
> + { AFE440X_LED2LEDENDC, 398 }, \
> + { AFE440X_LED2STC, 80 }, \
> + { AFE440X_LED2ENDC, 398 }, \
> + { AFE440X_ADCRSTSTCT0, 5600 }, \
> + { AFE440X_ADCRSTENDCT0, 5606 }, \
> + { AFE440X_LED2CONVST, 5607 }, \
> + { AFE440X_LED2CONVEND, 6066 }, \
> + { AFE4404_LED3LEDSTC, 400 }, \
> + { AFE4404_LED3LEDENDC, 798 }, \
> + { AFE440X_ALED2STC, 480 }, \
> + { AFE440X_ALED2ENDC, 798 }, \
> + { AFE440X_ADCRSTSTCT1, 6068 }, \
> + { AFE440X_ADCRSTENDCT1, 6074 }, \
> + { AFE440X_ALED2CONVST, 6075 }, \
> + { AFE440X_ALED2CONVEND, 6534 }, \
> + { AFE440X_LED1LEDSTC, 800 }, \
> + { AFE440X_LED1LEDENDC, 1198 }, \
> + { AFE440X_LED1STC, 880 }, \
> + { AFE440X_LED1ENDC, 1198 }, \
> + { AFE440X_ADCRSTSTCT2, 6536 }, \
> + { AFE440X_ADCRSTENDCT2, 6542 }, \
> + { AFE440X_LED1CONVST, 6543 }, \
> + { AFE440X_LED1CONVEND, 7003 }, \
> + { AFE440X_ALED1STC, 1280 }, \
> + { AFE440X_ALED1ENDC, 1598 }, \
> + { AFE440X_ADCRSTSTCT3, 7005 }, \
> + { AFE440X_ADCRSTENDCT3, 7011 }, \
> + { AFE440X_ALED1CONVST, 7012 }, \
> + { AFE440X_ALED1CONVEND, 7471 }, \
> + { AFE440X_PDNCYCLESTC, 7671 }, \
> + { AFE440X_PDNCYCLEENDC, 39199 }
> +
> +static const struct reg_sequence afe4404_reg_sequences[] = {
> + AFE4404_TIMING_PAIRS,
> + { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
> + { AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES_50_K },
> + { AFE440X_LEDCNTRL, (0xf << AFE4404_LEDCNTRL_ILED1_SHIFT) |
> + (0x3 << AFE4404_LEDCNTRL_ILED2_SHIFT) |
> + (0x3 << AFE4404_LEDCNTRL_ILED3_SHIFT) },
> + { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE },
> +};
> +
> +static const struct regmap_range afe4404_yes_ranges[] = {
> + regmap_reg_range(AFE440X_LED2VAL, AFE440X_LED1_ALED1VAL),
> + regmap_reg_range(AFE4404_AVG_LED2_ALED2VAL, AFE4404_AVG_LED1_ALED1VAL),
> +};
> +
> +static const struct regmap_access_table afe4404_volatile_table = {
> + .yes_ranges = afe4404_yes_ranges,
> + .n_yes_ranges = ARRAY_SIZE(afe4404_yes_ranges),
> +};
> +
> +static const struct regmap_config afe4404_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 24,
> +
> + .max_register = AFE4404_AVG_LED1_ALED1VAL,
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_table = &afe4404_volatile_table,
> +};
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id afe4404_of_match[] = {
> + { .compatible = "ti,afe4404", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, afe4404_of_match);
> +#endif
> +
> +static int afe4404_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_update_bits(afe->regmap, AFE440X_CONTROL2,
> + AFE440X_CONTROL2_PDN_AFE,
> + AFE440X_CONTROL2_PDN_AFE);
> + if (ret)
> + return ret;
> +
> + ret = regulator_disable(afe->regulator);
> + if (ret) {
> + dev_err(dev, "Unable to disable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int afe4404_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regulator_enable(afe->regulator);
> + if (ret) {
> + dev_err(dev, "Unable to enable regulator\n");
> + return ret;
> + }
> +
> + ret = regmap_update_bits(afe->regmap, AFE440X_CONTROL2,
> + AFE440X_CONTROL2_PDN_AFE, 0);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(afe4404_pm_ops, afe4404_suspend, afe4404_resume);
> +
Looking nice and clean in general - few bits not getting unwound in the
remove however.
> +static int afe4404_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct iio_dev *indio_dev;
> + struct afe4404_data *afe;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + afe = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> +
> + afe->dev = &client->dev;
> + afe->irq = client->irq;
> +
> + afe->regmap = devm_regmap_init_i2c(client, &afe4404_regmap_config);
> + if (IS_ERR(afe->regmap)) {
> + dev_err(afe->dev, "Unable to allocate register map\n");
> + return PTR_ERR(afe->regmap);
> + }
> +
> + afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
> + if (IS_ERR(afe->regulator)) {
> + dev_err(afe->dev, "Unable to get regulator\n");
> + return PTR_ERR(afe->regulator);
> + }
> + ret = regulator_enable(afe->regulator);
> + if (ret) {
> + dev_err(afe->dev, "Unable to enable regulator\n");
> + return ret;
> + }
> +
> + ret = regmap_write(afe->regmap, AFE440X_CONTROL0,
> + AFE440X_CONTROL0_SW_RESET);
> + if (ret) {
> + dev_err(afe->dev, "Unable to reset device\n");
> + return ret;
> + }
> +
> + ret = regmap_multi_reg_write(afe->regmap, afe4404_reg_sequences,
> + ARRAY_SIZE(afe4404_reg_sequences));
> + if (ret) {
> + dev_err(afe->dev, "Unable to set register defaults\n");
> + return ret;
> + }
> +
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->dev.parent = afe->dev;
> + indio_dev->channels = afe4404_channels;
> + indio_dev->num_channels = ARRAY_SIZE(afe4404_channels);
> + indio_dev->name = AFE4404_DRIVER_NAME;
> + indio_dev->info = &afe4404_iio_info;
> +
> + if (afe->irq > 0) {
> + afe->trig = devm_iio_trigger_alloc(afe->dev,
> + "%s-dev%d",
> + indio_dev->name,
> + indio_dev->id);
> + if (!afe->trig) {
> + dev_err(afe->dev, "Unable to allocate IIO trigger\n");
> + return -ENOMEM;
> + }
> +
> + iio_trigger_set_drvdata(afe->trig, indio_dev);
> +
> + afe->trig->ops = &afe4404_trigger_ops;
> + afe->trig->dev.parent = afe->dev;
> +
> + ret = iio_trigger_register(afe->trig);
> + if (ret) {
> + dev_err(afe->dev, "Unable to register IIO trigger\n");
> + return ret;
> + }
> +
> + ret = devm_request_threaded_irq(afe->dev, afe->irq,
> + iio_trigger_generic_data_rdy_poll,
> + NULL, IRQF_ONESHOT,
> + AFE4404_DRIVER_NAME,
> + afe->trig);
> + if (ret) {
> + dev_err(afe->dev, "Unable to request IRQ\n");
> + return ret;
> + }
> + }
> +
> + ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
> + afe4404_trigger_handler, NULL);
> + if (ret) {
> + dev_err(afe->dev, "Unable to setup buffer\n");
> + return ret;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(afe->dev, "Unable to register IIO device\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int afe4404_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct afe4404_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + iio_device_unregister(indio_dev);
> +
Would expect unwinding of the triggered_buffer here.
iio_triggered_buffer_cleanup()
> + iio_trigger_unregister(afe->trig);
This should be protected by a check on afe->irq as for the register in probe.
There is no protection against afe->trig == NULL in iio_trigger_unregister.
That is kind of deliberate as it makes sure the probe / remove in drivers
are balanced unlike here.
> +
> + ret = regulator_disable(afe->regulator);
> + if (ret) {
> + dev_err(afe->dev, "Unable to disable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id afe4404_ids[] = {
> + { "afe4404", 0 },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, afe4404_ids);
> +
> +static struct i2c_driver afe4404_i2c_driver = {
> + .driver = {
> + .name = AFE4404_DRIVER_NAME,
> + .of_match_table = of_match_ptr(afe4404_of_match),
> + .pm = &afe4404_pm_ops,
> + },
> + .probe = afe4404_probe,
> + .remove = afe4404_remove,
> + .id_table = afe4404_ids,
> +};
> +module_i2c_driver(afe4404_i2c_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_DESCRIPTION("TI AFE4404 Heart Rate and Pulse Oximeter");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
> new file mode 100644
> index 0000000..9641653
> --- /dev/null
> +++ b/drivers/iio/health/afe440x.h
> @@ -0,0 +1,193 @@
> +/*
> + * AFE440X Heart Rate Monitors and Low-Cost Pulse Oximeters
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#ifndef _AFE440X_H
> +#define _AFE440X_H
> +
> +/* AFE440X registers */
> +#define AFE440X_CONTROL0 0x00
> +#define AFE440X_LED2STC 0x01
> +#define AFE440X_LED2ENDC 0x02
> +#define AFE440X_LED1LEDSTC 0x03
> +#define AFE440X_LED1LEDENDC 0x04
> +#define AFE440X_ALED2STC 0x05
> +#define AFE440X_ALED2ENDC 0x06
> +#define AFE440X_LED1STC 0x07
> +#define AFE440X_LED1ENDC 0x08
> +#define AFE440X_LED2LEDSTC 0x09
> +#define AFE440X_LED2LEDENDC 0x0a
> +#define AFE440X_ALED1STC 0x0b
> +#define AFE440X_ALED1ENDC 0x0c
> +#define AFE440X_LED2CONVST 0x0d
> +#define AFE440X_LED2CONVEND 0x0e
> +#define AFE440X_ALED2CONVST 0x0f
> +#define AFE440X_ALED2CONVEND 0x10
> +#define AFE440X_LED1CONVST 0x11
> +#define AFE440X_LED1CONVEND 0x12
> +#define AFE440X_ALED1CONVST 0x13
> +#define AFE440X_ALED1CONVEND 0x14
> +#define AFE440X_ADCRSTSTCT0 0x15
> +#define AFE440X_ADCRSTENDCT0 0x16
> +#define AFE440X_ADCRSTSTCT1 0x17
> +#define AFE440X_ADCRSTENDCT1 0x18
> +#define AFE440X_ADCRSTSTCT2 0x19
> +#define AFE440X_ADCRSTENDCT2 0x1a
> +#define AFE440X_ADCRSTSTCT3 0x1b
> +#define AFE440X_ADCRSTENDCT3 0x1c
> +#define AFE440X_PRPCOUNT 0x1d
> +#define AFE440X_CONTROL1 0x1e
> +#define AFE440X_LEDCNTRL 0x22
> +#define AFE440X_CONTROL2 0x23
> +#define AFE440X_ALARM 0x29
> +#define AFE440X_LED2VAL 0x2a
> +#define AFE440X_ALED2VAL 0x2b
> +#define AFE440X_LED1VAL 0x2c
> +#define AFE440X_ALED1VAL 0x2d
> +#define AFE440X_LED2_ALED2VAL 0x2e
> +#define AFE440X_LED1_ALED1VAL 0x2f
> +#define AFE440X_CONTROL3 0x31
> +#define AFE440X_PDNCYCLESTC 0x32
> +#define AFE440X_PDNCYCLEENDC 0x33
> +
> +/* CONTROL0 register fields */
> +#define AFE440X_CONTROL0_REG_READ BIT(0)
> +#define AFE440X_CONTROL0_TM_COUNT_RST BIT(1)
> +#define AFE440X_CONTROL0_SW_RESET BIT(3)
> +
> +/* CONTROL1 register fields */
> +#define AFE440X_CONTROL1_TIMEREN BIT(8)
> +
> +/* TIAGAIN register fields */
> +#define AFE440X_TIAGAIN_ENSEPGAIN_MASK BIT(15)
> +#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT 15
> +
> +/* CONTROL2 register fields */
> +#define AFE440X_CONTROL2_PDN_AFE BIT(0)
> +#define AFE440X_CONTROL2_PDN_RX BIT(1)
> +#define AFE440X_CONTROL2_DYNAMIC4 BIT(3)
> +#define AFE440X_CONTROL2_DYNAMIC3 BIT(4)
> +#define AFE440X_CONTROL2_DYNAMIC2 BIT(14)
> +#define AFE440X_CONTROL2_DYNAMIC1 BIT(20)
> +
> +/* CONTROL3 register fields */
> +#define AFE440X_CONTROL3_CLKDIV GENMASK(2, 0)
> +
> +/* CONTROL0 values */
> +#define AFE440X_CONTROL0_WRITE 0x0
> +#define AFE440X_CONTROL0_READ 0x1
> +
> +struct afe440x_reg_info {
> + unsigned int reg;
> + unsigned int offreg;
> + unsigned int shift;
> + unsigned int mask;
> +};
> +
> +#define AFE440X_REG_INFO(_id, _reg, _offreg, _sm) \
> + [_id] = { \
> + .reg = _reg, \
> + .offreg = _offreg, \
> + .shift = _sm ## _SHIFT, \
> + .mask = _sm ## _MASK, \
> + }
I'm not overly keen on the hiding fo the indexing. I'd prefer
this was just the bit {...} and you had the fact it was filling in an
indexed element explicit where used.
> +
> +#define AFE440X_INTENSITY_CHAN(_index, _name, _mask) \
> + { \
> + .type = IIO_INTENSITY, \
> + .channel = _index, \
> + .address = _index, \
> + .scan_index = _index, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 24, \
> + .storagebits = 32, \
> + .endianness = IIO_CPU, \
> + }, \
> + .extend_name = _name, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + _mask, \
> + }
> +
> +#define AFE440X_CURRENT_CHAN(_index, _name) \
> + { \
> + .type = IIO_CURRENT, \
> + .channel = _index, \
> + .address = _index, \
> + .scan_index = _index, \
> + .extend_name = _name, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .output = true, \
> + }
> +
I'd prefix reg_type to avoid possible name clashes in future.
> +enum reg_type {
> + SIMPLE,
> + RESISTANCE,
> + CAPACITANCE,
> +};
> +
> +/* this could be made more general for other IIO drivers if needed --------- */
Yes, but lets do that at as a follow up.
> +struct afe440x_val_table {
> + int integer;
> + int fract;
> +};
> +
> +#define AFE440X_TABLE_ATTR(_name, _table) \
> +static ssize_t _name ## _show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + ssize_t len = 0; \
> + int i; \
> + \
> + for (i = 0; i < ARRAY_SIZE(_table); i++) \
> + len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ", \
> + _table[i].integer, \
> + _table[i].fract); \
> + \
> + buf[len - 1] = '\n'; \
> + \
> + return len; \
> +} \
> +static DEVICE_ATTR_RO(_name)
> +/* ------------------------------------------------------------------------- */
> +
> +struct afe440x_attr {
> + struct device_attribute dev_attr;
> + unsigned int reg;
> + unsigned int shift;
> + unsigned int mask;
> + enum reg_type type;
> + const struct afe440x_val_table *val_table;
> + unsigned int table_size;
> +};
> +
> +#define to_afe440x_attr(_dev_attr) \
> + container_of(_dev_attr, struct afe440x_attr, dev_attr)
> +
> +#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size) \
> + struct afe440x_attr afe440x_attr_##_name = { \
> + .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \
> + afe440x_show_register, \
> + afe440x_store_register), \
> + .reg = _reg, \
> + .shift = _field ## _SHIFT, \
> + .mask = _field ## _MASK, \
> + .type = _type, \
> + .val_table = _table, \
> + .table_size = _size, \
> + }
> +
> +#endif /* _AFE440X_H */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor
[not found] ` <1453742941-12086-5-git-send-email-afd@ti.com>
2016-01-25 20:21 ` [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor Peter Meerwald-Stadler
@ 2016-01-30 15:10 ` Jonathan Cameron
2016-02-02 17:38 ` Andrew F. Davis
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2016-01-30 15:10 UTC (permalink / raw)
To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Dan Murphy, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
Cc: linux-iio, linux-api, devicetree, linux-kernel
On 25/01/16 17:29, Andrew F. Davis wrote:
> Add driver for the TI AFE4403 heart rate monitor and pulse oximeter.
> This device detects reflected LED light fluctuations and presents an ADC
> value to the user space for further signal processing.
>
> Data sheet located here:
> http://www.ti.com/product/AFE4403/datasheet
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Hi Andrew,
As expected, the comment in here are pretty similar to in the previous
driver. I'm fairly sure you can't use ___cacheline_aligned as you have
here as I don't believe it has any effect on data store on the stack - or
at least not how you need it to (which is to ensure only the spi buffers
are in the relevant cacheline to avoid corruption of other memory in there).
Otherwise, looks pretty good.
Jonathan
> ---
> drivers/iio/health/Kconfig | 12 +
> drivers/iio/health/Makefile | 1 +
> drivers/iio/health/afe4403.c | 698 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 711 insertions(+)
> create mode 100644 drivers/iio/health/afe4403.c
>
> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig
> index 632a14b..f0c1977 100644
> --- a/drivers/iio/health/Kconfig
> +++ b/drivers/iio/health/Kconfig
> @@ -7,6 +7,18 @@ menu "Health Sensors"
>
> menu "Heart Rate Monitors"
>
> +config AFE4403
> + tristate "TI AFE4403 Heart Rate Monitor"
> + depends on SPI_MASTER
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes to choose the Texas Instruments AFE4403
> + heart rate monitor and low-cost pulse oximeter.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called afe4403.
> +
> config AFE4404
> tristate "TI AFE4404 heart rate and pulse oximeter sensor"
> depends on I2C
> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile
> index b37c0d5..9955a2a 100644
> --- a/drivers/iio/health/Makefile
> +++ b/drivers/iio/health/Makefile
> @@ -4,5 +4,6 @@
>
> # When adding new entries keep the list in alphabetical order
>
> +obj-$(CONFIG_AFE4403) += afe4403.o
> obj-$(CONFIG_AFE4404) += afe4404.o
> obj-$(CONFIG_MAX30100) += max30100.o
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> new file mode 100644
> index 0000000..2c8565f
> --- /dev/null
> +++ b/drivers/iio/health/afe4403.c
> @@ -0,0 +1,698 @@
> +/*
> + * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#include "afe440x.h"
> +
> +#define AFE4403_DRIVER_NAME "afe4403"
> +
> +/* AFE4403 Registers */
> +#define AFE4403_TIAGAIN 0x20
> +#define AFE4403_TIA_AMB_GAIN 0x21
> +
> +/* AFE4403 GAIN register fields */
> +#define AFE4403_TIAGAIN_RES_MASK GENMASK(2, 0)
> +#define AFE4403_TIAGAIN_RES_SHIFT 0
> +#define AFE4403_TIAGAIN_CAP_MASK GENMASK(7, 3)
> +#define AFE4403_TIAGAIN_CAP_SHIFT 3
> +
> +/* AFE4403 LEDCNTRL register fields */
> +#define AFE440X_LEDCNTRL_LED1_MASK GENMASK(15, 8)
> +#define AFE440X_LEDCNTRL_LED1_SHIFT 8
> +#define AFE440X_LEDCNTRL_LED2_MASK GENMASK(7, 0)
> +#define AFE440X_LEDCNTRL_LED2_SHIFT 0
> +#define AFE440X_LEDCNTRL_LED_RANGE_MASK GENMASK(17, 16)
> +#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT 16
> +
> +/* AFE4403 CONTROL2 register fields */
> +#define AFE440X_CONTROL2_PWR_DWN_TX BIT(2)
> +#define AFE440X_CONTROL2_EN_SLOW_DIAG BIT(8)
> +#define AFE440X_CONTROL2_DIAG_OUT_TRI BIT(10)
> +#define AFE440X_CONTROL2_TX_BRDG_MOD BIT(11)
> +#define AFE440X_CONTROL2_TX_REF_MASK GENMASK(18, 17)
> +#define AFE440X_CONTROL2_TX_REF_SHIFT 17
> +
> +/* AFE4404 NULL fields */
> +#define NULL_MASK 0
> +#define NULL_SHIFT 0
> +
> +/* AFE4403 LEDCNTRL values */
> +#define AFE440X_LEDCNTRL_RANGE_TX_HALF 0x1
> +#define AFE440X_LEDCNTRL_RANGE_TX_FULL 0x2
> +#define AFE440X_LEDCNTRL_RANGE_TX_OFF 0x3
> +
> +/* AFE4403 CONTROL2 values */
> +#define AFE440X_CONTROL2_TX_REF_025 0x0
> +#define AFE440X_CONTROL2_TX_REF_050 0x1
> +#define AFE440X_CONTROL2_TX_REF_100 0x2
> +#define AFE440X_CONTROL2_TX_REF_075 0x3
> +
> +/* AFE4403 CONTROL3 values */
> +#define AFE440X_CONTROL3_CLK_DIV_2 0x0
> +#define AFE440X_CONTROL3_CLK_DIV_4 0x2
> +#define AFE440X_CONTROL3_CLK_DIV_6 0x3
> +#define AFE440X_CONTROL3_CLK_DIV_8 0x4
> +#define AFE440X_CONTROL3_CLK_DIV_12 0x5
> +#define AFE440X_CONTROL3_CLK_DIV_1 0x7
> +
> +/* AFE4403 TIAGAIN_CAP values */
> +#define AFE4403_TIAGAIN_CAP_5_P 0x0
> +#define AFE4403_TIAGAIN_CAP_10_P 0x1
> +#define AFE4403_TIAGAIN_CAP_20_P 0x2
> +#define AFE4403_TIAGAIN_CAP_30_P 0x3
> +#define AFE4403_TIAGAIN_CAP_55_P 0x8
> +#define AFE4403_TIAGAIN_CAP_155_P 0x10
> +
> +/* AFE4403 TIAGAIN_RES values */
> +#define AFE4403_TIAGAIN_RES_500_K 0x0
> +#define AFE4403_TIAGAIN_RES_250_K 0x1
> +#define AFE4403_TIAGAIN_RES_100_K 0x2
> +#define AFE4403_TIAGAIN_RES_50_K 0x3
> +#define AFE4403_TIAGAIN_RES_25_K 0x4
> +#define AFE4403_TIAGAIN_RES_10_K 0x5
> +#define AFE4403_TIAGAIN_RES_1_M 0x6
> +#define AFE4403_TIAGAIN_RES_NONE 0x7
> +
> +/**
> + * struct afe4403_data
> + * @dev - Device structure
> + * @spi - SPI device handle
> + * @regmap - Register map of the device
> + * @regulator - Pointer to the regulator for the IC
> + * @trig - IIO trigger for this device
> + * @irq - ADC_RDY line interrupt number
> + */
> +struct afe4403_data {
> + struct device *dev;
> + struct spi_device *spi;
> + struct regmap *regmap;
> + struct regulator *regulator;
> + struct iio_trigger *trig;
> + int irq;
> +};
> +
> +enum afe4403_chan_id {
> + LED1,
> + ALED1,
> + LED2,
> + ALED2,
> + LED1_ALED1,
> + LED2_ALED2,
> + ILED1,
> + ILED2,
> +};
> +
> +static const struct afe440x_reg_info afe4403_reg_info[] = {
> + AFE440X_REG_INFO(LED1, AFE440X_LED1VAL, 0, NULL),
> + AFE440X_REG_INFO(ALED1, AFE440X_ALED1VAL, 0, NULL),
> + AFE440X_REG_INFO(LED2, AFE440X_LED2VAL, 0, NULL),
> + AFE440X_REG_INFO(ALED2, AFE440X_ALED2VAL, 0, NULL),
> + AFE440X_REG_INFO(LED1_ALED1, AFE440X_LED1_ALED1VAL, 0, NULL),
> + AFE440X_REG_INFO(LED2_ALED2, AFE440X_LED2_ALED2VAL, 0, NULL),
> + AFE440X_REG_INFO(ILED1, AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
> + AFE440X_REG_INFO(ILED2, AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
> +};
> +
> +static const struct iio_chan_spec afe4403_channels[] = {
> + /* ADC values */
> + AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
> + AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
> + AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> + AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> + /* LED current */
> + AFE440X_CURRENT_CHAN(ILED1, "led1"),
> + AFE440X_CURRENT_CHAN(ILED2, "led2"),
> +};
> +
> +static const struct afe440x_val_table afe4403_res_table[] = {
> + { 500000 }, { 250000 }, { 100000 }, { 50000 },
> + { 25000 }, { 10000 }, { 1000000 }, { 0 },
> +};
> +AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table);
> +
> +static const struct afe440x_val_table afe4403_cap_table[] = {
> + { 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 },
> + { 0, 30000 }, { 0, 35000 }, { 0, 45000 }, { 0, 50000 },
> + { 0, 55000 }, { 0, 60000 }, { 0, 70000 }, { 0, 75000 },
> + { 0, 80000 }, { 0, 85000 }, { 0, 95000 }, { 0, 100000 },
> + { 0, 155000 }, { 0, 160000 }, { 0, 170000 }, { 0, 175000 },
> + { 0, 180000 }, { 0, 185000 }, { 0, 195000 }, { 0, 200000 },
> + { 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 },
> + { 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 },
> +};
> +AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table);
1> +
> +static ssize_t afe440x_show_register(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> + unsigned int reg_val, type;
> + int vals[2];
> + int ret, val_len;
> +
> + ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val);
> + if (ret)
> + return ret;
> +
> + reg_val &= afe440x_attr->mask;
> + reg_val >>= afe440x_attr->shift;
> +
> + switch (afe440x_attr->type) {
> + case SIMPLE:
> + type = IIO_VAL_INT;
> + val_len = 1;
> + vals[0] = reg_val;
> + break;
> + case RESISTANCE:
> + case CAPACITANCE:
> + type = IIO_VAL_INT_PLUS_MICRO;
> + val_len = 2;
> + if (reg_val < afe440x_attr->table_size) {
> + vals[0] = afe440x_attr->val_table[reg_val].integer;
> + vals[1] = afe440x_attr->val_table[reg_val].fract;
> + break;
> + }
> + return -EINVAL;
> + default:
> + return -EINVAL;
> + }
> +
> + return iio_format_value(buf, type, val_len, vals);
> +}
> +
> +static ssize_t afe440x_store_register(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> + int val, integer, fract, ret;
> +
> + ret = iio_str_to_fixpoint(buf, 100000, &integer, &fract);
> + if (ret)
> + return ret;
> +
> + switch (afe440x_attr->type) {
> + case SIMPLE:
> + val = integer;
> + break;
> + case RESISTANCE:
> + case CAPACITANCE:
> + for (val = 0; val < afe440x_attr->table_size; val++)
> + if (afe440x_attr->val_table[val].integer == integer &&
> + afe440x_attr->val_table[val].fract == fract)
> + break;
> + if (val == afe440x_attr->table_size)
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
> + afe440x_attr->mask,
> + (val << afe440x_attr->shift));
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
> +
> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
> +
> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
> +
> +static struct attribute *afe440x_attributes[] = {
> + &afe440x_attr_tia_separate_en.dev_attr.attr,
> + &afe440x_attr_tia_resistance1.dev_attr.attr,
> + &afe440x_attr_tia_capacitance1.dev_attr.attr,
> + &afe440x_attr_tia_resistance2.dev_attr.attr,
> + &afe440x_attr_tia_capacitance2.dev_attr.attr,
> + &dev_attr_tia_resistance_available.attr,
> + &dev_attr_tia_capacitance_available.attr,
> + NULL
> +};
> +
> +static const struct attribute_group afe440x_attribute_group = {
> + .attrs = afe440x_attributes
> +};
> +
> +static int afe4403_read(struct afe4403_data *afe, unsigned int reg, u32 *val)
> +{
> + u8 tx[4] ____cacheline_aligned = {AFE440X_CONTROL0, 0x0, 0x0,
> + AFE440X_CONTROL0_READ};
hmm. Can you do this on the stack? Don't think so but maybe I'm wrong..
The cachline aligned trick relies on the start of the allocation on the
heap being aligned and then pads to ensure that the element so tagged
is also aligned appropriately.
> + u8 rx[3];
> + int ret;
Even if this were possible, ret is in the same cacheline as tx and rx so
chaos may well occur.
If you really want to avoid having allocations elsewhere, just use
spi_write_then_read(afe->spi, tx, 4, NULL, 0) and you should be fine
as spi_write_then_read uses safe bounce buffers.
> +
> + /* Enable reading from the device */
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto out;
> +
> + ret = spi_write_then_read(afe->spi, ®, 1, rx, 3);
> + if (ret)
> + goto out;
> +
> + *val = (rx[0] << 16) |
> + (rx[1] << 8) |
> + (rx[2]);
> +
> + /* Disable reading from the device */
> + tx[3] = AFE440X_CONTROL0_WRITE;
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto out;
> +
> +out:
> +
> + return ret;
> +}
> +
> +static int afe4403_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
> + int ret;
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = afe4403_read(afe, reg_info.reg, val);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OFFSET:
> + ret = regmap_read(afe->regmap, reg_info.offreg,
> + val);
> + if (ret)
> + return ret;
> + *val &= reg_info.mask;
> + *val >>= reg_info.shift;
> + return IIO_VAL_INT;
> + }
> + break;
> + case IIO_CURRENT:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = regmap_read(afe->regmap, reg_info.reg, val);
> + if (ret)
> + return ret;
> + *val &= reg_info.mask;
> + *val >>= reg_info.shift;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = 0;
> + *val2 = 800000;
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int afe4403_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
> +
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + switch (mask) {
> + case IIO_CHAN_INFO_OFFSET:
> + return regmap_update_bits(afe->regmap,
> + reg_info.offreg,
> + reg_info.mask,
> + (val << reg_info.shift));
> + }
> + break;
> + case IIO_CURRENT:
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return regmap_update_bits(afe->regmap,
> + reg_info.reg,
> + reg_info.mask,
> + (val << reg_info.shift));
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_info afe4403_iio_info = {
> + .attrs = &afe440x_attribute_group,
> + .read_raw = afe4403_read_raw,
> + .write_raw = afe4403_write_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static irqreturn_t afe4403_trigger_handler(int irq, void *private)
> +{
> + struct iio_poll_func *pf = private;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret, bit, i = 0;
> + s32 buffer[8];
> + u8 tx[4] ____cacheline_aligned = {AFE440X_CONTROL0, 0x0, 0x0,
> + AFE440X_CONTROL0_READ};
> + u8 rx[3];
> +
> + /* Enable reading from the device */
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto err;
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + ret = spi_write_then_read(afe->spi,
> + &afe4403_reg_info[bit].reg, 1,
> + rx, 3);
> + if (ret)
> + goto err;
> +
> + buffer[i++] = (rx[0] << 16) |
> + (rx[1] << 8) |
> + (rx[2]);
> + }
> +
> + /* Disable reading from the device */
> + tx[3] = AFE440X_CONTROL0_WRITE;
> + ret = spi_write(afe->spi, tx, 4);
> + if (ret)
> + goto err;
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buffer, pf->timestamp);
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct iio_trigger_ops afe4403_trigger_ops = {
> + .owner = THIS_MODULE,
> +};
> +
> +#define AFE4403_TIMING_PAIRS \
> + { AFE440X_LED2STC, 0x000050 }, \
> + { AFE440X_LED2ENDC, 0x0003e7 }, \
> + { AFE440X_LED1LEDSTC, 0x0007d0 }, \
> + { AFE440X_LED1LEDENDC, 0x000bb7 }, \
> + { AFE440X_ALED2STC, 0x000438 }, \
> + { AFE440X_ALED2ENDC, 0x0007cf }, \
> + { AFE440X_LED1STC, 0x000820 }, \
> + { AFE440X_LED1ENDC, 0x000bb7 }, \
> + { AFE440X_LED2LEDSTC, 0x000000 }, \
> + { AFE440X_LED2LEDENDC, 0x0003e7 }, \
> + { AFE440X_ALED1STC, 0x000c08 }, \
> + { AFE440X_ALED1ENDC, 0x000f9f }, \
> + { AFE440X_LED2CONVST, 0x0003ef }, \
> + { AFE440X_LED2CONVEND, 0x0007cf }, \
> + { AFE440X_ALED2CONVST, 0x0007d7 }, \
> + { AFE440X_ALED2CONVEND, 0x000bb7 }, \
> + { AFE440X_LED1CONVST, 0x000bbf }, \
> + { AFE440X_LED1CONVEND, 0x009c3f }, \
> + { AFE440X_ALED1CONVST, 0x000fa7 }, \
> + { AFE440X_ALED1CONVEND, 0x001387 }, \
> + { AFE440X_ADCRSTSTCT0, 0x0003e8 }, \
> + { AFE440X_ADCRSTENDCT0, 0x0003eb }, \
> + { AFE440X_ADCRSTSTCT1, 0x0007d0 }, \
> + { AFE440X_ADCRSTENDCT1, 0x0007d3 }, \
> + { AFE440X_ADCRSTSTCT2, 0x000bb8 }, \
> + { AFE440X_ADCRSTENDCT2, 0x000bbb }, \
> + { AFE440X_ADCRSTSTCT3, 0x000fa0 }, \
> + { AFE440X_ADCRSTENDCT3, 0x000fa3 }, \
> + { AFE440X_PRPCOUNT, 0x009c3f }, \
> + { AFE440X_PDNCYCLESTC, 0x001518 }, \
> + { AFE440X_PDNCYCLEENDC, 0x00991f }
> +
> +static const struct reg_sequence afe4403_reg_sequences[] = {
> + AFE4403_TIMING_PAIRS,
> + { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN | 0x000007},
> + { AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES_1_M },
> + { AFE440X_LEDCNTRL, (0x14 << AFE440X_LEDCNTRL_LED1_SHIFT) |
> + (0x14 << AFE440X_LEDCNTRL_LED2_SHIFT) },
> + { AFE440X_CONTROL2, AFE440X_CONTROL2_TX_REF_050 <<
> + AFE440X_CONTROL2_TX_REF_SHIFT },
> +};
> +
> +static const struct regmap_range afe4403_yes_ranges[] = {
> + regmap_reg_range(AFE440X_LED2VAL, AFE440X_LED1_ALED1VAL),
> +};
> +
> +static const struct regmap_access_table afe4403_volatile_table = {
> + .yes_ranges = afe4403_yes_ranges,
> + .n_yes_ranges = ARRAY_SIZE(afe4403_yes_ranges),
> +};
> +
> +static const struct regmap_config afe4403_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 24,
> +
> + .max_register = AFE440X_PDNCYCLEENDC,
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_table = &afe4403_volatile_table,
> +};
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id afe4403_of_match[] = {
> + { .compatible = "ti,afe4403", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, afe4403_of_match);
> +#endif
> +
> +static int afe4403_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_update_bits(afe->regmap, AFE440X_CONTROL2,
> + AFE440X_CONTROL2_PDN_AFE,
> + AFE440X_CONTROL2_PDN_AFE);
> + if (ret)
> + return ret;
> +
> + ret = regulator_disable(afe->regulator);
> + if (ret) {
> + dev_err(dev, "Unable to disable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int afe4403_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regulator_enable(afe->regulator);
> + if (ret) {
> + dev_err(dev, "Unable to enable regulator\n");
> + return ret;
> + }
> +
> + ret = regmap_update_bits(afe->regmap, AFE440X_CONTROL2,
> + AFE440X_CONTROL2_PDN_AFE, 0);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(afe4403_pm_ops, afe4403_suspend, afe4403_resume);
> +
> +static int afe4403_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct afe4403_data *afe;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + afe = iio_priv(indio_dev);
> + spi_set_drvdata(spi, indio_dev);
> +
> + afe->dev = &spi->dev;
> + afe->spi = spi;
> + afe->irq = spi->irq;
> +
> + afe->regmap = devm_regmap_init_spi(spi, &afe4403_regmap_config);
> + if (IS_ERR(afe->regmap)) {
> + dev_err(afe->dev, "Unable to allocate register map\n");
> + return PTR_ERR(afe->regmap);
> + }
> +
> + afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
> + if (IS_ERR(afe->regulator)) {
> + dev_err(afe->dev, "Unable to get regulator\n");
> + return PTR_ERR(afe->regulator);
> + }
> + ret = regulator_enable(afe->regulator);
> + if (ret) {
> + dev_err(afe->dev, "Unable to enable regulator\n");
> + return ret;
> + }
> +
> + ret = regmap_write(afe->regmap, AFE440X_CONTROL0,
> + AFE440X_CONTROL0_SW_RESET);
> + if (ret) {
> + dev_err(afe->dev, "Unable to reset device\n");
> + return ret;
> + }
> +
> + ret = regmap_multi_reg_write(afe->regmap, afe4403_reg_sequences,
> + ARRAY_SIZE(afe4403_reg_sequences));
> + if (ret) {
> + dev_err(afe->dev, "Unable to set register defaults\n");
> + return ret;
> + }
> +
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->dev.parent = afe->dev;
> + indio_dev->channels = afe4403_channels;
> + indio_dev->num_channels = ARRAY_SIZE(afe4403_channels);
> + indio_dev->name = AFE4403_DRIVER_NAME;
> + indio_dev->info = &afe4403_iio_info;
> +
> + if (afe->irq > 0) {
> + afe->trig = devm_iio_trigger_alloc(afe->dev,
> + "%s-dev%d",
> + indio_dev->name,
> + indio_dev->id);
> + if (!afe->trig) {
> + dev_err(afe->dev, "Unable to allocate IIO trigger\n");
> + return -ENOMEM;
> + }
> +
> + iio_trigger_set_drvdata(afe->trig, indio_dev);
> +
> + afe->trig->ops = &afe4403_trigger_ops;
> + afe->trig->dev.parent = afe->dev;
> +
> + ret = iio_trigger_register(afe->trig);
> + if (ret) {
> + dev_err(afe->dev, "Unable to register IIO trigger\n");
> + return ret;
> + }
> +
> + ret = devm_request_threaded_irq(afe->dev, afe->irq,
> + iio_trigger_generic_data_rdy_poll,
> + NULL, IRQF_ONESHOT,
> + AFE4403_DRIVER_NAME,
> + afe->trig);
> + if (ret) {
> + dev_err(afe->dev, "Unable to request IRQ\n");
> + return ret;
> + }
> + }
> +
> + ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
> + afe4403_trigger_handler, NULL);
> + if (ret) {
> + dev_err(afe->dev, "Unable to setup buffer\n");
> + return ret;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(afe->dev, "Unable to register IIO device\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int afe4403_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct afe4403_data *afe = iio_priv(indio_dev);
> + int ret;
> +
> + iio_device_unregister(indio_dev);
> +
iio_triggered_buffer_cleanup()
> + iio_trigger_unregister(afe->trig);
> +
> + ret = regulator_disable(afe->regulator);
> + if (ret) {
> + dev_err(afe->dev, "Unable to disable regulator\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct spi_device_id afe4403_ids[] = {
> + { "afe4403", 0 },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, afe4403_ids);
> +
> +static struct spi_driver afe4403_spi_driver = {
> + .driver = {
> + .name = AFE4403_DRIVER_NAME,
> + .of_match_table = of_match_ptr(afe4403_of_match),
> + .pm = &afe4403_pm_ops,
> + },
> + .probe = afe4403_probe,
> + .remove = afe4403_remove,
> + .id_table = afe4403_ids,
> +};
> +module_spi_driver(afe4403_spi_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_DESCRIPTION("TI AFE4403 Heart Rate and Pulse Oximeter");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/4] iio: health: Add driver for the TI AFE4404 heart monitor
2016-01-30 14:59 ` [PATCH v4 2/4] iio: health: Add driver for the TI AFE4404 " Jonathan Cameron
@ 2016-02-02 15:50 ` Andrew F. Davis
0 siblings, 0 replies; 6+ messages in thread
From: Andrew F. Davis @ 2016-02-02 15:50 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Dan Murphy, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
Cc: linux-iio, linux-api, devicetree, linux-kernel
On 01/30/2016 08:59 AM, Jonathan Cameron wrote:
> On 25/01/16 17:28, Andrew F. Davis wrote:
>> Add driver for the TI AFE4404 heart rate monitor and pulse oximeter.
>> This device detects reflected LED light fluctuations and presents an ADC
>> value to the user space for further signal processing.
>>
>> Datasheet: http://www.ti.com/product/AFE4404/datasheet
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Hi Andrew,
>
> This changed rather more than I was expecting, but fair enough.
>
> A few bits are missing from remove. Few other little bits inline,
> though mostly those are about readability rather than what the code
> is doing.
>
> I was a little suprised to see no control over whether the trigger is
> enabled or not. I'm trying to get my head around what the result of this will
> be. I guess we'll have interrupts pinging away merilly doing nothing until
> the buffer is attached then the demux will get set up to actually do
> something with the data.
>
> hmm. Might be fine, if I ususual. Normally devices have some means of
> masking the interrupt.
I wasn't sure about this ether, but it doesn't seem to break anything.
>
> Jonathan
>
>> ---
>> .../ABI/testing/sysfs-bus-iio-health-afe440x | 54 ++
>> drivers/iio/health/Kconfig | 19 +-
[...]
>> +
>> +static const struct afe440x_reg_info afe4404_reg_info[] = {
>
> As noted below, I'd find
> [LED1] = AFE440X_REG_INFO(AFE440_LED1VAL...
> more readable.
>
ACK
[...]
>> + int ret;
>> +
>> + iio_device_unregister(indio_dev);
>> +
> Would expect unwinding of the triggered_buffer here.
> iio_triggered_buffer_cleanup()
>
ACK
>> + iio_trigger_unregister(afe->trig);
> This should be protected by a check on afe->irq as for the register in probe.
> There is no protection against afe->trig == NULL in iio_trigger_unregister.
> That is kind of deliberate as it makes sure the probe / remove in drivers
> are balanced unlike here.
>
Makes sense, will add.
[...]
> I'm not overly keen on the hiding fo the indexing. I'd prefer
> this was just the bit {...} and you had the fact it was filling in an
> indexed element explicit where used.
That works.
[...]
> I'd prefix reg_type to avoid possible name clashes in future.
>> +enum reg_type {
>> + SIMPLE,
>> + RESISTANCE,
>> + CAPACITANCE,
>> +};
ACK
>> +
>> +/* this could be made more general for other IIO drivers if needed --------- */
>
> Yes, but lets do that at as a follow up.
>
Sure.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor
2016-01-30 15:10 ` Jonathan Cameron
@ 2016-02-02 17:38 ` Andrew F. Davis
0 siblings, 0 replies; 6+ messages in thread
From: Andrew F. Davis @ 2016-02-02 17:38 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Dan Murphy, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
Cc: linux-iio, linux-api, devicetree, linux-kernel
On 01/30/2016 09:10 AM, Jonathan Cameron wrote:
> On 25/01/16 17:29, Andrew F. Davis wrote:
>> +
>> +static int afe4403_read(struct afe4403_data *afe, unsigned int reg, u32 *val)
>> +{
>> + u8 tx[4] ____cacheline_aligned = {AFE440X_CONTROL0, 0x0, 0x0,
>> + AFE440X_CONTROL0_READ};
> hmm. Can you do this on the stack? Don't think so but maybe I'm wrong..
> The cachline aligned trick relies on the start of the allocation on the
> heap being aligned and then pads to ensure that the element so tagged
> is also aligned appropriately.
I am not sure ether, I think I borrowed this from some example that did
it like this, but I can't find it now. So, I'll change this to be safe.
>> + u8 rx[3];
>> + int ret;
> Even if this were possible, ret is in the same cacheline as tx and rx so
> chaos may well occur.
>
> If you really want to avoid having allocations elsewhere, just use
> spi_write_then_read(afe->spi, tx, 4, NULL, 0) and you should be fine
> as spi_write_then_read uses safe bounce buffers.
spi_write_then_read performs a memcpy to the rx buffer, even though
the length is 0 I believe it is still technically undefined behavior,
but it doesn't seem to cause any issues with the current implementation
so I'll do this.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-02 17:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1453742941-12086-1-git-send-email-afd@ti.com>
[not found] ` <1453742941-12086-5-git-send-email-afd@ti.com>
2016-01-25 20:21 ` [PATCH v4 4/4] iio: health: Add driver for the TI AFE4403 heart monitor Peter Meerwald-Stadler
2016-01-25 20:36 ` Andrew F. Davis
2016-01-30 15:10 ` Jonathan Cameron
2016-02-02 17:38 ` Andrew F. Davis
[not found] ` <1453742941-12086-3-git-send-email-afd@ti.com>
2016-01-30 14:59 ` [PATCH v4 2/4] iio: health: Add driver for the TI AFE4404 " Jonathan Cameron
2016-02-02 15:50 ` Andrew F. Davis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).