On 3/18/20 7:50 AM, Ardelean, Alexandru wrote:
On Tue, 2020-03-17 at 22:17 +0200, Alexandru Lazar wrote:
[External]
Add driver for the Maxim MAX1241 12-bit, single-channel ADC. The driver
includes support for this device's low-power operation mode.
hey,
overall looks good;
i'd run ./scripts/checpatch.pl on the patches a bit;
you can run it on the patch file, or on the git commit with
./scripts/checpatch.pl -g <git-commits>
i usually do ./scripts/checpatch.pl -g HEAD~2.. [or something like that] before
generating patches;
i sometimes forget to do that;
some more comments inline
Signed-off-by: Alexandru Lazar <alazar@xxxxxxxxxxxxx>
---
drivers/iio/adc/Kconfig | 12 +++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/max1241.c | 215 ++++++++++++++++++++++++++++++++++++++
3 files changed, 228 insertions(+)
create mode 100644 drivers/iio/adc/max1241.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 5d8540b7b427..3a55beec69c9 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -566,6 +566,18 @@ 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
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say yes here to build support for Maxim max1241 12-bit, single-channel
+ ADC.
nitpick: this looks inconsistently indented
+
+ To compile this driver as a module, choose M here: the module will be
+ called max1118.
+
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..2bd31f22fb2c
--- /dev/null
+++ b/drivers/iio/adc/max1241.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MAX1241 low-power, 12-bit serial ADC
+ *
+ * Copyright (c) 2020 Ioan-Alexandru Lazar <alazar@xxxxxxxxxxxxx>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
This license text is no longer needed.
The SPDX-License-Identifier header should handle that.
+ *
+ * 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/gpio/driver.h>
+#include <linux/gpio.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;
Jonathan may know better than me here, but you could technically avoid needing
to explicitly use the __be16 datatype; and just use u16;
i think the SPI framework should have some handling for that;
maybe using the 'bits_per_word' field;
you'd probably still need to do the shifting though;
i remember some discussion about this on ad7949.c
though there may be other drivers doing this as well
I'd keep it as it is. Which bits_per_word values are supported depends
on the SPI master driver. All of them support 8-bit, but many don't
support 16-bit.
- Lars
