[PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Jacopo Mondi]

Hello,
   sending out v4 splitting device tree bindings documentation and actual ADC
driver.
No changes in driver code since v3.

Same question for iio maintainers here:
I would like to have clarified the measure unit returned by read_raw().
Currently (value_raw * value_scale) return the ADC input value in mV.
While testing the patch I've been questioned if that should not actually
be in uV. This is easily achievable making _scale return a value in uV.
I have found no mention of this in the ABI documentation as it speaks of
generic voltage.
Can we have a final word on this?

Thanks Marek for having tested this.

v1 -> v2:
    - incorporated pmeerw's review comments
    - retrieve vref from dts and use that to convert read_raw result
      to mV
    - add device tree bindings documentation

v2 -> v3:
    - add _SCALE bit of read_raw function and change _RAW bit accordingly
    - call regulator_get_voltage when accessing the _SCALE part of read_raw
      and not during probe
    - add back remove function as regulator has to be disabled when detaching
      the module. Do not use devm_ version of iio_register/unregister functions
      anymore but do unregister in the remove.
    - remove mutex as access to SPI bus is protected by SPI core. Thanks marex

v3 -> v4:
    - split device tree binding documentation and actual ADC driver
    - add "reg" to the list of required properties and use a better
      namimg for the adc device node in bindings documentation as suggested
      by Geert.

Jacopo Mondi (2):
  iio: adc: Add Maxim MAX11100 driver
  dt-bindings: iio: document MAX11100 ADC

 .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
 drivers/iio/adc/Kconfig                            |   9 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
 4 files changed, 216 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Jacopo Mondi]

Hello,
   sending out v4 splitting device tree bindings documentation and actual ADC
driver.
No changes in driver code since v3.

Same question for iio maintainers here:
I would like to have clarified the measure unit returned by read_raw().
Currently (value_raw * value_scale) return the ADC input value in mV.
While testing the patch I've been questioned if that should not actually
be in uV. This is easily achievable making _scale return a value in uV.
I have found no mention of this in the ABI documentation as it speaks of
generic voltage.
Can we have a final word on this?

Thanks Marek for having tested this.

v1 -> v2:
    - incorporated pmeerw's review comments
    - retrieve vref from dts and use that to convert read_raw result
      to mV
    - add device tree bindings documentation

v2 -> v3:
    - add _SCALE bit of read_raw function and change _RAW bit accordingly
    - call regulator_get_voltage when accessing the _SCALE part of read_raw
      and not during probe
    - add back remove function as regulator has to be disabled when detaching
      the module. Do not use devm_ version of iio_register/unregister functions
      anymore but do unregister in the remove.
    - remove mutex as access to SPI bus is protected by SPI core. Thanks marex

v3 -> v4:
    - split device tree binding documentation and actual ADC driver
    - add "reg" to the list of required properties and use a better
      namimg for the adc device node in bindings documentation as suggested
      by Geert.

Jacopo Mondi (2):
  iio: adc: Add Maxim MAX11100 driver
  dt-bindings: iio: document MAX11100 ADC

 .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
 drivers/iio/adc/Kconfig                            |   9 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
 4 files changed, 216 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

-- 
2.7.4

[PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Jacopo Mondi]

From: Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Add iio driver for Maxim MAX11100 single-channel ADC.

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Tested-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
 drivers/iio/adc/Kconfig    |   9 +++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 38bc319..c32bc7a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -307,6 +307,15 @@ config MAX1027
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1027.
 
+config MAX11100
+	tristate "Maxim max11100 ADC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Maxim max11100 SPI ADC
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max11100.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d36c4be..5684369 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
 obj-$(CONFIG_MAX1027) += max1027.o
+obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
 obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
