[PATCH v2 1/2] bindings: iio: adc: Add max11205 documentation file

[PATCH v2 1/2] bindings: iio: adc: Add max11205 documentation file

[Ramona Bolboaca]

Add bindings documentation file and MAINTAINERS entry for MAX11205.

Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>
---
changes in v2:
 - reference /schemas/spi/spi-peripheral-props.yaml in allOf 
 - use unevaluatedProperties:false and remove additionalProperties:false
 - use generic node name and fix indentation in device-tree example
 - wrap lines nearer to 75-80 chars in documentation
 - add extra information to the description of the regulator
 .../bindings/iio/adc/maxim,max11205.yaml      | 69 +++++++++++++++++++
 MAINTAINERS                                   |  8 +++
 2 files changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
new file mode 100644
index 000000000000..7902f82da927
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/maxim,max11205.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX11205 ADC
+
+maintainers:
+  - Ramona Bolboaca <ramona.bolboaca@analog.com>
+
+description: |
+  The MAX11205 is an ultra-low-power (< 300FA max active current),
+  high-resolution, serial-output ADC.
+
+  https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - maxim,max11205a
+      - maxim,max11205b
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 5000000
+
+  spi-cpha: true
+
+  vref-supply:
+    description:
+      The regulator supply for the ADC reference voltage. This is a differential
+      reference. It is equal to the V_REFP - V_REFN. The maximum value is 3.6V.
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - spi-cpha
+  - interrupts
+  - vref-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        adc@0 {
+            compatible = "maxim,max11205a";
+            reg = <0>;
+            spi-max-frequency = <5000000>;
+            spi-cpha;
+            interrupt-parent = <&gpio>;
+            interrupts = <19 IRQ_TYPE_EDGE_FALLING>;
+            vref-supply = <&max11205_vref>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 96f47a7865d6..db1b5dc03988 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12341,6 +12341,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/proximity/maxbotix,mb1232.yaml
 F:	drivers/iio/proximity/mb1232.c
 
+MAXIM MAX11205 DRIVER
+M:	Ramona Bolboaca <ramona.bolboaca@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
+F:	drivers/iio/adc/max11205.c
+
 MAXIM MAX17040 FAMILY FUEL GAUGE DRIVERS
 R:	Iskren Chernev <iskren.chernev@gmail.com>
 R:	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
-- 
2.25.1

[PATCH v2 2/2] iio: adc: add max11205 adc driver

[Ramona Bolboaca]

Adding support for max11205 16-bit single-channel ultra-low power
delta-sigma adc.
The MAX11205 is compatible with the 2-wire interface and uses
SCLK and RDY/DOUT for serial communications. In this mode, all
controls are implemented by timing the high or low phase of the SCLK.
The 2-wire serial interface only allows for data to be read out through
the RDY/DOUT output.

Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>
---
changes in v2:
 - add chip_info null pointer check
 - add support for probing with ACPI table
 - remove function for module removal
 - remove irq flag from max11205_sigma_delta_info
 - add missing commas and missing spaces
 - remove redundant blank line
 - wrap text to 75-80 chars
 - removed typos in commit message
 drivers/iio/adc/Kconfig    |  14 +++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11205.c | 183 +++++++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/iio/adc/max11205.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7fe5930891e0..0c8f376b65d8 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -653,6 +653,20 @@ config MAX1118
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1118.
 
+config MAX11205
+	tristate "Maxim max11205 ADC driver"
+	depends on SPI
+	select AD_SIGMA_DELTA
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+
+	help
+	  Say yes here to build support for Maxim max11205 16-bit, single-channel
+	  ultra-low power delta-sigma ADC.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max11205.
+
 config MAX1241
 	tristate "Maxim max1241 ADC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 1772a549a3c8..bb681844e497 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1118) += max1118.o
+obj-$(CONFIG_MAX11205) += max11205.o
 obj-$(CONFIG_MAX1241) += max1241.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MAX9611) += max9611.o
