[PATCH 1/2] iio: add IIO_DISTANCE type

[PATCH 1/2] iio: add IIO_DISTANCE type

[Matt Ranostay]

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 drivers/iio/industrialio-core.c | 1 +
 include/linux/iio/types.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index acc911a..ac999ab 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -70,6 +70,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_CCT] = "cct",
 	[IIO_PRESSURE] = "pressure",
 	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
+	[IIO_DISTANCE] = "distance",
 };
 
 static const char * const iio_modifier_names[] = {
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 084d882..675c2d8 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -30,6 +30,7 @@ enum iio_chan_type {
 	IIO_CCT,
 	IIO_PRESSURE,
 	IIO_HUMIDITYRELATIVE,
+	IIO_DISTANCE,
 };
 
 enum iio_modifier {
-- 
1.8.3.2

[PATCH 2/2] iio: Add AS3935 lightning sensor support

[Matt Ranostay]

AS3935 chipset can detect lightning strikes and reports those
back as events and the esimated distance to the storm.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../devicetree/bindings/iio/distance/as3935.txt    |  25 ++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/distance/Kconfig                       |  19 +
 drivers/iio/distance/Makefile                      |   6 +
 drivers/iio/distance/as3935.c                      | 459 +++++++++++++++++++++
 6 files changed, 511 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/distance/as3935.txt
 create mode 100644 drivers/iio/distance/Kconfig
 create mode 100644 drivers/iio/distance/Makefile
 create mode 100644 drivers/iio/distance/as3935.c

diff --git a/Documentation/devicetree/bindings/iio/distance/as3935.txt b/Documentation/devicetree/bindings/iio/distance/as3935.txt
new file mode 100644
index 0000000..af35827
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/distance/as3935.txt
@@ -0,0 +1,25 @@
+Austrian Microsystems AS3935 Franklin lightning sensor device driver
+
+Required properties:
+	- compatible: must be "ams,as3935"
+	- reg: SPI chip select number for the device
+	- spi-max-frequency: Max SPI frequency to use
+	- spi-cpha: SPI Mode 1
+	- gpios: GPIO input of interrupt line from IRQ pin of AS3935 IC
+
+Optional properties:
+	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 15.
+	  Range of 0 to 120 pF, 8pF steps. This will require using the calibration
+	  data from the manufacturer.
+
+
+Example:
+
+		as3935@0 {
+			compatible = "ams,as3935";
+			reg = <0>;
+			spi-max-frequency = <100000>;
+			spi-cpha;
+			ams,tune-cap = /bits/ 8 <10>;
+			gpios = <&gpio1 16 10>;
+		};
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 5dd0e12..5164a68 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -63,6 +63,7 @@ source "drivers/iio/adc/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/dac/Kconfig"
+source "drivers/iio/distance/Kconfig"
 source "drivers/iio/frequency/Kconfig"
 source "drivers/iio/gyro/Kconfig"
 source "drivers/iio/humidity/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 887d390..1574731 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -16,6 +16,7 @@ obj-y += adc/
 obj-y += amplifiers/
 obj-y += common/
 obj-y += dac/
+obj-y += distance/
 obj-y += gyro/
 obj-y += frequency/
 obj-y += humidity/
diff --git a/drivers/iio/distance/Kconfig b/drivers/iio/distance/Kconfig
new file mode 100644
index 0000000..753352c
--- /dev/null
+++ b/drivers/iio/distance/Kconfig
@@ -0,0 +1,19 @@
+#
+# Distance sensors
+#
+
+menu "Lightning sensors"
+
+config AS3935
+	tristate "AS3935 Franklin lightning sensor"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	depends on SPI
+	help
+	 If you say yes here you get support for the Austrian Microsystems
+	 AS3935 lightning detection sensor.
+
+	 This driver can also be built as a module. If so, the module
+	 will be called as3935
+
+endmenu
diff --git a/drivers/iio/distance/Makefile b/drivers/iio/distance/Makefile
new file mode 100644
index 0000000..f63b70d
--- /dev/null
+++ b/drivers/iio/distance/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for IIO distance sensors
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AS3935)		+= as3935.o
diff --git a/drivers/iio/distance/as3935.c b/drivers/iio/distance/as3935.c
new file mode 100644
index 0000000..1a8f48d
--- /dev/null
+++ b/drivers/iio/distance/as3935.c
@@ -0,0 +1,459 @@
+/*
+ * as3935.c - Support for AS3935 Franklin lightning sensor
+ *
+ * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/workqueue.h>
+#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_gpio.h>
+
+
+#define AS3935_AFE_GAIN		0x00
+#define AS3935_AFE_MASK		0x3F
+#define AS3935_AFE_GAIN_MAX	0x1F
+
+#define AS3935_INT		0x03
+#define AS3935_INT_MASK		0x07
+#define AS3935_DATA		0x07
+#define AS3935_DATA_MASK	0x1F
+
+#define AS3935_TUNE_CAP		0x08
+#define AS3935_CALIBRATE	0x3D
+
+#define AS3935_WRITE_DATA	(0x1 << 15)
+#define AS3935_READ_DATA	(0x1 << 14)
+#define AS3935_ADDRESS(x)	(x << 8)
+
+#define AS3935_EVENT_INT	(1<<3)
+#define AS3935_NOISE_INT	(1<<0)
+
+struct as3935_state {
+	struct spi_device *spi;
+	struct mutex lock;
+
+	struct iio_trigger *trig;
+	struct delayed_work work;
+
+	u8 tune_cap;
+};
+
+static const struct iio_chan_spec as3935_channels[] = {
+	{
+		.type           = IIO_DISTANCE,
+		.channel        = 0,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW),
+		.scan_index     = 0,
+		.scan_type = {
+			.sign           = 'u',
+			.realbits       = 6,
+			.storagebits    = 6,
+		},
+		.modified = 0,
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static const unsigned long as3935_available_scan_masks[] = { 0x01, 0x0 };
+
+static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
+{
+	u8 tx, rx;
+	int ret;
+
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &tx,
+			.bits_per_word = 8,
+			.len = 1,
+		}, {
+			.rx_buf = &rx,
+			.bits_per_word = 8,
+			.len = 1,
+		},
+	};
+
+	mutex_lock(&st->lock);
+	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	mutex_unlock(&st->lock);
+
+	*val = rx;
+
+	return ret;
+};
+
+static int as3935_write(struct as3935_state *st,
+				unsigned int reg,
+				unsigned int val)
+{
+	u8 buf[2];
+	int ret;
+
+	mutex_lock(&st->lock);
+	buf[0] = AS3935_WRITE_DATA | AS3935_ADDRESS(reg) >> 8;
+	buf[1] = val;
+
+	ret = spi_write(st->spi, (u8 *) &buf, 2);
+	mutex_unlock(&st->lock);
+
+	return ret;
+};
+
+static ssize_t gain_boost_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val = (val & AS3935_AFE_MASK) >> 1;
+
+	return sprintf(buf, "%d\n", val);
+};
+
+static ssize_t gain_boost_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul((const char *) buf, 10, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val > AS3935_AFE_GAIN_MAX)
+		return -EINVAL;
+
+	as3935_write(st, AS3935_AFE_GAIN, val << 1);
+
+	return len;
+};
+
+static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
+	gain_boost_show, gain_boost_store, 0);
+
+
+static struct attribute *as3935_attributes[] = {
+	&iio_dev_attr_gain_boost.dev_attr.attr,
+	NULL,
+};
+
+static struct attribute_group as3935_attribute_group = {
+	.attrs = as3935_attributes,
+};
+
+static int as3935_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	if (m == IIO_CHAN_INFO_RAW) {
+		int ret;
+		*val = 0;
+		ret = as3935_read(st, AS3935_DATA, val2);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info as3935_info = {
+	.driver_module = THIS_MODULE,
+	.attrs = &as3935_attribute_group,
+	.read_raw = &as3935_read_raw,
+};
+
+static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
+	.postenable = &iio_triggered_buffer_postenable,
+	.predisable = &iio_triggered_buffer_predisable,
+};
+
+static irqreturn_t as3935_trigger_handler(int irq, void *private)
+{
+	struct iio_poll_func *pf = private;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_DATA, &val);
+	if (ret)
+		goto err_read;
+	val &= AS3935_DATA_MASK;
+	iio_push_to_buffers_with_timestamp(indio_dev, &val, iio_get_time_ns());
+
+	iio_trigger_notify_done(indio_dev->trig);
+err_read:
+
+	return IRQ_HANDLED;
+};
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static void as3935_event_work(struct work_struct *work)
+{
+	struct as3935_state *st;
+	struct spi_device *spi;
+	int val;
+
+	st = container_of(work, struct as3935_state, work.work);
+	spi = st->spi;
+
+	as3935_read(st, AS3935_INT, &val);
+	val &= AS3935_INT_MASK;
+
+	switch (val) {
+	case AS3935_EVENT_INT:
+		iio_trigger_poll(st->trig, 0);
+		break;
+	case AS3935_NOISE_INT:
+		dev_warn(&spi->dev, "noise level is too high");
+		break;
+	}
+};
+
+static irqreturn_t as3935_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	cancel_delayed_work(&st->work);
+	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
+	return IRQ_HANDLED;
+}
+
+static void calibrate_as3935(struct as3935_state *st)
+{
+	/* mask disturber interrupt bit */
+	as3935_write(st, AS3935_INT, 1 << 5);
+
+	as3935_write(st, AS3935_CALIBRATE, 0x96);
+	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
+
+	mdelay(2);
+	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val |= 0x01;
+
+	return as3935_write(st, AS3935_AFE_GAIN, val);
+}
+
+static int as3935_resume(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val &= ~1;
+
+	return as3935_write(st, AS3935_AFE_GAIN, val);
+}
+#else
+#define as3935_suspend	NULL
+#define as3935_resume	NULL
+#endif
+
+static int as3935_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct iio_trigger *trig;
+	struct as3935_state *st;
+	struct device_node *np = spi->dev.of_node;
+	int gpio;
+	int irq;
+	int ret;
+
+	/* Grab the GPIO to use for lightning event interrupt */
+	if (np)
+		gpio = of_get_gpio(spi->dev.of_node, 0);
+	else {
+		dev_err(&spi->dev, "unable to get interrupt gpio\n");
+		return -EINVAL;
+	}
+
+	/* GPIO event setup */
+	ret = devm_gpio_request(&spi->dev, gpio, "as3935");
+	if (ret) {
+		dev_err(&spi->dev, "failed to request GPIO %u\n", gpio);
+		return ret;
+	}
+
+	ret = gpio_direction_input(gpio);
+	if (ret) {
+		dev_err(&spi->dev, "failed to set pin direction\n");
+		return -EINVAL;
+	}
+
+	/* IRQ setup */
+	irq = gpio_to_irq(gpio);
+	if (irq < 0) {
+		dev_err(&spi->dev, "failed to map GPIO to IRQ: %d\n", irq);
+		return -EINVAL;
+	}
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->tune_cap = 0;
+	spi_set_drvdata(spi, indio_dev);
+	mutex_init(&st->lock);
+	INIT_DELAYED_WORK(&st->work, as3935_event_work);
+
+	of_property_read_u8(np, "ams,tune-cap", &st->tune_cap);
+	if (st->tune_cap > 15) {
+		dev_err(&spi->dev,
+			"wrong tune_cap setting of %d\n", st->tune_cap);
+		return -EINVAL;
+	}
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->channels = as3935_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = as3935_available_scan_masks;
+	indio_dev->info = &as3935_info;
+
+	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+				      indio_dev->name, indio_dev->id);
+
+	if (!trig)
+		return -ENOMEM;
+
+	st->trig = trig;
+	trig->dev.parent = indio_dev->dev.parent;
+	iio_trigger_set_drvdata(trig, indio_dev);
+	trig->ops = &iio_interrupt_trigger_ops;
+
+	ret = iio_trigger_register(trig);
+	if (ret) {
+		dev_err(&spi->dev, "failed to register trigger\n");
+		return ret;
+	}
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					&as3935_trigger_handler,
+					&iio_triggered_buffer_setup_ops);
+
+	if (ret) {
+		dev_err(&spi->dev, "cannot setup iio trigger\n");
+		return -EINVAL;
+	}
+
+	calibrate_as3935(st);
+
+	ret = devm_request_irq(&spi->dev, irq,
+				&as3935_interrupt_handler,
+				IRQF_TRIGGER_RISING,
+				dev_name(&spi->dev),
+				indio_dev);
+
+	if (ret) {
+		dev_err(&spi->dev, "unable to request irq\n");
+		goto uninit_buffer;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&spi->dev, "unable to register device\n");
+		goto uninit_buffer;
+	}
+
+	return 0;
+
+uninit_buffer:
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return ret;
+};
+
+static int as3935_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	return 0;
+};
+
+static const struct spi_device_id as3935_id[] = {
+	{"as3935", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, as3935_id);
+
+static struct spi_driver as3935_driver = {
+	.driver = {
+		.name	= "as3935",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= as3935_probe,
+	.remove		= as3935_remove,
+	.id_table	= as3935_id,
+	.suspend	= as3935_suspend,
+	.resume		= as3935_resume,
+};
+module_spi_driver(as3935_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("AS3935 lightning sensor");
+MODULE_LICENSE("GPL");
-- 
1.8.3.2

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Peter Meerwald]

> AS3935 chipset can detect lightning strikes and reports those
> back as events and the esimated distance to the storm.

pretty cool :)

the distance thing should be documented in 
Documentation/ABI/testing/sysfs-bus-iio -- which unit (meters?) for 
example

sysfs attributes such as boost should be documented as well; or better use 
a writable scale attribute if possible

nitpicking inline

> ---
>  .../devicetree/bindings/iio/distance/as3935.txt    |  25 ++
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/distance/Kconfig                       |  19 +
>  drivers/iio/distance/Makefile                      |   6 +
>  drivers/iio/distance/as3935.c                      | 459 +++++++++++++++++++++
>  6 files changed, 511 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/distance/as3935.txt
>  create mode 100644 drivers/iio/distance/Kconfig
>  create mode 100644 drivers/iio/distance/Makefile
>  create mode 100644 drivers/iio/distance/as3935.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/distance/as3935.txt b/Documentation/devicetree/bindings/iio/distance/as3935.txt
> new file mode 100644
> index 0000000..af35827
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/distance/as3935.txt
> @@ -0,0 +1,25 @@
> +Austrian Microsystems AS3935 Franklin lightning sensor device driver
> +
> +Required properties:
> +	- compatible: must be "ams,as3935"
> +	- reg: SPI chip select number for the device
> +	- spi-max-frequency: Max SPI frequency to use
> +	- spi-cpha: SPI Mode 1
> +	- gpios: GPIO input of interrupt line from IRQ pin of AS3935 IC
> +
> +Optional properties:
> +	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 15.
> +	  Range of 0 to 120 pF, 8pF steps. This will require using the calibration
> +	  data from the manufacturer.
> +
> +
> +Example:
> +
> +		as3935@0 {
> +			compatible = "ams,as3935";
> +			reg = <0>;
> +			spi-max-frequency = <100000>;
> +			spi-cpha;
> +			ams,tune-cap = /bits/ 8 <10>;
> +			gpios = <&gpio1 16 10>;
> +		};
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 5dd0e12..5164a68 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -63,6 +63,7 @@ source "drivers/iio/adc/Kconfig"
>  source "drivers/iio/amplifiers/Kconfig"
>  source "drivers/iio/common/Kconfig"
>  source "drivers/iio/dac/Kconfig"
> +source "drivers/iio/distance/Kconfig"
>  source "drivers/iio/frequency/Kconfig"
>  source "drivers/iio/gyro/Kconfig"
>  source "drivers/iio/humidity/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 887d390..1574731 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -16,6 +16,7 @@ obj-y += adc/
>  obj-y += amplifiers/
>  obj-y += common/
>  obj-y += dac/
> +obj-y += distance/
>  obj-y += gyro/
>  obj-y += frequency/
>  obj-y += humidity/
> diff --git a/drivers/iio/distance/Kconfig b/drivers/iio/distance/Kconfig
> new file mode 100644
> index 0000000..753352c
> --- /dev/null
> +++ b/drivers/iio/distance/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Distance sensors
> +#
> +
> +menu "Lightning sensors"
> +
> +config AS3935
> +	tristate "AS3935 Franklin lightning sensor"
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	depends on SPI
> +	help
> +	 If you say yes here you get support for the Austrian Microsystems
> +	 AS3935 lightning detection sensor.
> +
> +	 This driver can also be built as a module. If so, the module
> +	 will be called as3935
> +
> +endmenu
> diff --git a/drivers/iio/distance/Makefile b/drivers/iio/distance/Makefile
> new file mode 100644
> index 0000000..f63b70d
> --- /dev/null
> +++ b/drivers/iio/distance/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for IIO distance sensors
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_AS3935)		+= as3935.o
> diff --git a/drivers/iio/distance/as3935.c b/drivers/iio/distance/as3935.c
> new file mode 100644
> index 0000000..1a8f48d
> --- /dev/null
> +++ b/drivers/iio/distance/as3935.c
> @@ -0,0 +1,459 @@
> +/*
> + * as3935.c - Support for AS3935 Franklin lightning sensor
> + *
> + * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/workqueue.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/of_gpio.h>
> +
> +
> +#define AS3935_AFE_GAIN		0x00
> +#define AS3935_AFE_MASK		0x3F
> +#define AS3935_AFE_GAIN_MAX	0x1F
> +
> +#define AS3935_INT		0x03
> +#define AS3935_INT_MASK		0x07
> +#define AS3935_DATA		0x07
> +#define AS3935_DATA_MASK	0x1F
> +
> +#define AS3935_TUNE_CAP		0x08
> +#define AS3935_CALIBRATE	0x3D
> +
> +#define AS3935_WRITE_DATA	(0x1 << 15)
> +#define AS3935_READ_DATA	(0x1 << 14)
> +#define AS3935_ADDRESS(x)	(x << 8)
> +
> +#define AS3935_EVENT_INT	(1<<3)
> +#define AS3935_NOISE_INT	(1<<0)
> +
> +struct as3935_state {
> +	struct spi_device *spi;
> +	struct mutex lock;
> +
> +	struct iio_trigger *trig;
> +	struct delayed_work work;
> +
> +	u8 tune_cap;
> +};
> +
> +static const struct iio_chan_spec as3935_channels[] = {
> +	{
> +		.type           = IIO_DISTANCE,
> +		.channel        = 0,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW),
> +		.scan_index     = 0,
> +		.scan_type = {
> +			.sign           = 'u',
> +			.realbits       = 6,
> +			.storagebits    = 6,
> +		},
> +		.modified = 0,

channel and modified are not needed
storagebits should be a multiple of 8

> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
> +static const unsigned long as3935_available_scan_masks[] = { 0x01, 0x0 };

is this needed? there is just one channel

> +
> +static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
> +{
> +	u8 tx, rx;
> +	int ret;
> +
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = &tx,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		}, {
> +			.rx_buf = &rx,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		},
> +	};
> +
> +	mutex_lock(&st->lock);
> +	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +	mutex_unlock(&st->lock);
> +
> +	*val = rx;
> +
> +	return ret;
> +};
> +
> +static int as3935_write(struct as3935_state *st,
> +				unsigned int reg,
> +				unsigned int val)
> +{
> +	u8 buf[2];
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	buf[0] = AS3935_WRITE_DATA | AS3935_ADDRESS(reg) >> 8;
> +	buf[1] = val;
> +
> +	ret = spi_write(st->spi, (u8 *) &buf, 2);
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +};
> +
> +static ssize_t gain_boost_show(struct device *dev,

not prefixed with as3935_

> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val = (val & AS3935_AFE_MASK) >> 1;
> +
> +	return sprintf(buf, "%d\n", val);
> +};
> +
> +static ssize_t gain_boost_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul((const char *) buf, 10, &val);
> +	if (ret)
> +		return -EINVAL;
> +
> +	if (val > AS3935_AFE_GAIN_MAX)
> +		return -EINVAL;
> +
> +	as3935_write(st, AS3935_AFE_GAIN, val << 1);
> +
> +	return len;
> +};
> +
> +static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
> +	gain_boost_show, gain_boost_store, 0);
> +
> +
> +static struct attribute *as3935_attributes[] = {
> +	&iio_dev_attr_gain_boost.dev_attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group as3935_attribute_group = {
> +	.attrs = as3935_attributes,
> +};
> +
> +static int as3935_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
> +	if (m == IIO_CHAN_INFO_RAW) {
> +		int ret;
> +		*val = 0;
> +		ret = as3935_read(st, AS3935_DATA, val2);

huh, this always returns 0?
only val is return if IIO_VAL_INT

> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info as3935_info = {
> +	.driver_module = THIS_MODULE,
> +	.attrs = &as3935_attribute_group,
> +	.read_raw = &as3935_read_raw,
> +};
> +
> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
> +	.postenable = &iio_triggered_buffer_postenable,
> +	.predisable = &iio_triggered_buffer_predisable,
> +};
> +
> +static irqreturn_t as3935_trigger_handler(int irq, void *private)
> +{
> +	struct iio_poll_func *pf = private;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_DATA, &val);
> +	if (ret)
> +		goto err_read;
> +	val &= AS3935_DATA_MASK;
> +	iio_push_to_buffers_with_timestamp(indio_dev, &val, iio_get_time_ns());
> +
> +	iio_trigger_notify_done(indio_dev->trig);

iio_trigger_notify_done() must always be called, most past err_read

> +err_read:
> +
> +	return IRQ_HANDLED;
> +};
> +
> +static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
> +	.owner = THIS_MODULE,
> +};
> +
> +static void as3935_event_work(struct work_struct *work)
> +{
> +	struct as3935_state *st;
> +	struct spi_device *spi;
> +	int val;
> +
> +	st = container_of(work, struct as3935_state, work.work);
> +	spi = st->spi;
> +
> +	as3935_read(st, AS3935_INT, &val);
> +	val &= AS3935_INT_MASK;
> +
> +	switch (val) {
> +	case AS3935_EVENT_INT:
> +		iio_trigger_poll(st->trig, 0);
> +		break;
> +	case AS3935_NOISE_INT:
> +		dev_warn(&spi->dev, "noise level is too high");
> +		break;
> +	}
> +};
> +
> +static irqreturn_t as3935_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
> +	cancel_delayed_work(&st->work);
> +	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
> +	return IRQ_HANDLED;
> +}
> +
> +static void calibrate_as3935(struct as3935_state *st)
> +{
> +	/* mask disturber interrupt bit */
> +	as3935_write(st, AS3935_INT, 1 << 5);
> +
> +	as3935_write(st, AS3935_CALIBRATE, 0x96);
> +	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
> +
> +	mdelay(2);
> +	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val |= 0x01;
> +
> +	return as3935_write(st, AS3935_AFE_GAIN, val);
> +}
> +
> +static int as3935_resume(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val &= ~1;
> +
> +	return as3935_write(st, AS3935_AFE_GAIN, val);
> +}
> +#else
> +#define as3935_suspend	NULL
> +#define as3935_resume	NULL
> +#endif
> +
> +static int as3935_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct iio_trigger *trig;
> +	struct as3935_state *st;
> +	struct device_node *np = spi->dev.of_node;
> +	int gpio;
> +	int irq;
> +	int ret;
> +
> +	/* Grab the GPIO to use for lightning event interrupt */
> +	if (np)
> +		gpio = of_get_gpio(spi->dev.of_node, 0);
> +	else {
> +		dev_err(&spi->dev, "unable to get interrupt gpio\n");
> +		return -EINVAL;
> +	}
> +
> +	/* GPIO event setup */
> +	ret = devm_gpio_request(&spi->dev, gpio, "as3935");
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to request GPIO %u\n", gpio);
> +		return ret;
> +	}
> +
> +	ret = gpio_direction_input(gpio);
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to set pin direction\n");
> +		return -EINVAL;
> +	}
> +
> +	/* IRQ setup */
> +	irq = gpio_to_irq(gpio);
> +	if (irq < 0) {
> +		dev_err(&spi->dev, "failed to map GPIO to IRQ: %d\n", irq);
> +		return -EINVAL;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->tune_cap = 0;
> +	spi_set_drvdata(spi, indio_dev);
> +	mutex_init(&st->lock);
> +	INIT_DELAYED_WORK(&st->work, as3935_event_work);
> +
> +	of_property_read_u8(np, "ams,tune-cap", &st->tune_cap);
> +	if (st->tune_cap > 15) {
> +		dev_err(&spi->dev,
> +			"wrong tune_cap setting of %d\n", st->tune_cap);
> +		return -EINVAL;
> +	}
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->channels = as3935_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->available_scan_masks = as3935_available_scan_masks;
> +	indio_dev->info = &as3935_info;
> +
> +	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> +				      indio_dev->name, indio_dev->id);
> +
> +	if (!trig)
> +		return -ENOMEM;
> +
> +	st->trig = trig;
> +	trig->dev.parent = indio_dev->dev.parent;
> +	iio_trigger_set_drvdata(trig, indio_dev);
> +	trig->ops = &iio_interrupt_trigger_ops;
> +
> +	ret = iio_trigger_register(trig);
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to register trigger\n");
> +		return ret;
> +	}
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					&as3935_trigger_handler,
> +					&iio_triggered_buffer_setup_ops);
> +
> +	if (ret) {
> +		dev_err(&spi->dev, "cannot setup iio trigger\n");
> +		return -EINVAL;
> +	}
> +
> +	calibrate_as3935(st);
> +
> +	ret = devm_request_irq(&spi->dev, irq,
> +				&as3935_interrupt_handler,
> +				IRQF_TRIGGER_RISING,
> +				dev_name(&spi->dev),
> +				indio_dev);
> +
> +	if (ret) {
> +		dev_err(&spi->dev, "unable to request irq\n");
> +		goto uninit_buffer;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "unable to register device\n");
> +		goto uninit_buffer;
> +	}
> +
> +	return 0;
> +
> +uninit_buffer:
> +	iio_triggered_buffer_cleanup(indio_dev);