new file mode 100644
index 0000000..78e2a45
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,187 @@
+/*
+ * iio/adc/max11100.c
+ * Maxim max11100 ADC Driver with IIO interface
+ *
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ * Copyright (C) 2016 Jacopo Mondi
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/driver.h>
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref_mv / 2 ^ 16)
+ *
+ * LSB is used to calculate analog voltage value
+ * from the number of ADC steps count
+ *
+ * Ain = (count * LSB)
+ */
+#define MAX11100_LSB_DIV		(1 << 16)
+
+struct max11100_state {
+	const struct max11100_chip_desc *desc;
+	struct regulator *vref_reg;
+	struct spi_device *spi;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+	{ /* [0] */
+		.type = IIO_VOLTAGE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static struct max11100_chip_desc {
+	unsigned int num_chan;
+	const struct iio_chan_spec *channels;
+} max11100_desc = {
+	.num_chan = ARRAY_SIZE(max11100_channels),
+	.channels = max11100_channels,
+};
+
+static int max11100_read_single(struct iio_dev *indio_dev, int *val)
+{
+	int ret;
+	struct max11100_state *state = iio_priv(indio_dev);
+	uint8_t buffer[3];
+
+	ret = spi_read(state->spi, buffer, sizeof(buffer));
+	if (ret) {
+		dev_err(&indio_dev->dev, "SPI transfer failed\n");
+		return ret;
+	}
+
+	/* the first 8 bits sent out from ADC must be 0s */
+	if (buffer[0]) {
+		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
+		return -EINVAL;
+	}
+
+	*val = (buffer[1] << 8) | buffer[2];
+
+	return 0;
+}
+
+static int max11100_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	int ret, vref_uv;
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = max11100_read_single(indio_dev, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		vref_uv = regulator_get_voltage(state->vref_reg);
+		if (vref_uv < 0)
+			/* dummy regulator "get_voltage" returns -EINVAL */
+			return -EINVAL;
+
+		*val =  vref_uv / 1000;
+		*val2 = MAX11100_LSB_DIV;
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info max11100_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = max11100_read_raw,
+};
+
+static int max11100_probe(struct spi_device *spi)
+{
+	int ret;
+	struct iio_dev *indio_dev;
+	struct max11100_state *state;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	state = iio_priv(indio_dev);
+	state->spi = spi;
+	state->desc = &max11100_desc;
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.of_node = spi->dev.of_node;
+	indio_dev->info = &max11100_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = state->desc->channels;
+	indio_dev->num_channels = state->desc->num_chan;
+
+	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(state->vref_reg))
+		return PTR_ERR(state->vref_reg);
+
+	ret = regulator_enable(state->vref_reg);
+	if (ret)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto disable_regulator;
+
+	return 0;
+
+disable_regulator:
+	regulator_disable(state->vref_reg);
+
+	return ret;
+}
+
+static int max11100_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	regulator_disable(state->vref_reg);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct of_device_id max11100_ids[] = {
+	{.compatible = "maxim,max11100"},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, max11100_ids);
+
+static struct spi_driver max11100_driver = {
+	.driver = {
+		.name	= "max11100",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(max11100_ids),
+	},
+	.probe		= max11100_probe,
+	.remove		= max11100_remove,
+};
+
+module_spi_driver(max11100_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("Maxim max11100 ADC Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Jacopo Mondi]

From: Jacopo Mondi <jacopo@jmondi.org>

Add iio driver for Maxim MAX11100 single-channel ADC.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Tested-by: Marek Vasut <marek.vasut@gmail.com>

---
 drivers/iio/adc/Kconfig    |   9 +++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 38bc319..c32bc7a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -307,6 +307,15 @@ config MAX1027
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1027.
 
+config MAX11100
+	tristate "Maxim max11100 ADC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Maxim max11100 SPI ADC
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max11100.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d36c4be..5684369 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
 obj-$(CONFIG_MAX1027) += max1027.o
+obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
 obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
new file mode 100644
index 0000000..78e2a45
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,187 @@
+/*
+ * iio/adc/max11100.c
+ * Maxim max11100 ADC Driver with IIO interface
+ *
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ * Copyright (C) 2016 Jacopo Mondi
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/driver.h>
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref_mv / 2 ^ 16)
+ *
+ * LSB is used to calculate analog voltage value
+ * from the number of ADC steps count
+ *
+ * Ain = (count * LSB)
+ */
+#define MAX11100_LSB_DIV		(1 << 16)
+
+struct max11100_state {
+	const struct max11100_chip_desc *desc;
+	struct regulator *vref_reg;
+	struct spi_device *spi;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+	{ /* [0] */
+		.type = IIO_VOLTAGE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static struct max11100_chip_desc {
+	unsigned int num_chan;
+	const struct iio_chan_spec *channels;
+} max11100_desc = {
+	.num_chan = ARRAY_SIZE(max11100_channels),
+	.channels = max11100_channels,
+};
+
+static int max11100_read_single(struct iio_dev *indio_dev, int *val)
+{
+	int ret;
+	struct max11100_state *state = iio_priv(indio_dev);
+	uint8_t buffer[3];
+
+	ret = spi_read(state->spi, buffer, sizeof(buffer));
+	if (ret) {
+		dev_err(&indio_dev->dev, "SPI transfer failed\n");
+		return ret;
+	}
+
+	/* the first 8 bits sent out from ADC must be 0s */
+	if (buffer[0]) {
+		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
+		return -EINVAL;
+	}
+
+	*val = (buffer[1] << 8) | buffer[2];
+
+	return 0;
+}
+
+static int max11100_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	int ret, vref_uv;
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = max11100_read_single(indio_dev, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		vref_uv = regulator_get_voltage(state->vref_reg);
+		if (vref_uv < 0)
+			/* dummy regulator "get_voltage" returns -EINVAL */
+			return -EINVAL;
+
+		*val =  vref_uv / 1000;
+		*val2 = MAX11100_LSB_DIV;
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info max11100_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = max11100_read_raw,
+};
+
+static int max11100_probe(struct spi_device *spi)
+{
+	int ret;
+	struct iio_dev *indio_dev;
+	struct max11100_state *state;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	state = iio_priv(indio_dev);
+	state->spi = spi;
+	state->desc = &max11100_desc;
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.of_node = spi->dev.of_node;
+	indio_dev->info = &max11100_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = state->desc->channels;
+	indio_dev->num_channels = state->desc->num_chan;
+
+	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(state->vref_reg))
+		return PTR_ERR(state->vref_reg);
+
+	ret = regulator_enable(state->vref_reg);
+	if (ret)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto disable_regulator;
+
+	return 0;
+
+disable_regulator:
+	regulator_disable(state->vref_reg);
+
+	return ret;
+}
+
+static int max11100_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	regulator_disable(state->vref_reg);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct of_device_id max11100_ids[] = {
+	{.compatible = "maxim,max11100"},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, max11100_ids);
+
+static struct spi_driver max11100_driver = {
+	.driver = {
+		.name	= "max11100",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(max11100_ids),
+	},
+	.probe		= max11100_probe,
+	.remove		= max11100_remove,
+};
+
+module_spi_driver(max11100_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo@jmondi.org>");
+MODULE_DESCRIPTION("Maxim max11100 ADC Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC

[Jacopo Mondi]

Add device tree bindings documentation for Maxim MAX11100 single-channel
ADC

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../devicetree/bindings/iio/adc/max11100.txt          | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 0000000..ad0bc31
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,19 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "maxim,max11100"
+  - reg: the adc unit address
+  - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+  - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+max11100: adc@0 {
+        compatible = "maxim,max11100";
+        reg = <0>;
+        vref-supply = <&adc0_vref>;
+        spi-max-frequency = <240000>;
+};
+
-- 
2.7.4

Re: [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC

[Geert Uytterhoeven]

On Fri, Jan 13, 2017 at 10:50 AM, Jacopo Mondi
<jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> wrote:
> Add device tree bindings documentation for Maxim MAX11100 single-channel
> ADC
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC

[Geert Uytterhoeven]

On Fri, Jan 13, 2017 at 10:50 AM, Jacopo Mondi
<jacopo+renesas@jmondi.org> wrote:
> Add device tree bindings documentation for Maxim MAX11100 single-channel
> ADC
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Jonathan Cameron]

On 13/01/17 09:50, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> 
> Add iio driver for Maxim MAX11100 single-channel ADC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> Tested-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
Small issue with DMA using SPI controllers and the location of buffer
in your read.

Otherwise looks good to me.

Jonathan
> ---
>  drivers/iio/adc/Kconfig    |   9 +++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+)
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 38bc319..c32bc7a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -307,6 +307,15 @@ config MAX1027
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max1027.
>  
> +config MAX11100
> +	tristate "Maxim max11100 ADC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Maxim max11100 SPI ADC
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max11100.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d36c4be..5684369 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
>  obj-$(CONFIG_MAX1027) += max1027.o
> +obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> new file mode 100644
> index 0000000..78e2a45
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,187 @@
> +/*
> + * iio/adc/max11100.c
> + * Maxim max11100 ADC Driver with IIO interface
> + *
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/driver.h>
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref_mv / 2 ^ 16)
> + *
> + * LSB is used to calculate analog voltage value
> + * from the number of ADC steps count
> + *
> + * Ain = (count * LSB)
> + */
> +#define MAX11100_LSB_DIV		(1 << 16)
> +
> +struct max11100_state {
> +	const struct max11100_chip_desc *desc;
> +	struct regulator *vref_reg;
> +	struct spi_device *spi;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> +	{ /* [0] */
> +		.type = IIO_VOLTAGE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static struct max11100_chip_desc {
> +	unsigned int num_chan;
> +	const struct iio_chan_spec *channels;
> +} max11100_desc = {
> +	.num_chan = ARRAY_SIZE(max11100_channels),
> +	.channels = max11100_channels,
> +};
> +
> +static int max11100_read_single(struct iio_dev *indio_dev, int *val)
> +{
> +	int ret;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +	uint8_t buffer[3];
> +
> +	ret = spi_read(state->spi, buffer, sizeof(buffer));
Buffer needs to be __cacheline_aligned to safely use spi_read on spi
controllers that use DMA.

Two easy ways of doing this, either allocate buffer using kmalloc or
take advantage of the fact that spi_priv() is cache line aligned and
stick you buffer at the end of your private structure and mark it
__cacheline_aligned to ensure it also appropriately aligned in it's own
cacheline.

> +	if (ret) {
> +		dev_err(&indio_dev->dev, "SPI transfer failed\n");
> +		return ret;
> +	}
> +
> +	/* the first 8 bits sent out from ADC must be 0s */
> +	if (buffer[0]) {
> +		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
> +		return -EINVAL;
> +	}
> +
> +	*val = (buffer[1] << 8) | buffer[2];
> +
> +	return 0;
> +}
> +
> +static int max11100_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	int ret, vref_uv;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = max11100_read_single(indio_dev, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_uv = regulator_get_voltage(state->vref_reg);
> +		if (vref_uv < 0)
> +			/* dummy regulator "get_voltage" returns -EINVAL */
> +			return -EINVAL;
> +
> +		*val =  vref_uv / 1000;
> +		*val2 = MAX11100_LSB_DIV;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info max11100_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = max11100_read_raw,
> +};
> +
> +static int max11100_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct max11100_state *state;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	state = iio_priv(indio_dev);
> +	state->spi = spi;
> +	state->desc = &max11100_desc;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.of_node = spi->dev.of_node;
> +	indio_dev->info = &max11100_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = state->desc->channels;
> +	indio_dev->num_channels = state->desc->num_chan;
> +
> +	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
> +	if (IS_ERR(state->vref_reg))
> +		return PTR_ERR(state->vref_reg);
> +
> +	ret = regulator_enable(state->vref_reg);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto disable_regulator;
> +
> +	return 0;
> +
> +disable_regulator:
> +	regulator_disable(state->vref_reg);
> +
> +	return ret;
> +}
> +
> +static int max11100_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct max11100_state *state = iio_priv(indio_dev);
> +
> +	regulator_disable(state->vref_reg);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max11100_ids[] = {
> +	{.compatible = "maxim,max11100"},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, max11100_ids);
> +
> +static struct spi_driver max11100_driver = {
> +	.driver = {
> +		.name	= "max11100",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(max11100_ids),
> +	},
> +	.probe		= max11100_probe,
> +	.remove		= max11100_remove,
> +};
> +
> +module_spi_driver(max11100_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
> +MODULE_DESCRIPTION("Maxim max11100 ADC Driver");
> +MODULE_LICENSE("GPL v2");
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Jonathan Cameron]

On 13/01/17 09:50, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
> 
> Add iio driver for Maxim MAX11100 single-channel ADC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Tested-by: Marek Vasut <marek.vasut@gmail.com>
>
Small issue with DMA using SPI controllers and the location of buffer
in your read.

Otherwise looks good to me.

Jonathan
> ---
>  drivers/iio/adc/Kconfig    |   9 +++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+)
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 38bc319..c32bc7a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -307,6 +307,15 @@ config MAX1027
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max1027.
>  
> +config MAX11100
> +	tristate "Maxim max11100 ADC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Maxim max11100 SPI ADC
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max11100.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d36c4be..5684369 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
>  obj-$(CONFIG_MAX1027) += max1027.o
> +obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> new file mode 100644
> index 0000000..78e2a45
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,187 @@
> +/*
> + * iio/adc/max11100.c
> + * Maxim max11100 ADC Driver with IIO interface
> + *
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/driver.h>
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref_mv / 2 ^ 16)
> + *
> + * LSB is used to calculate analog voltage value
> + * from the number of ADC steps count
> + *
> + * Ain = (count * LSB)
> + */
> +#define MAX11100_LSB_DIV		(1 << 16)
> +
> +struct max11100_state {
> +	const struct max11100_chip_desc *desc;
> +	struct regulator *vref_reg;
> +	struct spi_device *spi;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> +	{ /* [0] */
> +		.type = IIO_VOLTAGE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static struct max11100_chip_desc {
> +	unsigned int num_chan;
> +	const struct iio_chan_spec *channels;
> +} max11100_desc = {
> +	.num_chan = ARRAY_SIZE(max11100_channels),
> +	.channels = max11100_channels,
> +};
> +
> +static int max11100_read_single(struct iio_dev *indio_dev, int *val)
> +{
> +	int ret;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +	uint8_t buffer[3];
> +
> +	ret = spi_read(state->spi, buffer, sizeof(buffer));
Buffer needs to be __cacheline_aligned to safely use spi_read on spi
controllers that use DMA.

Two easy ways of doing this, either allocate buffer using kmalloc or
take advantage of the fact that spi_priv() is cache line aligned and
stick you buffer at the end of your private structure and mark it
__cacheline_aligned to ensure it also appropriately aligned in it's own
cacheline.

> +	if (ret) {
> +		dev_err(&indio_dev->dev, "SPI transfer failed\n");
> +		return ret;
> +	}
> +
> +	/* the first 8 bits sent out from ADC must be 0s */
> +	if (buffer[0]) {
> +		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
> +		return -EINVAL;
> +	}
> +
> +	*val = (buffer[1] << 8) | buffer[2];
> +
> +	return 0;
> +}
> +
> +static int max11100_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	int ret, vref_uv;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = max11100_read_single(indio_dev, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_uv = regulator_get_voltage(state->vref_reg);
> +		if (vref_uv < 0)
> +			/* dummy regulator "get_voltage" returns -EINVAL */
> +			return -EINVAL;
> +
> +		*val =  vref_uv / 1000;
> +		*val2 = MAX11100_LSB_DIV;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info max11100_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = max11100_read_raw,
> +};
> +
> +static int max11100_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct max11100_state *state;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	state = iio_priv(indio_dev);
> +	state->spi = spi;
> +	state->desc = &max11100_desc;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.of_node = spi->dev.of_node;
> +	indio_dev->info = &max11100_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = state->desc->channels;
> +	indio_dev->num_channels = state->desc->num_chan;
> +
> +	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
> +	if (IS_ERR(state->vref_reg))
> +		return PTR_ERR(state->vref_reg);
> +
> +	ret = regulator_enable(state->vref_reg);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto disable_regulator;
> +
> +	return 0;
> +
> +disable_regulator:
> +	regulator_disable(state->vref_reg);
> +
> +	return ret;
> +}
> +
> +static int max11100_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct max11100_state *state = iio_priv(indio_dev);
> +
> +	regulator_disable(state->vref_reg);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max11100_ids[] = {
> +	{.compatible = "maxim,max11100"},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, max11100_ids);
> +
> +static struct spi_driver max11100_driver = {
> +	.driver = {
> +		.name	= "max11100",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(max11100_ids),
> +	},
> +	.probe		= max11100_probe,
> +	.remove		= max11100_remove,
> +};
> +
> +module_spi_driver(max11100_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo@jmondi.org>");
> +MODULE_DESCRIPTION("Maxim max11100 ADC Driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Lars-Peter Clausen]

On 01/13/2017 10:50 AM, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> 
> Add iio driver for Maxim MAX11100 single-channel ADC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> Tested-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> ---
>  drivers/iio/adc/Kconfig    |   9 +++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+)
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 38bc319..c32bc7a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -307,6 +307,15 @@ config MAX1027
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max1027.
>  
> +config MAX11100
> +	tristate "Maxim max11100 ADC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Maxim max11100 SPI ADC
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max11100.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d36c4be..5684369 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
>  obj-$(CONFIG_MAX1027) += max1027.o
> +obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> new file mode 100644
> index 0000000..78e2a45
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,187 @@
> +/*
> + * iio/adc/max11100.c
> + * Maxim max11100 ADC Driver with IIO interface
> + *
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/driver.h>
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref_mv / 2 ^ 16)
> + *
> + * LSB is used to calculate analog voltage value
> + * from the number of ADC steps count
> + *
> + * Ain = (count * LSB)
> + */
> +#define MAX11100_LSB_DIV		(1 << 16)
> +
> +struct max11100_state {
> +	const struct max11100_chip_desc *desc;
> +	struct regulator *vref_reg;
> +	struct spi_device *spi;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> +	{ /* [0] */
> +		.type = IIO_VOLTAGE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static struct max11100_chip_desc {
> +	unsigned int num_chan;
> +	const struct iio_chan_spec *channels;
> +} max11100_desc = {
> +	.num_chan = ARRAY_SIZE(max11100_channels),
> +	.channels = max11100_channels,
> +};
> +
> +static int max11100_read_single(struct iio_dev *indio_dev, int *val)
> +{
> +	int ret;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +	uint8_t buffer[3];
> +
> +	ret = spi_read(state->spi, buffer, sizeof(buffer));
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "SPI transfer failed\n");
> +		return ret;
> +	}
> +
> +	/* the first 8 bits sent out from ADC must be 0s */
> +	if (buffer[0]) {
> +		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
> +		return -EINVAL;
> +	}
> +
> +	*val = (buffer[1] << 8) | buffer[2];
> +
> +	return 0;
> +}
> +
> +static int max11100_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	int ret, vref_uv;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = max11100_read_single(indio_dev, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_uv = regulator_get_voltage(state->vref_reg);
> +		if (vref_uv < 0)
> +			/* dummy regulator "get_voltage" returns -EINVAL */
> +			return -EINVAL;
> +
> +		*val =  vref_uv / 1000;
> +		*val2 = MAX11100_LSB_DIV;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info max11100_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = max11100_read_raw,
> +};
> +
> +static int max11100_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct max11100_state *state;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	state = iio_priv(indio_dev);
> +	state->spi = spi;
> +	state->desc = &max11100_desc;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.of_node = spi->dev.of_node;
> +	indio_dev->info = &max11100_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = state->desc->channels;
> +	indio_dev->num_channels = state->desc->num_chan;

Consider setting indio_dev->name= "max11100"; so userspace can identify the
type of the device.

Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Lars-Peter Clausen]

On 01/13/2017 10:50 AM, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
> 
> Add iio driver for Maxim MAX11100 single-channel ADC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Tested-by: Marek Vasut <marek.vasut@gmail.com>
> 
> ---
>  drivers/iio/adc/Kconfig    |   9 +++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+)
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 38bc319..c32bc7a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -307,6 +307,15 @@ config MAX1027
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max1027.
>  
> +config MAX11100
> +	tristate "Maxim max11100 ADC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Maxim max11100 SPI ADC
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max11100.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d36c4be..5684369 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
>  obj-$(CONFIG_MAX1027) += max1027.o
> +obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> new file mode 100644
> index 0000000..78e2a45
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,187 @@
> +/*
> + * iio/adc/max11100.c
> + * Maxim max11100 ADC Driver with IIO interface
> + *
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/driver.h>
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref_mv / 2 ^ 16)
> + *
> + * LSB is used to calculate analog voltage value
> + * from the number of ADC steps count
> + *
> + * Ain = (count * LSB)
> + */
> +#define MAX11100_LSB_DIV		(1 << 16)
> +
> +struct max11100_state {
> +	const struct max11100_chip_desc *desc;
> +	struct regulator *vref_reg;
> +	struct spi_device *spi;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> +	{ /* [0] */
> +		.type = IIO_VOLTAGE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static struct max11100_chip_desc {
> +	unsigned int num_chan;
> +	const struct iio_chan_spec *channels;
> +} max11100_desc = {
> +	.num_chan = ARRAY_SIZE(max11100_channels),
> +	.channels = max11100_channels,
> +};
> +
> +static int max11100_read_single(struct iio_dev *indio_dev, int *val)
> +{
> +	int ret;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +	uint8_t buffer[3];
> +
> +	ret = spi_read(state->spi, buffer, sizeof(buffer));
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "SPI transfer failed\n");
> +		return ret;
> +	}
> +
> +	/* the first 8 bits sent out from ADC must be 0s */
> +	if (buffer[0]) {
> +		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
> +		return -EINVAL;
> +	}
> +
> +	*val = (buffer[1] << 8) | buffer[2];
> +
> +	return 0;
> +}
> +
> +static int max11100_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	int ret, vref_uv;
> +	struct max11100_state *state = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = max11100_read_single(indio_dev, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_uv = regulator_get_voltage(state->vref_reg);
> +		if (vref_uv < 0)
> +			/* dummy regulator "get_voltage" returns -EINVAL */
> +			return -EINVAL;
> +
> +		*val =  vref_uv / 1000;
> +		*val2 = MAX11100_LSB_DIV;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info max11100_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = max11100_read_raw,
> +};
> +
> +static int max11100_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct max11100_state *state;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	state = iio_priv(indio_dev);
> +	state->spi = spi;
> +	state->desc = &max11100_desc;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.of_node = spi->dev.of_node;
> +	indio_dev->info = &max11100_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = state->desc->channels;
> +	indio_dev->num_channels = state->desc->num_chan;

Consider setting indio_dev->name= "max11100"; so userspace can identify the
type of the device.

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[jacopo mondi]

Hi Jonathan,
    thanks for review,

On 13/01/2017 10:50, Jacopo Mondi wrote:
> Hello,
>    sending out v4 splitting device tree bindings documentation and actual ADC
> driver.
> No changes in driver code since v3.
>
> Same question for iio maintainers here:
> I would like to have clarified the measure unit returned by read_raw().
> Currently (value_raw * value_scale) return the ADC input value in mV.
> While testing the patch I've been questioned if that should not actually
> be in uV. This is easily achievable making _scale return a value in uV.
> I have found no mention of this in the ABI documentation as it speaks of
> generic voltage.
> Can we have a final word on this?

I see you have reviewed the driver without complaining for the 
read_raw() measure unit, so I assume this replies to the above question 
as well...

Thanks
    j


>
> Thanks Marek for having tested this.
>
> v1 -> v2:
>     - incorporated pmeerw's review comments
>     - retrieve vref from dts and use that to convert read_raw result
>       to mV
>     - add device tree bindings documentation
>
> v2 -> v3:
>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>       and not during probe
>     - add back remove function as regulator has to be disabled when detaching
>       the module. Do not use devm_ version of iio_register/unregister functions
>       anymore but do unregister in the remove.
>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>
> v3 -> v4:
>     - split device tree binding documentation and actual ADC driver
>     - add "reg" to the list of required properties and use a better
>       namimg for the adc device node in bindings documentation as suggested
>       by Geert.
>
> Jacopo Mondi (2):
>   iio: adc: Add Maxim MAX11100 driver
>   dt-bindings: iio: document MAX11100 ADC
>
>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>  drivers/iio/adc/Kconfig                            |   9 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>  4 files changed, 216 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[jacopo mondi]

Hi Jonathan,
    thanks for review,

On 13/01/2017 10:50, Jacopo Mondi wrote:
> Hello,
>    sending out v4 splitting device tree bindings documentation and actual ADC
> driver.
> No changes in driver code since v3.
>
> Same question for iio maintainers here:
> I would like to have clarified the measure unit returned by read_raw().
> Currently (value_raw * value_scale) return the ADC input value in mV.
> While testing the patch I've been questioned if that should not actually
> be in uV. This is easily achievable making _scale return a value in uV.
> I have found no mention of this in the ABI documentation as it speaks of
> generic voltage.
> Can we have a final word on this?

I see you have reviewed the driver without complaining for the 
read_raw() measure unit, so I assume this replies to the above question 
as well...

Thanks
    j


>
> Thanks Marek for having tested this.
>
> v1 -> v2:
>     - incorporated pmeerw's review comments
>     - retrieve vref from dts and use that to convert read_raw result
>       to mV
>     - add device tree bindings documentation
>
> v2 -> v3:
>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>       and not during probe
>     - add back remove function as regulator has to be disabled when detaching
>       the module. Do not use devm_ version of iio_register/unregister functions
>       anymore but do unregister in the remove.
>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>
> v3 -> v4:
>     - split device tree binding documentation and actual ADC driver
>     - add "reg" to the list of required properties and use a better
>       namimg for the adc device node in bindings documentation as suggested
>       by Geert.
>
> Jacopo Mondi (2):
>   iio: adc: Add Maxim MAX11100 driver
>   dt-bindings: iio: document MAX11100 ADC
>
>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>  drivers/iio/adc/Kconfig                            |   9 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>  4 files changed, 216 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
>

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Jonathan Cameron]

On 15/01/17 14:13, jacopo mondi wrote:
> Hi Jonathan,
>    thanks for review,
> 
> On 13/01/2017 10:50, Jacopo Mondi wrote:
>> Hello,
>>    sending out v4 splitting device tree bindings documentation and actual ADC
>> driver.
>> No changes in driver code since v3.
>>
>> Same question for iio maintainers here:
>> I would like to have clarified the measure unit returned by read_raw().
>> Currently (value_raw * value_scale) return the ADC input value in mV.
Good.
>> While testing the patch I've been questioned if that should not actually
>> be in uV. This is easily achievable making _scale return a value in uV.
>> I have found no mention of this in the ABI documentation as it speaks of
>> generic voltage. Can we have a final word on this?
> 
> I see you have reviewed the driver without complaining for the
> read_raw() measure unit, so I assume this replies to the above
> question as well...
> 
The units are specified in Documentation/ABI/testing/sysfs-bus-iio.

> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
> KernelVersion:	2.6.35
> Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Description:
> 		Raw (unscaled no bias removal etc.) voltage measurement from
> 		channel Y. In special cases where the channel does not
> 		correspond to externally available input one of the named
> 		versions may be used. The number must always be specified and
> 		unique to allow association with event codes. Units after
> 		application of scale and offset are millivolts.

So millivolts.  This comes from a, perhaps ill judged, decision to match
hwmon units were we could.

Jonathan

> Thanks j
> 
>>
>> Thanks Marek for having tested this.
>>
>> v1 -> v2:
>>     - incorporated pmeerw's review comments
>>     - retrieve vref from dts and use that to convert read_raw result
>>       to mV
>>     - add device tree bindings documentation
>>
>> v2 -> v3:
>>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>>       and not during probe
>>     - add back remove function as regulator has to be disabled when detaching
>>       the module. Do not use devm_ version of iio_register/unregister functions
>>       anymore but do unregister in the remove.
>>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>>
>> v3 -> v4:
>>     - split device tree binding documentation and actual ADC driver
>>     - add "reg" to the list of required properties and use a better
>>       namimg for the adc device node in bindings documentation as suggested
>>       by Geert.
>>
>> Jacopo Mondi (2):
>>   iio: adc: Add Maxim MAX11100 driver
>>   dt-bindings: iio: document MAX11100 ADC
>>
>>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>>  drivers/iio/adc/Kconfig                            |   9 +
>>  drivers/iio/adc/Makefile                           |   1 +
>>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>>  4 files changed, 216 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>  create mode 100644 drivers/iio/adc/max11100.c
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Jonathan Cameron]

On 15/01/17 14:13, jacopo mondi wrote:
> Hi Jonathan,
>    thanks for review,
> 
> On 13/01/2017 10:50, Jacopo Mondi wrote:
>> Hello,
>>    sending out v4 splitting device tree bindings documentation and actual ADC
>> driver.
>> No changes in driver code since v3.
>>
>> Same question for iio maintainers here:
>> I would like to have clarified the measure unit returned by read_raw().
>> Currently (value_raw * value_scale) return the ADC input value in mV.
Good.
>> While testing the patch I've been questioned if that should not actually
>> be in uV. This is easily achievable making _scale return a value in uV.
>> I have found no mention of this in the ABI documentation as it speaks of
>> generic voltage. Can we have a final word on this?
> 
> I see you have reviewed the driver without complaining for the
> read_raw() measure unit, so I assume this replies to the above
> question as well...
> 
The units are specified in Documentation/ABI/testing/sysfs-bus-iio.

> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
> KernelVersion:	2.6.35
> Contact:	linux-iio@vger.kernel.org
> Description:
> 		Raw (unscaled no bias removal etc.) voltage measurement from
> 		channel Y. In special cases where the channel does not
> 		correspond to externally available input one of the named
> 		versions may be used. The number must always be specified and
> 		unique to allow association with event codes. Units after
> 		application of scale and offset are millivolts.

So millivolts.  This comes from a, perhaps ill judged, decision to match
hwmon units were we could.

Jonathan

> Thanks j
> 
>>
>> Thanks Marek for having tested this.
>>
>> v1 -> v2:
>>     - incorporated pmeerw's review comments
>>     - retrieve vref from dts and use that to convert read_raw result
>>       to mV
>>     - add device tree bindings documentation
>>
>> v2 -> v3:
>>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>>       and not during probe
>>     - add back remove function as regulator has to be disabled when detaching
>>       the module. Do not use devm_ version of iio_register/unregister functions
>>       anymore but do unregister in the remove.
>>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>>
>> v3 -> v4:
>>     - split device tree binding documentation and actual ADC driver
>>     - add "reg" to the list of required properties and use a better
>>       namimg for the adc device node in bindings documentation as suggested
>>       by Geert.
>>
>> Jacopo Mondi (2):
>>   iio: adc: Add Maxim MAX11100 driver
>>   dt-bindings: iio: document MAX11100 ADC
>>
>>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>>  drivers/iio/adc/Kconfig                            |   9 +
>>  drivers/iio/adc/Makefile                           |   1 +
>>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>>  4 files changed, 216 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>  create mode 100644 drivers/iio/adc/max11100.c
>>
> 

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[jacopo mondi]

Hi Jonathan,

On 15/01/2017 15:31, Jonathan Cameron wrote:
> On 15/01/17 14:13, jacopo mondi wrote:
>> Hi Jonathan,
>>    thanks for review,
>>
>> On 13/01/2017 10:50, Jacopo Mondi wrote:
>>> Hello,
>>>    sending out v4 splitting device tree bindings documentation and actual ADC
>>> driver.
>>> No changes in driver code since v3.
>>>
>>> Same question for iio maintainers here:
>>> I would like to have clarified the measure unit returned by read_raw().
>>> Currently (value_raw * value_scale) return the ADC input value in mV.
> Good.
>>> While testing the patch I've been questioned if that should not actually
>>> be in uV. This is easily achievable making _scale return a value in uV.
>>> I have found no mention of this in the ABI documentation as it speaks of
>>> generic voltage. Can we have a final word on this?
>>
>> I see you have reviewed the driver without complaining for the
>> read_raw() measure unit, so I assume this replies to the above
>> question as well...
>>
> The units are specified in Documentation/ABI/testing/sysfs-bus-iio.
>
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
>> KernelVersion:	2.6.35
>> Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Description:
>> 		Raw (unscaled no bias removal etc.) voltage measurement from
>> 		channel Y. In special cases where the channel does not
>> 		correspond to externally available input one of the named
>> 		versions may be used. The number must always be specified and
>> 		unique to allow association with event codes. Units after
>> 		application of scale and offset are millivolts.
>
> So millivolts.  This comes from a, perhaps ill judged, decision to match
> hwmon units were we could.
>

Thank you! The "generic voltage" I said is mentioned in the ABI 
documentation is in _scale attributes description.
I assume then the measure unit of the value returned by _scale is not 
relevant as long as the _raw one with scale applied  returns millivolts.

Thanks, as soon as I have feedbacks on DTS binding docs, I'll send v5 
and we can hopefully close this one ;)

Thanks
    j




> Jonathan
>
>> Thanks j
>>
>>>
>>> Thanks Marek for having tested this.
>>>
>>> v1 -> v2:
>>>     - incorporated pmeerw's review comments
>>>     - retrieve vref from dts and use that to convert read_raw result
>>>       to mV
>>>     - add device tree bindings documentation
>>>
>>> v2 -> v3:
>>>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>>>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>>>       and not during probe
>>>     - add back remove function as regulator has to be disabled when detaching
>>>       the module. Do not use devm_ version of iio_register/unregister functions
>>>       anymore but do unregister in the remove.
>>>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>>>
>>> v3 -> v4:
>>>     - split device tree binding documentation and actual ADC driver
>>>     - add "reg" to the list of required properties and use a better
>>>       namimg for the adc device node in bindings documentation as suggested
>>>       by Geert.
>>>
>>> Jacopo Mondi (2):
>>>   iio: adc: Add Maxim MAX11100 driver
>>>   dt-bindings: iio: document MAX11100 ADC
>>>
>>>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>>>  drivers/iio/adc/Kconfig                            |   9 +
>>>  drivers/iio/adc/Makefile                           |   1 +
>>>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>>>  4 files changed, 216 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>>  create mode 100644 drivers/iio/adc/max11100.c
>>>
>>
>

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[jacopo mondi]

Hi Jonathan,

On 15/01/2017 15:31, Jonathan Cameron wrote:
> On 15/01/17 14:13, jacopo mondi wrote:
>> Hi Jonathan,
>>    thanks for review,
>>
>> On 13/01/2017 10:50, Jacopo Mondi wrote:
>>> Hello,
>>>    sending out v4 splitting device tree bindings documentation and actual ADC
>>> driver.
>>> No changes in driver code since v3.
>>>
>>> Same question for iio maintainers here:
>>> I would like to have clarified the measure unit returned by read_raw().
>>> Currently (value_raw * value_scale) return the ADC input value in mV.
> Good.
>>> While testing the patch I've been questioned if that should not actually
>>> be in uV. This is easily achievable making _scale return a value in uV.
>>> I have found no mention of this in the ABI documentation as it speaks of
>>> generic voltage. Can we have a final word on this?
>>
>> I see you have reviewed the driver without complaining for the
>> read_raw() measure unit, so I assume this replies to the above
>> question as well...
>>
> The units are specified in Documentation/ABI/testing/sysfs-bus-iio.
>
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
>> KernelVersion:	2.6.35
>> Contact:	linux-iio@vger.kernel.org
>> Description:
>> 		Raw (unscaled no bias removal etc.) voltage measurement from
>> 		channel Y. In special cases where the channel does not
>> 		correspond to externally available input one of the named
>> 		versions may be used. The number must always be specified and
>> 		unique to allow association with event codes. Units after
>> 		application of scale and offset are millivolts.
>
> So millivolts.  This comes from a, perhaps ill judged, decision to match
> hwmon units were we could.
>

Thank you! The "generic voltage" I said is mentioned in the ABI 
documentation is in _scale attributes description.
I assume then the measure unit of the value returned by _scale is not 
relevant as long as the _raw one with scale applied  returns millivolts.

Thanks, as soon as I have feedbacks on DTS binding docs, I'll send v5 
and we can hopefully close this one ;)

Thanks
    j




> Jonathan
>
>> Thanks j
>>
>>>
>>> Thanks Marek for having tested this.
>>>
>>> v1 -> v2:
>>>     - incorporated pmeerw's review comments
>>>     - retrieve vref from dts and use that to convert read_raw result
>>>       to mV
>>>     - add device tree bindings documentation
>>>
>>> v2 -> v3:
>>>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>>>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>>>       and not during probe
>>>     - add back remove function as regulator has to be disabled when detaching
>>>       the module. Do not use devm_ version of iio_register/unregister functions
>>>       anymore but do unregister in the remove.
>>>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>>>
>>> v3 -> v4:
>>>     - split device tree binding documentation and actual ADC driver
>>>     - add "reg" to the list of required properties and use a better
>>>       namimg for the adc device node in bindings documentation as suggested
>>>       by Geert.
>>>
>>> Jacopo Mondi (2):
>>>   iio: adc: Add Maxim MAX11100 driver
>>>   dt-bindings: iio: document MAX11100 ADC
>>>
>>>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>>>  drivers/iio/adc/Kconfig                            |   9 +
>>>  drivers/iio/adc/Makefile                           |   1 +
>>>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>>>  4 files changed, 216 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>>  create mode 100644 drivers/iio/adc/max11100.c
>>>
>>
>

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Jonathan Cameron]

