Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2017-01-02 Thread Jonathan Cameron 
On 02/01/17 09:19, jacopo mondi wrote:
> Hi Jonathan,
>thanks for review
> 
> On 30/12/2016 17:31, Jonathan Cameron wrote:
>> On 14/12/16 12:00, jac...@jmondi.org wrote:
>>> Hello Peter,
>>>thanks for review
>>>
>>> On 13/12/2016 21:33, Peter Meerwald-Stadler wrote:

> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.

 some more comments

> Signed-off-by: Jacopo Mondi 
> ---
>
> 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
>
> ---
>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>  drivers/iio/adc/Kconfig|   9 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 166 
> +
>  4 files changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> new file mode 100644
> index 000..6877c11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> @@ -0,0 +1,17 @@
> +* Maxim max11100 Analog to Digital Converter (ADC)
> +
> +Required properties:
> +  - compatible: Should be "maxim,max11100"
> +  - vref-supply: phandle to the regulator that provides reference voltage
> +
> +Optional properties:
> +  - spi-max-frequency: SPI maximum frequency
> +
> +Example:
> +
> +adc0: max11100@0 {
> +compatible = "maxim,max11100";
> +vref-supply = <_vref>;
> +spi-max-frequency = <24>;
> +};
> +
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..a909484 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -285,6 +285,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

 SPI_MASTER is more precise I think

> +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 7a40c04..1463044 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,166 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV(1 << 16)
> +#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)

 maybe parenthesis around vref