iio_trigger_unregister()?

> +
> +	return ret;
> +};
> +
> +static int as3935_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +

iio_trigger_unregister()?
iio_triggered_buffer_cleanup()?

> +	iio_device_unregister(indio_dev);
> +	return 0;
> +};
> +
> +static const struct spi_device_id as3935_id[] = {
> +	{"as3935", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, as3935_id);
> +
> +static struct spi_driver as3935_driver = {
> +	.driver = {
> +		.name	= "as3935",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= as3935_probe,
> +	.remove		= as3935_remove,
> +	.id_table	= as3935_id,
> +	.suspend	= as3935_suspend,
> +	.resume		= as3935_resume,
> +};
> +module_spi_driver(as3935_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("AS3935 lightning sensor");
> +MODULE_LICENSE("GPL");
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Jonathan Cameron]

On January 30, 2014 10:37:19 AM GMT+00:00, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>
>> AS3935 chipset can detect lightning strikes and reports those
>> back as events and the esimated distance to the storm.
>
>pretty cool :)
Indeed!
>
>the distance thing should be documented in 
>Documentation/ABI/testing/sysfs-bus-iio -- which unit (meters?) for 
>example
>
>sysfs attributes such as boost should be documented as well; or better
>use 
>a writable scale attribute if possible
>
>nitpicking inline

Couple of quick comments for now.

The driver should not be doing the gpio irq setup. There is no reason it needs to be a gpio that I can see. DT should hence also have interrupt not gpio.

Why all the pinctrl headers.

Curious question... What controls max spi fre
q? I would have thought that was fixed for the part hence not needed in device tree?

Do make sure to add sysfs attr docs as Peter suggested.
>
>> ---
>>  .../devicetree/bindings/iio/distance/as3935.txt    |  25 ++
>>  drivers/iio/Kconfig                                |   1 +
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/distance/Kconfig                       |  19 +
>>  drivers/iio/distance/Makefile                      |   6 +
>>  drivers/iio/distance/as3935.c                      | 459
>+++++++++++++++++++++
>>  6 files changed, 511 insertions(+)
>>  create mode 100644
>Documentation/devicetree/bindings/iio/distance/as3935.txt
>>  create mode 100644 drivers/iio/distance/Kconfig
>>  create mode 100644 drivers/iio/distance/Makefile
>>  create mode 100644 drivers/iio/distance/as3935.c
>> 
>> diff --git
>a/Documentation/devicetree/bindings/iio/distance/as3935.txt
>b/Documentation/devicetree/bindings/iio/distance/as3935.txt
>> new file mode 100644
>> index 0000000..af35827
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/distance/as3935.txt
>> @@ -0,0 +1,25 @@
>> +Austrian Microsystems AS3935 Franklin lightning sensor device driver
>> +
>> +Required properties:
>> +	- compatible: must be "ams,as3935"
>> +	- reg: SPI chip select number for the device
>> +	- spi-max-frequency: Max SPI frequency to use
>> +	- spi-cpha: SPI Mode 1
>> +	- gpios: GPIO input of interrupt line from IRQ pin of AS3935 IC
>> +
>> +Optional properties:
>> +	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 15.
>> +	  Range of 0 to 120 pF, 8pF steps. This will require using the
>calibration
>> +	  data from the manufacturer.
>> +
>> +
>> +Example:
>> +
>> +		as3935@0 {
>> +			compatible = "ams,as3935";
>> +			reg = <0>;
>> +			spi-max-frequency = <100000>;
>> +			spi-cpha;
>> +			ams,tune-cap = /bits/ 8 <10>;
>> +			gpios = <&gpio1 16 10>;
>> +		};
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 5dd0e12..5164a68 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -63,6 +63,7 @@ source "drivers/iio/adc/Kconfig"
>>  source "drivers/iio/amplifiers/Kconfig"
>>  source "drivers/iio/common/Kconfig"
>>  source "drivers/iio/dac/Kconfig"
>> +source "drivers/iio/distance/Kconfig"
>>  source "drivers/iio/frequency/Kconfig"
>>  source "drivers/iio/gyro/Kconfig"
>>  source "drivers/iio/humidity/Kconfig"
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 887d390..1574731 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -16,6 +16,7 @@ obj-y += adc/
>>  obj-y += amplifiers/
>>  obj-y += common/
>>  obj-y += dac/
>> +obj-y += distance/
>>  obj-y += gyro/
>>  obj-y += frequency/
>>  obj-y += humidity/
>> diff --git a/drivers/iio/distance/Kconfig
>b/drivers/iio/distance/Kconfig
>> new file mode 100644
>> index 0000000..753352c
>> --- /dev/null
>> +++ b/drivers/iio/distance/Kconfig
>> @@ -0,0 +1,19 @@
>> +#
>> +# Distance sensors
>> +#
>> +
>> +menu "Lightning sensors"
>> +
>> +config AS3935
>> +	tristate "AS3935 Franklin lightning sensor"
>> +	select IIO_BUFFER
>> +	select IIO_TRIGGERED_BUFFER
>> +	depends on SPI
>> +	help
>> +	 If you say yes here you get support for the Austrian Microsystems
>> +	 AS3935 lightning detection sensor.
>> +
>> +	 This driver can also be built as a module. If so, the module
>> +	 will be called as3935
>> +
>> +endmenu
>> diff --git a/drivers/iio/distance/Makefile
>b/drivers/iio/distance/Makefile
>> new file mode 100644
>> index 0000000..f63b70d
>> --- /dev/null
>> +++ b/drivers/iio/distance/Makefile
>> @@ -0,0 +1,6 @@
>> +#
>> +# Makefile for IIO distance sensors
>> +#
>> +
>> +# When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_AS3935)		+= as3935.o
>> diff --git a/drivers/iio/distance/as3935.c
>b/drivers/iio/distance/as3935.c
>> new file mode 100644
>> index 0000000..1a8f48d
>> --- /dev/null
>> +++ b/drivers/iio/distance/as3935.c
>> @@ -0,0 +1,459 @@
>> +/*
>> + * as3935.c - Support for AS3935 Franklin lightning sensor
>> + *
>> + * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>modify
>> + * it under the terms of the GNU General Public License as published
>by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/mutex.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/err.h>
>> +#include <linux/irq.h>
>> +#include <linux/gpio.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/pinctrl/pinctrl.h>
>> +#include <linux/pinctrl/pinmux.h>
>> +#include <linux/pinctrl/consumer.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/of_gpio.h>
>> +
>> +
>> +#define AS3935_AFE_GAIN		0x00
>> +#define AS3935_AFE_MASK		0x3F
>> +#define AS3935_AFE_GAIN_MAX	0x1F
>> +
>> +#define AS3935_INT		0x03
>> +#define AS3935_INT_MASK		0x07
>> +#define AS3935_DATA		0x07
>> +#define AS3935_DATA_MASK	0x1F
>> +
>> +#define AS3935_TUNE_CAP		0x08
>> +#define AS3935_CALIBRATE	0x3D
>> +
>> +#define AS3935_WRITE_DATA	(0x1 << 15)
>> +#define AS3935_READ_DATA	(0x1 << 14)
>> +#define AS3935_ADDRESS(x)	(x << 8)
>> +
>> +#define AS3935_EVENT_INT	(1<<3)
>> +#define AS3935_NOISE_INT	(1<<0)
>> +
>> +struct as3935_state {
>> +	struct spi_device *spi;
>> +	struct mutex lock;
>> +
>> +	struct iio_trigger *trig;
>> +	struct delayed_work work;
>> +
>> +	u8 tune_cap;
>> +};
>> +
>> +static const struct iio_chan_spec as3935_channels[] = {
>> +	{
>> +		.type           = IIO_DISTANCE,
>> +		.channel        = 0,
>> +		.info_mask_separate =
>> +			BIT(IIO_CHAN_INFO_RAW),
>> +		.scan_index     = 0,
>> +		.scan_type = {
>> +			.sign           = 'u',
>> +			.realbits       = 6,
>> +			.storagebits    = 6,
>> +		},
>> +		.modified = 0,
>
>channel and modified are not needed
>storagebits should be a multiple of 8
>
>> +	},
>> +	IIO_CHAN_SOFT_TIMESTAMP(1),
>> +};
>> +
>> +static const unsigned long as3935_available_scan_masks[] = { 0x01,
>0x0 };
>
>is this needed? there is just one channel
>
>> +
>> +static int as3935_read(struct as3935_state *st, unsigned int reg,
>int *val)
>> +{
>> +	u8 tx, rx;
>> +	int ret;
>> +
>> +	struct spi_transfer xfers[] = {
>> +		{
>> +			.tx_buf = &tx,
>> +			.bits_per_word = 8,
>> +			.len = 1,
>> +		}, {
>> +			.rx_buf = &rx,
>> +			.bits_per_word = 8,
>> +			.len = 1,
>> +		},
>> +	};
>> +
>> +	mutex_lock(&st->lock);
>> +	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
>> +
>> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>> +	mutex_unlock(&st->lock);
>> +
>> +	*val = rx;
>> +
>> +	return ret;
>> +};
>> +
>> +static int as3935_write(struct as3935_state *st,
>> +				unsigned int reg,
>> +				unsigned int val)
>> +{
>> +	u8 buf[2];
>> +	int ret;
>> +
>> +	mutex_lock(&st->lock);
>> +	buf[0] = AS3935_WRITE_DATA | AS3935_ADDRESS(reg) >> 8;
>> +	buf[1] = val;
>> +
>> +	ret = spi_write(st->spi, (u8 *) &buf, 2);
>> +	mutex_unlock(&st->lock);
>> +
>> +	return ret;
>> +};
>> +
>> +static ssize_t gain_boost_show(struct device *dev,
>
>not prefixed with as3935_
>
>> +				struct device_attribute *attr,
>> +				char *buf)
>> +{
>> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
>> +	int val, ret;
>> +
>> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +	if (ret)
>> +		return ret;
>> +	val = (val & AS3935_AFE_MASK) >> 1;
>> +
>> +	return sprintf(buf, "%d\n", val);
>> +};
>> +
>> +static ssize_t gain_boost_store(struct device *dev,
>> +					struct device_attribute *attr,
>> +					const char *buf, size_t len)
>> +{
>> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
>> +	unsigned long val;
>> +	int ret;
>> +
>> +	ret = kstrtoul((const char *) buf, 10, &val);
>> +	if (ret)
>> +		return -EINVAL;
>> +
>> +	if (val > AS3935_AFE_GAIN_MAX)
>> +		return -EINVAL;
>> +
>> +	as3935_write(st, AS3935_AFE_GAIN, val << 1);
>> +
>> +	return len;
>> +};
>> +
>> +static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
>> +	gain_boost_show, gain_boost_store, 0);
>> +
>> +
>> +static struct attribute *as3935_attributes[] = {
>> +	&iio_dev_attr_gain_boost.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static struct attribute_group as3935_attribute_group = {
>> +	.attrs = as3935_attributes,
>> +};
>> +
>> +static int as3935_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val,
>> +			   int *val2,
>> +			   long m)
>> +{
>> +	struct as3935_state *st = iio_priv(indio_dev);
>> +
>> +	if (m == IIO_CHAN_INFO_RAW) {
>> +		int ret;
>> +		*val = 0;
>> +		ret = as3935_read(st, AS3935_DATA, val2);
>
>huh, this always returns 0?
>only val is return if IIO_VAL_INT
>
>> +		if (ret)
>> +			return ret;
>> +		return IIO_VAL_INT;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct iio_info as3935_info = {
>> +	.driver_module = THIS_MODULE,
>> +	.attrs = &as3935_attribute_group,
>> +	.read_raw = &as3935_read_raw,
>> +};
>> +
>> +static const struct iio_buffer_setup_ops
>iio_triggered_buffer_setup_ops = {
>> +	.postenable = &iio_triggered_buffer_postenable,
>> +	.predisable = &iio_triggered_buffer_predisable,
>> +};
>> +
>> +static irqreturn_t as3935_trigger_handler(int irq, void *private)
>> +{
>> +	struct iio_poll_func *pf = private;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct as3935_state *st = iio_priv(indio_dev);
>> +	int val, ret;
>> +
>> +	ret = as3935_read(st, AS3935_DATA, &val);
>> +	if (ret)
>> +		goto err_read;
>> +	val &= AS3935_DATA_MASK;
>> +	iio_push_to_buffers_with_timestamp(indio_dev, &val,
>iio_get_time_ns());
>> +
>> +	iio_trigger_notify_done(indio_dev->trig);
>
>iio_trigger_notify_done() must always be called, most past err_read
>
>> +err_read:
>> +
>> +	return IRQ_HANDLED;
>> +};
>> +
>> +static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
>> +	.owner = THIS_MODULE,
>> +};
>> +
>> +static void as3935_event_work(struct work_struct *work)
>> +{
>> +	struct as3935_state *st;
>> +	struct spi_device *spi;
>> +	int val;
>> +
>> +	st = container_of(work, struct as3935_state, work.work);
>> +	spi = st->spi;
>> +
>> +	as3935_read(st, AS3935_INT, &val);
>> +	val &= AS3935_INT_MASK;
>> +
>> +	switch (val) {
>> +	case AS3935_EVENT_INT:
>> +		iio_trigger_poll(st->trig, 0);
>> +		break;
>> +	case AS3935_NOISE_INT:
>> +		dev_warn(&spi->dev, "noise level is too high");
>> +		break;
>> +	}
>> +};
>> +
>> +static irqreturn_t as3935_interrupt_handler(int irq, void *private)
>> +{
>> +	struct iio_dev *indio_dev = private;
>> +	struct as3935_state *st = iio_priv(indio_dev);
>> +
>> +	cancel_delayed_work(&st->work);
>> +	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static void calibrate_as3935(struct as3935_state *st)
>> +{
>> +	/* mask disturber interrupt bit */
>> +	as3935_write(st, AS3935_INT, 1 << 5);
>> +
>> +	as3935_write(st, AS3935_CALIBRATE, 0x96);
>> +	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
>> +
>> +	mdelay(2);
>> +	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
>> +{
>> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +	struct as3935_state *st = iio_priv(indio_dev);
>> +	int val, ret;
>> +
>> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +	if (ret)
>> +		return ret;
>> +	val |= 0x01;
>> +
>> +	return as3935_write(st, AS3935_AFE_GAIN, val);
>> +}
>> +
>> +static int as3935_resume(struct spi_device *spi)
>> +{
>> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +	struct as3935_state *st = iio_priv(indio_dev);
>> +	int val, ret;
>> +
>> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +	if (ret)
>> +		return ret;
>> +	val &= ~1;
>> +
>> +	return as3935_write(st, AS3935_AFE_GAIN, val);
>> +}
>> +#else
>> +#define as3935_suspend	NULL
>> +#define as3935_resume	NULL
>> +#endif
>> +
>> +static int as3935_probe(struct spi_device *spi)
>> +{
>> +	struct iio_dev *indio_dev;
>> +	struct iio_trigger *trig;
>> +	struct as3935_state *st;
>> +	struct device_node *np = spi->dev.of_node;
>> +	int gpio;
>> +	int irq;
>> +	int ret;
>> +
>> +	/* Grab the GPIO to use for lightning event interrupt */
>> +	if (np)
>> +		gpio = of_get_gpio(spi->dev.of_node, 0);
>> +	else {
>> +		dev_err(&spi->dev, "unable to get interrupt gpio\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* GPIO event setup */
>> +	ret = devm_gpio_request(&spi->dev, gpio, "as3935");
>> +	if (ret) {
>> +		dev_err(&spi->dev, "failed to request GPIO %u\n", gpio);
>> +		return ret;
>> +	}
>> +
>> +	ret = gpio_direction_input(gpio);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "failed to set pin direction\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* IRQ setup */
>> +	irq = gpio_to_irq(gpio);
>> +	if (irq < 0) {
>> +		dev_err(&spi->dev, "failed to map GPIO to IRQ: %d\n", irq);
>> +		return -EINVAL;
>> +	}
>> +
>> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	st = iio_priv(indio_dev);
>> +	st->spi = spi;
>> +	st->tune_cap = 0;
>> +	spi_set_drvdata(spi, indio_dev);
>> +	mutex_init(&st->lock);
>> +	INIT_DELAYED_WORK(&st->work, as3935_event_work);
>> +
>> +	of_property_read_u8(np, "ams,tune-cap", &st->tune_cap);
>> +	if (st->tune_cap > 15) {
>> +		dev_err(&spi->dev,
>> +			"wrong tune_cap setting of %d\n", st->tune_cap);
>> +		return -EINVAL;
>> +	}
>> +
>> +	indio_dev->dev.parent = &spi->dev;
>> +	indio_dev->name = spi_get_device_id(spi)->name;
>> +	indio_dev->channels = as3935_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->available_scan_masks = as3935_available_scan_masks;
>> +	indio_dev->info = &as3935_info;
>> +
>> +	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
>> +				      indio_dev->name, indio_dev->id);
>> +
>> +	if (!trig)
>> +		return -ENOMEM;
>> +
>> +	st->trig = trig;
>> +	trig->dev.parent = indio_dev->dev.parent;
>> +	iio_trigger_set_drvdata(trig, indio_dev);
>> +	trig->ops = &iio_interrupt_trigger_ops;
>> +
>> +	ret = iio_trigger_register(trig);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "failed to register trigger\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +					&as3935_trigger_handler,
>> +					&iio_triggered_buffer_setup_ops);
>> +
>> +	if (ret) {
>> +		dev_err(&spi->dev, "cannot setup iio trigger\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	calibrate_as3935(st);
>> +
>> +	ret = devm_request_irq(&spi->dev, irq,
>> +				&as3935_interrupt_handler,
>> +				IRQF_TRIGGER_RISING,
>> +				dev_name(&spi->dev),
>> +				indio_dev);
>> +
>> +	if (ret) {
>> +		dev_err(&spi->dev, "unable to request irq\n");
>> +		goto uninit_buffer;
>> +	}
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret < 0) {
>> +		dev_err(&spi->dev, "unable to register device\n");
>> +		goto uninit_buffer;
>> +	}
>> +
>> +	return 0;
>> +
>> +uninit_buffer:
>> +	iio_triggered_buffer_cleanup(indio_dev);
>
>iio_trigger_unregister()?
>
>> +
>> +	return ret;
>> +};
>> +
>> +static int as3935_remove(struct spi_device *spi)
>> +{
>> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +
>
>iio_trigger_unregister()?
>iio_triggered_buffer_cleanup()?
>
>> +	iio_device_unregister(indio_dev);
>> +	return 0;
>> +};
>> +
>> +static const struct spi_device_id as3935_id[] = {
>> +	{"as3935", 0},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(spi, as3935_id);
>> +
>> +static struct spi_driver as3935_driver = {
>> +	.driver = {
>> +		.name	= "as3935",
>> +		.owner	= THIS_MODULE,
>> +	},
>> +	.probe		= as3935_probe,
>> +	.remove		= as3935_remove,
>> +	.id_table	= as3935_id,
>> +	.suspend	= as3935_suspend,
>> +	.resume		= as3935_resume,
>> +};
>> +module_spi_driver(as3935_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("AS3935 lightning sensor");
>> +MODULE_LICENSE("GPL");
>> 

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Alexandre Belloni]

Hi Matt,

On 30/01/2014 at 02:11:13 -0800, Matt Ranostay wrote :
> AS3935 chipset can detect lightning strikes and reports those
> back as events and the esimated distance to the storm.
> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  .../devicetree/bindings/iio/distance/as3935.txt    |  25 ++

Maybe I'm wrong but wasn't the agreement that the bindings documentation
has to be sent separately to the devicetree mailing list ?

>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/distance/Kconfig                       |  19 +
>  drivers/iio/distance/Makefile                      |   6 +
>  drivers/iio/distance/as3935.c                      | 459 +++++++++++++++++++++
>  6 files changed, 511 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/distance/as3935.txt
>  create mode 100644 drivers/iio/distance/Kconfig
>  create mode 100644 drivers/iio/distance/Makefile
>  create mode 100644 drivers/iio/distance/as3935.c

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Matt Porter]

On Fri, Jan 31, 2014 at 10:05:07PM +0100, Alexandre Belloni wrote:
> Hi Matt,
> 
> On 30/01/2014 at 02:11:13 -0800, Matt Ranostay wrote :
> > AS3935 chipset can detect lightning strikes and reports those
> > back as events and the esimated distance to the storm.
> > 
> > Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> > ---
> >  .../devicetree/bindings/iio/distance/as3935.txt    |  25 ++
> 
> Maybe I'm wrong but wasn't the agreement that the bindings documentation
> has to be sent separately to the devicetree mailing list ?

That's correct. It needs to be separated for ease of review.

-anothermatt

[PATCH 1/2] iio: add IIO_DISTANCE type

[Matt Ranostay]

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 drivers/iio/industrialio-core.c | 1 +
 include/linux/iio/types.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index acc911a..ac999ab 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -70,6 +70,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_CCT] = "cct",
 	[IIO_PRESSURE] = "pressure",
 	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
+	[IIO_DISTANCE] = "distance",
 };
 
 static const char * const iio_modifier_names[] = {
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 084d882..675c2d8 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -30,6 +30,7 @@ enum iio_chan_type {
 	IIO_CCT,
 	IIO_PRESSURE,
 	IIO_HUMIDITYRELATIVE,
+	IIO_DISTANCE,
 };
 
 enum iio_modifier {
-- 
1.8.3.2

[PATCH 2/2] iio: Add AS3935 lightning sensor support

[Matt Ranostay]

AS3935 chipset can detect lightning strikes and reports those
back as events and the estimated distance to the storm.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../ABI/testing/sysfs-bus-iio-distance-as3935      |  19 +
 .../devicetree/bindings/iio/distance/as3935.txt    |  25 ++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/distance/Kconfig                       |  19 +
 drivers/iio/distance/Makefile                      |   6 +
 drivers/iio/distance/as3935.c                      | 458 +++++++++++++++++++++
 7 files changed, 529 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-distance-as3935
 create mode 100644 Documentation/devicetree/bindings/iio/distance/as3935.txt
 create mode 100644 drivers/iio/distance/Kconfig
 create mode 100644 drivers/iio/distance/Makefile
 create mode 100644 drivers/iio/distance/as3935.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-distance-as3935 b/Documentation/ABI/testing/sysfs-bus-iio-distance-as3935
new file mode 100644
index 0000000..cf9a67c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-distance-as3935
@@ -0,0 +1,19 @@
+What		/sys/bus/iio/devices/iio:deviceX/in_distance_raw
+Date:		January 2014
+KernelVersion:	3.15
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the current distance in kilometers of storm
+		0    = no storm activity
+		1    = storm overhead
+		1-40 = distance in kilometers
+		63   = out of range
+
+What		/sys/bus/iio/devices/iio:deviceX/gain_boost
+Date:		January 2014
+KernelVersion:	3.15
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Show or set the gain boost of the amp, from 0-31 range.
+		18 = indoors (default)
+		14 = outdoors
diff --git a/Documentation/devicetree/bindings/iio/distance/as3935.txt b/Documentation/devicetree/bindings/iio/distance/as3935.txt
new file mode 100644
index 0000000..af35827
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/distance/as3935.txt
@@ -0,0 +1,25 @@
+Austrian Microsystems AS3935 Franklin lightning sensor device driver
+
+Required properties:
+	- compatible: must be "ams,as3935"
+	- reg: SPI chip select number for the device
+	- spi-max-frequency: Max SPI frequency to use
+	- spi-cpha: SPI Mode 1
+	- gpios: GPIO input of interrupt line from IRQ pin of AS3935 IC
+
+Optional properties:
+	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 15.
+	  Range of 0 to 120 pF, 8pF steps. This will require using the calibration
+	  data from the manufacturer.
+
+
+Example:
+
+		as3935@0 {
+			compatible = "ams,as3935";
+			reg = <0>;
+			spi-max-frequency = <100000>;
+			spi-cpha;
+			ams,tune-cap = /bits/ 8 <10>;
+			gpios = <&gpio1 16 10>;
+		};
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 5dd0e12..5164a68 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -63,6 +63,7 @@ source "drivers/iio/adc/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/dac/Kconfig"
+source "drivers/iio/distance/Kconfig"
 source "drivers/iio/frequency/Kconfig"
 source "drivers/iio/gyro/Kconfig"
 source "drivers/iio/humidity/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 887d390..1574731 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -16,6 +16,7 @@ obj-y += adc/
 obj-y += amplifiers/
 obj-y += common/
 obj-y += dac/
+obj-y += distance/
 obj-y += gyro/
 obj-y += frequency/
 obj-y += humidity/
diff --git a/drivers/iio/distance/Kconfig b/drivers/iio/distance/Kconfig
new file mode 100644
index 0000000..753352c
--- /dev/null
+++ b/drivers/iio/distance/Kconfig
@@ -0,0 +1,19 @@
+#
+# Distance sensors
+#
+
+menu "Lightning sensors"
+
+config AS3935
+	tristate "AS3935 Franklin lightning sensor"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	depends on SPI
+	help
+	 If you say yes here you get support for the Austrian Microsystems
+	 AS3935 lightning detection sensor.
+
+	 This driver can also be built as a module. If so, the module
+	 will be called as3935
+
+endmenu
diff --git a/drivers/iio/distance/Makefile b/drivers/iio/distance/Makefile
new file mode 100644
index 0000000..f63b70d
--- /dev/null
+++ b/drivers/iio/distance/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for IIO distance sensors
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AS3935)		+= as3935.o
diff --git a/drivers/iio/distance/as3935.c b/drivers/iio/distance/as3935.c
new file mode 100644
index 0000000..b9c2097
--- /dev/null
+++ b/drivers/iio/distance/as3935.c
@@ -0,0 +1,458 @@
+/*
+ * as3935.c - Support for AS3935 Franklin lightning sensor
+ *
+ * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/workqueue.h>
+#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_gpio.h>
+
+
+#define AS3935_AFE_GAIN		0x00
+#define AS3935_AFE_MASK		0x3F
+#define AS3935_AFE_GAIN_MAX	0x1F
+
+#define AS3935_INT		0x03
+#define AS3935_INT_MASK		0x07
+#define AS3935_DATA		0x07
+#define AS3935_DATA_MASK	0x1F
+
+#define AS3935_TUNE_CAP		0x08
+#define AS3935_CALIBRATE	0x3D
+
+#define AS3935_WRITE_DATA	(0x1 << 15)
+#define AS3935_READ_DATA	(0x1 << 14)
+#define AS3935_ADDRESS(x)	(x << 8)
+
+#define AS3935_EVENT_INT	(1<<3)
+#define AS3935_NOISE_INT	(1<<0)
+
+struct as3935_state {
+	struct spi_device *spi;
+	struct mutex lock;
+
+	struct iio_trigger *trig;
+	struct delayed_work work;
+
+	u8 tune_cap;
+};
+
+static const struct iio_chan_spec as3935_channels[] = {
+	{
+		.type           = IIO_DISTANCE,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW),
+		.scan_index     = 0,
+		.scan_type = {
+			.sign           = 'u',
+			.realbits       = 8,
+			.storagebits    = 8,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
+{
+	u8 tx, rx;
+	int ret;
+
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &tx,
+			.bits_per_word = 8,
+			.len = 1,
+		}, {
+			.rx_buf = &rx,
+			.bits_per_word = 8,
+			.len = 1,
+		},
+	};
+
+	mutex_lock(&st->lock);
+	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	mutex_unlock(&st->lock);
+
+	*val = rx;
+
+	return ret;
+};
+
+static int as3935_write(struct as3935_state *st,
+				unsigned int reg,
+				unsigned int val)
+{
+	u8 buf[2];
+	int ret;
+
+	mutex_lock(&st->lock);
+	buf[0] = AS3935_WRITE_DATA | AS3935_ADDRESS(reg) >> 8;
+	buf[1] = val;
+
+	ret = spi_write(st->spi, (u8 *) &buf, 2);
+	mutex_unlock(&st->lock);
+
+	return ret;
+};
+
+static ssize_t as3935_gain_boost_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val = (val & AS3935_AFE_MASK) >> 1;
+
+	return sprintf(buf, "%d\n", val);
+};
+
+static ssize_t as3935_gain_boost_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul((const char *) buf, 10, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val > AS3935_AFE_GAIN_MAX)
+		return -EINVAL;
+
+	as3935_write(st, AS3935_AFE_GAIN, val << 1);
+
+	return len;
+};
+
+static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
+	as3935_gain_boost_show, as3935_gain_boost_store, 0);
+
+
+static struct attribute *as3935_attributes[] = {
+	&iio_dev_attr_gain_boost.dev_attr.attr,
+	NULL,
+};
+
+static struct attribute_group as3935_attribute_group = {
+	.attrs = as3935_attributes,
+};
+
+static int as3935_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	if (m == IIO_CHAN_INFO_RAW) {
+		int ret;
+		*val2 = 0;
+		ret = as3935_read(st, AS3935_DATA, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info as3935_info = {
+	.driver_module = THIS_MODULE,
+	.attrs = &as3935_attribute_group,
+	.read_raw = &as3935_read_raw,
+};
+
+static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
+	.postenable = &iio_triggered_buffer_postenable,
+	.predisable = &iio_triggered_buffer_predisable,
+};
+
+static irqreturn_t as3935_trigger_handler(int irq, void *private)
+{
+	struct iio_poll_func *pf = private;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_DATA, &val);
+	if (ret)
+		goto err_read;
+	val &= AS3935_DATA_MASK;
+	iio_push_to_buffers_with_timestamp(indio_dev, &val, iio_get_time_ns());
+err_read:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+};
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static void as3935_event_work(struct work_struct *work)
+{
+	struct as3935_state *st;
+	struct spi_device *spi;
+	int val;
+
+	st = container_of(work, struct as3935_state, work.work);
+	spi = st->spi;
+
+	as3935_read(st, AS3935_INT, &val);
+	val &= AS3935_INT_MASK;
+
+	switch (val) {
+	case AS3935_EVENT_INT:
+		iio_trigger_poll(st->trig, 0);
+		break;
+	case AS3935_NOISE_INT:
+		dev_warn(&spi->dev, "noise level is too high");
+		break;
+	}
+};
+
+static irqreturn_t as3935_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	cancel_delayed_work(&st->work);
+	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
+	return IRQ_HANDLED;
+}
+
+static void calibrate_as3935(struct as3935_state *st)
+{
+	/* mask disturber interrupt bit */
+	as3935_write(st, AS3935_INT, 1 << 5);
+
+	as3935_write(st, AS3935_CALIBRATE, 0x96);
+	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
+
+	mdelay(2);
+	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val |= 0x01;
+
+	return as3935_write(st, AS3935_AFE_GAIN, val);
+}
+
+static int as3935_resume(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val &= ~1;
+
+	return as3935_write(st, AS3935_AFE_GAIN, val);
+}
+#else
+#define as3935_suspend	NULL
+#define as3935_resume	NULL
+#endif
+
+static int as3935_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct iio_trigger *trig;
+	struct as3935_state *st;
+	struct device_node *np = spi->dev.of_node;
+	int gpio;
+	int irq;
+	int ret;
+
+	/* Grab the GPIO to use for lightning event interrupt */
+	if (np)
+		gpio = of_get_gpio(spi->dev.of_node, 0);
+	else {
+		dev_err(&spi->dev, "unable to get interrupt gpio\n");
+		return -EINVAL;
+	}
+
+	/* GPIO event setup */
+	ret = devm_gpio_request(&spi->dev, gpio, "as3935");
+	if (ret) {
+		dev_err(&spi->dev, "failed to request GPIO %u\n", gpio);
+		return ret;
+	}
+
+	ret = gpio_direction_input(gpio);
+	if (ret) {
+		dev_err(&spi->dev, "failed to set pin direction\n");
+		return -EINVAL;
+	}
+
+	/* IRQ setup */
+	irq = gpio_to_irq(gpio);
+	if (irq < 0) {
+		dev_err(&spi->dev, "failed to map GPIO to IRQ: %d\n", irq);
+		return -EINVAL;
+	}
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->tune_cap = 0;
+	spi_set_drvdata(spi, indio_dev);
+	mutex_init(&st->lock);
+	INIT_DELAYED_WORK(&st->work, as3935_event_work);
+
+	of_property_read_u8(np, "ams,tune-cap", &st->tune_cap);
+	if (st->tune_cap > 15) {
+		dev_err(&spi->dev,
+			"wrong tune_cap setting of %d\n", st->tune_cap);
+		return -EINVAL;
+	}
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->channels = as3935_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &as3935_info;
+
+	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+				      indio_dev->name, indio_dev->id);
+
+	if (!trig)
+		return -ENOMEM;
+
+	st->trig = trig;
+	trig->dev.parent = indio_dev->dev.parent;
+	iio_trigger_set_drvdata(trig, indio_dev);
+	trig->ops = &iio_interrupt_trigger_ops;
+
+	ret = iio_trigger_register(trig);
+	if (ret) {
+		dev_err(&spi->dev, "failed to register trigger\n");
+		return ret;
+	}
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					&as3935_trigger_handler,
+					&iio_triggered_buffer_setup_ops);
+
+	if (ret) {
+		dev_err(&spi->dev, "cannot setup iio trigger\n");
+		goto unregister_trigger;
+	}
+
+	calibrate_as3935(st);
+
+	ret = devm_request_irq(&spi->dev, irq,
+				&as3935_interrupt_handler,
+				IRQF_TRIGGER_RISING,
+				dev_name(&spi->dev),
+				indio_dev);
+
+	if (ret) {
+		dev_err(&spi->dev, "unable to request irq\n");
+		goto unregister_trigger;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&spi->dev, "unable to register device\n");
+		goto unregister_trigger;
+	}
+
+	return 0;
+
+unregister_trigger:
+	iio_trigger_unregister(st->trig);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return ret;
+};
+
+static int as3935_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	iio_trigger_unregister(st->trig);
+	iio_triggered_buffer_cleanup(indio_dev);
+	iio_device_unregister(indio_dev);
+
+	return 0;
+};
+
+static const struct spi_device_id as3935_id[] = {
+	{"as3935", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, as3935_id);
+
+static struct spi_driver as3935_driver = {
+	.driver = {
+		.name	= "as3935",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= as3935_probe,
+	.remove		= as3935_remove,
+	.id_table	= as3935_id,
+	.suspend	= as3935_suspend,
+	.resume		= as3935_resume,
+};
+module_spi_driver(as3935_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("AS3935 lightning sensor");
+MODULE_LICENSE("GPL");
-- 
1.8.3.2

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Marek Vasut]

On Friday, January 31, 2014 at 04:38:23 PM, Matt Ranostay wrote:

[...]

> diff --git a/Documentation/devicetree/bindings/iio/distance/as3935.txt
> b/Documentation/devicetree/bindings/iio/distance/as3935.txt new file mode
> 100644
> index 0000000..af35827
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/distance/as3935.txt
> @@ -0,0 +1,25 @@
> +Austrian Microsystems AS3935 Franklin lightning sensor device driver
> +
> +Required properties:
> +	- compatible: must be "ams,as3935"
> +	- reg: SPI chip select number for the device
> +	- spi-max-frequency: Max SPI frequency to use
> +	- spi-cpha: SPI Mode 1
> +	- gpios: GPIO input of interrupt line from IRQ pin of AS3935 IC
> +
> +Optional properties:
> +	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 15.
> +	  Range of 0 to 120 pF, 8pF steps. This will require using the
> calibration +	  data from the manufacturer.
> +
> +
> +Example:
> +
> +		as3935@0 {
> +			compatible = "ams,as3935";
> +			reg = <0>;
> +			spi-max-frequency = <100000>;
> +			spi-cpha;
> +			ams,tune-cap = /bits/ 8 <10>;
> +			gpios = <&gpio1 16 10>;
> +		};

You should CC devicetree-discuss with new bindings! Such a grave flub ;-)

[...]

> +config AS3935
> +	tristate "AS3935 Franklin lightning sensor"
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	depends on SPI
> +	help
> +	 If you say yes here you get support for the Austrian Microsystems
> +	 AS3935 lightning detection sensor.
> +
> +	 This driver can also be built as a module. If so, the module
> +	 will be called as3935

Fullstop's missing at the end of the above sentence .

[...]

> +#define AS3935_AFE_GAIN		0x00
> +#define AS3935_AFE_MASK		0x3F
> +#define AS3935_AFE_GAIN_MAX	0x1F
> +
> +#define AS3935_INT		0x03
> +#define AS3935_INT_MASK		0x07
> +#define AS3935_DATA		0x07
> +#define AS3935_DATA_MASK	0x1F
> +
> +#define AS3935_TUNE_CAP		0x08
> +#define AS3935_CALIBRATE	0x3D
> +
> +#define AS3935_WRITE_DATA	(0x1 << 15)
> +#define AS3935_READ_DATA	(0x1 << 14)
> +#define AS3935_ADDRESS(x)	(x << 8)

((x) << 8)

[...]

> +static int as3935_write(struct as3935_state *st,
> +				unsigned int reg,
> +				unsigned int val)
> +{
> +	u8 buf[2];
> +	int ret;
> +
> +	mutex_lock(&st->lock);

You don't need to protect the writes to buf[] with a mutex, since the buf[] is 
on stack here. Each thread will have it's own local copy of that. DTTO for $reg 
variable.

Actually, I think you don't even need mutex around all of the the SPI sync 
stuff. The SPI framework already has a mutex in it.

> +	buf[0] = AS3935_WRITE_DATA | AS3935_ADDRESS(reg) >> 8;
> +	buf[1] = val;
> +
> +	ret = spi_write(st->spi, (u8 *) &buf, 2);
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +};
> +

[...]

> +static int as3935_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
> +	if (m == IIO_CHAN_INFO_RAW) {

if (m != IIO...)
 return -EINVAL;

... rest of the code ...

This will cut down on the depth of indent.

> +		int ret;
> +		*val2 = 0;
> +		ret = as3935_read(st, AS3935_DATA, val);
> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}

[...]

> +#ifdef CONFIG_PM_SLEEP
> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val |= 0x01;

What's this hexadecimal magic constant here?

> +
> +	return as3935_write(st, AS3935_AFE_GAIN, val);
> +}
> +
> +static int as3935_resume(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val &= ~1;

What's this decimal magic constant here?

> +	return as3935_write(st, AS3935_AFE_GAIN, val);
> +}
> +#else
> +#define as3935_suspend	NULL
> +#define as3935_resume	NULL
> +#endif
> +
> +static int as3935_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct iio_trigger *trig;
> +	struct as3935_state *st;
> +	struct device_node *np = spi->dev.of_node;
> +	int gpio;
> +	int irq;
> +	int ret;
> +
> +	/* Grab the GPIO to use for lightning event interrupt */
> +	if (np)
> +		gpio = of_get_gpio(spi->dev.of_node, 0);
> +	else {
> +		dev_err(&spi->dev, "unable to get interrupt gpio\n");
> +		return -EINVAL;
> +	}