On 15/01/17 14:41, jacopo mondi wrote:
> Hi Jonathan,
> 
> On 15/01/2017 15:31, Jonathan Cameron wrote:
>> On 15/01/17 14:13, jacopo mondi wrote:
>>> Hi Jonathan,
>>>    thanks for review,
>>>
>>> On 13/01/2017 10:50, Jacopo Mondi wrote:
>>>> Hello,
>>>>    sending out v4 splitting device tree bindings documentation and actual ADC
>>>> driver.
>>>> No changes in driver code since v3.
>>>>
>>>> Same question for iio maintainers here:
>>>> I would like to have clarified the measure unit returned by read_raw().
>>>> Currently (value_raw * value_scale) return the ADC input value in mV.
>> Good.
>>>> While testing the patch I've been questioned if that should not actually
>>>> be in uV. This is easily achievable making _scale return a value in uV.
>>>> I have found no mention of this in the ABI documentation as it speaks of
>>>> generic voltage. Can we have a final word on this?
>>>
>>> I see you have reviewed the driver without complaining for the
>>> read_raw() measure unit, so I assume this replies to the above
>>> question as well...
>>>
>> The units are specified in Documentation/ABI/testing/sysfs-bus-iio.
>>
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
>>> KernelVersion:    2.6.35
>>> Contact:    linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Description:
>>>         Raw (unscaled no bias removal etc.) voltage measurement from
>>>         channel Y. In special cases where the channel does not
>>>         correspond to externally available input one of the named
>>>         versions may be used. The number must always be specified and
>>>         unique to allow association with event codes. Units after
>>>         application of scale and offset are millivolts.
>>
>> So millivolts.  This comes from a, perhaps ill judged, decision to match
>> hwmon units were we could.
>>
> 
> Thank you! The "generic voltage" I said is mentioned in the ABI
> documentation is in _scale attributes description. I assume then the
> measure unit of the value returned by _scale is not relevant as long
> as the _raw one with scale applied returns millivolts.
Yes.  Scale is inherently a ratio really rather than a quantity with units
at all.  It just happens to correspond to the voltage represented by 1LSB.
> 
> Thanks, as soon as I have feedbacks on DTS binding docs, I'll send v5
> and we can hopefully close this one ;)
Cool.  Got a while in this cycle yet, but I know it's always nice to get
patches out of your personal queue!

