> On Feb 5, 2017, at 08:24, Tomasz Duszynski <tduszyns@xxxxxxxxx> wrote: > > Thanks for review! > >> On Sun, Feb 05, 2017 at 04:19:47PM +0100, Peter Meerwald-Stadler wrote: >> >>> This patch adds support for dsm501 and ppd42ns particle sensors. >> >> quick comments below >> G >>> Both sensors work on the same principle. Heater (resistor) heats up air >>> in sensor chamber which induces upward flow. Particles convect up through >>> a light beam provided by internal infra-red LED. Light scattered by >>> particles is picked by photodiode and internal electronics outputs PWM >>> signal. >>> >>> Measuring low time occupancy of the signal during 30s measurement period >>> particles number in cubic meter can be computed. >>> This a SI unit? Also this maybe could be better off in drivers/iio/light... as much I like to have other people in drivers/iio/chemical 😀 >>> Signed-off-by: Tomasz Duszynski <tduszyns@xxxxxxxxx> >>> --- >>> drivers/iio/chemical/Kconfig | 10 ++ >>> drivers/iio/chemical/Makefile | 1 + >>> drivers/iio/chemical/dsm501.c | 231 ++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 242 insertions(+) >>> create mode 100644 drivers/iio/chemical/dsm501.c >>> >>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >>> index cea7f9857a1f..986d612aa77f 100644 >>> --- a/drivers/iio/chemical/Kconfig >>> +++ b/drivers/iio/chemical/Kconfig >>> @@ -21,6 +21,16 @@ config ATLAS_PH_SENSOR >>> To compile this driver as module, choose M here: the >>> module will be called atlas-ph-sensor. >>> >>> +config DSM501 >>> + tristate "Samyoung DSM501 particle sensor" >>> + depends on GPIOLIB >>> + help >>> + Say yes here to build support for the Samyoung DSM501 >>> + particle sensor. >>> + >>> + To compile this driver as a module, choose M here: the module >>> + will be called dsm501. >>> + >>> config IAQCORE >>> tristate "AMS iAQ-Core VOC sensors" >>> depends on I2C >>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile >>> index b02202b41289..76f50ff8ba7d 100644 >>> --- a/drivers/iio/chemical/Makefile >>> +++ b/drivers/iio/chemical/Makefile >>> @@ -4,5 +4,6 @@ >>> >>> # When adding new entries keep the list in alphabetical order >>> obj-$(CONFIG_ATLAS_PH_SENSOR) += atlas-ph-sensor.o >>> +obj-$(CONFIG_DSM501) += dsm501.o >>> obj-$(CONFIG_IAQCORE) += ams-iaq-core.o >>> obj-$(CONFIG_VZ89X) += vz89x.o >>> diff --git a/drivers/iio/chemical/dsm501.c b/drivers/iio/chemical/dsm501.c >>> new file mode 100644 >>> index 000000000000..013f6b3bfd48 >>> --- /dev/null >>> +++ b/drivers/iio/chemical/dsm501.c >>> @@ -0,0 +1,231 @@ >>> +/* >>> + * Samyoung DSM501 particle sensor driver >>> + * >>> + * Copyright (C) 2017 Tomasz Duszynski <tduszyns@xxxxxxxxx> >>> + * >>> + * Datasheets: >>> + * http://www.samyoungsnc.com/products/3-1%20Specification%20DSM501.pdf >>> + * http://wiki.timelab.org/images/f/f9/PPD42NS.pdf >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; either version 2 of the License, or >>> + * (at your option) any later version. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + */ >>> + >>> +#include <linux/delay.h> >>> +#include <linux/gpio/consumer.h> >>> +#include <linux/iio/iio.h> >>> +#include <linux/init.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/module.h> >>> +#include <linux/mutex.h> >>> +#include <linux/of_device.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/timekeeping.h> >>> + >>> +#define DSM501_DRV_NAME "dsm501" >>> +#define DSM501_IRQ_NAME "dsm501_irq" >> >> replace tab by space > Ack. >> >>> + >>> +#define DSM501_DEFAULT_MEASUREMENT_TIME 30 /* seconds */ >>> + >>> +struct dsm501_data { >>> + ktime_t ts; >>> + ktime_t low_time; >>> + ktime_t meas_time; >>> + >>> + int irq; >>> + struct gpio_desc *gpio; >>> + >>> + struct mutex lock; >>> + >>> + int (*number_concentration)(struct dsm501_data *data); >>> +}; >>> + >>> +/* >>> + * Series of data points in Fig. 8-3 (Low Ratio vs Particle) >>> + * can be approximated by following polynomials: >>> + * >>> + * p(r) = 0 (undefined) for r < 4 >>> + * p(r) = 2353564.2r - 4373814.7 for 4 <= r < 20 >>> + * p(r) = 4788112.4r - 53581390 for r >= 20 >>> + * >>> + * Note: Result is in pcs/m3. To convert to pcs/0.01cf multiply >>> + * by 0.0002831685. >> >> is cf needed? > No. I've put that sidenote so that others looking at figures in > datahseets won't scratch their heads why units here are pcs/m3 and > pcs/0.01cf there. Anyway I am happy to drop that part. >> >>> + */ >>> +static int dsm501_number_concentartion(struct dsm501_data *data) >>> +{ >>> + s64 retval = 0, r = div64_s64(ktime_to_ns(data->low_time) * 100, >>> + ktime_to_ns(data->meas_time)); >>> + >>> + if (r >= 4 && r < 20) >>> + retval = 23535642 * r - 43738147; >>> + else if (r >= 20) >>> + retval = 47881124 * r - 535813900; >>> + >>> + return div_s64(retval, 10); >>> +} >>> + >>> +/* >>> + * Series of data points in Fig. 2 (Lo Pulse Occupancy Time vs Concentration) >>> + * can be approximated by following polynomial: >>> + * >>> + * p(r) = 3844.2r^3 - 16201.3r^2 + 1848746.1r + 52497.2 >>> + * >>> + * Note: Result is in pcs/m3. To convert to pcs/0.01cf multiply >>> + * by 0.0002831685. >>> + */ >>> +static int ppd42ns_number_concentration(struct dsm501_data *data) >>> +{ >>> + s64 retval, r3, r2, r = div64_s64(ktime_to_ns(data->low_time) * 100, >>> + ktime_to_ns(data->meas_time)); >>> + >>> + r2 = r * r; >>> + r3 = r2 * r; >>> + >>> + retval = 38442 * r3; >>> + retval -= 162013 * r2; >>> + retval += 18487461 * r; >>> + retval += 524972; >>> + >>> + return div_s64(retval, 10); >>> +} >>> + >>> +static irqreturn_t dsm501_irq(int irq, void *dev_id) >>> +{ >>> + struct dsm501_data *data = iio_priv(dev_id); >>> + int val = gpiod_get_value(data->gpio); >>> + ktime_t dt, ts = ktime_get(); >>> + >>> + if (ktime_to_ns(data->ts) == 0) { >>> + data->ts = ts; >>> + data->low_time = ktime_set(0, 0); >>> + } >>> + >>> + if (val) { >>> + dt = ktime_sub(ts, data->ts); >>> + data->low_time = ktime_add(data->low_time, dt); >>> + } else { >>> + data->ts = ts; >>> + } >> >> {} needed? > According to CodingStyle yes. {} could be dropped if there was single > statement under if (val) { ... >> >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static int dsm501_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, int *val, >>> + int *val2, long mask) >>> +{ >>> + struct dsm501_data *data = iio_priv(indio_dev); >>> + struct device *dev = indio_dev->dev.parent; >>> + unsigned long irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING; >>> + int ret; >>> + >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_PROCESSED: >>> + mutex_lock(&data->lock); >>> + data->ts = ktime_set(0, 0); >>> + >>> + ret = devm_request_irq(dev, data->irq, dsm501_irq, irqflags, >>> + DSM501_IRQ_NAME, indio_dev); >> >> why devm_()? if the irq is explicitly freed below? >> requesting the irq for every measurement seems weird even if it only >> happens every 30s > Ack. >> >>> + if (ret) { >>> + dev_err(dev, "Failed to request interrupt %d\n", data->irq); >>> + mutex_unlock(&data->lock); >>> + return ret; >>> + } >>> + >>> + msleep_interruptible(ktime_to_ms(data->meas_time)); >>> + devm_free_irq(dev, data->irq, indio_dev); >>> + >>> + *val = data->number_concentration(data); >>> + mutex_unlock(&data->lock); >>> + >>> + return IIO_VAL_INT; >>> + default: >>> + return -EINVAL; >>> + } >>> +} >>> + >>> +static const struct iio_info dsm501_info = { >>> + .driver_module = THIS_MODULE, >>> + .read_raw = dsm501_read_raw, >>> +}; >>> + >>> +static const struct iio_chan_spec dsm501_channels[] = { >>> + { >>> + .type = IIO_NUMBERCONCENTRATION, >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), >>> + }, >>> +}; >>> + >>> +static int dsm501_probe(struct platform_device *pdev) >>> +{ >>> + struct dsm501_data *data; >>> + struct iio_dev *indio_dev; >>> + struct device *dev = &pdev->dev; >>> + >>> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*data)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + data = iio_priv(indio_dev); >>> + platform_set_drvdata(pdev, indio_dev); >>> + >>> + data->gpio = devm_gpiod_get_index(dev, NULL, 0, GPIOD_IN); >>> + if (IS_ERR(data->gpio)) { >>> + dev_err(dev, "Failed to get GPIO\n"); >>> + return PTR_ERR(data->gpio); >>> + } >>> + >>> + data->irq = gpiod_to_irq(data->gpio); >>> + if (data->irq < 0) { >>> + dev_err(dev, "GPIO has no interrupt\n"); >>> + return data->irq; >>> + } >>> + >>> + data->meas_time = ktime_set(DSM501_DEFAULT_MEASUREMENT_TIME, 0); >>> + data->number_concentration = of_device_get_match_data(dev); >>> + mutex_init(&data->lock); >>> + >>> + indio_dev->name = DSM501_DRV_NAME; >>> + indio_dev->dev.parent = dev; >>> + indio_dev->info = &dsm501_info; >>> + indio_dev->modes = INDIO_DIRECT_MODE; >>> + indio_dev->channels = dsm501_channels; >>> + indio_dev->num_channels = ARRAY_SIZE(dsm501_channels); >>> + >>> + return devm_iio_device_register(&pdev->dev, indio_dev); >>> +} >>> + >>> +static const struct of_device_id dsm501_id[] = { >>> + { >>> + .compatible = "samyoung,dsm501", >>> + .data = dsm501_number_concentartion, >> >> type: concentration > Are you refering to function naming here? *_number_concentration was named > after returned physical quantity. >> >>> + }, >>> + { >>> + .compatible = "shinyei,ppd42ns", >>> + .data = ppd42ns_number_concentration, >>> + }, >>> + { } >>> +}; >>> +MODULE_DEVICE_TABLE(of, dsm501_id); >>> + >>> +static struct platform_driver dsm501_driver = { >>> + .driver = { >>> + .name = DSM501_DRV_NAME, >>> + .of_match_table = of_match_ptr(dsm501_id) >>> + }, >>> + .probe = dsm501_probe >>> +}; >>> +module_platform_driver(dsm501_driver); >>> + >>> +MODULE_AUTHOR("Tomasz Duszynski <tduszyns@xxxxxxxxx>"); >>> +MODULE_DESCRIPTION("Samyoung DSM501 particle sensor driver"); >>> +MODULE_LICENSE("GPL v2"); >>> >> >> -- >> >> Peter Meerwald-Stadler >> +43-664-2444418 (mobile) > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