> +
> +struct max11100_state {
> +const struct max11100_chip_desc *desc;
> +struct spi_device *spi;
> +int vref_uv;
> +struct mutex lock;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> +{ /* [0] */
> +.type = IIO_VOLTAGE,
> +.scan_type = {

 scan_type not needed since driver does not support buffered reads

> +.sign = 'u',
> +.realbits = 16,
> +.storagebits = 24,
> +

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2017-01-02 Thread jacopo mondi 

Hi Jonathan,
   thanks for review

On 30/12/2016 17:31, Jonathan Cameron wrote:

On 14/12/16 12:00, jac...@jmondi.org wrote:

Hello Peter,
   thanks for review

On 13/12/2016 21:33, Peter Meerwald-Stadler wrote:



Add IIO driver for Maxim MAX11100 single-channel ADC.
Add DT bindings documentation.


some more comments


Signed-off-by: Jacopo Mondi 
---

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

---
 .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
 drivers/iio/adc/Kconfig|   9 ++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 166 +
 4 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 000..6877c11
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,17 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "maxim,max11100"
+  - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+  - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+adc0: max11100@0 {
+compatible = "maxim,max11100";
+vref-supply = <_vref>;
+spi-max-frequency = <24>;
+};
+
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..a909484 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -285,6 +285,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


SPI_MASTER is more precise I think


+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 7a40c04..1463044 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -29,6 +29,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 000..f372ad8
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,166 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref / 2 ^ 16)
+ * AIN = (DIN * LSB)
+ */
+#define MAX11100_LSB_DIV(1 << 16)
+#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)


maybe parenthesis around vref


+
+struct max11100_state {
+const struct max11100_chip_desc *desc;
+struct spi_device *spi;
+int vref_uv;
+struct mutex lock;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+{ /* [0] */
+.type = IIO_VOLTAGE,
+.scan_type = {


scan_type not needed since driver does not support buffered reads


+.sign = 'u',
+.realbits = 16,
+.storagebits = 24,
+.shift = 8,
+.repeat = 1,
+.endianness = IIO_BE,
+},
+.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+},
+};
+
+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_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+int ret;
+struct max11100_state *state = iio_priv(indio_dev);
+uint8_t buffer[3];
+
+mutex_lock(>lock);
+
+ret = spi_read(state->spi, buffer, sizeof(buffer));
+if (ret) {
+mutex_unlock(>lock);
+dev_err(_dev->dev, "SPI transfer failed\n");
+return ret;
+}
+mutex_unlock(>lock);
+
+

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2017-01-02 Thread jacopo mondi 

Hi Jonathan,

On 30/12/2016 17:08, Jonathan Cameron wrote:

On 13/12/16 12:37, Jacopo Mondi wrote:

Add IIO driver for Maxim MAX11100 single-channel ADC.
Add DT bindings documentation.

Signed-off-by: Jacopo Mondi 

A quick process related thing.  Please don't post a new
version of a driver as a reply to an old version. It very rapidly
leads to deep and difficult to follow threads.

Much easier to just start a new thread.



Noted!
Thanks for suggestion ;)
   j



---

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

---
 .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
 drivers/iio/adc/Kconfig|   9 ++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 166 +
 4 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 000..6877c11
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,17 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "maxim,max11100"
+  - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+  - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+adc0: max11100@0 {
+compatible = "maxim,max11100";
+vref-supply = <_vref>;
+spi-max-frequency = <24>;
+};
+
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..a909484 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -285,6 +285,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
+   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 7a40c04..1463044 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -29,6 +29,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 000..f372ad8
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,166 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref / 2 ^ 16)
+ * AIN = (DIN * LSB)
+ */
+#define MAX11100_LSB_DIV   (1 << 16)
+#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)
+
+struct max11100_state {
+   const struct max11100_chip_desc *desc;
+   struct spi_device *spi;
+   int vref_uv;
+   struct mutex lock;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+   { /* [0] */
+   .type = IIO_VOLTAGE,
+   .scan_type = {
+   .sign = 'u',
+   .realbits = 16,
+   .storagebits = 24,
+   .shift = 8,
+   .repeat = 1,
+   .endianness = IIO_BE,
+   },
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+   },
+};
+
+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_raw(struct iio_dev *indio_dev,
+struct iio_chan_spec const *chan,
+int *val, int *val2, long mask)
+{
+   int ret;
+   struct max11100_state *state = iio_priv(indio_dev);
+   uint8_t buffer[3];
+
+   mutex_lock(>lock);
+
+   ret = spi_read(state->spi, buffer,

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 13/12/16 12:37, Jacopo Mondi wrote:
> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 
Nothing significant to add, but a few comments inline.

Jonathan
> ---
> 
> 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
> 
> ---
>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>  drivers/iio/adc/Kconfig|   9 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 166 
> +
>  4 files changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> new file mode 100644
> index 000..6877c11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> @@ -0,0 +1,17 @@
> +* Maxim max11100 Analog to Digital Converter (ADC)
> +
> +Required properties:
> +  - compatible: Should be "maxim,max11100"
> +  - vref-supply: phandle to the regulator that provides reference voltage
> +
> +Optional properties:
> +  - spi-max-frequency: SPI maximum frequency
> +
> +Example:
> +
> +adc0: max11100@0 {
> +compatible = "maxim,max11100";
> +vref-supply = <_vref>;
> +spi-max-frequency = <24>;
> +};
> +
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..a909484 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -285,6 +285,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
> + 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 7a40c04..1463044 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,166 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV (1 << 16)
> +#define MAX11100_LSB(vref)   (vref / MAX11100_LSB_DIV)
> +
> +struct max11100_state {
> + const struct max11100_chip_desc *desc;
> + struct spi_device *spi;
> + int vref_uv;
> + struct mutex lock;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> + { /* [0] */
> + .type = IIO_VOLTAGE,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 24,
> + .shift = 8,
> + .repeat = 1,
> + .endianness = IIO_BE,
> + },
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + },
> +};
> +
> +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_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int *val, int *val2, long mask)
> +{
> + int ret;
> + struct max11100_state *state = iio_priv(indio_dev);
> + uint8_t buffer[3];
> +
> + mutex_lock(>lock);
> +
> + ret = spi_read(state->spi, buffer, sizeof(buffer));
> +

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 14/12/16 12:00, jac...@jmondi.org wrote:
> Hello Peter,
>thanks for review
> 
> On 13/12/2016 21:33, Peter Meerwald-Stadler wrote:
>>
>>> Add IIO driver for Maxim MAX11100 single-channel ADC.
>>> Add DT bindings documentation.
>>
>> some more comments
>>
>>> Signed-off-by: Jacopo Mondi 
>>> ---
>>>
>>> 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
>>>
>>> ---
>>>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>>>  drivers/iio/adc/Kconfig|   9 ++
>>>  drivers/iio/adc/Makefile   |   1 +
>>>  drivers/iio/adc/max11100.c | 166 
>>> +
>>>  4 files changed, 193 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>>  create mode 100644 drivers/iio/adc/max11100.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
>>> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
>>> new file mode 100644
>>> index 000..6877c11
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
>>> @@ -0,0 +1,17 @@
>>> +* Maxim max11100 Analog to Digital Converter (ADC)
>>> +
>>> +Required properties:
>>> +  - compatible: Should be "maxim,max11100"
>>> +  - vref-supply: phandle to the regulator that provides reference voltage
>>> +
>>> +Optional properties:
>>> +  - spi-max-frequency: SPI maximum frequency
>>> +
>>> +Example:
>>> +
>>> +adc0: max11100@0 {
>>> +compatible = "maxim,max11100";
>>> +vref-supply = <_vref>;
>>> +spi-max-frequency = <24>;
>>> +};
>>> +
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 99c0514..a909484 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -285,6 +285,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
>>
>> SPI_MASTER is more precise I think
>>
>>> +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 7a40c04..1463044 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -29,6 +29,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 000..f372ad8
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/max11100.c
>>> @@ -0,0 +1,166 @@
>>> +/*
>>> + * 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 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +/*
>>> + * LSB is the ADC single digital step
>>> + * 1 LSB = (vref / 2 ^ 16)
>>> + * AIN = (DIN * LSB)
>>> + */
>>> +#define MAX11100_LSB_DIV(1 << 16)
>>> +#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)
>>
>> maybe parenthesis around vref
>>
>>> +
>>> +struct max11100_state {
>>> +const struct max11100_chip_desc *desc;
>>> +struct spi_device *spi;
>>> +int vref_uv;
>>> +struct mutex lock;
>>> +};
>>> +
>>> +static struct iio_chan_spec max11100_channels[] = {
>>> +{ /* [0] */
>>> +.type = IIO_VOLTAGE,
>>> +.scan_type = {
>>
>> scan_type not needed since driver does not support buffered reads
>>
>>> +.sign = 'u',
>>> +.realbits = 16,
>>> +.storagebits = 24,
>>> +.shift = 8,
>>> +.repeat = 1,
>>> +.endianness = IIO_BE,
>>> +},
>>> +.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>> +},
>>> +};
>>> +
>>> +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,
>>> +};

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 13/12/16 15:53, Wolfram Sang wrote:
> 
>>> +struct max11100_state {
>>> +   const struct max11100_chip_desc *desc;
>>> +   struct spi_device *spi;
>>> +   int vref_uv;
>>
>> unsi
>> It's good practice to move the smaller members to the end of the structure,
>> to avoid gaps due to alignment rules (struct mutex needs to be 64-bit aligned
>> on 64-bit platforms).
> 
> One option. Another idea is to put the most used members to the front to
> increase chances of being in the same cacheline.
> 
Or more cynically, just put them in the order that makes most logical sense
as that gives you fewest grumpy reviewers.
We aren't really talking high performance stuff here!

Jonathan

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 13/12/16 12:37, Jacopo Mondi wrote:
> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 
A quick process related thing.  Please don't post a new
version of a driver as a reply to an old version. It very rapidly
leads to deep and difficult to follow threads.

Much easier to just start a new thread.

> ---
> 
> 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
> 
> ---
>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>  drivers/iio/adc/Kconfig|   9 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 166 
> +
>  4 files changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> new file mode 100644
> index 000..6877c11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> @@ -0,0 +1,17 @@
> +* Maxim max11100 Analog to Digital Converter (ADC)
> +
> +Required properties:
> +  - compatible: Should be "maxim,max11100"
> +  - vref-supply: phandle to the regulator that provides reference voltage
> +
> +Optional properties:
> +  - spi-max-frequency: SPI maximum frequency
> +
> +Example:
> +
> +adc0: max11100@0 {
> +compatible = "maxim,max11100";
> +vref-supply = <_vref>;
> +spi-max-frequency = <24>;
> +};
> +
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..a909484 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -285,6 +285,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
> + 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 7a40c04..1463044 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,166 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV (1 << 16)
> +#define MAX11100_LSB(vref)   (vref / MAX11100_LSB_DIV)
> +
> +struct max11100_state {
> + const struct max11100_chip_desc *desc;
> + struct spi_device *spi;
> + int vref_uv;
> + struct mutex lock;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> + { /* [0] */
> + .type = IIO_VOLTAGE,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 24,
> + .shift = 8,
> + .repeat = 1,
> + .endianness = IIO_BE,
> + },
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + },
> +};
> +
> +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_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int *val, int *val2, long mask)
> +{
> + int ret;
> + struct max11100_state *state =

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-14 Thread Geert Uytterhoeven 
Hi Jacopo,

On Wed, Dec 14, 2016 at 12:54 PM, jac...@jmondi.org  wrote:
>>> +struct max11100_state {
>>> +   const struct max11100_chip_desc *desc;
>>> +   struct spi_device *spi;
>>> +   int vref_uv;
>>
>> unsi
>
> Was this a suggestion to turn "vref_uv" into unsigned?

It was a suggestion I started writing, and aborted improperly when noticing
the below ;-)

> As you can see in probe function it can get assigned to -EINVAL when a dummy
> regulator is returned from regulator-core.
> I had to make it signed for this reason

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-14 Thread jac...@jmondi.org 

Hi Geert,

On 13/12/2016 14:21, Geert Uytterhoeven wrote:

Hi Jacopo,

On Tue, Dec 13, 2016 at 1:37 PM, Jacopo Mondi  wrote:

Add IIO driver for Maxim MAX11100 single-channel ADC.
Add DT bindings documentation.

Signed-off-by: Jacopo Mondi 



--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -29,6 +29,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 000..f372ad8
--- /dev/null
+++ b/drivers/iio/adc/max11100.c



+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref / 2 ^ 16)
+ * AIN = (DIN * LSB)
+ */
+#define MAX11100_LSB_DIV   (1 << 16)
+#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)