Jonathan
> 
> Thanks j
> 
> 
> 
>> Jonathan
>>
>>> Thanks j
>>>
>>>>
>>>> Thanks Marek for having tested this.
>>>>
>>>> v1 -> v2:
>>>>     - incorporated pmeerw's review comments
>>>>     - retrieve vref from dts and use that to convert read_raw result
>>>>       to mV
>>>>     - add device tree bindings documentation
>>>>
>>>> v2 -> v3:
>>>>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>>>>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>>>>       and not during probe
>>>>     - add back remove function as regulator has to be disabled when detaching
>>>>       the module. Do not use devm_ version of iio_register/unregister functions
>>>>       anymore but do unregister in the remove.
>>>>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>>>>
>>>> v3 -> v4:
>>>>     - split device tree binding documentation and actual ADC driver
>>>>     - add "reg" to the list of required properties and use a better
>>>>       namimg for the adc device node in bindings documentation as suggested
>>>>       by Geert.
>>>>
>>>> Jacopo Mondi (2):
>>>>   iio: adc: Add Maxim MAX11100 driver
>>>>   dt-bindings: iio: document MAX11100 ADC
>>>>
>>>>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>>>>  drivers/iio/adc/Kconfig                            |   9 +
>>>>  drivers/iio/adc/Makefile                           |   1 +
>>>>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>>>>  4 files changed, 216 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>>>  create mode 100644 drivers/iio/adc/max11100.c
>>>>
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Jonathan Cameron]