The logic is a bit weird here. Why don't you check this like so:

if (!np) {
 ... bail ...
}

gpio = ...

> +	/* GPIO event setup */
> +	ret = devm_gpio_request(&spi->dev, gpio, "as3935");
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to request GPIO %u\n", gpio);
> +		return ret;
> +	}
> +
> +	ret = gpio_direction_input(gpio);
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to set pin direction\n");
> +		return -EINVAL;
> +	}
> +
> +	/* IRQ setup */
> +	irq = gpio_to_irq(gpio);
> +	if (irq < 0) {
> +		dev_err(&spi->dev, "failed to map GPIO to IRQ: %d\n", irq);
> +		return -EINVAL;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->tune_cap = 0;
> +	spi_set_drvdata(spi, indio_dev);
> +	mutex_init(&st->lock);
> +	INIT_DELAYED_WORK(&st->work, as3935_event_work);
> +
> +	of_property_read_u8(np, "ams,tune-cap", &st->tune_cap);

This can fail, check the retval please.

> +	if (st->tune_cap > 15) {
> +		dev_err(&spi->dev,
> +			"wrong tune_cap setting of %d\n", st->tune_cap);
> +		return -EINVAL;
> +	}

[...]

> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("AS3935 lightning sensor");
> +MODULE_LICENSE("GPL");

MODULE_ALIAS() is missing , no?

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Marek Vasut]