DIV_ROUND_CLOSEST()?


+struct max11100_state {
+   const struct max11100_chip_desc *desc;
+   struct spi_device *spi;
+   int vref_uv;


unsi


Was this a suggestion to turn "vref_uv" into unsigned?
As you can see in probe function it can get assigned to -EINVAL when a 
dummy regulator is returned from regulator-core.

I had to make it signed for this reason

Thanks
  j



It's good practice to move the smaller members to the end of the structure,
to avoid gaps due to alignment rules (struct mutex needs to be 64-bit aligned
on 64-bit platforms).


+   struct mutex lock;
+};



+static struct max11100_chip_desc {
+   unsigned int num_chan;
+   const struct iio_chan_spec *channels;


Same here (but it won't have any effect for now).


+} max11100_desc = {
+   .num_chan = ARRAY_SIZE(max11100_channels),
+   .channels = max11100_channels,
+};



+static int max11100_read_raw(struct iio_dev *indio_dev,
+struct iio_chan_spec const *chan,
+int *val, int *val2, long mask)
+{
+   int ret;
+   struct max11100_state *state = iio_priv(indio_dev);
+   uint8_t buffer[3];



+   *val = be16_to_cpu(*(uint16_t *)[1]);


Reading the uint16_t will be an unaligned load, which is not supported on all
platforms. So you either have to use get_unaligned_le16(), or assemble the
value yourself, like you did in v1.


+   *val = *val * MAX11100_LSB(state->vref_uv) / 1000;


DIV_ROUND_CLOSEST()?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-14 Thread jac...@jmondi.org 

Hello Peter,
   thanks for review

On 13/12/2016 21:33, Peter Meerwald-Stadler wrote:



Add IIO driver for Maxim MAX11100 single-channel ADC.
Add DT bindings documentation.


some more comments


Signed-off-by: Jacopo Mondi 
---

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

---
 .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
 drivers/iio/adc/Kconfig|   9 ++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 166 +
 4 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 000..6877c11
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,17 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "maxim,max11100"
+  - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+  - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+adc0: max11100@0 {
+compatible = "maxim,max11100";
+vref-supply = <_vref>;
+spi-max-frequency = <24>;
+};
+
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..a909484 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -285,6 +285,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


SPI_MASTER is more precise I think


+   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 7a40c04..1463044 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -29,6 +29,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 000..f372ad8
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,166 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref / 2 ^ 16)
+ * AIN = (DIN * LSB)
+ */
+#define MAX11100_LSB_DIV   (1 << 16)
+#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)


maybe parenthesis around vref


+
+struct max11100_state {
+   const struct max11100_chip_desc *desc;
+   struct spi_device *spi;
+   int vref_uv;
+   struct mutex lock;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+   { /* [0] */
+   .type = IIO_VOLTAGE,
+   .scan_type = {


scan_type not needed since driver does not support buffered reads


+   .sign = 'u',
+   .realbits = 16,
+   .storagebits = 24,
+   .shift = 8,
+   .repeat = 1,
+   .endianness = IIO_BE,
+   },
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+   },
+};
+
+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_raw(struct iio_dev *indio_dev,
+struct iio_chan_spec const *chan,
+int *val, int *val2, long mask)
+{
+   int ret;
+   struct max11100_state *state = iio_priv(indio_dev);
+   uint8_t buffer[3];
+
+   mutex_lock(>lock);
+
+   ret = spi_read(state->spi, buffer, sizeof(buffer));
+   if (ret) {
+   mutex_unlock(>lock);
+

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-13 Thread Peter Meerwald-Stadler 

> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.

some more comments
 
> Signed-off-by: Jacopo Mondi 
> ---
> 
> 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
> 
> ---
>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>  drivers/iio/adc/Kconfig|   9 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 166 
> +
>  4 files changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> new file mode 100644
> index 000..6877c11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> @@ -0,0 +1,17 @@
> +* Maxim max11100 Analog to Digital Converter (ADC)
> +
> +Required properties:
> +  - compatible: Should be "maxim,max11100"
> +  - vref-supply: phandle to the regulator that provides reference voltage
> +
> +Optional properties:
> +  - spi-max-frequency: SPI maximum frequency
> +
> +Example:
> +
> +adc0: max11100@0 {
> +compatible = "maxim,max11100";
> +vref-supply = <_vref>;
> +spi-max-frequency = <24>;
> +};
> +
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..a909484 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -285,6 +285,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

SPI_MASTER is more precise I think

> + 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 7a40c04..1463044 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,166 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV (1 << 16)
> +#define MAX11100_LSB(vref)   (vref / MAX11100_LSB_DIV)

maybe parenthesis around vref

> +
> +struct max11100_state {
> + const struct max11100_chip_desc *desc;
> + struct spi_device *spi;
> + int vref_uv;
> + struct mutex lock;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> + { /* [0] */
> + .type = IIO_VOLTAGE,
> + .scan_type = {

scan_type not needed since driver does not support buffered reads

> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 24,
> + .shift = 8,
> + .repeat = 1,
> + .endianness = IIO_BE,
> + },
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + },
> +};
> +
> +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_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int *val, int *val2, long mask)
> +{
> + int ret;
> + struct max11100_state *state = iio_priv(indio_dev);
> + uint8_t buffer[3];
> +
> + mutex_lock(>lock);
> +
> + ret

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-13 Thread Wolfram Sang 

> > +struct max11100_state {
> > +   const struct max11100_chip_desc *desc;
> > +   struct spi_device *spi;
> > +   int vref_uv;
> 
> unsi
> It's good practice to move the smaller members to the end of the structure,
> to avoid gaps due to alignment rules (struct mutex needs to be 64-bit aligned
> on 64-bit platforms).

One option. Another idea is to put the most used members to the front to
increase chances of being in the same cacheline.



signature.asc
Description: PGP signature

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-13 Thread Geert Uytterhoeven 
Hi Jacopo,

On Tue, Dec 13, 2016 at 1:37 PM, Jacopo Mondi  wrote:
> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.
>
> Signed-off-by: Jacopo Mondi 

> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c

> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV   (1 << 16)
> +#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)

DIV_ROUND_CLOSEST()?

> +struct max11100_state {
> +   const struct max11100_chip_desc *desc;
> +   struct spi_device *spi;
> +   int vref_uv;

unsi
It's good practice to move the smaller members to the end of the structure,
to avoid gaps due to alignment rules (struct mutex needs to be 64-bit aligned
on 64-bit platforms).

> +   struct mutex lock;
> +};

> +static struct max11100_chip_desc {
> +   unsigned int num_chan;
> +   const struct iio_chan_spec *channels;

Same here (but it won't have any effect for now).

> +} max11100_desc = {
> +   .num_chan = ARRAY_SIZE(max11100_channels),
> +   .channels = max11100_channels,
> +};

> +static int max11100_read_raw(struct iio_dev *indio_dev,
> +struct iio_chan_spec const *chan,
> +int *val, int *val2, long mask)
> +{
> +   int ret;
> +   struct max11100_state *state = iio_priv(indio_dev);
> +   uint8_t buffer[3];

> +   *val = be16_to_cpu(*(uint16_t *)[1]);

Reading the uint16_t will be an unaligned load, which is not supported on all
platforms. So you either have to use get_unaligned_le16(), or assemble the
value yourself, like you did in v1.

> +   *val = *val * MAX11100_LSB(state->vref_uv) / 1000;

DIV_ROUND_CLOSEST()?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

[PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-13 Thread Jacopo Mondi 
Add IIO driver for Maxim MAX11100 single-channel ADC.
Add DT bindings documentation.

Signed-off-by: Jacopo Mondi 
---

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

---
 .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
 drivers/iio/adc/Kconfig|   9 ++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 166 +
 4 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 000..6877c11
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,17 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "maxim,max11100"
+  - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+  - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+adc0: max11100@0 {
+compatible = "maxim,max11100";
+vref-supply = <_vref>;
+spi-max-frequency = <24>;
+};
+
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..a909484 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -285,6 +285,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
+   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 7a40c04..1463044 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -29,6 +29,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 000..f372ad8
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,166 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref / 2 ^ 16)
+ * AIN = (DIN * LSB)
+ */
+#define MAX11100_LSB_DIV   (1 << 16)
+#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)
+
+struct max11100_state {
+   const struct max11100_chip_desc *desc;
+   struct spi_device *spi;
+   int vref_uv;
+   struct mutex lock;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+   { /* [0] */
+   .type = IIO_VOLTAGE,
+   .scan_type = {
+   .sign = 'u',
+   .realbits = 16,
+   .storagebits = 24,
+   .shift = 8,
+   .repeat = 1,
+   .endianness = IIO_BE,
+   },
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+   },
+};
+
+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_raw(struct iio_dev *indio_dev,
+struct iio_chan_spec const *chan,
+int *val, int *val2, long mask)
+{
+   int ret;
+   struct max11100_state *state = iio_priv(indio_dev);
+   uint8_t buffer[3];
+
+   mutex_lock(>lock);
+
+   ret = spi_read(state->spi, buffer, sizeof(buffer));
+   if (ret) {
+   mutex_unlock(>lock);
+   dev_err(_dev->dev, "SPI transfer failed\n");
+   return ret;
+   }
+   mutex_unlock(>lock);
+
+   /* the first 8 bits sent out from ADC must be 0s */
+   if (buffer[0]) {
+   dev_err(_dev->dev, "Invalid value: buffer[0] !=