On 15/01/17 14:41, jacopo mondi wrote:
> Hi Jonathan,
> 
> On 15/01/2017 15:31, Jonathan Cameron wrote:
>> On 15/01/17 14:13, jacopo mondi wrote:
>>> Hi Jonathan,
>>>    thanks for review,
>>>
>>> On 13/01/2017 10:50, Jacopo Mondi wrote:
>>>> Hello,
>>>>    sending out v4 splitting device tree bindings documentation and actual ADC
>>>> driver.
>>>> No changes in driver code since v3.
>>>>
>>>> Same question for iio maintainers here:
>>>> I would like to have clarified the measure unit returned by read_raw().
>>>> Currently (value_raw * value_scale) return the ADC input value in mV.
>> Good.
>>>> While testing the patch I've been questioned if that should not actually
>>>> be in uV. This is easily achievable making _scale return a value in uV.
>>>> I have found no mention of this in the ABI documentation as it speaks of
>>>> generic voltage. Can we have a final word on this?
>>>
>>> I see you have reviewed the driver without complaining for the
>>> read_raw() measure unit, so I assume this replies to the above
>>> question as well...
>>>
>> The units are specified in Documentation/ABI/testing/sysfs-bus-iio.
>>
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>>> What:        /sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
>>> KernelVersion:    2.6.35
>>> Contact:    linux-iio@vger.kernel.org
>>> Description:
>>>         Raw (unscaled no bias removal etc.) voltage measurement from
>>>         channel Y. In special cases where the channel does not
>>>         correspond to externally available input one of the named
>>>         versions may be used. The number must always be specified and
>>>         unique to allow association with event codes. Units after
>>>         application of scale and offset are millivolts.
>>
>> So millivolts.  This comes from a, perhaps ill judged, decision to match
>> hwmon units were we could.
>>
> 
> Thank you! The "generic voltage" I said is mentioned in the ABI
> documentation is in _scale attributes description. I assume then the
> measure unit of the value returned by _scale is not relevant as long
> as the _raw one with scale applied returns millivolts.
Yes.  Scale is inherently a ratio really rather than a quantity with units
at all.  It just happens to correspond to the voltage represented by 1LSB.
> 
> Thanks, as soon as I have feedbacks on DTS binding docs, I'll send v5
> and we can hopefully close this one ;)
Cool.  Got a while in this cycle yet, but I know it's always nice to get
patches out of your personal queue!

