All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Maxim MAX1241 driver
@ 2020-03-19 16:29 Alexandru Lazar
  2020-03-19 16:29 ` [PATCH v3 1/2] iio: adc: Add " Alexandru Lazar
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexandru Lazar @ 2020-03-19 16:29 UTC (permalink / raw)
  To: linux-iio
  Cc: jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland, Alexandru Lazar

A silly typo found its way into the previous version of this patch
series. Fortunately, Alexandru Ardelean spotted it. I'm a little
embarrassed to show my face in public right now -- thankfully I'm
staying home these days anyway.

Other than the Kconfig text (which claimed this module was named max1118
-- like I said, I wear my code monkey badge with pride...) nothing's
changed since the last version. For reference, the last version is here:

https://lore.kernel.org/linux-iio/20200318202837.512428-1-alazar@startmail.com/

Thanks!

Alex

Alexandru Lazar (2):
  iio: adc: Add MAX1241 driver
  dt-bindings: iio: adc: Add MAX1241 device tree bindings in
    documentation

 .../bindings/iio/adc/maxim,max1241.yaml       |  62 ++++++
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/max1241.c                     | 206 ++++++++++++++++++
 4 files changed, 279 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
 create mode 100644 drivers/iio/adc/max1241.c

-- 
2.25.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/2] iio: adc: Add MAX1241 driver
  2020-03-19 16:29 [PATCH v3 0/2] Maxim MAX1241 driver Alexandru Lazar
@ 2020-03-19 16:29 ` Alexandru Lazar
  2020-03-20  7:06   ` Ardelean, Alexandru
  2020-03-19 16:29 ` [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation Alexandru Lazar
  2020-03-20  7:02 ` [PATCH v3 0/2] Maxim MAX1241 driver Ardelean, Alexandru
  2 siblings, 1 reply; 9+ messages in thread
From: Alexandru Lazar @ 2020-03-19 16:29 UTC (permalink / raw)
  To: linux-iio
  Cc: jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland, Alexandru Lazar

Add driver for the Maxim MAX1241 12-bit, single-channel ADC. The driver
includes support for this device's low-power operation mode.

Signed-off-by: Alexandru Lazar <alazar@startmail.com>
---
 drivers/iio/adc/Kconfig   |  10 ++
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/max1241.c | 206 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+)
 create mode 100644 drivers/iio/adc/max1241.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 5d8540b7b427..55f6462cd93f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -566,6 +566,16 @@ config MAX1118
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1118.
 
+config MAX1241
+	tristate "Maxim max1241 ADC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Maxim max1241 12-bit, single-channel
+	  ADC.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max1241.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a1f1fbec0f87..37d6f17559dc 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_LTC2497) += ltc2497.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1118) += max1118.o
+obj-$(CONFIG_MAX1241) += max1241.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MAX9611) += max9611.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
diff --git a/drivers/iio/adc/max1241.c b/drivers/iio/adc/max1241.c
new file mode 100644
index 000000000000..0278510ec346
--- /dev/null
+++ b/drivers/iio/adc/max1241.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MAX1241 low-power, 12-bit serial ADC
+ *
+ * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1240-MAX1241.pdf
+ */
+
+#include <linux/regulator/consumer.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+#define MAX1241_VAL_MASK 0xFFF
+#define MAX1241_SHDN_DELAY_USEC 4
+
+enum max1241_id {
+	max1241,
+};
+
+struct max1241 {
+	struct spi_device *spi;
+	struct mutex lock;
+	struct regulator *reg;
+	struct gpio_desc *shdn;
+
+	__be16 data ____cacheline_aligned;
+};
+
+static const struct iio_chan_spec max1241_channels[] = {
+	{
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 12,
+			.storagebits = 16,
+		},
+	},
+};
+
+static int max1241_read(struct max1241 *adc)
+{
+	struct spi_transfer xfers[] = {
+		/*
+		 * Begin conversion by bringing /CS low for at least
+		 * tconv us.
+		 */
+		{
+			.len = 0,
+			.delay.value = 8,
+			.delay.unit = SPI_DELAY_UNIT_USECS,
+		},
+		/*
+		 * Then read two bytes of data in our RX buffer.
+		 */
+		{
+			.rx_buf = &adc->data,
+			.len = 2,
+		},
+	};
+
+	return spi_sync_transfer(adc->spi, xfers, ARRAY_SIZE(xfers));
+}
+
+static int max1241_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val, int *val2, long mask)
+{
+	int ret, vref_uV;
+	struct max1241 *adc = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&adc->lock);
+
+		if (adc->shdn) {
+			gpiod_set_value(adc->shdn, 0);
+			udelay(MAX1241_SHDN_DELAY_USEC);
+		}
+
+		ret = max1241_read(adc);
+
+		if (adc->shdn)
+			gpiod_set_value(adc->shdn, 1);
+
+		if (ret) {
+			mutex_unlock(&adc->lock);
+			return ret;
+		}
+
+		*val = (be16_to_cpu(adc->data) >> 3) & MAX1241_VAL_MASK;
+
+		mutex_unlock(&adc->lock);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		vref_uV = regulator_get_voltage(adc->reg);
+
+		if (vref_uV < 0)
+			return vref_uV;
+
+		*val = vref_uV / 1000;
+		*val2 = 12;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info max1241_info = {
+	.read_raw = max1241_read_raw,
+};
+
+static void max1241_disable_reg_action(void *data)
+{
+	struct max1241 *adc = data;
+	int err;
+
+	err = regulator_disable(adc->reg);
+	if (err)
+		dev_err(&adc->spi->dev, "could not disable reference regulator.\n");
+}
+
+static int max1241_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct max1241 *adc;
+	int ret = 0;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	adc->spi = spi;
+	mutex_init(&adc->lock);
+
+	spi_set_drvdata(spi, indio_dev);
+
+	adc->reg = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(adc->reg)) {
+		dev_err(&spi->dev, "failed to get vref regulator\n");
+		return PTR_ERR(adc->reg);
+	}
+
+	ret = regulator_enable(adc->reg);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&spi->dev, max1241_disable_reg_action,
+					adc);
+	if (ret) {
+		dev_err(&spi->dev, "could not set up regulator cleanup action!\n");
+		return ret;
+	}
+
+	adc->shdn = devm_gpiod_get_optional(&spi->dev, "shdn", GPIOD_OUT_HIGH);
+
+	if (IS_ERR(adc->shdn))
+		return PTR_ERR(adc->shdn);
+
+	if (!adc->shdn)
+		dev_dbg(&spi->dev, "no shdn pin passed, low-power mode disabled");
+	else
+		dev_dbg(&spi->dev, "shdn pin passed, low-power mode enabled");
+
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->info = &max1241_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = max1241_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max1241_channels);
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id max1241_id[] = {
+	{ "max1241", max1241 },
+	{},
+};
+
+static const struct of_device_id max1241_dt_ids[] = {
+	{ .compatible = "maxim,max1241" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max1241_dt_ids);
+
+static struct spi_driver max1241_spi_driver = {
+	.driver = {
+		.name = "max1241",
+		.of_match_table = max1241_dt_ids,
+	},
+	.probe = max1241_probe,
+	.id_table = max1241_id,
+};
+module_spi_driver(max1241_spi_driver);
+
+MODULE_AUTHOR("Alexandru Lazar <alazar@startmail.com>");
+MODULE_DESCRIPTION("MAX1241 ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation
  2020-03-19 16:29 [PATCH v3 0/2] Maxim MAX1241 driver Alexandru Lazar
  2020-03-19 16:29 ` [PATCH v3 1/2] iio: adc: Add " Alexandru Lazar
@ 2020-03-19 16:29 ` Alexandru Lazar
  2020-03-20  7:12   ` Ardelean, Alexandru
  2020-03-20  7:02 ` [PATCH v3 0/2] Maxim MAX1241 driver Ardelean, Alexandru
  2 siblings, 1 reply; 9+ messages in thread
From: Alexandru Lazar @ 2020-03-19 16:29 UTC (permalink / raw)
  To: linux-iio
  Cc: jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland, Alexandru Lazar

Add device-tree bindings documentation for the MAX1241 device driver.

Signed-off-by: Alexandru Lazar <alazar@startmail.com>
---
 .../bindings/iio/adc/maxim,max1241.yaml       | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
new file mode 100644
index 000000000000..28c73ed795aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2020 Ioan-Alexandru Lazar
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/maxim,max1241.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX1241 12-bit, single-channel analog to digital converter
+
+maintainers:
+  - Ioan-Alexandru Lazar <alazar@startmail.com>
+
+description: |
+  Bindings for the max1241 12-bit, single-channel ADC device. This
+  driver supports voltage reading and can optionally be configured for
+  power-down mode operation. The datasheet can be found at:
+    https://datasheets.maximintegrated.com/en/ds/MAX1240-MAX1241.pdf
+
+properties:
+  compatible:
+    enum:
+      - maxim,max1241
+
+  reg:
+    description: SPI chip select number for this device
+    maxItems: 1
+
+  vref-supply:
+    description:
+      Device tree identifier of the regulator that provides the external
+      reference voltage.
+    maxItems: 1
+
+  shdn-gpios:
+    description:
+      GPIO spec for the GPIO pin connected to the ADC's /SHDN pin. If
+      specified, the /SHDN pin will be asserted between conversions,
+      thus enabling power-down mode.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vref-supply
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+        adc@0 {
+            compatible = "maxim,max1241";
+            reg = <0>;
+            vref-supply = <&vdd_3v3_reg>;
+            spi-max-frequency = <1000000>;
+            shdn-gpios = <&gpio 26 1>;
+        };
+    };
+
+
-- 
2.25.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/2] Maxim MAX1241 driver
  2020-03-19 16:29 [PATCH v3 0/2] Maxim MAX1241 driver Alexandru Lazar
  2020-03-19 16:29 ` [PATCH v3 1/2] iio: adc: Add " Alexandru Lazar
  2020-03-19 16:29 ` [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation Alexandru Lazar
@ 2020-03-20  7:02 ` Ardelean, Alexandru
  2 siblings, 0 replies; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-03-20  7:02 UTC (permalink / raw)
  To: alazar, linux-iio; +Cc: jic23, mark.rutland, knaack.h, lars, pmeerw, robh+dt

On Thu, 2020-03-19 at 18:29 +0200, Alexandru Lazar wrote:
> A silly typo found its way into the previous version of this patch
> series. Fortunately, Alexandru Ardelean spotted it. I'm a little
> embarrassed to show my face in public right now -- thankfully I'm
> staying home these days anyway.

I still win in embarrassment [getting embarrassed] contests.

> 
> Other than the Kconfig text (which claimed this module was named max1118
> -- like I said, I wear my code monkey badge with pride...) nothing's
> changed since the last version. For reference, the last version is here:
> 
> https://lore.kernel.org/linux-iio/20200318202837.512428-1-alazar@startmail.com/
> 

For future patches: it's usually a good idea to include changelog in each
V2,3,4,N

Example: 
https://lore.kernel.org/linux-iio/20200316155035.25500-1-alexandru.ardelean@analog.com/T/

[this one [of mine] got up-to V10 :p ; I'll admit I didn't do a good job right
from the start]

> Thanks!
> 
> Alex
> 
> Alexandru Lazar (2):
>   iio: adc: Add MAX1241 driver
>   dt-bindings: iio: adc: Add MAX1241 device tree bindings in
>     documentation
> 
>  .../bindings/iio/adc/maxim,max1241.yaml       |  62 ++++++
>  drivers/iio/adc/Kconfig                       |  10 +
>  drivers/iio/adc/Makefile                      |   1 +
>  drivers/iio/adc/max1241.c                     | 206 ++++++++++++++++++
>  4 files changed, 279 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
>  create mode 100644 drivers/iio/adc/max1241.c
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] iio: adc: Add MAX1241 driver
  2020-03-19 16:29 ` [PATCH v3 1/2] iio: adc: Add " Alexandru Lazar
@ 2020-03-20  7:06   ` Ardelean, Alexandru
  2020-03-20  7:13     ` Ardelean, Alexandru
  0 siblings, 1 reply; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-03-20  7:06 UTC (permalink / raw)
  To: alazar, linux-iio; +Cc: jic23, mark.rutland, knaack.h, lars, pmeerw, robh+dt

On Thu, 2020-03-19 at 18:29 +0200, Alexandru Lazar wrote:
> Add driver for the Maxim MAX1241 12-bit, single-channel ADC. The driver
> includes support for this device's low-power operation mode.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Alexandru Lazar <alazar@startmail.com>
> ---
>  drivers/iio/adc/Kconfig   |  10 ++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/max1241.c | 206 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 217 insertions(+)
>  create mode 100644 drivers/iio/adc/max1241.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 5d8540b7b427..55f6462cd93f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -566,6 +566,16 @@ config MAX1118
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max1118.
>  
> +config MAX1241
> +	tristate "Maxim max1241 ADC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Maxim max1241 12-bit, single-channel
> +	  ADC.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max1241.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index a1f1fbec0f87..37d6f17559dc 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_LTC2497) += ltc2497.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1118) += max1118.o
> +obj-$(CONFIG_MAX1241) += max1241.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MAX9611) += max9611.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
> diff --git a/drivers/iio/adc/max1241.c b/drivers/iio/adc/max1241.c
> new file mode 100644
> index 000000000000..0278510ec346
> --- /dev/null
> +++ b/drivers/iio/adc/max1241.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * MAX1241 low-power, 12-bit serial ADC
> + *
> + * Datasheet: 
> https://datasheets.maximintegrated.com/en/ds/MAX1240-MAX1241.pdf
> + */
> +
> +#include <linux/regulator/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +#define MAX1241_VAL_MASK 0xFFF
> +#define MAX1241_SHDN_DELAY_USEC 4
> +
> +enum max1241_id {
> +	max1241,
> +};
> +
> +struct max1241 {
> +	struct spi_device *spi;
> +	struct mutex lock;
> +	struct regulator *reg;
> +	struct gpio_desc *shdn;
> +
> +	__be16 data ____cacheline_aligned;
> +};
> +
> +static const struct iio_chan_spec max1241_channels[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 12,
> +			.storagebits = 16,
> +		},
> +	},
> +};
> +
> +static int max1241_read(struct max1241 *adc)
> +{
> +	struct spi_transfer xfers[] = {
> +		/*
> +		 * Begin conversion by bringing /CS low for at least
> +		 * tconv us.
> +		 */
> +		{
> +			.len = 0,
> +			.delay.value = 8,
> +			.delay.unit = SPI_DELAY_UNIT_USECS,
> +		},
> +		/*
> +		 * Then read two bytes of data in our RX buffer.
> +		 */
> +		{
> +			.rx_buf = &adc->data,
> +			.len = 2,
> +		},
> +	};
> +
> +	return spi_sync_transfer(adc->spi, xfers, ARRAY_SIZE(xfers));
> +}
> +
> +static int max1241_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val, int *val2, long mask)
> +{
> +	int ret, vref_uV;
> +	struct max1241 *adc = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&adc->lock);
> +
> +		if (adc->shdn) {
> +			gpiod_set_value(adc->shdn, 0);
> +			udelay(MAX1241_SHDN_DELAY_USEC);
> +		}
> +
> +		ret = max1241_read(adc);
> +
> +		if (adc->shdn)
> +			gpiod_set_value(adc->shdn, 1);
> +
> +		if (ret) {
> +			mutex_unlock(&adc->lock);
> +			return ret;
> +		}
> +
> +		*val = (be16_to_cpu(adc->data) >> 3) & MAX1241_VAL_MASK;
> +
> +		mutex_unlock(&adc->lock);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_uV = regulator_get_voltage(adc->reg);
> +
> +		if (vref_uV < 0)
> +			return vref_uV;
> +
> +		*val = vref_uV / 1000;
> +		*val2 = 12;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info max1241_info = {
> +	.read_raw = max1241_read_raw,
> +};
> +
> +static void max1241_disable_reg_action(void *data)
> +{
> +	struct max1241 *adc = data;
> +	int err;
> +
> +	err = regulator_disable(adc->reg);
> +	if (err)
> +		dev_err(&adc->spi->dev, "could not disable reference
> regulator.\n");
> +}
> +
> +static int max1241_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct max1241 *adc;
> +	int ret = 0;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	adc->spi = spi;
> +	mutex_init(&adc->lock);
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	adc->reg = devm_regulator_get(&spi->dev, "vref");
> +	if (IS_ERR(adc->reg)) {
> +		dev_err(&spi->dev, "failed to get vref regulator\n");
> +		return PTR_ERR(adc->reg);
> +	}
> +
> +	ret = regulator_enable(adc->reg);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(&spi->dev, max1241_disable_reg_action,
> +					adc);
> +	if (ret) {
> +		dev_err(&spi->dev, "could not set up regulator cleanup
> action!\n");
> +		return ret;
> +	}
> +
> +	adc->shdn = devm_gpiod_get_optional(&spi->dev, "shdn", GPIOD_OUT_HIGH);
> +
> +	if (IS_ERR(adc->shdn))
> +		return PTR_ERR(adc->shdn);
> +
> +	if (!adc->shdn)
> +		dev_dbg(&spi->dev, "no shdn pin passed, low-power mode
> disabled");
> +	else
> +		dev_dbg(&spi->dev, "shdn pin passed, low-power mode enabled");
> +
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->info = &max1241_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = max1241_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max1241_channels);
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id max1241_id[] = {
> +	{ "max1241", max1241 },
> +	{},
> +};
> +
> +static const struct of_device_id max1241_dt_ids[] = {
> +	{ .compatible = "maxim,max1241" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, max1241_dt_ids);
> +
> +static struct spi_driver max1241_spi_driver = {
> +	.driver = {
> +		.name = "max1241",
> +		.of_match_table = max1241_dt_ids,
> +	},
> +	.probe = max1241_probe,
> +	.id_table = max1241_id,
> +};
> +module_spi_driver(max1241_spi_driver);
> +
> +MODULE_AUTHOR("Alexandru Lazar <alazar@startmail.com>");
> +MODULE_DESCRIPTION("MAX1241 ADC driver");
> +MODULE_LICENSE("GPL v2");

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation
  2020-03-19 16:29 ` [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation Alexandru Lazar
@ 2020-03-20  7:12   ` Ardelean, Alexandru
  2020-03-20  8:33     ` Alexandru Lazar
  0 siblings, 1 reply; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-03-20  7:12 UTC (permalink / raw)
  To: alazar, linux-iio; +Cc: jic23, mark.rutland, knaack.h, lars, pmeerw, robh+dt

On Thu, 2020-03-19 at 18:29 +0200, Alexandru Lazar wrote:
> Add device-tree bindings documentation for the MAX1241 device driver.
> 
> Signed-off-by: Alexandru Lazar <alazar@startmail.com>
> ---
>  .../bindings/iio/adc/maxim,max1241.yaml       | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
> b/Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
> new file mode 100644
> index 000000000000..28c73ed795aa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max1241.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright 2020 Ioan-Alexandru Lazar
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/maxim,max1241.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX1241 12-bit, single-channel analog to digital converter
> +
> +maintainers:
> +  - Ioan-Alexandru Lazar <alazar@startmail.com>
> +
> +description: |
> +  Bindings for the max1241 12-bit, single-channel ADC device. This
> +  driver supports voltage reading and can optionally be configured for
> +  power-down mode operation. The datasheet can be found at:
> +    https://datasheets.maximintegrated.com/en/ds/MAX1240-MAX1241.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max1241
> +
> +  reg:
> +    description: SPI chip select number for this device

Rob will probably complain that you don't need to document[add description] to
standard attributes [like SPI's 'reg' parameter]
Just listing it here, should be sufficient. So, you can drop the description.

> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      Device tree identifier of the regulator that provides the external
> +      reference voltage.
> +    maxItems: 1
> +
> +  shdn-gpios:
> +    description:
> +      GPIO spec for the GPIO pin connected to the ADC's /SHDN pin. If
> +      specified, the /SHDN pin will be asserted between conversions,
> +      thus enabling power-down mode.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vref-supply
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi0 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "maxim,max1241";
> +            reg = <0>;
> +            vref-supply = <&vdd_3v3_reg>;
> +            spi-max-frequency = <1000000>;
> +            shdn-gpios = <&gpio 26 1>;
> +        };
> +    };
> +
> +

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] iio: adc: Add MAX1241 driver
  2020-03-20  7:06   ` Ardelean, Alexandru
@ 2020-03-20  7:13     ` Ardelean, Alexandru
  0 siblings, 0 replies; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-03-20  7:13 UTC (permalink / raw)
  To: alazar, linux-iio; +Cc: robh+dt, jic23, mark.rutland, knaack.h, pmeerw, lars