On Saturday, February 01, 2014 at 06:03:22 AM, Matt Ranostay wrote:
> Duly noted. Will be fixed in the next rev... mutexs are pointless as you
> say.. only corner case would be if you were changing gain_boost and a event
> came in at the same time. But that wouldn't harm anything except slightly
> delay the gain_boost being updated by some  x microseconds.

Please stop top-posting ;-)

btw I think you might need a mutex around the entire:

+static void calibrate_as3935(struct as3935_state *st)
+{
+       /* mask disturber interrupt bit */
+       as3935_write(st, AS3935_INT, 1 << 5);
+
+       as3935_write(st, AS3935_CALIBRATE, 0x96);
+       as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
+
+       mdelay(2);
+       as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
+}

and similar functions where you do a bunch of register accesses. This is because 
you probably want to protect them against concurent execution so the chip won't 
be confused if a user were to poke something via SYSFS twice.

Best regards,
Marek Vasut

[PATCH 0/2] AS3935 lightning sensor support

[Matt Ranostay]

This series adds support for the AMS AS3935 lightning sensor that allows
reporting back estimated storm distance and strike events.

Matt Ranostay (2):
  iio:as3935: Add DT binding docs for AS3935 driver
  iio: Add AS3935 lightning sensor support

 .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
 .../devicetree/bindings/iio/proximity/as3935.txt   |  28 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/proximity/Kconfig                      |  19 +
 drivers/iio/proximity/Makefile                     |   6 +
 drivers/iio/proximity/as3935.c                     | 437 +++++++++++++++++++++
 8 files changed, 511 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/as3935.txt
 create mode 100644 drivers/iio/proximity/Kconfig
 create mode 100644 drivers/iio/proximity/Makefile
 create mode 100644 drivers/iio/proximity/as3935.c