Jonathan
> 
> Thanks j
> 
> 
> 
>> Jonathan
>>
>>> Thanks j
>>>
>>>>
>>>> Thanks Marek for having tested this.
>>>>
>>>> v1 -> v2:
>>>>     - incorporated pmeerw's review comments
>>>>     - retrieve vref from dts and use that to convert read_raw result
>>>>       to mV
>>>>     - add device tree bindings documentation
>>>>
>>>> v2 -> v3:
>>>>     - add _SCALE bit of read_raw function and change _RAW bit accordingly
>>>>     - call regulator_get_voltage when accessing the _SCALE part of read_raw
>>>>       and not during probe
>>>>     - add back remove function as regulator has to be disabled when detaching
>>>>       the module. Do not use devm_ version of iio_register/unregister functions
>>>>       anymore but do unregister in the remove.
>>>>     - remove mutex as access to SPI bus is protected by SPI core. Thanks marex
>>>>
>>>> v3 -> v4:
>>>>     - split device tree binding documentation and actual ADC driver
>>>>     - add "reg" to the list of required properties and use a better
>>>>       namimg for the adc device node in bindings documentation as suggested
>>>>       by Geert.
>>>>
>>>> Jacopo Mondi (2):
>>>>   iio: adc: Add Maxim MAX11100 driver
>>>>   dt-bindings: iio: document MAX11100 ADC
>>>>
>>>>  .../devicetree/bindings/iio/adc/max11100.txt       |  19 +++
>>>>  drivers/iio/adc/Kconfig                            |   9 +
>>>>  drivers/iio/adc/Makefile                           |   1 +
>>>>  drivers/iio/adc/max11100.c                         | 187 +++++++++++++++++++++
>>>>  4 files changed, 216 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>>>  create mode 100644 drivers/iio/adc/max11100.c
>>>>
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Marek Vasut]