On Fri, 2020-03-20 at 07:06 +0000, Ardelean, Alexandru wrote:
> [External]
> 
> On Thu, 2020-03-19 at 18:29 +0200, Alexandru Lazar wrote:
> > Add driver for the Maxim MAX1241 12-bit, single-channel ADC. The driver
> > includes support for this device's low-power operation mode.
> > 
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>


before I forget;
if you do a V4 [or above] you can add my Reviewed-by tag to this patch

> 
> > Signed-off-by: Alexandru Lazar <alazar@startmail.com>
> > ---
> >  drivers/iio/adc/Kconfig   |  10 ++
> >  drivers/iio/adc/Makefile  |   1 +
> >  drivers/iio/adc/max1241.c | 206 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 217 insertions(+)
> >  create mode 100644 drivers/iio/adc/max1241.c
> > 
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 5d8540b7b427..55f6462cd93f 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -566,6 +566,16 @@ config MAX1118
> >  	  To compile this driver as a module, choose M here: the module will be
> >  	  called max1118.
> >  
> > +config MAX1241
> > +	tristate "Maxim max1241 ADC driver"
> > +	depends on SPI_MASTER
> > +	help
> > +	  Say yes here to build support for Maxim max1241 12-bit, single-channel
> > +	  ADC.
> > +
> > +	  To compile this driver as a module, choose M here: the module will be
> > +	  called max1241.
> > +
> >  config MAX1363
> >  	tristate "Maxim max1363 ADC driver"
> >  	depends on I2C
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index a1f1fbec0f87..37d6f17559dc 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -54,6 +54,7 @@ obj-$(CONFIG_LTC2497) += ltc2497.o
> >  obj-$(CONFIG_MAX1027) += max1027.o
> >  obj-$(CONFIG_MAX11100) += max11100.o
> >  obj-$(CONFIG_MAX1118) += max1118.o
> > +obj-$(CONFIG_MAX1241) += max1241.o
> >  obj-$(CONFIG_MAX1363) += max1363.o
> >  obj-$(CONFIG_MAX9611) += max9611.o
> >  obj-$(CONFIG_MCP320X) += mcp320x.o
> > diff --git a/drivers/iio/adc/max1241.c b/drivers/iio/adc/max1241.c
> > new file mode 100644
> > index 000000000000..0278510ec346
> > --- /dev/null
> > +++ b/drivers/iio/adc/max1241.c
> > @@ -0,0 +1,206 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * MAX1241 low-power, 12-bit serial ADC
> > + *
> > + * Datasheet: 
> > https://datasheets.maximintegrated.com/en/ds/MAX1240-MAX1241.pdf
> > + */
> > +
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/module.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#define MAX1241_VAL_MASK 0xFFF
> > +#define MAX1241_SHDN_DELAY_USEC 4
> > +
> > +enum max1241_id {
> > +	max1241,
> > +};
> > +
> > +struct max1241 {
> > +	struct spi_device *spi;
> > +	struct mutex lock;
> > +	struct regulator *reg;
> > +	struct gpio_desc *shdn;
> > +
> > +	__be16 data ____cacheline_aligned;
> > +};
> > +
> > +static const struct iio_chan_spec max1241_channels[] = {
> > +	{
> > +		.type = IIO_VOLTAGE,
> > +		.indexed = 1,
> > +		.channel = 0,
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +				BIT(IIO_CHAN_INFO_SCALE),
> > +		.scan_index = 0,
> > +		.scan_type = {
> > +			.sign = 'u',
> > +			.realbits = 12,
> > +			.storagebits = 16,
> > +		},
> > +	},
> > +};
> > +
> > +static int max1241_read(struct max1241 *adc)
> > +{
> > +	struct spi_transfer xfers[] = {
> > +		/*
> > +		 * Begin conversion by bringing /CS low for at least
> > +		 * tconv us.
> > +		 */
> > +		{
> > +			.len = 0,
> > +			.delay.value = 8,
> > +			.delay.unit = SPI_DELAY_UNIT_USECS,
> > +		},
> > +		/*
> > +		 * Then read two bytes of data in our RX buffer.
> > +		 */
> > +		{
> > +			.rx_buf = &adc->data,
> > +			.len = 2,
> > +		},
> > +	};
> > +
> > +	return spi_sync_transfer(adc->spi, xfers, ARRAY_SIZE(xfers));
> > +}
> > +
> > +static int max1241_read_raw(struct iio_dev *indio_dev,
> > +			struct iio_chan_spec const *chan,
> > +			int *val, int *val2, long mask)
> > +{
> > +	int ret, vref_uV;
> > +	struct max1241 *adc = iio_priv(indio_dev);
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		mutex_lock(&adc->lock);
> > +
> > +		if (adc->shdn) {
> > +			gpiod_set_value(adc->shdn, 0);
> > +			udelay(MAX1241_SHDN_DELAY_USEC);
> > +		}
> > +
> > +		ret = max1241_read(adc);
> > +
> > +		if (adc->shdn)
> > +			gpiod_set_value(adc->shdn, 1);
> > +
> > +		if (ret) {
> > +			mutex_unlock(&adc->lock);
> > +			return ret;
> > +		}
> > +
> > +		*val = (be16_to_cpu(adc->data) >> 3) & MAX1241_VAL_MASK;
> > +
> > +		mutex_unlock(&adc->lock);
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_SCALE:
> > +		vref_uV = regulator_get_voltage(adc->reg);
> > +
> > +		if (vref_uV < 0)
> > +			return vref_uV;
> > +
> > +		*val = vref_uV / 1000;
> > +		*val2 = 12;
> > +
> > +		return IIO_VAL_FRACTIONAL_LOG2;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static const struct iio_info max1241_info = {
> > +	.read_raw = max1241_read_raw,
> > +};
> > +
> > +static void max1241_disable_reg_action(void *data)
> > +{
> > +	struct max1241 *adc = data;
> > +	int err;
> > +
> > +	err = regulator_disable(adc->reg);
> > +	if (err)
> > +		dev_err(&adc->spi->dev, "could not disable reference
> > regulator.\n");
> > +}
> > +
> > +static int max1241_probe(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct max1241 *adc;
> > +	int ret = 0;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	adc = iio_priv(indio_dev);
> > +	adc->spi = spi;
> > +	mutex_init(&adc->lock);
> > +
> > +	spi_set_drvdata(spi, indio_dev);
> > +
> > +	adc->reg = devm_regulator_get(&spi->dev, "vref");
> > +	if (IS_ERR(adc->reg)) {
> > +		dev_err(&spi->dev, "failed to get vref regulator\n");
> > +		return PTR_ERR(adc->reg);
> > +	}
> > +
> > +	ret = regulator_enable(adc->reg);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_add_action_or_reset(&spi->dev, max1241_disable_reg_action,
> > +					adc);
> > +	if (ret) {
> > +		dev_err(&spi->dev, "could not set up regulator cleanup
> > action!\n");
> > +		return ret;
> > +	}
> > +
> > +	adc->shdn = devm_gpiod_get_optional(&spi->dev, "shdn", GPIOD_OUT_HIGH);
> > +
> > +	if (IS_ERR(adc->shdn))
> > +		return PTR_ERR(adc->shdn);
> > +
> > +	if (!adc->shdn)
> > +		dev_dbg(&spi->dev, "no shdn pin passed, low-power mode
> > disabled");
> > +	else
> > +		dev_dbg(&spi->dev, "shdn pin passed, low-power mode enabled");
> > +
> > +	indio_dev->name = spi_get_device_id(spi)->name;
> > +	indio_dev->dev.parent = &spi->dev;
> > +	indio_dev->info = &max1241_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->channels = max1241_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(max1241_channels);
> > +
> > +	return devm_iio_device_register(&spi->dev, indio_dev);
> > +}
> > +
> > +static const struct spi_device_id max1241_id[] = {
> > +	{ "max1241", max1241 },
> > +	{},
> > +};
> > +
> > +static const struct of_device_id max1241_dt_ids[] = {
> > +	{ .compatible = "maxim,max1241" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, max1241_dt_ids);
> > +
> > +static struct spi_driver max1241_spi_driver = {
> > +	.driver = {
> > +		.name = "max1241",
> > +		.of_match_table = max1241_dt_ids,
> > +	},
> > +	.probe = max1241_probe,
> > +	.id_table = max1241_id,
> > +};
> > +module_spi_driver(max1241_spi_driver);
> > +
> > +MODULE_AUTHOR("Alexandru Lazar <alazar@startmail.com>");
> > +MODULE_DESCRIPTION("MAX1241 ADC driver");
> > +MODULE_LICENSE("GPL v2");

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation
  2020-03-20  7:12   ` Ardelean, Alexandru
@ 2020-03-20  8:33     ` Alexandru Lazar
  2020-03-20  8:45       ` Ardelean, Alexandru
  0 siblings, 1 reply; 9+ messages in thread
From: Alexandru Lazar @ 2020-03-20  8:33 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: linux-iio, jic23, mark.rutland, knaack.h, lars, pmeerw, robh+dt

> > +  reg:
> > +    description: SPI chip select number for this device
> 
> Rob will probably complain that you don't need to document[add description] to
> standard attributes [like SPI's 'reg' parameter]
> Just listing it here, should be sufficient. So, you can drop the
> description.

Makes sense -- should've thought of it, considering I dropped the doc
for spi-frequency as well. My brain is still wired to the non-standard
txt docs it seems, sorry!

I'm going to make a v4 for this (I think I need to reorder the patch
series anyway -- the dt-bindings should come first) and Cc: the
dt-bindings list with as well.

Thanks again for all your help!

Best regards,
Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation
  2020-03-20  8:33     ` Alexandru Lazar
@ 2020-03-20  8:45       ` Ardelean, Alexandru
  0 siblings, 0 replies; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-03-20  8:45 UTC (permalink / raw)
  To: alazar; +Cc: pmeerw, robh+dt, jic23, mark.rutland, linux-iio, lars, knaack.h

On Fri, 2020-03-20 at 10:33 +0200, Alexandru Lazar wrote:
> [External]
> 
> > > +  reg:
> > > +    description: SPI chip select number for this device
> > 
> > Rob will probably complain that you don't need to document[add description]
> > to
> > standard attributes [like SPI's 'reg' parameter]
> > Just listing it here, should be sufficient. So, you can drop the
> > description.
> 
> Makes sense -- should've thought of it, considering I dropped the doc
> for spi-frequency as well. My brain is still wired to the non-standard
> txt docs it seems, sorry!
> 

I think many people aren't yet super-comfortable with the new yaml format, which
probably makes Rob's life a bit difficult.

I guess it should become easier once version 5.4+ becomes more popular.

> I'm going to make a v4 for this (I think I need to reorder the patch
> series anyway -- the dt-bindings should come first) and Cc: the
> dt-bindings list with as well.

The order of the bindings vs code is a bit relaxed [atm]. But may become more
strict [later at some point]. I think the official version is bindings-first &
driver-next, but it's still accepted to do it the other way around.

> 
> Thanks again for all your help!
> 
> Best regards,
> Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-03-20  8:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 16:29 [PATCH v3 0/2] Maxim MAX1241 driver Alexandru Lazar
2020-03-19 16:29 ` [PATCH v3 1/2] iio: adc: Add " Alexandru Lazar
2020-03-20  7:06   ` Ardelean, Alexandru
2020-03-20  7:13     ` Ardelean, Alexandru
2020-03-19 16:29 ` [PATCH v3 2/2] dt-bindings: iio: adc: Add MAX1241 device tree bindings in documentation Alexandru Lazar
2020-03-20  7:12   ` Ardelean, Alexandru
2020-03-20  8:33     ` Alexandru Lazar
2020-03-20  8:45       ` Ardelean, Alexandru
2020-03-20  7:02 ` [PATCH v3 0/2] Maxim MAX1241 driver Ardelean, Alexandru

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.