diff --git a/drivers/iio/adc/max11205.c b/drivers/iio/adc/max11205.c
new file mode 100644
index 000000000000..68e6082e70e5
--- /dev/null
+++ b/drivers/iio/adc/max11205.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * max11205 16-Bit Delta-Sigma ADC
+ *
+ * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1240-max11205.pdf
+ * Copyright (C) 2022 Analog Devices, Inc.
+ * Author: Ramona Bolboaca <ramona.bolboaca@analog.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/adc/ad_sigma_delta.h>
+
+#define MAX11205_BIT_SCALE	15
+#define MAX11205A_OUT_DATA_RATE	116
+#define MAX11205B_OUT_DATA_RATE	13
+
+enum chip_type {
+	TYPE_MAX11205A,
+	TYPE_MAX11205B,
+};
+
+struct chip_info {
+	unsigned int	out_data_rate;
+	const char	*name;
+};
+
+struct max11205_state {
+	const struct chip_info	*chip_info;
+	struct regulator	*vref;
+	struct ad_sigma_delta	sd;
+};
+
+static const struct ad_sigma_delta_info max11205_sigma_delta_info = {
+	.has_registers = false,
+};
+
+static int max11205_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct max11205_state *st = iio_priv(indio_dev);
+	int reg_mv;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return ad_sigma_delta_single_conversion(indio_dev, chan, val);
+	case IIO_CHAN_INFO_SCALE:
+		reg_mv = regulator_get_voltage(st->vref);
+		if (reg_mv < 0)
+			return reg_mv;
+		reg_mv /= 1000;
+		*val = reg_mv;
+		*val2 = MAX11205_BIT_SCALE;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = st->chip_info->out_data_rate;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info max11205_iio_info = {
+	.read_raw = max11205_read_raw,
+	.validate_trigger = ad_sd_validate_trigger,
+};
+
+static const struct iio_chan_spec max11205_channels[] = {
+	{
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_BE
+		},
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+		BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static const struct chip_info max11205_chip_info[] = {
+	[TYPE_MAX11205A] = {
+		.out_data_rate = MAX11205A_OUT_DATA_RATE,
+		.name = "max11205a",
+	},
+	[TYPE_MAX11205B] = {
+		.out_data_rate = MAX11205B_OUT_DATA_RATE,
+		.name = "max11205b",
+	},
+};
+
+static void max11205_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
+static int max11205_probe(struct spi_device *spi)
+{
+	struct max11205_state *st;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	ad_sd_init(&st->sd, indio_dev, spi, &max11205_sigma_delta_info);
+
+	st->chip_info = device_get_match_data(&spi->dev);
+
+	if (!st->chip_info)
+		st->chip_info = (const struct chip_info *)spi_get_device_id(spi)->driver_data;
+
+	indio_dev->name = st->chip_info->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = max11205_channels;
+	indio_dev->num_channels = 1;
+	indio_dev->info = &max11205_iio_info;
+
+	st->vref = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(st->vref))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->vref),
+				     "Failed to get vref regulator\n");
+
+	ret = regulator_enable(st->vref);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&spi->dev, max11205_reg_disable, st->vref);
+	if (ret)
+		return ret;
+
+	ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id max11205_spi_ids[] = {
+	{ "max11205a", (kernel_ulong_t)&max11205_chip_info[TYPE_MAX11205A] },
+	{ "max11205b", (kernel_ulong_t)&max11205_chip_info[TYPE_MAX11205B] },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, max11205_spi_ids);
+
+static const struct of_device_id max11205_dt_ids[] = {
+	{
+		.compatible = "maxim,max11205a",
+		.data = &max11205_chip_info[TYPE_MAX11205A],
+	},
+	{
+		.compatible = "maxim,max11205b",
+		.data = &max11205_chip_info[TYPE_MAX11205B],
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max11205_dt_ids);
+
+static struct spi_driver max11205_spi_driver = {
+	.driver = {
+		.name = "max11205",
+		.of_match_table = max11205_dt_ids,
+	},
+	.probe = max11205_probe,
+	.id_table = max11205_spi_ids,
+};
+module_spi_driver(max11205_spi_driver);
+
+MODULE_AUTHOR("Ramona Bolboaca <ramona.bolboaca@analog.com>");
+MODULE_DESCRIPTION("MAX11205 ADC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
-- 
2.25.1

Re: [PATCH v2 1/2] bindings: iio: adc: Add max11205 documentation file

[Krzysztof Kozlowski]

On 31/08/2022 16:30, Ramona Bolboaca wrote:
> Add bindings documentation file and MAINTAINERS entry for MAX11205.
> 
> Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>


> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max11205a
> +      - maxim,max11205b
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 5000000
> +
> +  spi-cpha: true
> +
> +  vref-supply:
> +    description:
> +      The regulator supply for the ADC reference voltage. This is a differential
> +      reference. It is equal to the V_REFP - V_REFN. The maximum value is 3.6V.
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - spi-cpha
> +  - interrupts
> +  - vref-supply

If you are going to send a new version, please put the properties here
in the same order as they appear in top-level "properties:".

In any case:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/2] iio: adc: add max11205 adc driver

[Andy Shevchenko]

On Wed, Aug 31, 2022 at 4:36 PM Ramona Bolboaca
<ramona.bolboaca@analog.com> wrote:
>
> Adding support for max11205 16-bit single-channel ultra-low power
> delta-sigma adc.
> The MAX11205 is compatible with the 2-wire interface and uses
> SCLK and RDY/DOUT for serial communications. In this mode, all
> controls are implemented by timing the high or low phase of the SCLK.
> The 2-wire serial interface only allows for data to be read out through
> the RDY/DOUT output.

Couple of minor comments, if you are going to address them,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
> Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>
> ---
> changes in v2:
>  - add chip_info null pointer check
>  - add support for probing with ACPI table
>  - remove function for module removal
>  - remove irq flag from max11205_sigma_delta_info
>  - add missing commas and missing spaces
>  - remove redundant blank line
>  - wrap text to 75-80 chars
>  - removed typos in commit message
>  drivers/iio/adc/Kconfig    |  14 +++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11205.c | 183 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 198 insertions(+)
>  create mode 100644 drivers/iio/adc/max11205.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 7fe5930891e0..0c8f376b65d8 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -653,6 +653,20 @@ config MAX1118
>           To compile this driver as a module, choose M here: the module will be
>           called max1118.
>
> +config MAX11205
> +       tristate "Maxim max11205 ADC driver"
> +       depends on SPI
> +       select AD_SIGMA_DELTA
> +       select IIO_BUFFER
> +       select IIO_TRIGGERED_BUFFER
> +
> +       help
> +         Say yes here to build support for Maxim max11205 16-bit, single-channel
> +         ultra-low power delta-sigma ADC.
> +
> +         To compile this driver as a module, choose M here: the module will be
> +         called max11205.
> +
>  config MAX1241
>         tristate "Maxim max1241 ADC driver"
>         depends on SPI_MASTER
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 1772a549a3c8..bb681844e497 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1118) += max1118.o
> +obj-$(CONFIG_MAX11205) += max11205.o
>  obj-$(CONFIG_MAX1241) += max1241.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MAX9611) += max9611.o
> diff --git a/drivers/iio/adc/max11205.c b/drivers/iio/adc/max11205.c
> new file mode 100644
> index 000000000000..68e6082e70e5
> --- /dev/null
> +++ b/drivers/iio/adc/max11205.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * max11205 16-Bit Delta-Sigma ADC