On 01/15/2017 03:31 PM, Jonathan Cameron wrote:
> On 15/01/17 14:13, jacopo mondi wrote:
>> Hi Jonathan,
>>    thanks for review,
>>
>> On 13/01/2017 10:50, Jacopo Mondi wrote:
>>> Hello,
>>>    sending out v4 splitting device tree bindings documentation and actual ADC
>>> driver.
>>> No changes in driver code since v3.
>>>
>>> Same question for iio maintainers here:
>>> I would like to have clarified the measure unit returned by read_raw().
>>> Currently (value_raw * value_scale) return the ADC input value in mV.
> Good.
>>> While testing the patch I've been questioned if that should not actually
>>> be in uV. This is easily achievable making _scale return a value in uV.
>>> I have found no mention of this in the ABI documentation as it speaks of
>>> generic voltage. Can we have a final word on this?
>>
>> I see you have reviewed the driver without complaining for the
>> read_raw() measure unit, so I assume this replies to the above
>> question as well...
>>
> The units are specified in Documentation/ABI/testing/sysfs-bus-iio.
> 
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
>> KernelVersion:	2.6.35
>> Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Description:
>> 		Raw (unscaled no bias removal etc.) voltage measurement from
>> 		channel Y. In special cases where the channel does not
>> 		correspond to externally available input one of the named
>> 		versions may be used. The number must always be specified and
>> 		unique to allow association with event codes. Units after
>> 		application of scale and offset are millivolts.
> 
> So millivolts.  This comes from a, perhaps ill judged, decision to match
> hwmon units were we could.