-- 
1.8.3.2

[PATCH 2/2] iio: Add AS3935 lightning sensor support

[Matt Ranostay]

AS3935 chipset can detect lightning strikes and reports those back as
events and the estimated distance to the storm.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/proximity/Kconfig                      |  19 +
 drivers/iio/proximity/Makefile                     |   6 +
 drivers/iio/proximity/as3935.c                     | 437 +++++++++++++++++++++
 6 files changed, 482 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
 create mode 100644 drivers/iio/proximity/Kconfig
 create mode 100644 drivers/iio/proximity/Makefile
 create mode 100644 drivers/iio/proximity/as3935.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935 b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
new file mode 100644
index 0000000..f6d9e6f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
@@ -0,0 +1,18 @@
+What		/sys/bus/iio/devices/iio:deviceX/in_proximity_raw
+Date:		January 2014
+KernelVersion:	3.15
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the current distance in kilometers of storm
+		1    = storm overhead
+		1-40 = distance in kilometers
+		63   = out of range
+
+What		/sys/bus/iio/devices/iio:deviceX/gain_boost
+Date:		January 2014
+KernelVersion:	3.15
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Show or set the gain boost of the amp, from 0-31 range.
+		18 = indoors (default)
+		14 = outdoors
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 5dd0e12..743485e 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -74,6 +74,7 @@ if IIO_TRIGGER
    source "drivers/iio/trigger/Kconfig"
 endif #IIO_TRIGGER
 source "drivers/iio/pressure/Kconfig"
+source "drivers/iio/proximity/Kconfig"
 source "drivers/iio/temperature/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 887d390..698afc2 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -24,5 +24,6 @@ obj-y += light/
 obj-y += magnetometer/
 obj-y += orientation/
 obj-y += pressure/