Can you spell the vendor and part number verbosely?
Something like "Maxim MAX11205" or similar.

> + *
> + * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1240-max11205.pdf
> + * Copyright (C) 2022 Analog Devices, Inc.
> + * Author: Ramona Bolboaca <ramona.bolboaca@analog.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/adc/ad_sigma_delta.h>
> +
> +#define MAX11205_BIT_SCALE     15
> +#define MAX11205A_OUT_DATA_RATE        116
> +#define MAX11205B_OUT_DATA_RATE        13
> +
> +enum chip_type {
> +       TYPE_MAX11205A,
> +       TYPE_MAX11205B,
> +};
> +
> +struct chip_info {
> +       unsigned int    out_data_rate;
> +       const char      *name;
> +};
> +
> +struct max11205_state {
> +       const struct chip_info  *chip_info;
> +       struct regulator        *vref;
> +       struct ad_sigma_delta   sd;
> +};
> +
> +static const struct ad_sigma_delta_info max11205_sigma_delta_info = {
> +       .has_registers = false,
> +};
> +
> +static int max11205_read_raw(struct iio_dev *indio_dev,
> +                            struct iio_chan_spec const *chan,
> +                            int *val, int *val2, long mask)
> +{
> +       struct max11205_state *st = iio_priv(indio_dev);
> +       int reg_mv;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               return ad_sigma_delta_single_conversion(indio_dev, chan, val);
> +       case IIO_CHAN_INFO_SCALE:
> +               reg_mv = regulator_get_voltage(st->vref);
> +               if (reg_mv < 0)
> +                       return reg_mv;
> +               reg_mv /= 1000;
> +               *val = reg_mv;
> +               *val2 = MAX11205_BIT_SCALE;
> +               return IIO_VAL_FRACTIONAL_LOG2;
> +       case IIO_CHAN_INFO_SAMP_FREQ:
> +               *val = st->chip_info->out_data_rate;
> +               return IIO_VAL_INT;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static const struct iio_info max11205_iio_info = {
> +       .read_raw = max11205_read_raw,
> +       .validate_trigger = ad_sd_validate_trigger,
> +};
> +
> +static const struct iio_chan_spec max11205_channels[] = {
> +       {
> +               .type = IIO_VOLTAGE,
> +               .indexed = 1,
> +               .scan_type = {
> +                       .sign = 's',
> +                       .realbits = 16,
> +                       .storagebits = 16,
> +                       .endianness = IIO_BE
> +               },
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +               BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +               BIT(IIO_CHAN_INFO_SCALE),
> +       },
> +};
> +
> +static const struct chip_info max11205_chip_info[] = {
> +       [TYPE_MAX11205A] = {
> +               .out_data_rate = MAX11205A_OUT_DATA_RATE,
> +               .name = "max11205a",
> +       },
> +       [TYPE_MAX11205B] = {
> +               .out_data_rate = MAX11205B_OUT_DATA_RATE,
> +               .name = "max11205b",
> +       },
> +};
> +
> +static void max11205_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
> +static int max11205_probe(struct spi_device *spi)
> +{
> +       struct max11205_state *st;
> +       struct iio_dev *indio_dev;
> +       int ret;
> +
> +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       st = iio_priv(indio_dev);
> +
> +       ad_sd_init(&st->sd, indio_dev, spi, &max11205_sigma_delta_info);

> +       st->chip_info = device_get_match_data(&spi->dev);

> +

Unneeded blank line.

> +       if (!st->chip_info)
> +               st->chip_info = (const struct chip_info *)spi_get_device_id(spi)->driver_data;
> +
> +       indio_dev->name = st->chip_info->name;
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +       indio_dev->channels = max11205_channels;
> +       indio_dev->num_channels = 1;
> +       indio_dev->info = &max11205_iio_info;
> +
> +       st->vref = devm_regulator_get(&spi->dev, "vref");

devm_regulator_get_enable() ?

> +       if (IS_ERR(st->vref))
> +               return dev_err_probe(&spi->dev, PTR_ERR(st->vref),
> +                                    "Failed to get vref regulator\n");
> +
> +       ret = regulator_enable(st->vref);
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_add_action_or_reset(&spi->dev, max11205_reg_disable, st->vref);
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
> +       if (ret)
> +               return ret;
> +
> +       return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id max11205_spi_ids[] = {
> +       { "max11205a", (kernel_ulong_t)&max11205_chip_info[TYPE_MAX11205A] },
> +       { "max11205b", (kernel_ulong_t)&max11205_chip_info[TYPE_MAX11205B] },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(spi, max11205_spi_ids);
> +
> +static const struct of_device_id max11205_dt_ids[] = {
> +       {
> +               .compatible = "maxim,max11205a",
> +               .data = &max11205_chip_info[TYPE_MAX11205A],
> +       },
> +       {
> +               .compatible = "maxim,max11205b",
> +               .data = &max11205_chip_info[TYPE_MAX11205B],
> +       },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, max11205_dt_ids);
> +
> +static struct spi_driver max11205_spi_driver = {
> +       .driver = {
> +               .name = "max11205",
> +               .of_match_table = max11205_dt_ids,
> +       },
> +       .probe = max11205_probe,
> +       .id_table = max11205_spi_ids,
> +};
> +module_spi_driver(max11205_spi_driver);
> +
> +MODULE_AUTHOR("Ramona Bolboaca <ramona.bolboaca@analog.com>");
> +MODULE_DESCRIPTION("MAX11205 ADC driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
> --
> 2.25.1
>


-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2 2/2] iio: adc: add max11205 adc driver

[Bolboaca, Ramona]

> On Wed, Aug 31, 2022 at 4:36 PM Ramona Bolboaca
> <ramona.bolboaca@analog.com> wrote:
> >
> > Adding support for max11205 16-bit single-channel ultra-low power
> > delta-sigma adc.
> > The MAX11205 is compatible with the 2-wire interface and uses
> > SCLK and RDY/DOUT for serial communications. In this mode, all
> > controls are implemented by timing the high or low phase of the SCLK.
> > The 2-wire serial interface only allows for data to be read out through
> > the RDY/DOUT output.
> 
> Couple of minor comments, if you are going to address them,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> > Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
> > Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>
> > ---
> > changes in v2:
> >  - add chip_info null pointer check
> >  - add support for probing with ACPI table
> >  - remove function for module removal
> >  - remove irq flag from max11205_sigma_delta_info
> >  - add missing commas and missing spaces
> >  - remove redundant blank line
> >  - wrap text to 75-80 chars
> >  - removed typos in commit message
> >  drivers/iio/adc/Kconfig    |  14 +++
> >  drivers/iio/adc/Makefile   |   1 +
> >  drivers/iio/adc/max11205.c | 183 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 198 insertions(+)
> >  create mode 100644 drivers/iio/adc/max11205.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 7fe5930891e0..0c8f376b65d8 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -653,6 +653,20 @@ config MAX1118
> >           To compile this driver as a module, choose M here: the module will be
> >           called max1118.
> >
> > +config MAX11205
> > +       tristate "Maxim max11205 ADC driver"
> > +       depends on SPI
> > +       select AD_SIGMA_DELTA
> > +       select IIO_BUFFER
> > +       select IIO_TRIGGERED_BUFFER
> > +
> > +       help
> > +         Say yes here to build support for Maxim max11205 16-bit, single-channel
> > +         ultra-low power delta-sigma ADC.
> > +
> > +         To compile this driver as a module, choose M here: the module will be
> > +         called max11205.
> > +
> >  config MAX1241
> >         tristate "Maxim max1241 ADC driver"
> >         depends on SPI_MASTER
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 1772a549a3c8..bb681844e497 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -61,6 +61,7 @@ obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
> >  obj-$(CONFIG_MAX1027) += max1027.o
> >  obj-$(CONFIG_MAX11100) += max11100.o
> >  obj-$(CONFIG_MAX1118) += max1118.o
> > +obj-$(CONFIG_MAX11205) += max11205.o
> >  obj-$(CONFIG_MAX1241) += max1241.o
> >  obj-$(CONFIG_MAX1363) += max1363.o
> >  obj-$(CONFIG_MAX9611) += max9611.o
> > diff --git a/drivers/iio/adc/max11205.c b/drivers/iio/adc/max11205.c
> > new file mode 100644
> > index 000000000000..68e6082e70e5
> > --- /dev/null
> > +++ b/drivers/iio/adc/max11205.c
> > @@ -0,0 +1,183 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * max11205 16-Bit Delta-Sigma ADC
> 
> Can you spell the vendor and part number verbosely?
> Something like "Maxim MAX11205" or similar.
> 
> > + *
> > + * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1240-max11205.pdf
> > + * Copyright (C) 2022 Analog Devices, Inc.
> > + * Author: Ramona Bolboaca <ramona.bolboaca@analog.com>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/adc/ad_sigma_delta.h>
> > +
> > +#define MAX11205_BIT_SCALE     15
> > +#define MAX11205A_OUT_DATA_RATE        116
> > +#define MAX11205B_OUT_DATA_RATE        13
> > +
> > +enum chip_type {
> > +       TYPE_MAX11205A,
> > +       TYPE_MAX11205B,
> > +};
> > +
> > +struct chip_info {
> > +       unsigned int    out_data_rate;
> > +       const char      *name;
> > +};
> > +
> > +struct max11205_state {
> > +       const struct chip_info  *chip_info;
> > +       struct regulator        *vref;
> > +       struct ad_sigma_delta   sd;
> > +};
> > +
> > +static const struct ad_sigma_delta_info max11205_sigma_delta_info = {
> > +       .has_registers = false,
> > +};
> > +
> > +static int max11205_read_raw(struct iio_dev *indio_dev,
> > +                            struct iio_chan_spec const *chan,
> > +                            int *val, int *val2, long mask)
> > +{
> > +       struct max11205_state *st = iio_priv(indio_dev);
> > +       int reg_mv;
> > +
> > +       switch (mask) {
> > +       case IIO_CHAN_INFO_RAW:
> > +               return ad_sigma_delta_single_conversion(indio_dev, chan, val);
> > +       case IIO_CHAN_INFO_SCALE:
> > +               reg_mv = regulator_get_voltage(st->vref);
> > +               if (reg_mv < 0)
> > +                       return reg_mv;
> > +               reg_mv /= 1000;
> > +               *val = reg_mv;
> > +               *val2 = MAX11205_BIT_SCALE;
> > +               return IIO_VAL_FRACTIONAL_LOG2;
> > +       case IIO_CHAN_INFO_SAMP_FREQ:
> > +               *val = st->chip_info->out_data_rate;
> > +               return IIO_VAL_INT;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > +
> > +static const struct iio_info max11205_iio_info = {
> > +       .read_raw = max11205_read_raw,
> > +       .validate_trigger = ad_sd_validate_trigger,
> > +};
> > +
> > +static const struct iio_chan_spec max11205_channels[] = {
> > +       {
> > +               .type = IIO_VOLTAGE,
> > +               .indexed = 1,
> > +               .scan_type = {
> > +                       .sign = 's',
> > +                       .realbits = 16,
> > +                       .storagebits = 16,
> > +                       .endianness = IIO_BE
> > +               },
> > +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +               BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > +               BIT(IIO_CHAN_INFO_SCALE),
> > +       },
> > +};
> > +
> > +static const struct chip_info max11205_chip_info[] = {
> > +       [TYPE_MAX11205A] = {
> > +               .out_data_rate = MAX11205A_OUT_DATA_RATE,
> > +               .name = "max11205a",
> > +       },
> > +       [TYPE_MAX11205B] = {
> > +               .out_data_rate = MAX11205B_OUT_DATA_RATE,
> > +               .name = "max11205b",
> > +       },
> > +};
> > +
> > +static void max11205_reg_disable(void *reg)
> > +{
> > +       regulator_disable(reg);
> > +}
> > +
> > +static int max11205_probe(struct spi_device *spi)
> > +{
> > +       struct max11205_state *st;
> > +       struct iio_dev *indio_dev;
> > +       int ret;
> > +
> > +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +       if (!indio_dev)
> > +               return -ENOMEM;
> > +
> > +       st = iio_priv(indio_dev);
> > +
> > +       ad_sd_init(&st->sd, indio_dev, spi, &max11205_sigma_delta_info);
> 
> > +       st->chip_info = device_get_match_data(&spi->dev);
> 
> > +
> 
> Unneeded blank line.
> 
> > +       if (!st->chip_info)
> > +               st->chip_info = (const struct chip_info *)spi_get_device_id(spi)->driver_data;
> > +
> > +       indio_dev->name = st->chip_info->name;
> > +       indio_dev->modes = INDIO_DIRECT_MODE;
> > +       indio_dev->channels = max11205_channels;
> > +       indio_dev->num_channels = 1;
> > +       indio_dev->info = &max11205_iio_info;
> > +
> > +       st->vref = devm_regulator_get(&spi->dev, "vref");
> 
> devm_regulator_get_enable() ?
I found the patches which implement devm_regulator_get_enable. However, I need to get the voltage of the regulator using regulator_get_voltage(struct regulator *regulator) and if I use devm_regulator_get_enable I do not have access to the regulator pointer. What should I do in this case? Is there an API which works like devm_regulator_get_enable but also gives access to the regulator's pointer?
Thank you for you review!
Kind Regards,
Ramona Bolboaca
> 
> > +       if (IS_ERR(st->vref))
> > +               return dev_err_probe(&spi->dev, PTR_ERR(st->vref),
> > +                                    "Failed to get vref regulator\n");
> > +
> > +       ret = regulator_enable(st->vref);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = devm_add_action_or_reset(&spi->dev, max11205_reg_disable, st->vref);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
> > +       if (ret)
> > +               return ret;
> > +
> > +       return devm_iio_device_register(&spi->dev, indio_dev);
> > +}
> > +
> > +static const struct spi_device_id max11205_spi_ids[] = {
> > +       { "max11205a", (kernel_ulong_t)&max11205_chip_info[TYPE_MAX11205A] },
> > +       { "max11205b", (kernel_ulong_t)&max11205_chip_info[TYPE_MAX11205B] },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(spi, max11205_spi_ids);
> > +
> > +static const struct of_device_id max11205_dt_ids[] = {
> > +       {
> > +               .compatible = "maxim,max11205a",
> > +               .data = &max11205_chip_info[TYPE_MAX11205A],
> > +       },
> > +       {
> > +               .compatible = "maxim,max11205b",
> > +               .data = &max11205_chip_info[TYPE_MAX11205B],
> > +       },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(of, max11205_dt_ids);
> > +
> > +static struct spi_driver max11205_spi_driver = {
> > +       .driver = {
> > +               .name = "max11205",
> > +               .of_match_table = max11205_dt_ids,
> > +       },
> > +       .probe = max11205_probe,
> > +       .id_table = max11205_spi_ids,
> > +};
> > +module_spi_driver(max11205_spi_driver);
> > +
> > +MODULE_AUTHOR("Ramona Bolboaca <ramona.bolboaca@analog.com>");
> > +MODULE_DESCRIPTION("MAX11205 ADC driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
> > --
> > 2.25.1
> >
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2 2/2] iio: adc: add max11205 adc driver

[Jonathan Cameron]

> >   
> > > +       if (!st->chip_info)
> > > +               st->chip_info = (const struct chip_info *)spi_get_device_id(spi)->driver_data;
> > > +
> > > +       indio_dev->name = st->chip_info->name;
> > > +       indio_dev->modes = INDIO_DIRECT_MODE;
> > > +       indio_dev->channels = max11205_channels;
> > > +       indio_dev->num_channels = 1;
> > > +       indio_dev->info = &max11205_iio_info;
> > > +
> > > +       st->vref = devm_regulator_get(&spi->dev, "vref");  
> > 

> > devm_regulator_get_enable() ?  

> I found the patches which implement devm_regulator_get_enable.
> However, I need to get the voltage of the regulator using
> regulator_get_voltage(struct regulator *regulator) and if I use
> devm_regulator_get_enable I do not have access to the regulator
> pointer. What should I do in this case? Is there an API which works
> like devm_regulator_get_enable but also gives access to the
> regulator's pointer? Thank you for you review!

Ah.  You've run into a fun long running 'discussion'.  There was
great resistance to adding devm_regulator_get_enable() because it
was felt that it was too easy to get the handling wrong and end up
with underflowing reference counters etc.  So the 'solution' was
to make it less useful than it would otherwise have been by making
sure it could not be combined with other accesses to the regulator.

Upshot is you are correct that it cannot be applied in this case.

Also relevant is that the patch in question is going through the
regulator tree so won't be generally available to the rest of the
kernel until next cycle.  As such we'd have had to make such a
change as a follow up patch if it were possible.

Jonathan

p.s. Wrap your replies at 80 chars as well as the code.
Kernel developers have an annoying habit of reading their email in
one small window even when they have very large monitors :)
Also, where sensible crop unnecessary parts of the email to only
include the bit you are responding to. Saves on lots of scrolling..

> Kind Regards, Ramona
> Bolboaca

Re: [PATCH v2 1/2] bindings: iio: adc: Add max11205 documentation file

[Jonathan Cameron]

On Wed, 31 Aug 2022 16:30:20 +0300
Ramona Bolboaca <ramona.bolboaca@analog.com> wrote:

> Add bindings documentation file and MAINTAINERS entry for MAX11205.

Binding patch titles tend to be
dt-bindings: iio:

etc.
I'll fix whilst applying.

> 
> Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>
> ---
> changes in v2:
>  - reference /schemas/spi/spi-peripheral-props.yaml in allOf 
>  - use unevaluatedProperties:false and remove additionalProperties:false
>  - use generic node name and fix indentation in device-tree example
>  - wrap lines nearer to 75-80 chars in documentation
>  - add extra information to the description of the regulator
>  .../bindings/iio/adc/maxim,max11205.yaml      | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  8 +++
>  2 files changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
> new file mode 100644
> index 000000000000..7902f82da927
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/maxim,max11205.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX11205 ADC
> +
> +maintainers:
> +  - Ramona Bolboaca <ramona.bolboaca@analog.com>
> +
> +description: |
> +  The MAX11205 is an ultra-low-power (< 300FA max active current),
> +  high-resolution, serial-output ADC.
> +
> +  https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max11205a
> +      - maxim,max11205b
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 5000000
> +
> +  spi-cpha: true
> +
> +  vref-supply:
> +    description:
> +      The regulator supply for the ADC reference voltage. This is a differential
> +      reference. It is equal to the V_REFP - V_REFN. The maximum value is 3.6V.
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - spi-cpha
> +  - interrupts
> +  - vref-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        adc@0 {
> +            compatible = "maxim,max11205a";
> +            reg = <0>;
> +            spi-max-frequency = <5000000>;
> +            spi-cpha;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <19 IRQ_TYPE_EDGE_FALLING>;
> +            vref-supply = <&max11205_vref>;
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 96f47a7865d6..db1b5dc03988 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12341,6 +12341,14 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/proximity/maxbotix,mb1232.yaml
>  F:	drivers/iio/proximity/mb1232.c
>  
> +MAXIM MAX11205 DRIVER
> +M:	Ramona Bolboaca <ramona.bolboaca@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/adc/maxim,max11205.yaml
> +F:	drivers/iio/adc/max11205.c
> +
>  MAXIM MAX17040 FAMILY FUEL GAUGE DRIVERS
>  R:	Iskren Chernev <iskren.chernev@gmail.com>
>  R:	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Re: [PATCH v2 1/2] bindings: iio: adc: Add max11205 documentation file

[Jonathan Cameron]

On Wed, 31 Aug 2022 16:36:25 +0300
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 31/08/2022 16:30, Ramona Bolboaca wrote:
> > Add bindings documentation file and MAINTAINERS entry for MAX11205.
> > 
> > Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>  
> 
> 
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - maxim,max11205a
> > +      - maxim,max11205b
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 5000000
> > +
> > +  spi-cpha: true
> > +
> > +  vref-supply:
> > +    description:
> > +      The regulator supply for the ADC reference voltage. This is a differential
> > +      reference. It is equal to the V_REFP - V_REFN. The maximum value is 3.6V.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - spi-max-frequency
> > +  - spi-cpha
> > +  - interrupts
> > +  - vref-supply  
> 
> If you are going to send a new version, please put the properties here
> in the same order as they appear in top-level "properties:".
Tweaked whilst applying.

J
> 
> In any case:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 2/2] iio: adc: add max11205 adc driver

[Jonathan Cameron]

On Wed, 31 Aug 2022 16:30:21 +0300
Ramona Bolboaca <ramona.bolboaca@analog.com> wrote:

> Adding support for max11205 16-bit single-channel ultra-low power
> delta-sigma adc.
> The MAX11205 is compatible with the 2-wire interface and uses
> SCLK and RDY/DOUT for serial communications. In this mode, all
> controls are implemented by timing the high or low phase of the SCLK.
> The 2-wire serial interface only allows for data to be read out through
> the RDY/DOUT output.
> 
> Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
> Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>

Given the requested changes below and those from Andy and Kryzstof are minor, I'll just
tweak them whilst applying.

Diff for this patch was below.  The long line for chip_info is a bit ugly but
not too bad...

diff --git a/drivers/iio/adc/max11205.c b/drivers/iio/adc/max11205.c
index 68e6082e70e5..fc90fed81eb6 100644
--- a/drivers/iio/adc/max11205.c
+++ b/drivers/iio/adc/max11205.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * max11205 16-Bit Delta-Sigma ADC
+ * Maxim MAX11205 16-Bit Delta-Sigma ADC
  *
  * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1240-max11205.pdf
  * Copyright (C) 2022 Analog Devices, Inc.
@@ -19,20 +19,20 @@
 #define MAX11205A_OUT_DATA_RATE        116
 #define MAX11205B_OUT_DATA_RATE        13
 
-enum chip_type {
+enum max11205_chip_type {
        TYPE_MAX11205A,
        TYPE_MAX11205B,
 };
 
-struct chip_info {
+struct max11205_chip_info {
        unsigned int    out_data_rate;
        const char      *name;
 };
 
 struct max11205_state {
-       const struct chip_info  *chip_info;
-       struct regulator        *vref;
-       struct ad_sigma_delta   sd;
+       const struct max11205_chip_info *chip_info;
+       struct regulator                *vref;
+       struct ad_sigma_delta           sd;
 };
 
 static const struct ad_sigma_delta_info max11205_sigma_delta_info = {
@@ -81,12 +81,12 @@ static const struct iio_chan_spec max11205_channels[] = {
                        .endianness = IIO_BE
                },
                .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-               BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-               BIT(IIO_CHAN_INFO_SCALE),
+                                     BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+                                     BIT(IIO_CHAN_INFO_SCALE),
        },
 };
 
-static const struct chip_info max11205_chip_info[] = {
+static const struct max11205_chip_info max11205_chip_info[] = {
        [TYPE_MAX11205A] = {
                .out_data_rate = MAX11205A_OUT_DATA_RATE,
                .name = "max11205a",
@@ -117,9 +117,9 @@ static int max11205_probe(struct spi_device *spi)
        ad_sd_init(&st->sd, indio_dev, spi, &max11205_sigma_delta_info);
 
        st->chip_info = device_get_match_data(&spi->dev);
-
        if (!st->chip_info)
-               st->chip_info = (const struct chip_info *)spi_get_device_id(spi)->driver_data;
+               st->chip_info =
+                       (const struct max11205_chip_info *)spi_get_device_id(spi)->driver_data;
 
        indio_dev->name = st->chip_info->name;
        indio_dev->modes = INDIO_DIRECT_MODE;

> ---
> changes in v2:
>  - add chip_info null pointer check
>  - add support for probing with ACPI table
>  - remove function for module removal
>  - remove irq flag from max11205_sigma_delta_info
>  - add missing commas and missing spaces
>  - remove redundant blank line
>  - wrap text to 75-80 chars
>  - removed typos in commit message
>  drivers/iio/adc/Kconfig    |  14 +++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11205.c | 183 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 198 insertions(+)
>  create mode 100644 drivers/iio/adc/max11205.c
> 

> +enum chip_type {
> +	TYPE_MAX11205A,
> +	TYPE_MAX11205B,
> +};
> +
> +struct chip_info {
> +	unsigned int	out_data_rate;
> +	const char	*name;
> +};
Prefix these enums and structures with max11205.

They have very generic names, so it's definitely possible that
something with the same name might be added to a header included
by this driver
> +
> +struct max11205_state {
> +	const struct chip_info	*chip_info;
> +	struct regulator	*vref;
> +	struct ad_sigma_delta	sd;
> +};
> +


> +static const struct iio_chan_spec max11205_channels[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.indexed = 1,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_BE
> +		},
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |

Code editors always get confused on these, but please add an
indent to this line and the next one. Either align it with the BIT()
above, or just push it in one tab.

> +		BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};

Re: [PATCH v2 2/2] iio: adc: add max11205 adc driver

[Jonathan Cameron]

On Sun, 4 Sep 2022 16:50:03 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 31 Aug 2022 16:30:21 +0300
> Ramona Bolboaca <ramona.bolboaca@analog.com> wrote:
> 
> > Adding support for max11205 16-bit single-channel ultra-low power
> > delta-sigma adc.
> > The MAX11205 is compatible with the 2-wire interface and uses
> > SCLK and RDY/DOUT for serial communications. In this mode, all
> > controls are implemented by timing the high or low phase of the SCLK.
> > The 2-wire serial interface only allows for data to be read out through
> > the RDY/DOUT output.
> > 
> > Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX11205.pdf
> > Signed-off-by: Ramona Bolboaca <ramona.bolboaca@analog.com>  
> 
> Given the requested changes below and those from Andy and Kryzstof are minor, I'll just
> tweak them whilst applying.

On that note, applied to the togreg branch of iio.git (with changes as noted)
and pushed out as testing for 0-day to see if it can find anything we missed.

Thanks,

Jonathan

> 
> Diff for this patch was below.  The long line for chip_info is a bit ugly but
> not too bad...
> 
> diff --git a/drivers/iio/adc/max11205.c b/drivers/iio/adc/max11205.c
> index 68e6082e70e5..fc90fed81eb6 100644
> --- a/drivers/iio/adc/max11205.c
> +++ b/drivers/iio/adc/max11205.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * max11205 16-Bit Delta-Sigma ADC
> + * Maxim MAX11205 16-Bit Delta-Sigma ADC
>   *
>   * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1240-max11205.pdf
>   * Copyright (C) 2022 Analog Devices, Inc.
> @@ -19,20 +19,20 @@
>  #define MAX11205A_OUT_DATA_RATE        116
>  #define MAX11205B_OUT_DATA_RATE        13
>  
> -enum chip_type {
> +enum max11205_chip_type {
>         TYPE_MAX11205A,
>         TYPE_MAX11205B,
>  };
>  
> -struct chip_info {
> +struct max11205_chip_info {
>         unsigned int    out_data_rate;
>         const char      *name;
>  };
>  
>  struct max11205_state {
> -       const struct chip_info  *chip_info;
> -       struct regulator        *vref;
> -       struct ad_sigma_delta   sd;
> +       const struct max11205_chip_info *chip_info;
> +       struct regulator                *vref;
> +       struct ad_sigma_delta           sd;
>  };
>  
>  static const struct ad_sigma_delta_info max11205_sigma_delta_info = {
> @@ -81,12 +81,12 @@ static const struct iio_chan_spec max11205_channels[] = {
>                         .endianness = IIO_BE
>                 },
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> -               BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> -               BIT(IIO_CHAN_INFO_SCALE),
> +                                     BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +                                     BIT(IIO_CHAN_INFO_SCALE),
>         },
>  };
>  
> -static const struct chip_info max11205_chip_info[] = {
> +static const struct max11205_chip_info max11205_chip_info[] = {
>         [TYPE_MAX11205A] = {
>                 .out_data_rate = MAX11205A_OUT_DATA_RATE,
>                 .name = "max11205a",
> @@ -117,9 +117,9 @@ static int max11205_probe(struct spi_device *spi)
>         ad_sd_init(&st->sd, indio_dev, spi, &max11205_sigma_delta_info);
>  
>         st->chip_info = device_get_match_data(&spi->dev);
> -
>         if (!st->chip_info)
> -               st->chip_info = (const struct chip_info *)spi_get_device_id(spi)->driver_data;
> +               st->chip_info =
> +                       (const struct max11205_chip_info *)spi_get_device_id(spi)->driver_data;
>  
>         indio_dev->name = st->chip_info->name;
>         indio_dev->modes = INDIO_DIRECT_MODE;
> 
> > ---
> > changes in v2:
> >  - add chip_info null pointer check
> >  - add support for probing with ACPI table
> >  - remove function for module removal
> >  - remove irq flag from max11205_sigma_delta_info
> >  - add missing commas and missing spaces
> >  - remove redundant blank line
> >  - wrap text to 75-80 chars
> >  - removed typos in commit message
> >  drivers/iio/adc/Kconfig    |  14 +++
> >  drivers/iio/adc/Makefile   |   1 +
> >  drivers/iio/adc/max11205.c | 183 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 198 insertions(+)
> >  create mode 100644 drivers/iio/adc/max11205.c
> >   
> 
> > +enum chip_type {
> > +	TYPE_MAX11205A,
> > +	TYPE_MAX11205B,
> > +};
> > +
> > +struct chip_info {
> > +	unsigned int	out_data_rate;
> > +	const char	*name;
> > +};  
> Prefix these enums and structures with max11205.
> 
> They have very generic names, so it's definitely possible that
> something with the same name might be added to a header included
> by this driver
> > +
> > +struct max11205_state {
> > +	const struct chip_info	*chip_info;
> > +	struct regulator	*vref;
> > +	struct ad_sigma_delta	sd;
> > +};
> > +  
> 
> 
> > +static const struct iio_chan_spec max11205_channels[] = {
> > +	{
> > +		.type = IIO_VOLTAGE,
> > +		.indexed = 1,
> > +		.scan_type = {
> > +			.sign = 's',
> > +			.realbits = 16,
> > +			.storagebits = 16,
> > +			.endianness = IIO_BE
> > +		},
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |  
> 
> Code editors always get confused on these, but please add an
> indent to this line and the next one. Either align it with the BIT()
> above, or just push it in one tab.
> 
> > +		BIT(IIO_CHAN_INFO_SCALE),
> > +	},
> > +};  
> 
> 

Re: [PATCH v2 2/2] iio: adc: add max11205 adc driver

[Andy Shevchenko]

On Sun, Sep 4, 2022 at 7:31 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Sun, 4 Sep 2022 16:50:03 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> > On Wed, 31 Aug 2022 16:30:21 +0300
> > Ramona Bolboaca <ramona.bolboaca@analog.com> wrote:

...

> > Given the requested changes below and those from Andy and Kryzstof are minor, I'll just
> > tweak them whilst applying.
>
> On that note, applied to the togreg branch of iio.git (with changes as noted)
> and pushed out as testing for 0-day to see if it can find anything we missed.

If you are not too lazy and have time to tweak one completely minor
thingy, see below.

...

> > @@ -81,12 +81,12 @@ static const struct iio_chan_spec max11205_channels[] = {
> >                         .endianness = IIO_BE

+ A comma here :-)


> >                 },
> >                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > -               BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > -               BIT(IIO_CHAN_INFO_SCALE),
> > +                                     BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > +                                     BIT(IIO_CHAN_INFO_SCALE),
> >         },

-- 
With Best Regards,
Andy Shevchenko