Fun, regulator framework returns values in uV to add to the chaos :)


-- 
Best regards,
Marek Vasut

Re: [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver

[Marek Vasut]

On 01/15/2017 03:31 PM, Jonathan Cameron wrote:
> On 15/01/17 14:13, jacopo mondi wrote:
>> Hi Jonathan,
>>    thanks for review,
>>
>> On 13/01/2017 10:50, Jacopo Mondi wrote:
>>> Hello,
>>>    sending out v4 splitting device tree bindings documentation and actual ADC
>>> driver.
>>> No changes in driver code since v3.
>>>
>>> Same question for iio maintainers here:
>>> I would like to have clarified the measure unit returned by read_raw().
>>> Currently (value_raw * value_scale) return the ADC input value in mV.
> Good.
>>> While testing the patch I've been questioned if that should not actually
>>> be in uV. This is easily achievable making _scale return a value in uV.
>>> I have found no mention of this in the ABI documentation as it speaks of
>>> generic voltage. Can we have a final word on this?
>>
>> I see you have reviewed the driver without complaining for the
>> read_raw() measure unit, so I assume this replies to the above
>> question as well...
>>
> The units are specified in Documentation/ABI/testing/sysfs-bus-iio.
> 
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>> What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
>> KernelVersion:	2.6.35
>> Contact:	linux-iio@vger.kernel.org
>> Description:
>> 		Raw (unscaled no bias removal etc.) voltage measurement from
>> 		channel Y. In special cases where the channel does not
>> 		correspond to externally available input one of the named
>> 		versions may be used. The number must always be specified and
>> 		unique to allow association with event codes. Units after
>> 		application of scale and offset are millivolts.
> 
> So millivolts.  This comes from a, perhaps ill judged, decision to match
> hwmon units were we could.

Fun, regulator framework returns values in uV to add to the chaos :)


-- 
Best regards,
Marek Vasut

Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Wolfram Sang]

> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi

In case you need to resend: 2016-17?

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver

[Wolfram Sang]

> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi

In case you need to resend: 2016-17?

Re: [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC

[Wolfram Sang]

On Fri, Jan 13, 2017 at 10:50:38AM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Maxim MAX11100 single-channel
> ADC
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Acked-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

Re: [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC

[Wolfram Sang]

On Fri, Jan 13, 2017 at 10:50:38AM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Maxim MAX11100 single-channel
> ADC
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Re: [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC

[Wolfram Sang]

On Fri, Jan 13, 2017 at 10:50:38AM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Maxim MAX11100 single-channel
> ADC
>=20
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>