+obj-y += proximity/
 obj-y += temperature/
 obj-y += trigger/
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
new file mode 100644
index 0000000..0c8cdf5
--- /dev/null
+++ b/drivers/iio/proximity/Kconfig
@@ -0,0 +1,19 @@
+#
+# Proximity sensors
+#
+
+menu "Lightning sensors"
+
+config AS3935
+	tristate "AS3935 Franklin lightning sensor"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	depends on SPI
+	help
+	  Say Y here to build SPI interface support for the Austrian
+	  Microsystems AS3935 lightning detection sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called as3935
+
+endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
new file mode 100644
index 0000000..743adee
--- /dev/null
+++ b/drivers/iio/proximity/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for IIO proximity sensors
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AS3935)		+= as3935.o
diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
new file mode 100644
index 0000000..109759e
--- /dev/null
+++ b/drivers/iio/proximity/as3935.c
@@ -0,0 +1,437 @@
+/*
+ * as3935.c - Support for AS3935 Franklin lightning sensor
+ *
+ * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/workqueue.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_gpio.h>
+
+
+#define AS3935_AFE_GAIN		0x00
+#define AS3935_AFE_MASK		0x3F
+#define AS3935_AFE_GAIN_MAX	0x1F
+#define AS3935_AFE_PWR_BIT	BIT(0)
+
+#define AS3935_INT		0x03
+#define AS3935_INT_MASK		0x07
+#define AS3935_EVENT_INT	BIT(3)
+#define AS3935_NOISE_INT	BIT(1)
+
+#define AS3935_DATA		0x07
+#define AS3935_DATA_MASK	0x3F
+
+#define AS3935_TUNE_CAP		0x08
+#define AS3935_CALIBRATE	0x3D
+
+#define AS3935_WRITE_DATA	BIT(15)
+#define AS3935_READ_DATA	BIT(14)
+#define AS3935_ADDRESS(x)	(x<<8)
+
+struct as3935_state {
+	struct spi_device *spi;
+	struct iio_trigger *trig;
+	struct mutex lock;
+	struct delayed_work work;
+
+	u32 tune_cap;
+};
+
+static const struct iio_chan_spec as3935_channels[] = {
+	{
+		.type           = IIO_PROXIMITY,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW),
+		.scan_index     = 0,
+		.scan_type = {
+			.sign           = 'u',
+			.realbits       = 6,
+			.storagebits    = 8,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
+{
+	u8 tx, rx;
+	int ret;
+
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &tx,
+			.bits_per_word = 8,
+			.len = 1,
+		}, {
+			.rx_buf = &rx,
+			.bits_per_word = 8,
+			.len = 1,
+		},
+	};
+	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	*val = rx;
+
+	return ret;
+};
+
+static int as3935_write(struct as3935_state *st,
+				unsigned int reg,
+				unsigned int val)
+{
+	u8 buf[2];
+
+	buf[0] = (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8;
+	buf[1] = val;
+
+	return spi_write(st->spi, (u8 *) &buf, 2);
+};
+
+static ssize_t as3935_gain_boost_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val = (val & AS3935_AFE_MASK) >> 1;
+
+	return sprintf(buf, "%d\n", val);
+};
+
+static ssize_t as3935_gain_boost_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul((const char *) buf, 10, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val > AS3935_AFE_GAIN_MAX)
+		return -EINVAL;
+
+	as3935_write(st, AS3935_AFE_GAIN, val << 1);
+
+	return len;
+};
+
+static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
+	as3935_gain_boost_show, as3935_gain_boost_store, 0);
+
+
+static struct attribute *as3935_attributes[] = {
+	&iio_dev_attr_gain_boost.dev_attr.attr,
+	NULL,
+};
+
+static struct attribute_group as3935_attribute_group = {
+	.attrs = as3935_attributes,
+};
+
+static int as3935_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct as3935_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (m != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	*val2 = 0;
+	ret = as3935_read(st, AS3935_DATA, val);
+	if (ret)
+		return ret;
+	return IIO_VAL_INT;
+}
+
+static const struct iio_info as3935_info = {
+	.driver_module = THIS_MODULE,
+	.attrs = &as3935_attribute_group,
+	.read_raw = &as3935_read_raw,
+};
+
+static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
+	.postenable = &iio_triggered_buffer_postenable,
+	.predisable = &iio_triggered_buffer_predisable,
+};
+
+static irqreturn_t as3935_trigger_handler(int irq, void *private)
+{
+	struct iio_poll_func *pf = private;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_DATA, &val);
+	if (ret)
+		goto err_read;
+	val &= AS3935_DATA_MASK;
+	iio_push_to_buffers_with_timestamp(indio_dev, &val, iio_get_time_ns());
+err_read:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+};
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static void as3935_event_work(struct work_struct *work)
+{
+	struct as3935_state *st;
+	struct spi_device *spi;
+	int val;
+
+	st = container_of(work, struct as3935_state, work.work);
+	spi = st->spi;
+
+	as3935_read(st, AS3935_INT, &val);
+	val &= AS3935_INT_MASK;
+
+	switch (val) {
+	case AS3935_EVENT_INT:
+		iio_trigger_poll(st->trig, 0);
+		break;
+	case AS3935_NOISE_INT:
+		dev_warn(&spi->dev, "noise level is too high");
+		break;
+	}
+};
+
+static irqreturn_t as3935_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	cancel_delayed_work(&st->work);
+	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
+	return IRQ_HANDLED;
+}
+
+static void calibrate_as3935(struct as3935_state *st)
+{
+	mutex_lock(&st->lock);
+
+	/* mask disturber interrupt bit */
+	as3935_write(st, AS3935_INT, 1 << 5);
+
+	as3935_write(st, AS3935_CALIBRATE, 0x96);
+	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
+
+	mdelay(2);
+	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
+
+	mutex_unlock(&st->lock);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	mutex_lock(&st->lock);
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val |= AS3935_AFE_PWR_BIT;
+
+	ret = as3935_write(st, AS3935_AFE_GAIN, val);
+	mutex_unlock(&st->lock);
+	return ret;
+}
+
+static int as3935_resume(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	mutex_lock(&st->lock);
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val &= ~AS3935_AFE_PWR_BIT;
+	ret = as3935_write(st, AS3935_AFE_GAIN, val);
+	mutex_unlock(&st->lock);
+	return ret;
+}
+#else
+#define as3935_suspend	NULL
+#define as3935_resume	NULL
+#endif
+
+static int as3935_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct iio_trigger *trig;
+	struct as3935_state *st;
+	struct device_node *np = spi->dev.of_node;
+	int ret;
+
+	/* Be sure lightning event interrupt */
+	if (!spi->irq) {
+		dev_err(&spi->dev, "unable to get event interrupt\n");
+		return -EINVAL;
+	}
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->tune_cap = 0;
+
+	spi_set_drvdata(spi, indio_dev);
+	mutex_init(&st->lock);
+	INIT_DELAYED_WORK(&st->work, as3935_event_work);
+
+	ret = of_property_read_u32(np, "ams,tune-cap", &st->tune_cap);
+	if (ret) {
+		st->tune_cap = 0;
+		dev_warn(&spi->dev,
+			"no tune-cap set, defaulting to %d", st->tune_cap);
+	}
+
+	if (st->tune_cap > 15) {
+		dev_err(&spi->dev,
+			"wrong tune-cap setting of %d\n", st->tune_cap);
+		return -EINVAL;
+	}
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->channels = as3935_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &as3935_info;
+
+	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+				      indio_dev->name, indio_dev->id);
+
+	if (!trig)
+		return -ENOMEM;
+
+	st->trig = trig;
+	trig->dev.parent = indio_dev->dev.parent;
+	iio_trigger_set_drvdata(trig, indio_dev);
+	trig->ops = &iio_interrupt_trigger_ops;
+
+	ret = iio_trigger_register(trig);
+	if (ret) {
+		dev_err(&spi->dev, "failed to register trigger\n");
+		return ret;
+	}
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					&as3935_trigger_handler,
+					&iio_triggered_buffer_setup_ops);
+
+	if (ret) {
+		dev_err(&spi->dev, "cannot setup iio trigger\n");
+		goto unregister_trigger;
+	}
+
+	calibrate_as3935(st);
+
+	ret = devm_request_irq(&spi->dev, spi->irq,
+				&as3935_interrupt_handler,
+				IRQF_TRIGGER_RISING,
+				dev_name(&spi->dev),
+				indio_dev);
+
+	if (ret) {
+		dev_err(&spi->dev, "unable to request irq\n");
+		goto unregister_trigger;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&spi->dev, "unable to register device\n");
+		goto unregister_trigger;
+	}
+	return 0;
+
+unregister_trigger:
+	iio_trigger_unregister(st->trig);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return ret;
+};
+
+static int as3935_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	iio_trigger_unregister(st->trig);
+	iio_triggered_buffer_cleanup(indio_dev);
+	iio_device_unregister(indio_dev);
+
+	return 0;
+};
+
+static const struct spi_device_id as3935_id[] = {
+	{"as3935", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, as3935_id);
+
+static struct spi_driver as3935_driver = {
+	.driver = {
+		.name	= "as3935",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= as3935_probe,
+	.remove		= as3935_remove,
+	.id_table	= as3935_id,
+	.suspend	= as3935_suspend,
+	.resume		= as3935_resume,
+};
+module_spi_driver(as3935_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("AS3935 lightning sensor");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:as3935");
-- 
1.8.3.2

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Jonathan Cameron]

On 06/02/14 07:00, Matt Ranostay wrote:
> AS3935 chipset can detect lightning strikes and reports those back as
> events and the estimated distance to the storm.
>
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
Hi Matt,

Sorry for my somewhat late entry in the discussion of the interface for this
device, but I'm wondering if we don't have an opportunity to create a more
flexible interface for 'position' measurements.  Afterall sooner or later
we'll get a magetic position sensor or similar coming through.

So how about defining a new iio_chan_type IIO_POSITION and adding some
more modifiers to allow for polar coordinates (spherical coordinates I guess
given we are in 3D). For now we'd only need IIO_MOD_R though later I guess
we would have IIO_MOD_THETA and IIO_MOD_PHI.  That would in this case give
us.

in_position_r_raw with the unit after applying scaling being meters.

Clearly the ideal would have been to bring this suggestion up before we
had the proximity sensors but such is life.  Those tend to simply indicate
something is 'near' rather than at an explicity distance.  The difference
is clearly minor though.

What do people think of this approach?

Comments on the code inline. Note I review from the end (usually makes
more sense) so comments may only make sense in that direction!

Beware of the quirks of spi buses as highlighted inline.  You have to allow
for the buffers you provide being used directly for DMA transfers (unlike
say i2c which copies everything). As such there are some restrictions on
where these buffers must lie.

Jonathan
> ---
>   .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
>   drivers/iio/Kconfig                                |   1 +
>   drivers/iio/Makefile                               |   1 +
>   drivers/iio/proximity/Kconfig                      |  19 +
>   drivers/iio/proximity/Makefile                     |   6 +
>   drivers/iio/proximity/as3935.c                     | 437 +++++++++++++++++++++
>   6 files changed, 482 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>   create mode 100644 drivers/iio/proximity/Kconfig
>   create mode 100644 drivers/iio/proximity/Makefile
>   create mode 100644 drivers/iio/proximity/as3935.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935 b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
> new file mode 100644
> index 0000000..f6d9e6f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
> @@ -0,0 +1,18 @@
> +What		/sys/bus/iio/devices/iio:deviceX/in_proximity_raw
> +Date:		January 2014
> +KernelVersion:	3.15
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the current distance in kilometers of storm
> +		1    = storm overhead
> +		1-40 = distance in kilometers
> +		63   = out of range
This attribute should be a general purpose one given it will apply to lots
of other drivers over time.  Write it as such and distances should definitely
be in meters not kilometers.

> +
> +What		/sys/bus/iio/devices/iio:deviceX/gain_boost
> +Date:		January 2014
> +KernelVersion:	3.15
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Show or set the gain boost of the amp, from 0-31 range.
> +		18 = indoors (default)
> +		14 = outdoors
What does this gain boost actually mean?  I couldn't immediately tell from
the datasheet.  Does it effect the distance estimates, or is it about
sensitivity to detect them in the first place?  Even though you have defined
it just for this one device, we are always looking to generalize interfaces
so it is a good to have an idea of what it actually is.
   
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 5dd0e12..743485e 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -74,6 +74,7 @@ if IIO_TRIGGER
>      source "drivers/iio/trigger/Kconfig"
>   endif #IIO_TRIGGER
>   source "drivers/iio/pressure/Kconfig"
> +source "drivers/iio/proximity/Kconfig"
>   source "drivers/iio/temperature/Kconfig"
>
>   endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 887d390..698afc2 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -24,5 +24,6 @@ obj-y += light/
>   obj-y += magnetometer/
>   obj-y += orientation/
>   obj-y += pressure/
> +obj-y += proximity/
>   obj-y += temperature/
>   obj-y += trigger/
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> new file mode 100644
> index 0000000..0c8cdf5
> --- /dev/null
> +++ b/drivers/iio/proximity/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Proximity sensors
> +#
> +
> +menu "Lightning sensors"
> +
> +config AS3935
> +	tristate "AS3935 Franklin lightning sensor"
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	depends on SPI
> +	help
> +	  Say Y here to build SPI interface support for the Austrian
> +	  Microsystems AS3935 lightning detection sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called as3935
> +
> +endmenu
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> new file mode 100644
> index 0000000..743adee
> --- /dev/null
> +++ b/drivers/iio/proximity/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for IIO proximity sensors
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_AS3935)		+= as3935.o
> diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
> new file mode 100644
> index 0000000..109759e
> --- /dev/null
> +++ b/drivers/iio/proximity/as3935.c
> @@ -0,0 +1,437 @@
> +/*
> + * as3935.c - Support for AS3935 Franklin lightning sensor
> + *
> + * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/workqueue.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/of_gpio.h>
> +
> +
> +#define AS3935_AFE_GAIN		0x00
> +#define AS3935_AFE_MASK		0x3F
> +#define AS3935_AFE_GAIN_MAX	0x1F
> +#define AS3935_AFE_PWR_BIT	BIT(0)
> +
> +#define AS3935_INT		0x03
> +#define AS3935_INT_MASK		0x07
> +#define AS3935_EVENT_INT	BIT(3)
> +#define AS3935_NOISE_INT	BIT(1)
> +
> +#define AS3935_DATA		0x07
> +#define AS3935_DATA_MASK	0x3F
> +
> +#define AS3935_TUNE_CAP		0x08
> +#define AS3935_CALIBRATE	0x3D
> +
> +#define AS3935_WRITE_DATA	BIT(15)
> +#define AS3935_READ_DATA	BIT(14)
((x) << 8) to protect against strange elements being passed in as x.
Obviously you have this tightly controlled here, but who knows what others
might add in future, so best to do it right!
> +#define AS3935_ADDRESS(x)	(x<<8)
> +
> +struct as3935_state {
> +	struct spi_device *spi;
> +	struct iio_trigger *trig;
> +	struct mutex lock;
> +	struct delayed_work work;
> +
> +	u32 tune_cap;
> +};
> +
> +static const struct iio_chan_spec as3935_channels[] = {
> +	{
> +		.type           = IIO_PROXIMITY,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW),
> +		.scan_index     = 0,
> +		.scan_type = {
> +			.sign           = 'u',
> +			.realbits       = 6,
> +			.storagebits    = 8,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
This looks very similar to val = spi_w8r8(st->spi, reg);

> +static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
> +{
> +	u8 tx, rx;
Spi buffers should never be on the stack. You can't guarantee alignment and
they must be cache aligned.  spi_w8r8 deals with this by using a small
buffer allocated on the heap.  The alternative is to carefully allocate
a cache line aligned buffer in your state structure.  Here I'd definitely
use spi_w8r8 though!
> +	int ret;
> +
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = &tx,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		}, {
> +			.rx_buf = &rx,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		},
> +	};
> +	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +	*val = rx;
> +
> +	return ret;
> +};
> +

> +static int as3935_write(struct as3935_state *st,
> +				unsigned int reg,
> +				unsigned int val)
> +{
> +	u8 buf[2];
You could use a C99 style asignment here (entirely optional!)
e.g.
	u8 buf[2] = { (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8,
	   	      val };
However, this buffer must be cacheline aligned seeing as spi_write
does not use a suitably aligned bounce buffer (unlike spi_w8r8 which does).
This can cause nasty, difficult to track down corruptions with spi controllers
that do DMA.
> +
> +	buf[0] = (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8;
> +	buf[1] = val;
> +
> +	return spi_write(st->spi, (u8 *) &buf, 2);
> +};
> +
> +static ssize_t as3935_gain_boost_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val = (val & AS3935_AFE_MASK) >> 1;
> +
> +	return sprintf(buf, "%d\n", val);
> +};
> +
> +static ssize_t as3935_gain_boost_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul((const char *) buf, 10, &val);
> +	if (ret)
> +		return -EINVAL;
> +
> +	if (val > AS3935_AFE_GAIN_MAX)
> +		return -EINVAL;
> +
> +	as3935_write(st, AS3935_AFE_GAIN, val << 1);
> +
> +	return len;
> +};
> +
> +static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
> +	as3935_gain_boost_show, as3935_gain_boost_store, 0);
> +
> +
> +static struct attribute *as3935_attributes[] = {
> +	&iio_dev_attr_gain_boost.dev_attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group as3935_attribute_group = {
> +	.attrs = as3935_attributes,
> +};
> +
> +static int as3935_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (m != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	*val2 = 0;
> +	ret = as3935_read(st, AS3935_DATA, val);
> +	if (ret)
> +		return ret;
> +	return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info as3935_info = {
> +	.driver_module = THIS_MODULE,
> +	.attrs = &as3935_attribute_group,
> +	.read_raw = &as3935_read_raw,
> +};
> +
> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
> +	.postenable = &iio_triggered_buffer_postenable,
> +	.predisable = &iio_triggered_buffer_predisable,
> +};
> +
> +static irqreturn_t as3935_trigger_handler(int irq, void *private)
> +{
> +	struct iio_poll_func *pf = private;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_DATA, &val);
> +	if (ret)
> +		goto err_read;
> +	val &= AS3935_DATA_MASK;
This timestamp is probably rather late given I think the interrupt indicated
the actual point in time?
> +	iio_push_to_buffers_with_timestamp(indio_dev, &val, iio_get_time_ns());
> +err_read:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +};
> +
> +static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
> +	.owner = THIS_MODULE,
> +};
> +
> +static void as3935_event_work(struct work_struct *work)
> +{
> +	struct as3935_state *st;
> +	struct spi_device *spi;
> +	int val;
> +
> +	st = container_of(work, struct as3935_state, work.work);
> +	spi = st->spi;
Don't bother with the local variable for spi. It doesn't add any clarity.
> +
> +	as3935_read(st, AS3935_INT, &val);
> +	val &= AS3935_INT_MASK;
> +
> +	switch (val) {
> +	case AS3935_EVENT_INT:
You could save a timestamp in the interrupt handler then pass it on here.
> +		iio_trigger_poll(st->trig, 0);
> +		break;
> +	case AS3935_NOISE_INT:
Hmm. This brings me back to one of my favourite kernel questions,
'Why isn't there a better way of handling hardware errors?'
There isn't as far as I know!
> +		dev_warn(&spi->dev, "noise level is too high");
> +		break;
> +	}
> +};
> +
> +static irqreturn_t as3935_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
This is 'unusual' so can you add a comment explaining what is going on here.
My guess is that you want to avoid grabing new data for a short window after
a strike is detected?  The cancel is to avoid running the delayed work twice?

If so the reason a sleep in a threaded handler wouldn't work is that it would
not 'cancel' if new data arrived I guess.  You still have a window for problems
if the interrupt occurs whilst the work queue is actually running the
scheduled work.

I'm wondering if this is overcomplicating things and it is better to just
occasionally handle an extra interrupt and do this with a conventional
threaded interrupt handler.  Of course I may be missing something.
> +	cancel_delayed_work(&st->work);
I'm guessing this should be msecs_to_jiffies seeing as schedule_delayed_work
takes a value in jiffies.
> +	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
blank line here.
> +	return IRQ_HANDLED;
> +}
> +
> +static void calibrate_as3935(struct as3935_state *st)
> +{
> +	mutex_lock(&st->lock);
> +
> +	/* mask disturber interrupt bit */
> +	as3935_write(st, AS3935_INT, 1 << 5);
> +
> +	as3935_write(st, AS3935_CALIBRATE, 0x96);
> +	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
> +
> +	mdelay(2);
> +	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
> +
> +	mutex_unlock(&st->lock);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	mutex_lock(&st->lock);
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
Mutex not released.

> +	val |= AS3935_AFE_PWR_BIT;
> +
> +	ret = as3935_write(st, AS3935_AFE_GAIN, val);
> +	mutex_unlock(&st->lock);
> +	return ret;
> +}
> +
> +static int as3935_resume(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	mutex_lock(&st->lock);
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
You haven't released the mutex in this error path.  This is a classic
case for using a goto rather than returning directly here.

> +	val &= ~AS3935_AFE_PWR_BIT;
> +	ret = as3935_write(st, AS3935_AFE_GAIN, val);
> +	mutex_unlock(&st->lock);
blank line before returns in simple cases like this.  Just makes things
ever so slightly easier to read ;)
> +	return ret;
> +}
> +#else
> +#define as3935_suspend	NULL
> +#define as3935_resume	NULL
> +#endif
> +
> +static int as3935_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct iio_trigger *trig;
> +	struct as3935_state *st;
> +	struct device_node *np = spi->dev.of_node;
> +	int ret;
> +
/* Make sure the lightning event interrupt is specified. */
You probably don't need to do this as the request will fail if supplied
with a 0. Even if you want to keep this, it would be cleaner to have it
down where the request for the irq is made rather than here at the start.
> +	/* Be sure lightning event interrupt */
> +	if (!spi->irq) {
> +		dev_err(&spi->dev, "unable to get event interrupt\n");
> +		return -EINVAL;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->tune_cap = 0;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	mutex_init(&st->lock);
> +	INIT_DELAYED_WORK(&st->work, as3935_event_work);
> +
> +	ret = of_property_read_u32(np, "ams,tune-cap", &st->tune_cap);
> +	if (ret) {
> +		st->tune_cap = 0;
> +		dev_warn(&spi->dev,
> +			"no tune-cap set, defaulting to %d", st->tune_cap);
> +	}
> +
> +	if (st->tune_cap > 15) {
> +		dev_err(&spi->dev,
> +			"wrong tune-cap setting of %d\n", st->tune_cap);
> +		return -EINVAL;
> +	}
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->channels = as3935_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &as3935_info;
> +
> +	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> +				      indio_dev->name, indio_dev->id);
> +
> +	if (!trig)
> +		return -ENOMEM;
> +
> +	st->trig = trig;
> +	trig->dev.parent = indio_dev->dev.parent;
> +	iio_trigger_set_drvdata(trig, indio_dev);
> +	trig->ops = &iio_interrupt_trigger_ops;
> +
> +	ret = iio_trigger_register(trig);
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to register trigger\n");
> +		return ret;
> +	}
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					&as3935_trigger_handler,
> +					&iio_triggered_buffer_setup_ops);
> +
> +	if (ret) {
> +		dev_err(&spi->dev, "cannot setup iio trigger\n");
> +		goto unregister_trigger;
> +	}
> +
> +	calibrate_as3935(st);
> +
> +	ret = devm_request_irq(&spi->dev, spi->irq,
> +				&as3935_interrupt_handler,
> +				IRQF_TRIGGER_RISING,
> +				dev_name(&spi->dev),
> +				indio_dev);
> +
> +	if (ret) {
> +		dev_err(&spi->dev, "unable to request irq\n");
> +		goto unregister_trigger;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "unable to register device\n");
> +		goto unregister_trigger;
> +	}
> +	return 0;
> +
> +unregister_trigger:
> +	iio_trigger_unregister(st->trig);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	return ret;
> +};
> +
> +static int as3935_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
> +	iio_trigger_unregister(st->trig);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	iio_device_unregister(indio_dev);
This should be in the reverse order of what goes on in the probe.
This helps both for review purposes and to avoid actual bugs. Here for
example, the buffer has been destroyed before the userspace interface
is removed in the device_unregister.

> +
> +	return 0;
> +};
> +
> +static const struct spi_device_id as3935_id[] = {
> +	{"as3935", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, as3935_id);
> +
> +static struct spi_driver as3935_driver = {
> +	.driver = {
> +		.name	= "as3935",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= as3935_probe,
> +	.remove		= as3935_remove,
> +	.id_table	= as3935_id,
> +	.suspend	= as3935_suspend,
> +	.resume		= as3935_resume,
> +};
> +module_spi_driver(as3935_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("AS3935 lightning sensor");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("spi:as3935");
>

Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support

[Matt Ranostay]

On Sun, Feb 9, 2014 at 1:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 06/02/14 07:00, Matt Ranostay wrote:
>>
>> AS3935 chipset can detect lightning strikes and reports those back as
>> events and the estimated distance to the storm.
>>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>
> Hi Matt,
>
> Sorry for my somewhat late entry in the discussion of the interface for this
> device, but I'm wondering if we don't have an opportunity to create a more
> flexible interface for 'position' measurements.  Afterall sooner or later
> we'll get a magetic position sensor or similar coming through.
>
> So how about defining a new iio_chan_type IIO_POSITION and adding some
> more modifiers to allow for polar coordinates (spherical coordinates I guess
> given we are in 3D). For now we'd only need IIO_MOD_R though later I guess
> we would have IIO_MOD_THETA and IIO_MOD_PHI.  That would in this case give
> us.
>
> in_position_r_raw with the unit after applying scaling being meters.
>
> Clearly the ideal would have been to bring this suggestion up before we
> had the proximity sensors but such is life.  Those tend to simply indicate
> something is 'near' rather than at an explicity distance.  The difference
> is clearly minor though.
>
> What do people think of this approach?
>
> Comments on the code inline. Note I review from the end (usually makes
> more sense) so comments may only make sense in that direction!
>
> Beware of the quirks of spi buses as highlighted inline.  You have to allow
> for the buffers you provide being used directly for DMA transfers (unlike
> say i2c which copies everything). As such there are some restrictions on
> where these buffers must lie.
>
> Jonathan
>
>> ---
>>   .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
>>   drivers/iio/Kconfig                                |   1 +
>>   drivers/iio/Makefile                               |   1 +
>>   drivers/iio/proximity/Kconfig                      |  19 +
>>   drivers/iio/proximity/Makefile                     |   6 +
>>   drivers/iio/proximity/as3935.c                     | 437
>> +++++++++++++++++++++
>>   6 files changed, 482 insertions(+)
>>   create mode 100644
>> Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>>   create mode 100644 drivers/iio/proximity/Kconfig
>>   create mode 100644 drivers/iio/proximity/Makefile
>>   create mode 100644 drivers/iio/proximity/as3935.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>> b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>> new file mode 100644
>> index 0000000..f6d9e6f
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>> @@ -0,0 +1,18 @@
>> +What           /sys/bus/iio/devices/iio:deviceX/in_proximity_raw
>> +Date:          January 2014
>> +KernelVersion: 3.15
>> +Contact:       Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +               Get the current distance in kilometers of storm
>> +               1    = storm overhead
>> +               1-40 = distance in kilometers
>> +               63   = out of range
>
> This attribute should be a general purpose one given it will apply to lots
> of other drivers over time.  Write it as such and distances should
> definitely
> be in meters not kilometers.
>
>
>> +
>> +What           /sys/bus/iio/devices/iio:deviceX/gain_boost
>> +Date:          January 2014
>> +KernelVersion: 3.15
>> +Contact:       Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +               Show or set the gain boost of the amp, from 0-31 range.
>> +               18 = indoors (default)
>> +               14 = outdoors
>
> What does this gain boost actually mean?  I couldn't immediately tell from
> the datasheet.  Does it effect the distance estimates, or is it about
> sensitivity to detect them in the first place?  Even though you have defined
> it just for this one device, we are always looking to generalize interfaces
> so it is a good to have an idea of what it actually is.
>

Controls the goo

>
>>
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 5dd0e12..743485e 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -74,6 +74,7 @@ if IIO_TRIGGER
>>      source "drivers/iio/trigger/Kconfig"
>>   endif #IIO_TRIGGER
>>   source "drivers/iio/pressure/Kconfig"
>> +source "drivers/iio/proximity/Kconfig"
>>   source "drivers/iio/temperature/Kconfig"
>>
>>   endif # IIO
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 887d390..698afc2 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -24,5 +24,6 @@ obj-y += light/
>>   obj-y += magnetometer/
>>   obj-y += orientation/
>>   obj-y += pressure/
>> +obj-y += proximity/
>>   obj-y += temperature/
>>   obj-y += trigger/
>> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
>> new file mode 100644
>> index 0000000..0c8cdf5
>> --- /dev/null
>> +++ b/drivers/iio/proximity/Kconfig
>> @@ -0,0 +1,19 @@
>> +#
>> +# Proximity sensors
>> +#
>> +
>> +menu "Lightning sensors"
>> +
>> +config AS3935
>> +       tristate "AS3935 Franklin lightning sensor"
>> +       select IIO_BUFFER
>> +       select IIO_TRIGGERED_BUFFER
>> +       depends on SPI
>> +       help
>> +         Say Y here to build SPI interface support for the Austrian
>> +         Microsystems AS3935 lightning detection sensor.
>> +
>> +         To compile this driver as a module, choose M here: the
>> +         module will be called as3935
>> +
>> +endmenu
>> diff --git a/drivers/iio/proximity/Makefile
>> b/drivers/iio/proximity/Makefile
>> new file mode 100644
>> index 0000000..743adee
>> --- /dev/null
>> +++ b/drivers/iio/proximity/Makefile
>> @@ -0,0 +1,6 @@
>> +#
>> +# Makefile for IIO proximity sensors
>> +#
>> +
>> +# When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_AS3935)           += as3935.o
>> diff --git a/drivers/iio/proximity/as3935.c
>> b/drivers/iio/proximity/as3935.c
>> new file mode 100644
>> index 0000000..109759e
>> --- /dev/null
>> +++ b/drivers/iio/proximity/as3935.c
>> @@ -0,0 +1,437 @@
>> +/*
>> + * as3935.c - Support for AS3935 Franklin lightning sensor
>> + *
>> + * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/irq.h>
>> +#include <linux/gpio.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/of_gpio.h>
>> +
>> +
>> +#define AS3935_AFE_GAIN                0x00
>> +#define AS3935_AFE_MASK                0x3F
>> +#define AS3935_AFE_GAIN_MAX    0x1F
>> +#define AS3935_AFE_PWR_BIT     BIT(0)
>> +
>> +#define AS3935_INT             0x03
>> +#define AS3935_INT_MASK                0x07
>> +#define AS3935_EVENT_INT       BIT(3)
>> +#define AS3935_NOISE_INT       BIT(1)
>> +
>> +#define AS3935_DATA            0x07
>> +#define AS3935_DATA_MASK       0x3F
>> +
>> +#define AS3935_TUNE_CAP                0x08
>> +#define AS3935_CALIBRATE       0x3D
>> +
>> +#define AS3935_WRITE_DATA      BIT(15)
>> +#define AS3935_READ_DATA       BIT(14)
>
> ((x) << 8) to protect against strange elements being passed in as x.
> Obviously you have this tightly controlled here, but who knows what others
> might add in future, so best to do it right!
>
>> +#define AS3935_ADDRESS(x)      (x<<8)
>> +
>> +struct as3935_state {
>> +       struct spi_device *spi;
>> +       struct iio_trigger *trig;
>> +       struct mutex lock;
>> +       struct delayed_work work;
>> +
>> +       u32 tune_cap;
>> +};
>> +
>> +static const struct iio_chan_spec as3935_channels[] = {
>> +       {
>> +               .type           = IIO_PROXIMITY,
>> +               .info_mask_separate =
>> +                       BIT(IIO_CHAN_INFO_RAW),
>> +               .scan_index     = 0,
>> +               .scan_type = {
>> +                       .sign           = 'u',
>> +                       .realbits       = 6,
>> +                       .storagebits    = 8,
>> +               },
>> +       },
>> +       IIO_CHAN_SOFT_TIMESTAMP(1),
>> +};
>> +
>
> This looks very similar to val = spi_w8r8(st->spi, reg);
>
>
>> +static int as3935_read(struct as3935_state *st, unsigned int reg, int
>> *val)
>> +{
>> +       u8 tx, rx;
>
> Spi buffers should never be on the stack. You can't guarantee alignment and
> they must be cache aligned.  spi_w8r8 deals with this by using a small
> buffer allocated on the heap.  The alternative is to carefully allocate
> a cache line aligned buffer in your state structure.  Here I'd definitely
> use spi_w8r8 though!
>

Noted will do the former.. having it the in the as3935_state struct
would make sense if the buffer was needed all over.

>> +       int ret;
>> +
>> +       struct spi_transfer xfers[] = {
>> +               {
>> +                       .tx_buf = &tx,
>> +                       .bits_per_word = 8,
>> +                       .len = 1,
>> +               }, {
>> +                       .rx_buf = &rx,
>> +                       .bits_per_word = 8,
>> +                       .len = 1,
>> +               },
>> +       };
>> +       tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
>> +
>> +       ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>> +       *val = rx;
>> +
>> +       return ret;
>> +};
>> +
>
>
>> +static int as3935_write(struct as3935_state *st,
>> +                               unsigned int reg,
>> +                               unsigned int val)
>> +{
>> +       u8 buf[2];
>
> You could use a C99 style asignment here (entirely optional!)
> e.g.
>         u8 buf[2] = { (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8,
>                       val };
> However, this buffer must be cacheline aligned seeing as spi_write
> does not use a suitably aligned bounce buffer (unlike spi_w8r8 which does).
> This can cause nasty, difficult to track down corruptions with spi
> controllers
> that do DMA.
>
>> +
>> +       buf[0] = (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8;
>> +       buf[1] = val;
>> +
>> +       return spi_write(st->spi, (u8 *) &buf, 2);
>> +};
>> +
>> +static ssize_t as3935_gain_boost_show(struct device *dev,
>> +                               struct device_attribute *attr,
>> +                               char *buf)
>> +{
>> +       struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
>> +       int val, ret;
>> +
>> +       ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +       if (ret)
>> +               return ret;
>> +       val = (val & AS3935_AFE_MASK) >> 1;
>> +
>> +       return sprintf(buf, "%d\n", val);
>> +};
>> +
>> +static ssize_t as3935_gain_boost_store(struct device *dev,
>> +                                       struct device_attribute *attr,
>> +                                       const char *buf, size_t len)
>> +{
>> +       struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
>> +       unsigned long val;
>> +       int ret;
>> +
>> +       ret = kstrtoul((const char *) buf, 10, &val);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       if (val > AS3935_AFE_GAIN_MAX)
>> +               return -EINVAL;
>> +
>> +       as3935_write(st, AS3935_AFE_GAIN, val << 1);
>> +
>> +       return len;
>> +};
>> +
>> +static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
>> +       as3935_gain_boost_show, as3935_gain_boost_store, 0);
>> +
>> +
>> +static struct attribute *as3935_attributes[] = {
>> +       &iio_dev_attr_gain_boost.dev_attr.attr,
>> +       NULL,
>> +};
>> +
>> +static struct attribute_group as3935_attribute_group = {
>> +       .attrs = as3935_attributes,
>> +};
>> +
>> +static int as3935_read_raw(struct iio_dev *indio_dev,
>> +                          struct iio_chan_spec const *chan,
>> +                          int *val,
>> +                          int *val2,
>> +                          long m)
>> +{
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int ret;
>> +
>> +       if (m != IIO_CHAN_INFO_RAW)
>> +               return -EINVAL;
>> +
>> +       *val2 = 0;
>> +       ret = as3935_read(st, AS3935_DATA, val);
>> +       if (ret)
>> +               return ret;
>> +       return IIO_VAL_INT;
>> +}
>> +
>> +static const struct iio_info as3935_info = {
>> +       .driver_module = THIS_MODULE,
>> +       .attrs = &as3935_attribute_group,
>> +       .read_raw = &as3935_read_raw,
>> +};
>> +
>> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops =
>> {
>> +       .postenable = &iio_triggered_buffer_postenable,
>> +       .predisable = &iio_triggered_buffer_predisable,
>> +};
>> +
>> +static irqreturn_t as3935_trigger_handler(int irq, void *private)
>> +{
>> +       struct iio_poll_func *pf = private;
>> +       struct iio_dev *indio_dev = pf->indio_dev;
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int val, ret;
>> +
>> +       ret = as3935_read(st, AS3935_DATA, &val);
>> +       if (ret)
>> +               goto err_read;
>> +       val &= AS3935_DATA_MASK;
>
> This timestamp is probably rather late given I think the interrupt indicated
> the actual point in time?
>
>> +       iio_push_to_buffers_with_timestamp(indio_dev, &val,
>> iio_get_time_ns());
>> +err_read:
>> +       iio_trigger_notify_done(indio_dev->trig);
>> +
>> +       return IRQ_HANDLED;
>> +};
>> +
>> +static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
>> +       .owner = THIS_MODULE,
>> +};
>> +
>> +static void as3935_event_work(struct work_struct *work)
>> +{
>> +       struct as3935_state *st;
>> +       struct spi_device *spi;
>> +       int val;
>> +
>> +       st = container_of(work, struct as3935_state, work.work);
>> +       spi = st->spi;
>
> Don't bother with the local variable for spi. It doesn't add any clarity.
>
>> +
>> +       as3935_read(st, AS3935_INT, &val);
>> +       val &= AS3935_INT_MASK;
>> +
>> +       switch (val) {
>> +       case AS3935_EVENT_INT:
>
> You could save a timestamp in the interrupt handler then pass it on here.
>
Noted.

>> +               iio_trigger_poll(st->trig, 0);
>> +               break;
>> +       case AS3935_NOISE_INT:
>
> Hmm. This brings me back to one of my favourite kernel questions,
> 'Why isn't there a better way of handling hardware errors?'
> There isn't as far as I know!
>
>> +               dev_warn(&spi->dev, "noise level is too high");
>> +               break;
>> +       }
>> +};
>> +
>> +static irqreturn_t as3935_interrupt_handler(int irq, void *private)
>> +{
>> +       struct iio_dev *indio_dev = private;
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +
>
> This is 'unusual' so can you add a comment explaining what is going on here.
> My guess is that you want to avoid grabing new data for a short window after
> a strike is detected?  The cancel is to avoid running the delayed work
> twice?
>
> If so the reason a sleep in a threaded handler wouldn't work is that it
> would
> not 'cancel' if new data arrived I guess.  You still have a window for
> problems
> if the interrupt occurs whilst the work queue is actually running the
> scheduled work.
>
> I'm wondering if this is overcomplicating things and it is better to just
> occasionally handle an extra interrupt and do this with a conventional
> threaded interrupt handler.  Of course I may be missing something.
>>
>> +       cancel_delayed_work(&st->work);
>
> I'm guessing this should be msecs_to_jiffies seeing as schedule_delayed_work
> takes a value in jiffies.
>>
>> +       schedule_delayed_work(&st->work, jiffies_to_msecs(3));
>

Yes it should be msec_to_jiffies(3) :/
Noted the cancel_delayed_work() probably is verbose and will drop in
the next version.

Reason for this is the datasheet requires (suggests?) a 2+ millisecond
delay before reading
estimated distance value.

> blank line here.
>
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static void calibrate_as3935(struct as3935_state *st)
>> +{
>> +       mutex_lock(&st->lock);
>> +
>> +       /* mask disturber interrupt bit */
>> +       as3935_write(st, AS3935_INT, 1 << 5);
>> +
>> +       as3935_write(st, AS3935_CALIBRATE, 0x96);
>> +       as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
>> +
>> +       mdelay(2);
>> +       as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
>> +
>> +       mutex_unlock(&st->lock);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
>> +{
>> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int val, ret;
>> +
>> +       mutex_lock(&st->lock);
>> +       ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +       if (ret)
>> +               return ret;
>
> Mutex not released.
>
>
>> +       val |= AS3935_AFE_PWR_BIT;
>> +
>> +       ret = as3935_write(st, AS3935_AFE_GAIN, val);
>> +       mutex_unlock(&st->lock);
>> +       return ret;
>> +}
>> +
>> +static int as3935_resume(struct spi_device *spi)
>> +{
>> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int val, ret;
>> +
>> +       mutex_lock(&st->lock);
>> +       ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +       if (ret)
>> +               return ret;
>
> You haven't released the mutex in this error path.  This is a classic
> case for using a goto rather than returning directly here.
>

Yeah this a rookie mistake on my behalf.

>
>> +       val &= ~AS3935_AFE_PWR_BIT;
>> +       ret = as3935_write(st, AS3935_AFE_GAIN, val);
>> +       mutex_unlock(&st->lock);
>
> blank line before returns in simple cases like this.  Just makes things
> ever so slightly easier to read ;)
>>
>> +       return ret;
>> +}
>>
>> +#else
>> +#define as3935_suspend NULL
>> +#define as3935_resume  NULL
>> +#endif
>> +
>> +static int as3935_probe(struct spi_device *spi)
>> +{
>> +       struct iio_dev *indio_dev;
>> +       struct iio_trigger *trig;
>> +       struct as3935_state *st;
>> +       struct device_node *np = spi->dev.of_node;
>> +       int ret;
>> +
>
> /* Make sure the lightning event interrupt is specified. */
> You probably don't need to do this as the request will fail if supplied
> with a 0. Even if you want to keep this, it would be cleaner to have it
> down where the request for the irq is made rather than here at the start.
>

Didn't know that.. all for no

>> +       /* Be sure lightning event interrupt */
>> +       if (!spi->irq) {
>> +               dev_err(&spi->dev, "unable to get event interrupt\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
>> +       if (!indio_dev)
>> +               return -ENOMEM;
>> +
>> +       st = iio_priv(indio_dev);
>> +       st->spi = spi;
>> +       st->tune_cap = 0;
>> +
>> +       spi_set_drvdata(spi, indio_dev);
>> +       mutex_init(&st->lock);
>> +       INIT_DELAYED_WORK(&st->work, as3935_event_work);
>> +
>> +       ret = of_property_read_u32(np, "ams,tune-cap", &st->tune_cap);
>> +       if (ret) {
>> +               st->tune_cap = 0;
>> +               dev_warn(&spi->dev,
>> +                       "no tune-cap set, defaulting to %d",
>> st->tune_cap);
>> +       }
>> +
>> +       if (st->tune_cap > 15) {
>> +               dev_err(&spi->dev,
>> +                       "wrong tune-cap setting of %d\n", st->tune_cap);
>> +               return -EINVAL;
>> +       }
>> +
>> +       indio_dev->dev.parent = &spi->dev;
>> +       indio_dev->name = spi_get_device_id(spi)->name;
>> +       indio_dev->channels = as3935_channels;
>> +       indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
>> +       indio_dev->modes = INDIO_DIRECT_MODE;
>> +       indio_dev->info = &as3935_info;
>> +
>> +       trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
>> +                                     indio_dev->name, indio_dev->id);
>> +
>> +       if (!trig)
>> +               return -ENOMEM;
>> +
>> +       st->trig = trig;
>> +       trig->dev.parent = indio_dev->dev.parent;
>> +       iio_trigger_set_drvdata(trig, indio_dev);
>> +       trig->ops = &iio_interrupt_trigger_ops;
>> +
>> +       ret = iio_trigger_register(trig);
>> +       if (ret) {
>> +               dev_err(&spi->dev, "failed to register trigger\n");
>> +               return ret;
>> +       }
>> +
>> +       ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +                                       &as3935_trigger_handler,
>> +                                       &iio_triggered_buffer_setup_ops);
>> +
>> +       if (ret) {
>> +               dev_err(&spi->dev, "cannot setup iio trigger\n");
>> +               goto unregister_trigger;
>> +       }
>> +
>> +       calibrate_as3935(st);
>> +
>> +       ret = devm_request_irq(&spi->dev, spi->irq,
>> +                               &as3935_interrupt_handler,
>> +                               IRQF_TRIGGER_RISING,
>> +                               dev_name(&spi->dev),
>> +                               indio_dev);
>> +
>> +       if (ret) {
>> +               dev_err(&spi->dev, "unable to request irq\n");
>> +               goto unregister_trigger;
>> +       }
>> +
>> +       ret = iio_device_register(indio_dev);
>> +       if (ret < 0) {
>> +               dev_err(&spi->dev, "unable to register device\n");
>> +               goto unregister_trigger;
>> +       }
>> +       return 0;
>> +
>> +unregister_trigger:
>> +       iio_trigger_unregister(st->trig);
>> +       iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +       return ret;
>> +};
>> +
>> +static int as3935_remove(struct spi_device *spi)
>> +{
>> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +
>> +       iio_trigger_unregister(st->trig);
>> +       iio_triggered_buffer_cleanup(indio_dev);
>> +       iio_device_unregister(indio_dev);
>
> This should be in the reverse order of what goes on in the probe.
> This helps both for review purposes and to avoid actual bugs. Here for
> example, the buffer has been destroyed before the userspace interface
> is removed in the device_unregister.
>

Noted.

>> +
>> +       return 0;
>> +};
>> +
>> +static const struct spi_device_id as3935_id[] = {
>> +       {"as3935", 0},
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(spi, as3935_id);
>> +
>> +static struct spi_driver as3935_driver = {
>> +       .driver = {
>> +               .name   = "as3935",
>> +               .owner  = THIS_MODULE,
>> +       },
>> +       .probe          = as3935_probe,
>> +       .remove         = as3935_remove,
>> +       .id_table       = as3935_id,
>> +       .suspend        = as3935_suspend,
>> +       .resume         = as3935_resume,
>> +};
>> +module_spi_driver(as3935_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("AS3935 lightning sensor");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("spi:as3935");
>>
>