[PATCH v1 1/2] dt:bindings:iio:frequency: Add ADMV4420 doc

[PATCH v1 1/2] dt:bindings:iio:frequency: Add ADMV4420 doc

[Cristian Pop]

Add device tree bindings for the ADMV4420 K band downconverter.

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
 .../bindings/iio/frequency/adi,admv4420.yaml  | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml
new file mode 100644
index 000000000000..69f1b4a41c5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,admv4420.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADMV4420 K Band Downconverter
+
+maintainers:
+- Cristian Pop <cristian.pop@analog.com>
+
+description: |
+    The ADMV4420 is a highly integrated, double balanced, active
+    mixer with an integrated fractional-N synthesizer, ideally suited
+    for next generation K band satellite communications
+
+properties:
+  compatible:
+    enum:
+      - adi,admv4420
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 1000000
+
+  adi,ref_single_ended:
+    description: Reference clock type.
+    type: boolean
+
+  adi,ref_freq_hz:
+    description: Reference clock frequency.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  adi,ref_doubler_en:
+    description: Reference multiplied by 2.
+    type: boolean
+
+  adi,ref_divide_by_2_en:
+    description: Reference divided by 2.
+    type: boolean
+
+  adi,ref_divider:
+    description: Reference divider value.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  adi,N_counter_int_val:
+    description: N counted int val.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  adi,N_counter_frac_val:
+    description: N counted frac val.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  adi,N_counter_mod_val:
+    description: N counted mod val.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  adi,mux_sel:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 4, 5, 8]
+    description: |
+      Multiplexer output allows access to various internal signals:
+      0: Output Logic Low
+      1: Digital Lock Detect
+      4: RDiv-by-2 to Mux Out, Frequency = REFIN/(2 x R)
+      5: NDiv-by-2 to Mux Out, Frequency = VCO/(2 x N)
+      8: Output Logic High.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      admv4420@0 {
+        compatible = "adi,admv4420";
+        reg = <0>;
+        spi-max-frequency = <1000000>;
+
+        /* reference block config */
+        adi,ref_freq_hz = <50000000>;
+        adi,ref_single_ended = <0>;
+        adi,ref_divider = <1>;
+
+        /* N counter config*/
+        adi,N_counter_int_val = <0xA7>;
+        adi,N_counter_frac_val = <0x02>;
+        adi,N_counter_mod_val = <0x04>;
+
+        adi,mux_sel = <1>;
+      };
+    };
+...
-- 
2.17.1

[PATCH v1 2/2] iio:frequency:admv4420.c: Add support for ADMV4420

[Cristian Pop]

Add support for K Band Downconverter with Integrated
Fractional-N PLL and VCO.
More info:
https://www.analog.com/en/products/admv4420.html

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
 drivers/iio/frequency/Kconfig    |  10 +
 drivers/iio/frequency/Makefile   |   1 +
 drivers/iio/frequency/admv4420.c | 413 +++++++++++++++++++++++++++++++
 3 files changed, 424 insertions(+)
 create mode 100644 drivers/iio/frequency/admv4420.c

diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
index 2c9e0559e8a4..e2a8428a72df 100644
--- a/drivers/iio/frequency/Kconfig
+++ b/drivers/iio/frequency/Kconfig
@@ -50,6 +50,16 @@ config ADF4371
 	  To compile this driver as a module, choose M here: the
 	  module will be called adf4371.
 
+config ADMV4420
+	tristate "Analog Devices ADMV4420 K Band Downconverter"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices K Band
+	  Downconverter with integrated Fractional-N PLL and VCO.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called admv4420.
+
 config ADRF6780
         tristate "Analog Devices ADRF6780 Microwave Upconverter"
         depends on SPI
diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
index ae3136c79202..f582305b3086 100644
--- a/drivers/iio/frequency/Makefile
+++ b/drivers/iio/frequency/Makefile
@@ -7,4 +7,5 @@
 obj-$(CONFIG_AD9523) += ad9523.o
 obj-$(CONFIG_ADF4350) += adf4350.o
 obj-$(CONFIG_ADF4371) += adf4371.o
+obj-$(CONFIG_ADMV4420) += admv4420.o
 obj-$(CONFIG_ADRF6780) += adrf6780.o
diff --git a/drivers/iio/frequency/admv4420.c b/drivers/iio/frequency/admv4420.c
new file mode 100644
index 000000000000..728e86ec93b6
--- /dev/null
+++ b/drivers/iio/frequency/admv4420.c
@@ -0,0 +1,413 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * ADMV4420
+ *
+ * Copyright 2021 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+/* ADMV4420 Register Map */
+#define ADMV4420_SPI_CONFIG_1			0x00
+#define ADMV4420_SPI_CONFIG_2			0x01
+#define ADMV4420_CHIPTYPE			0x03
+#define ADMV4420_PRODUCT_ID_L			0x04
+#define ADMV4420_PRODUCT_ID_H			0x05
+#define ADMV4420_SCRATCHPAD			0x0A
+#define ADMV4420_SPI_REV			0x0B
+#define ADMV4420_ENABLES			0x103
+#define ADMV4420_SDO_LEVEL			0x108
+#define ADMV4420_INT_L				0x200
+#define ADMV4420_INT_H				0x201
+#define ADMV4420_FRAC_L				0x202
+#define ADMV4420_FRAC_M				0x203
+#define ADMV4420_FRAC_H				0x204
+#define ADMV4420_MOD_L				0x208
+#define ADMV4420_MOD_M				0x209
+#define ADMV4420_MOD_H				0x20A
+#define ADMV4420_R_DIV_L			0x20C
+#define ADMV4420_R_DIV_H			0x20D
+#define ADMV4420_REFERENCE			0x20E
+#define ADMV4420_VCO_DATA_READBACK1		0x211
+#define ADMV4420_VCO_DATA_READBACK2		0x212
+#define ADMV4420_PLL_MUX_SEL			0x213
+#define ADMV4420_LOCK_DETECT			0x214
+#define ADMV4420_BAND_SELECT			0x215
+#define ADMV4420_VCO_ALC_TIMEOUT		0x216
+#define ADMV4420_VCO_MANUAL			0x217
+#define ADMV4420_ALC				0x219
+#define ADMV4420_VCO_TIMEOUT1			0x21C
+#define ADMV4420_VCO_TIMEOUT2			0x21D
+#define ADMV4420_VCO_BAND_DIV			0x21E
+#define ADMV4420_VCO_READBACK_SEL		0x21F
+#define ADMV4420_AUTOCAL			0x226
+#define ADMV4420_CP_STATE			0x22C
+#define ADMV4420_CP_BLEED_EN			0x22D
+#define ADMV4420_CP_CURRENT			0x22E
+#define ADMV4420_CP_BLEED			0x22F
+
+#define ADMV4420_SPI_CONFIG_1_SOFTRESET		BIT(1)
+#define ADMV4420_SPI_CONFIG_1_SOFTRESET_	BIT(7)
+
+#define ADMV4420_REFERENCE_IN_MODE(x)		(x << 1)
+#define ADMV4420_REFERENCE_DOUBLER(x)		(x << 2)
+#define ADMV4420_REFERENCE_DIVIDE_BY_2_MASK	BIT(0)
+#define ADMV4420_REFERENCE_MODE_MASK		BIT(1)
+#define ADMV4420_REFERENCE_DOUBLER_MASK		BIT(2)
+#define ADMV4420_REF_DIVIDER_MAX_VAL		GENMASK(9, 0)
+#define ADMV4420_N_COUNTER_INT_MAX		GENMASK(15, 0)
+#define ADMV4420_N_COUNTER_FRAC_MAX		GENMASK(23, 0)
+#define ADMV4420_N_COUNTER_MOD_MAX		GENMASK(23, 0)
+
+#define ADMV4420_L_MASK				GENMASK(7, 0)
+#define ADMV4420_H_MASK				GENMASK(15, 8)
+#define ADMV4420_FRAC_L_MASK			GENMASK(7, 0)
+#define ADMV4420_FRAC_M_MASK			GENMASK(15, 8)
+#define ADMV4420_FRAC_H_MASK			GENMASK(23, 16)
+#define ADMV4420_MOD_L_MASK			GENMASK(7, 0)
+#define ADMV4420_MOD_M_MASK			GENMASK(15, 8)
+#define ADMV4420_MOD_H_MASK			GENMASK(23, 16)
+
+#define ENABLE_PLL				BIT(6)
+#define ENABLE_LO				BIT(5)
+#define ENABLE_VCO				BIT(3)
+#define ENABLE_IFAMP				BIT(2)
+#define ENABLE_MIXER				BIT(1)
+#define ENABLE_LNA				BIT(0)
+
+#define ADAR1000_SCRATCH_PAD_VAL_1		0xAD
+#define ADAR1000_SCRATCH_PAD_VAL_2		0xEA
+#define ADMV4420_DEF_REF_HZ			50000000
+#define ADMV4420_DEF_REF_DIVIDER		1
+#define ADMV4420_DEF_NC_INT			0xA7
+#define ADMV4420_DEF_NC_FRAC			0x02
+#define ADMV4420_DEF_NC_MOD			0x04
+
+enum admv4420_option_st {
+	ADMV4420_DISABLED,
+	ADMV4420_ENABLED,
+};
+
+enum admv4420_ref_op {
+	ADMV4420_DOUBLER,
+	ADMV4420_DIVIDE_BY_2,
+	ADMV4420_REF_SINGLE_ENDED,
+};
+
+enum admv4420_mux_sel {
+	ADMV4420_LOW = 0,
+	ADMV4420_LOCK_DTCT = 1,
+	ADMV4420_R_COUNTER_PER_2 = 4,
+	ADMV4420_N_CONUTER_PER_2 = 5,
+	ADMV4420_HIGH = 8,
+};
+
+enum admv4420_n_counter_par {
+	ADMV4420_N_COUNTER_INT,
+	ADMV4420_N_COUNTER_FRAC,
+	ADMV4420_N_COUNTER_MOD,
+};
+
+struct admv4420_reference_block {
+	bool doubler_en;
+	bool divide_by_2_en;
+	bool ref_single_ended;
+	u32 freq_hz;
+	u32 divider;
+};
+
+struct admv4420_n_counter {
+	u32 int_val;
+	u32 frac_val;
+	u32 mod_val;
+	u32 n_counter;
+};
+
+struct admv4420_state {
+	struct spi_device		*spi;
+	struct regmap			*regmap;
+	u64				pfd_freq_hz;
+	u64				vco_freq_hz;
+	u64				lo_freq_hz;
+	struct admv4420_reference_block ref_block;
+	struct admv4420_n_counter	n_counter;
+	enum admv4420_mux_sel		mux_sel;
+	struct mutex			lock;
+};
+
+static const struct regmap_config admv4420_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = BIT(7),
+};
+
+static int admv4420_reg_access(struct iio_dev *indio_dev,
+			       u32 reg, u32 writeval,
+			       u32 *readval)
+{
+	struct admv4420_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+	else
+		return regmap_write(st->regmap, reg, writeval);
+}
+
+static void admv4420_calc_vco_freq(struct admv4420_state *st)
+{
+	u64 tmp;
+
+	tmp = div_u64((st->pfd_freq_hz * st->n_counter.frac_val), st->n_counter.mod_val);
+	tmp += st->pfd_freq_hz * st->n_counter.int_val;
+	st->vco_freq_hz = tmp;
+}
+
+static void admv4420_calc_pfd_freq(struct admv4420_state *st)
+{
+	u32 tmp;
+
+	tmp = st->ref_block.freq_hz * (st->ref_block.doubler_en ? 2 : 1);
+	tmp = DIV_ROUND_CLOSEST(tmp, st->ref_block.divider *
+				(st->ref_block.divide_by_2_en ? 2 : 1));
+	st->pfd_freq_hz = tmp;
+
+	admv4420_calc_vco_freq(st);
+	st->lo_freq_hz = st->vco_freq_hz * 2;
+}
+
+static int admv4420_set_n_counter(struct admv4420_state *st, u32 int_val, u32 frac_val, u32 mod_val)
+{
+	int ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_FRAC_H, FIELD_GET(ADMV4420_FRAC_H_MASK, frac_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_FRAC_M, FIELD_GET(ADMV4420_FRAC_M_MASK, frac_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_FRAC_L, FIELD_GET(ADMV4420_FRAC_L_MASK, frac_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_MOD_H, FIELD_GET(ADMV4420_MOD_H_MASK, mod_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_MOD_M, FIELD_GET(ADMV4420_MOD_M_MASK, mod_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_MOD_L, FIELD_GET(ADMV4420_MOD_L_MASK, mod_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_INT_H, FIELD_GET(ADMV4420_H_MASK, int_val));
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, ADMV4420_INT_L, FIELD_GET(ADMV4420_L_MASK, int_val));
+}
+
+static int admv4420_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	struct admv4420_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_FREQUENCY:
+		*val = div_u64(st->lo_freq_hz, 1000000);
+		div_u64_rem(st->lo_freq_hz, 1000000, val2);
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info admv4420_info = {
+	.read_raw = admv4420_read_raw,
+	.debugfs_reg_access = &admv4420_reg_access,
+};
+
+#define ADMV4420_CHAN_LO(_channel) {				\
+	.type = IIO_ALTVOLTAGE,					\
+	.output = 0,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY)	\
+}
+
+static const struct iio_chan_spec admv4420_channels[] = {
+	ADMV4420_CHAN_LO(0),
+};
+
+static void admv4420_dt_parse(struct admv4420_state *st)
+{
+	struct spi_device *spi = st->spi;
+
+	st->ref_block.ref_single_ended = of_property_read_bool(spi->dev.of_node,
+							       "adi,ref_single_ended");
+	st->ref_block.doubler_en = of_property_read_bool(spi->dev.of_node, "adi,ref_doubler_en");
+	st->ref_block.divide_by_2_en = of_property_read_bool(spi->dev.of_node,
+							     "adi,ref_divide_by_2_en");
+	device_property_read_u32(&spi->dev, "adi,ref_freq_hz", &st->ref_block.freq_hz);
+	device_property_read_u32(&spi->dev, "adi,ref_divider", &st->ref_block.divider);
+	device_property_read_u32(&spi->dev, "adi,N_counter_int_val", &st->n_counter.int_val);
+	device_property_read_u32(&spi->dev, "adi,N_counter_frac_val", &st->n_counter.frac_val);
+	device_property_read_u32(&spi->dev, "adi,N_counter_mod_val", &st->n_counter.mod_val);
+	device_property_read_u32(&spi->dev, "adi,mux_sel", &st->mux_sel);
+}
+
+static int admv4420_setup(struct iio_dev *indio_dev)
+{
+	struct admv4420_state *st = iio_priv(indio_dev);
+	u32 val = 0;
+	int ret;
+
+	/* Software reset and activate SDO */
+	ret = regmap_write(st->regmap, ADMV4420_SPI_CONFIG_1,
+			   ADMV4420_SPI_CONFIG_1_SOFTRESET_ | ADMV4420_SPI_CONFIG_1_SOFTRESET);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADAR1000_SCRATCH_PAD_VAL_1);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
+	if (ret)
+		return ret;
+
+	if (val != ADAR1000_SCRATCH_PAD_VAL_1) {
+		dev_err(indio_dev->dev.parent, "Failed ADMV4420 to read/write scratchpad %x ", val);
+		return -EIO;
+	}
+
+	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADAR1000_SCRATCH_PAD_VAL_2);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
+	if (ret)
+		return ret;
+
+	if (val != ADAR1000_SCRATCH_PAD_VAL_2) {
+		dev_err(indio_dev->dev.parent, "Failed ADMV4420 to read/write scratchpad %x ", val);
+		return -EIO;
+	}
+
+	st->ref_block.freq_hz = ADMV4420_DEF_REF_HZ;
+	st->ref_block.ref_single_ended = false;
+	st->ref_block.doubler_en = false;
+	st->ref_block.divide_by_2_en = false;
+	st->ref_block.divider = ADMV4420_DEF_REF_DIVIDER;
+
+	st->n_counter.int_val = ADMV4420_DEF_NC_INT;
+	st->n_counter.frac_val = ADMV4420_DEF_NC_FRAC;
+	st->n_counter.mod_val = ADMV4420_DEF_NC_MOD;
+
+	st->mux_sel = ADMV4420_LOCK_DTCT;
+
+	admv4420_dt_parse(st);
+
+	ret = regmap_write(st->regmap, ADMV4420_R_DIV_L,
+			   FIELD_GET(ADMV4420_L_MASK, st->ref_block.divider));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_R_DIV_H,
+			   FIELD_GET(ADMV4420_H_MASK, st->ref_block.divider));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_REFERENCE,
+			   st->ref_block.divide_by_2_en |
+			   ADMV4420_REFERENCE_IN_MODE(st->ref_block.ref_single_ended) |
+			   ADMV4420_REFERENCE_DOUBLER(st->ref_block.doubler_en));
+	if (ret)
+		return ret;
+
+	ret = admv4420_set_n_counter(st, st->n_counter.int_val, st->n_counter.frac_val,
+				     st->n_counter.mod_val);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_PLL_MUX_SEL, st->mux_sel);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_ENABLES, ENABLE_PLL | ENABLE_LO | ENABLE_VCO |
+			   ENABLE_IFAMP | ENABLE_MIXER | ENABLE_LNA);
+	if (ret)
+		return ret;
+
+	admv4420_calc_pfd_freq(st);
+
+	return 0;
+}
+
+static int admv4420_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct admv4420_state *st;
+	struct regmap *regmap;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_spi(spi, &admv4420_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "Error  ADMV4420 initializing spi regmap: %ld\n",
+			PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->regmap = regmap;
+	mutex_init(&st->lock);
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = "admv4420";
+	indio_dev->info = &admv4420_info;
+	indio_dev->channels = admv4420_channels;
+	indio_dev->num_channels = ARRAY_SIZE(admv4420_channels);
+
+	ret = admv4420_setup(indio_dev);
+	if (ret) {
+		dev_err(&spi->dev, "Setup ADMV4420 failed (%d)\n", ret);
+		return ret;
+	}
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id admv4420_of_match[] = {
+	{ .compatible = "adi,admv4420" },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, admv4420_of_match);
+
+static struct spi_driver admv4420_driver = {
+	.driver = {
+		.name	= "admv4420",
+		.of_match_table = admv4420_of_match,
+	},
+	.probe		= admv4420_probe,
+};
+module_spi_driver(admv4420_driver);
+
+MODULE_AUTHOR("Cristian Pop <cristian.pop@analog.com>");
+MODULE_DESCRIPTION("Analog Devices ADMV44200 K Band Downconverter");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.17.1

Re: [PATCH v1 1/2] dt:bindings:iio:frequency: Add ADMV4420 doc

[Rob Herring]

On Fri, 19 Nov 2021 13:40:10 +0200, Cristian Pop wrote:
> Add device tree bindings for the ADMV4420 K band downconverter.
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> ---
>  .../bindings/iio/frequency/adi,admv4420.yaml  | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml:10:1: [warning] wrong indentation: expected 2 but found 0 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.example.dt.yaml: admv4420@0: adi,ref_single_ended: [[0]] is not of type 'boolean'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1557045

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v1 1/2] dt:bindings:iio:frequency: Add ADMV4420 doc

[Jonathan Cameron]

On Fri, 19 Nov 2021 13:40:10 +0200
Cristian Pop <cristian.pop@analog.com> wrote:

> Add device tree bindings for the ADMV4420 K band downconverter.
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> ---
>  .../bindings/iio/frequency/adi,admv4420.yaml  | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml
> new file mode 100644
> index 000000000000..69f1b4a41c5c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,admv4420.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,admv4420.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADMV4420 K Band Downconverter
> +
> +maintainers:
> +- Cristian Pop <cristian.pop@analog.com>

Rob's scripted checks picked this up so I'll assume you'll add the 2 spaces.

> +
> +description: |
> +    The ADMV4420 is a highly integrated, double balanced, active
> +    mixer with an integrated fractional-N synthesizer, ideally suited
> +    for next generation K band satellite communications
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,admv4420
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 1000000
> +
> +  adi,ref_single_ended:

- rather than _ through out.

I 'think' this picking between a crystal and a clock.  We have other parts
doing this and IIRC one approach to handling this was to have an optional clock
source. If it isn't provided we know this is a crystal and can pick on that basis.
example is adc/microchip/mcp3911.yaml



> +    description: Reference clock type.
> +    type: boolean
> +
> +  adi,ref_freq_hz:
> +    description: Reference clock frequency.
> +    $ref: /schemas/types.yaml#/definitions/uint32

with -hz this will be covered by standard units suffixes so 
you won't need the $ref.

I'm curious on this. Datasheet seems to say the only valid frequency for
this is 50MHz either from a crystal or a reference clock.


> +
> +  adi,ref_doubler_en:
> +    description: Reference multiplied by 2.
> +    type: boolean
> +
> +  adi,ref_divide_by_2_en:
> +    description: Reference divided by 2.
> +    type: boolean
> +
> +  adi,ref_divider:
> +    description: Reference divider value.
> +    $ref: /schemas/types.yaml#/definitions/uint32

This lot correspond to clock signal doubling/ halving and
count based division.  Can we lift something standard from the
clk dt-bindings to describe this?

> +
> +  adi,N_counter_int_val:
> +    description: N counted int val.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  adi,N_counter_frac_val:
> +    description: N counted frac val.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  adi,N_counter_mod_val:
> +    description: N counted mod val.
> +    $ref: /schemas/types.yaml#/definitions/uint32

Sigh, my 20 year old memories of demodulation aren't coming back enough to
remember enough to describe these clearly - but we definitely need more information
here and I'm not sure these are things that should be in dt at all..

Are they characteristics of the wiring, or more closely related to what the input
is we are down converting?  All this stuff is about generating a very precise tuned
frequency given the PLL filter that we have no visibility of (and lets not try 
to describe that in the binding as that would be a nightmark).

So honestly I have no idea how to describe this.  Maybe more info would help?

> +
> +  adi,mux_sel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 4, 5, 8]
> +    description: |
> +      Multiplexer output allows access to various internal signals:
> +      0: Output Logic Low
> +      1: Digital Lock Detect
> +      4: RDiv-by-2 to Mux Out, Frequency = REFIN/(2 x R)
> +      5: NDiv-by-2 to Mux Out, Frequency = VCO/(2 x N)
> +      8: Output Logic High.

Hmm. So low and high are just using this as a gpio.  We 'could' support that
but I'd be highly surprised if that is ever used except for circuit debug so
I'd just drop those.

The digital lock is probably something that could be optionally wired to a gpio
to allow detection of lock.

The two frequency signals are probably for debug, but those might make sense
to describe in this fashion in DT.  I'd be tempted to not bother though as I'm not
sure what use they would be except during circuit debug.

So I'd just support the lock detect as an optional gpio connected signal.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      admv4420@0 {
> +        compatible = "adi,admv4420";
> +        reg = <0>;
> +        spi-max-frequency = <1000000>;
> +
> +        /* reference block config */
> +        adi,ref_freq_hz = <50000000>;
> +        adi,ref_single_ended = <0>;

Rob's scripts point out this isn't a boolean value.

> +        adi,ref_divider = <1>;
> +
> +        /* N counter config*/
> +        adi,N_counter_int_val = <0xA7>;
> +        adi,N_counter_frac_val = <0x02>;
> +        adi,N_counter_mod_val = <0x04>;
> +
> +        adi,mux_sel = <1>;
> +      };
> +    };
> +...

Re: [PATCH v1 2/2] iio:frequency:admv4420.c: Add support for ADMV4420

[Jonathan Cameron]

On Fri, 19 Nov 2021 13:40:11 +0200
Cristian Pop <cristian.pop@analog.com> wrote:

> Add support for K Band Downconverter with Integrated
> Fractional-N PLL and VCO.
> More info:
> https://www.analog.com/en/products/admv4420.html

Datasheet: https://www.analog.com/en/products/admv4420.html

> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>

Having looked at the datasheet I'm not sure how we can realistically fit this
into a remotely standard ABI.  A user would care about controlling the tuning
frequency and that's not going to be trivial to describe.

So having read this I'm not sure I understand why the IIO part of the driver
is useful.  If the only interest is in fixed frequency operation why expose
any standard(ish) userspace?

Thanks,

Jonathan


> ---
>  drivers/iio/frequency/Kconfig    |  10 +
>  drivers/iio/frequency/Makefile   |   1 +
>  drivers/iio/frequency/admv4420.c | 413 +++++++++++++++++++++++++++++++
>  3 files changed, 424 insertions(+)
>  create mode 100644 drivers/iio/frequency/admv4420.c
...

> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

Why this include?

> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +
> +#define ADMV4420_REFERENCE_IN_MODE(x)		(x << 1)

Prefer to see these specified as masks and then defines for the values
followed by use of FIELD_PREP to set the actual bits.

> +#define ADMV4420_REFERENCE_DOUBLER(x)		(x << 2)
> +#define ADMV4420_REFERENCE_DIVIDE_BY_2_MASK	BIT(0)
> +#define ADMV4420_REFERENCE_MODE_MASK		BIT(1)
> +#define ADMV4420_REFERENCE_DOUBLER_MASK		BIT(2)


> +
> +struct admv4420_reference_block {
> +	bool doubler_en;
> +	bool divide_by_2_en;
> +	bool ref_single_ended;
> +	u32 freq_hz;
> +	u32 divider;
> +};
> +
> +struct admv4420_n_counter {
> +	u32 int_val;
> +	u32 frac_val;
> +	u32 mod_val;
> +	u32 n_counter;
> +};
> +

> +
> +static void admv4420_calc_vco_freq(struct admv4420_state *st)
> +{
> +	u64 tmp;
> +
> +	tmp = div_u64((st->pfd_freq_hz * st->n_counter.frac_val), st->n_counter.mod_val);
> +	tmp += st->pfd_freq_hz * st->n_counter.int_val;
> +	st->vco_freq_hz = tmp;
> +}
> +
> +static void admv4420_calc_pfd_freq(struct admv4420_state *st)
> +{
> +	u32 tmp;
> +
> +	tmp = st->ref_block.freq_hz * (st->ref_block.doubler_en ? 2 : 1);
> +	tmp = DIV_ROUND_CLOSEST(tmp, st->ref_block.divider *
> +				(st->ref_block.divide_by_2_en ? 2 : 1));
> +	st->pfd_freq_hz = tmp;
> +
> +	admv4420_calc_vco_freq(st);
> +	st->lo_freq_hz = st->vco_freq_hz * 2;
> +}
> +
> +static int admv4420_set_n_counter(struct admv4420_state *st, u32 int_val, u32 frac_val, u32 mod_val)
> +{
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_FRAC_H, FIELD_GET(ADMV4420_FRAC_H_MASK, frac_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_FRAC_M, FIELD_GET(ADMV4420_FRAC_M_MASK, frac_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_FRAC_L, FIELD_GET(ADMV4420_FRAC_L_MASK, frac_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_MOD_H, FIELD_GET(ADMV4420_MOD_H_MASK, mod_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_MOD_M, FIELD_GET(ADMV4420_MOD_M_MASK, mod_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_MOD_L, FIELD_GET(ADMV4420_MOD_L_MASK, mod_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_INT_H, FIELD_GET(ADMV4420_H_MASK, int_val));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->regmap, ADMV4420_INT_L, FIELD_GET(ADMV4420_L_MASK, int_val));
> +}
> +
> +static int admv4420_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	struct admv4420_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_FREQUENCY:
> +		*val = div_u64(st->lo_freq_hz, 1000000);
> +		div_u64_rem(st->lo_freq_hz, 1000000, val2);

Why is it useful to describe a fixed frequency via an IIO device?



> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info admv4420_info = {
> +	.read_raw = admv4420_read_raw,
> +	.debugfs_reg_access = &admv4420_reg_access,
> +};
> +
> +#define ADMV4420_CHAN_LO(_channel) {				\
> +	.type = IIO_ALTVOLTAGE,					\
> +	.output = 0,						\
> +	.indexed = 1,						\
> +	.channel = _channel,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY)	\
> +}
> +
> +static const struct iio_chan_spec admv4420_channels[] = {
> +	ADMV4420_CHAN_LO(0),
> +};
> +
> +static void admv4420_dt_parse(struct admv4420_state *st)
> +{
> +	struct spi_device *spi = st->spi;
> +
> +	st->ref_block.ref_single_ended = of_property_read_bool(spi->dev.of_node,
> +							       "adi,ref_single_ended");
> +	st->ref_block.doubler_en = of_property_read_bool(spi->dev.of_node, "adi,ref_doubler_en");
> +	st->ref_block.divide_by_2_en = of_property_read_bool(spi->dev.of_node,
> +							     "adi,ref_divide_by_2_en");
> +	device_property_read_u32(&spi->dev, "adi,ref_freq_hz", &st->ref_block.freq_hz);
> +	device_property_read_u32(&spi->dev, "adi,ref_divider", &st->ref_block.divider);
> +	device_property_read_u32(&spi->dev, "adi,N_counter_int_val", &st->n_counter.int_val);
> +	device_property_read_u32(&spi->dev, "adi,N_counter_frac_val", &st->n_counter.frac_val);
> +	device_property_read_u32(&spi->dev, "adi,N_counter_mod_val", &st->n_counter.mod_val);
> +	device_property_read_u32(&spi->dev, "adi,mux_sel", &st->mux_sel);
> +}
> +
> +static int admv4420_setup(struct iio_dev *indio_dev)
> +{
> +	struct admv4420_state *st = iio_priv(indio_dev);
> +	u32 val = 0;
> +	int ret;
> +
> +	/* Software reset and activate SDO */
> +	ret = regmap_write(st->regmap, ADMV4420_SPI_CONFIG_1,
> +			   ADMV4420_SPI_CONFIG_1_SOFTRESET_ | ADMV4420_SPI_CONFIG_1_SOFTRESET);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADAR1000_SCRATCH_PAD_VAL_1);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val != ADAR1000_SCRATCH_PAD_VAL_1) {
> +		dev_err(indio_dev->dev.parent, "Failed ADMV4420 to read/write scratchpad %x ", val);

Try to keep lines under 80 chars unless it hurts readability.  Breaking this one before
the string doesn't hurt readability so please do so.

> +		return -EIO;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADAR1000_SCRATCH_PAD_VAL_2);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val != ADAR1000_SCRATCH_PAD_VAL_2) {
> +		dev_err(indio_dev->dev.parent, "Failed ADMV4420 to read/write scratchpad %x ", val);
> +		return -EIO;
> +	}
> +
> +	st->ref_block.freq_hz = ADMV4420_DEF_REF_HZ;
> +	st->ref_block.ref_single_ended = false;
> +	st->ref_block.doubler_en = false;
> +	st->ref_block.divide_by_2_en = false;
> +	st->ref_block.divider = ADMV4420_DEF_REF_DIVIDER;
> +
> +	st->n_counter.int_val = ADMV4420_DEF_NC_INT;
> +	st->n_counter.frac_val = ADMV4420_DEF_NC_FRAC;
> +	st->n_counter.mod_val = ADMV4420_DEF_NC_MOD;
> +
> +	st->mux_sel = ADMV4420_LOCK_DTCT;
> +
> +	admv4420_dt_parse(st);
> +
> +	ret = regmap_write(st->regmap, ADMV4420_R_DIV_L,
> +			   FIELD_GET(ADMV4420_L_MASK, st->ref_block.divider));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_R_DIV_H,
> +			   FIELD_GET(ADMV4420_H_MASK, st->ref_block.divider));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_REFERENCE,
> +			   st->ref_block.divide_by_2_en |
> +			   ADMV4420_REFERENCE_IN_MODE(st->ref_block.ref_single_ended) |
> +			   ADMV4420_REFERENCE_DOUBLER(st->ref_block.doubler_en));
> +	if (ret)
> +		return ret;
> +
> +	ret = admv4420_set_n_counter(st, st->n_counter.int_val, st->n_counter.frac_val,
> +				     st->n_counter.mod_val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_PLL_MUX_SEL, st->mux_sel);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_ENABLES, ENABLE_PLL | ENABLE_LO | ENABLE_VCO |
> +			   ENABLE_IFAMP | ENABLE_MIXER | ENABLE_LNA);
> +	if (ret)
> +		return ret;
> +
> +	admv4420_calc_pfd_freq(st);
> +
> +	return 0;
> +}
> +
> +static int admv4420_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct admv4420_state *st;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_spi(spi, &admv4420_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "Error  ADMV4420 initializing spi regmap: %ld\n",
> +			PTR_ERR(regmap));
> +		return PTR_ERR(regmap);
> +	}
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->regmap = regmap;
> +	mutex_init(&st->lock);
> +
> +	indio_dev->dev.parent = &spi->dev;

The IIO core should set that for you.

> +	indio_dev->name = "admv4420";
> +	indio_dev->info = &admv4420_info;
> +	indio_dev->channels = admv4420_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(admv4420_channels);
> +
> +	ret = admv4420_setup(indio_dev);
> +	if (ret) {
> +		dev_err(&spi->dev, "Setup ADMV4420 failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +

> +
> +MODULE_AUTHOR("Cristian Pop <cristian.pop@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices ADMV44200 K Band Downconverter");
> +MODULE_LICENSE("Dual BSD/GPL");

Re: [PATCH v1 2/2] iio:frequency:admv4420.c: Add support for ADMV4420

[Jonathan Cameron]

On Mon, 17 Jan 2022 18:52:47 +0200
Cristian Pop <cristian.pop@analog.com> wrote:

> Add support for K Band Downconverter with Integrated
> Fractional-N PLL and VCO.
> More info:
> https://www.analog.com/en/products/admv4420.html
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>

Hi Cristian,

A few comments inline from a very quick look.

Thanks,

Jonathan


> ---
>  drivers/iio/frequency/Kconfig    |  10 +
>  drivers/iio/frequency/Makefile   |   1 +
>  drivers/iio/frequency/admv4420.c | 412 +++++++++++++++++++++++++++++++
>  3 files changed, 423 insertions(+)
>  create mode 100644 drivers/iio/frequency/admv4420.c
> 
> diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
> index b44036f843af..55aa63548c00 100644
> --- a/drivers/iio/frequency/Kconfig
> +++ b/drivers/iio/frequency/Kconfig
> @@ -60,6 +60,16 @@ config ADMV1013
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called admv1013.
>  
> +config ADMV4420
> +	tristate "Analog Devices ADMV4420 K Band Downconverter"
> +	depends on SPI && COMMON_CLK

Why COMMON_CLK?  I can't see any usage.

However, I'm not sure just providing an input frequency is that
sensible as it is likely to be connect to something variable
(to some degree at least).

> +	help
> +	  Say yes here to build support for Analog Devices K Band
> +	  Downconverter with integrated Fractional-N PLL and VCO.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called admv4420.
> +
>  config ADRF6780
>          tristate "Analog Devices ADRF6780 Microwave Upconverter"
>          depends on SPI
> diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
> index ae6899856c99..782e5baa1630 100644
> --- a/drivers/iio/frequency/Makefile
> +++ b/drivers/iio/frequency/Makefile
> @@ -8,4 +8,5 @@ obj-$(CONFIG_AD9523) += ad9523.o
>  obj-$(CONFIG_ADF4350) += adf4350.o
>  obj-$(CONFIG_ADF4371) += adf4371.o
>  obj-$(CONFIG_ADMV1013) += admv1013.o
> +obj-$(CONFIG_ADMV4420) += admv4420.o
>  obj-$(CONFIG_ADRF6780) += adrf6780.o
> diff --git a/drivers/iio/frequency/admv4420.c b/drivers/iio/frequency/admv4420.c
> new file mode 100644
> index 000000000000..f32039ab2cd3
> --- /dev/null
> +++ b/drivers/iio/frequency/admv4420.c
> @@ -0,0 +1,412 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +/*
> + * ADMV4420
> + *
> + * Copyright 2021 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +/* ADMV4420 Register Map */
> +#define ADMV4420_SPI_CONFIG_1			0x00
> +#define ADMV4420_SPI_CONFIG_2			0x01
> +#define ADMV4420_CHIPTYPE			0x03
> +#define ADMV4420_PRODUCT_ID_L			0x04
> +#define ADMV4420_PRODUCT_ID_H			0x05
> +#define ADMV4420_SCRATCHPAD			0x0A
> +#define ADMV4420_SPI_REV			0x0B
> +#define ADMV4420_ENABLES			0x103
> +#define ADMV4420_SDO_LEVEL			0x108
> +#define ADMV4420_INT_L				0x200
> +#define ADMV4420_INT_H				0x201
> +#define ADMV4420_FRAC_L				0x202
> +#define ADMV4420_FRAC_M				0x203
> +#define ADMV4420_FRAC_H				0x204
> +#define ADMV4420_MOD_L				0x208
> +#define ADMV4420_MOD_M				0x209
> +#define ADMV4420_MOD_H				0x20A
> +#define ADMV4420_R_DIV_L			0x20C
> +#define ADMV4420_R_DIV_H			0x20D
> +#define ADMV4420_REFERENCE			0x20E
> +#define ADMV4420_VCO_DATA_READBACK1		0x211
> +#define ADMV4420_VCO_DATA_READBACK2		0x212
> +#define ADMV4420_PLL_MUX_SEL			0x213
> +#define ADMV4420_LOCK_DETECT			0x214
> +#define ADMV4420_BAND_SELECT			0x215
> +#define ADMV4420_VCO_ALC_TIMEOUT		0x216
> +#define ADMV4420_VCO_MANUAL			0x217
> +#define ADMV4420_ALC				0x219
> +#define ADMV4420_VCO_TIMEOUT1			0x21C
> +#define ADMV4420_VCO_TIMEOUT2			0x21D
> +#define ADMV4420_VCO_BAND_DIV			0x21E
> +#define ADMV4420_VCO_READBACK_SEL		0x21F
> +#define ADMV4420_AUTOCAL			0x226
> +#define ADMV4420_CP_STATE			0x22C
> +#define ADMV4420_CP_BLEED_EN			0x22D
> +#define ADMV4420_CP_CURRENT			0x22E
> +#define ADMV4420_CP_BLEED			0x22F
> +
> +
> +#define ADMV4420_SPI_CONFIG_1_SOFTRESET_	BIT(7)
> +#define ADMV4420_SPI_CONFIG_1_SDOACTIVE_	BIT(4)
> +#define ADMV4420_SPI_CONFIG_1_SDOACTIVE		BIT(3)
> +#define ADMV4420_SPI_CONFIG_1_SOFTRESET		BIT(1)
> +
> +#define ADMV4420_REFERENCE_IN_MODE(x)		(x << 1)
> +#define ADMV4420_REFERENCE_DOUBLER(x)		(x << 2)
> +#define ADMV4420_REFERENCE_DIVIDE_BY_2_MASK	BIT(0)
> +#define ADMV4420_REFERENCE_MODE_MASK		BIT(1)
> +#define ADMV4420_REFERENCE_DOUBLER_MASK		BIT(2)
> +#define ADMV4420_REF_DIVIDER_MAX_VAL		GENMASK(9, 0)
> +#define ADMV4420_N_COUNTER_INT_MAX		GENMASK(15, 0)
> +#define ADMV4420_N_COUNTER_FRAC_MAX		GENMASK(23, 0)
> +#define ADMV4420_N_COUNTER_MOD_MAX		GENMASK(23, 0)
> +
> +#define ADMV4420_INT_L_MASK			GENMASK(7, 0)
> +#define ADMV4420_INT_H_MASK			GENMASK(15, 8)
> +#define ADMV4420_FRAC_L_MASK			GENMASK(7, 0)
> +#define ADMV4420_FRAC_M_MASK			GENMASK(15, 8)
> +#define ADMV4420_FRAC_H_MASK			GENMASK(23, 16)
> +#define ADMV4420_MOD_L_MASK			GENMASK(7, 0)
> +#define ADMV4420_MOD_M_MASK			GENMASK(15, 8)
> +#define ADMV4420_MOD_H_MASK			GENMASK(23, 16)
> +
> +#define ENABLE_PLL				BIT(6)
> +#define ENABLE_LO				BIT(5)
> +#define ENABLE_VCO				BIT(3)
> +#define ENABLE_IFAMP				BIT(2)
> +#define ENABLE_MIXER				BIT(1)
> +#define ENABLE_LNA				BIT(0)
> +
> +#define ADMV4420_SCRATCH_PAD_VAL_1              0xAD
> +#define ADMV4420_SCRATCH_PAD_VAL_2              0xEA
> +
> +#define ADMV4420_REF_FREQ_HZ                    50000000
> +#define MAX_N_COUNTER                           655360UL
> +#define MAX_R_DIVIDER                           1024
> +#define ADMV4420_DEFAULT_LO_FREQ_HZ		16750000000ULL
> +
> +enum admv4420_mux_sel {
> +	ADMV4420_LOW = 0,
> +	ADMV4420_LOCK_DTCT = 1,
> +	ADMV4420_R_COUNTER_PER_2 = 4,
> +	ADMV4420_N_CONUTER_PER_2 = 5,
> +	ADMV4420_HIGH = 8,
> +};
> +
> +struct admv4420_reference_block {
> +	bool doubler_en;
> +	bool divide_by_2_en;
> +	bool ref_single_ended;
> +	u32 divider;
> +};
> +
> +struct admv4420_n_counter {
> +	u32 int_val;
> +	u32 frac_val;
> +	u32 mod_val;
> +	u32 n_counter;
> +};
> +
> +struct admv4420_state {
> +	struct spi_device		*spi;
> +	struct regmap			*regmap;
> +	u64				pfd_freq_hz;
> +	u64				vco_freq_hz;
> +	u64				lo_freq_hz;
> +	struct admv4420_reference_block ref_block;
> +	struct admv4420_n_counter	n_counter;
> +	enum admv4420_mux_sel		mux_sel;
> +	struct mutex			lock;

All locks need a comment saying what exactly their intended scope is.

> +};
> +
> +static const struct regmap_config admv4420_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.read_flag_mask = BIT(7),
> +};
> +
> +static int admv4420_reg_access(struct iio_dev *indio_dev,
> +			       u32 reg, u32 writeval,
> +			       u32 *readval)
> +{
> +	struct admv4420_state *st = iio_priv(indio_dev);
> +
> +	if (readval)
> +		return regmap_read(st->regmap, reg, readval);
> +	else
> +		return regmap_write(st->regmap, reg, writeval);
> +}
> +
> +static int admv4420_set_n_counter(struct admv4420_state *st, u32 int_val, u32 frac_val, u32 mod_val)

Shorter lines preferred. Under 80 chars unless it significantly affects readability or breaks
up a string we might want to grep for.

> +{
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_FRAC_H, FIELD_GET(ADMV4420_FRAC_H_MASK, frac_val));

I'm not that keen on doing FIELD_GET for a simple breaking up into 8 bit chunks.
It is a rare occasion where it makes it harder to see what is going on. 

Can't you do a bulk write to set 3 registers in one function call?
Will require a DMA safe buffer but it's probably easier to read than this
repetition


> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_FRAC_M, FIELD_GET(ADMV4420_FRAC_M_MASK, frac_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_FRAC_L, FIELD_GET(ADMV4420_FRAC_L_MASK, frac_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_MOD_H, FIELD_GET(ADMV4420_MOD_H_MASK, mod_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_MOD_M, FIELD_GET(ADMV4420_MOD_M_MASK, mod_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_MOD_L, FIELD_GET(ADMV4420_MOD_L_MASK, mod_val));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_INT_H, FIELD_GET(ADMV4420_INT_H_MASK, int_val));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->regmap, ADMV4420_INT_L, FIELD_GET(ADMV4420_INT_L_MASK, int_val));
> +}
> +
> +static int admv4420_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	struct admv4420_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_FREQUENCY:
> +		*val = div_u64(st->lo_freq_hz, 1000000);
> +		div_u64_rem(st->lo_freq_hz, 1000000, val2);
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info admv4420_info = {
> +	.read_raw = admv4420_read_raw,
> +	.debugfs_reg_access = &admv4420_reg_access,
> +};
> +
> +static const struct iio_chan_spec admv4420_channels[] = {
> +	{
> +		.type = IIO_ALTVOLTAGE,
> +		.output = 0,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY),
> +	},
> +};
> +
> +static void admv4420_dt_parse(struct admv4420_state *st)
> +{
> +	struct spi_device *spi = st->spi;
> +
> +	device_property_read_u64(&spi->dev, "adi,lo_freq_hz", &st->lo_freq_hz);
> +	st->ref_block.ref_single_ended = of_property_read_bool(spi->dev.of_node,
> +							       "adi,ref_ext_single_ended_en");

Use generic fw properties from include/linux/property.h as
then we get support for other firmware types for free.

> +}
> +
> +static inline uint64_t admv4420_calc_pfd_vco(struct admv4420_state *st)
> +{
> +	return div_u64(st->vco_freq_hz * 10, st->n_counter.n_counter);
> +}
> +
> +static inline uint64_t admv4420_calc_pfd_ref(struct admv4420_state *st)
> +{
> +	uint64_t tmp;
> +
> +	tmp = ADMV4420_REF_FREQ_HZ * (st->ref_block.doubler_en ? 2 : 1);
> +	return div_u64(tmp, st->ref_block.divider * (st->ref_block.divide_by_2_en ? 2 : 1));

Pull that ternary operating out on it's own line. There is too much going on in
one statement to make it easy to read!

> +}
> +
> +static int admv4420_calc_parameters(struct admv4420_state *st)
> +{
> +	u64 pfd_ref, pfd_vco;
> +	bool sol_found = false;
> +
> +	st->ref_block.doubler_en = false;
> +	st->ref_block.divide_by_2_en = false;
> +	st->vco_freq_hz = div_u64(st->lo_freq_hz, 2);
> +
> +	for (st->ref_block.divider = 1; st->ref_block.divider < MAX_R_DIVIDER;
> +	    st->ref_block.divider++) {
> +		pfd_ref = admv4420_calc_pfd_ref(st);
> +		for (st->n_counter.n_counter = 1; st->n_counter.n_counter < MAX_N_COUNTER;
> +		    st->n_counter.n_counter++) {
> +			pfd_vco = admv4420_calc_pfd_vco(st);
> +			if (pfd_ref == pfd_vco) {
> +				sol_found = true;
> +				break;
> +			}
> +		}
> +
> +		if (sol_found)
> +			break;
> +
> +		st->n_counter.n_counter = 1;
> +	}
> +	if (!sol_found)
> +		return -1;
> +
> +	st->n_counter.int_val = div_u64_rem(st->n_counter.n_counter, 10, &st->n_counter.frac_val);
> +	st->n_counter.mod_val = 1;
> +
> +	return 0;
> +}
> +
> +static int admv4420_setup(struct iio_dev *indio_dev)
> +{
> +	struct admv4420_state *st = iio_priv(indio_dev);
> +	u32 val = 0;

This will always be set before use.  If you are seeing a compiler warning then fine to leave
it here but perhaps add a comment so no one 'cleans it up'.

> +	int ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_SPI_CONFIG_1,
> +			   ADMV4420_SPI_CONFIG_1_SOFTRESET_ | ADMV4420_SPI_CONFIG_1_SOFTRESET);

In general try to keep below 80 chars where it is easy to do like here.

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_SPI_CONFIG_1,
> +			   ADMV4420_SPI_CONFIG_1_SDOACTIVE_ | ADMV4420_SPI_CONFIG_1_SDOACTIVE);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADMV4420_SCRATCH_PAD_VAL_1);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val != ADMV4420_SCRATCH_PAD_VAL_1) {
> +		dev_err(indio_dev->dev.parent, "Failed ADMV4420 to read/write scratchpad %x ", val);
> +		return -EIO;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADMV4420_SCRATCH_PAD_VAL_2);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val != ADMV4420_SCRATCH_PAD_VAL_2) {
> +		dev_err(indio_dev->dev.parent, "Failed to read/write scratchpad %x ", val);
> +		return -EIO;
> +	}
> +
> +	st->mux_sel = ADMV4420_LOCK_DTCT;
> +	st->lo_freq_hz = ADMV4420_DEFAULT_LO_FREQ_HZ;
> +
> +	admv4420_dt_parse(st);

I doubt this needs to be dt specific so _fw_parse()

> +
> +	ret = admv4420_calc_parameters(st);
> +	if (ret) {
> +		dev_err(indio_dev->dev.parent, "Failed calc parameters for %lld ", st->vco_freq_hz);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADMV4420_R_DIV_L,
> +			   FIELD_GET(0xFF, st->ref_block.divider));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_R_DIV_H,
> +			   FIELD_GET(0xFF00, st->ref_block.divider));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_REFERENCE,
> +			   st->ref_block.divide_by_2_en |
> +			   ADMV4420_REFERENCE_IN_MODE(st->ref_block.ref_single_ended) |
> +			   ADMV4420_REFERENCE_DOUBLER(st->ref_block.doubler_en));
> +	if (ret)
> +		return ret;
> +
> +	ret = admv4420_set_n_counter(st, st->n_counter.int_val, st->n_counter.frac_val, st->n_counter.mod_val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_PLL_MUX_SEL, st->mux_sel);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADMV4420_ENABLES, ENABLE_PLL | ENABLE_LO | ENABLE_VCO |
> +			   ENABLE_IFAMP | ENABLE_MIXER | ENABLE_LNA);

return regmap_write()

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int admv4420_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct admv4420_state *st;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_spi(spi, &admv4420_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "Error  ADMV4420 initializing spi regmap: %ld\n",
> +			PTR_ERR(regmap));
> +		return PTR_ERR(regmap);

For probe errors, it is often easier to just use dev_err_probe() for all of them
whether or not they can defer.


> +	}
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->regmap = regmap;
> +	mutex_init(&st->lock);
> +
> +	indio_dev->dev.parent = &spi->dev;

We set that automatically in devm_iio_device_alloc() as it's almost always
the same value as passed in as dev to that function and in rare case
where it is different we can override. Here they are the same.

> +	indio_dev->name = "admv4420";
> +	indio_dev->info = &admv4420_info;
> +	indio_dev->channels = admv4420_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(admv4420_channels);
> +
> +	ret = admv4420_setup(indio_dev);
> +	if (ret) {
> +		dev_err(&spi->dev, "Setup ADMV4420 failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +

[PATCH v1 2/2] iio:frequency:admv4420.c: Add support for ADMV4420

[Cristian Pop]

Add support for K Band Downconverter with Integrated
Fractional-N PLL and VCO.
More info:
https://www.analog.com/en/products/admv4420.html

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
 drivers/iio/frequency/Kconfig    |  10 +
 drivers/iio/frequency/Makefile   |   1 +
 drivers/iio/frequency/admv4420.c | 412 +++++++++++++++++++++++++++++++
 3 files changed, 423 insertions(+)
 create mode 100644 drivers/iio/frequency/admv4420.c

diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
index b44036f843af..55aa63548c00 100644
--- a/drivers/iio/frequency/Kconfig
+++ b/drivers/iio/frequency/Kconfig
@@ -60,6 +60,16 @@ config ADMV1013
 	  To compile this driver as a module, choose M here: the
 	  module will be called admv1013.
 
+config ADMV4420
+	tristate "Analog Devices ADMV4420 K Band Downconverter"
+	depends on SPI && COMMON_CLK
+	help
+	  Say yes here to build support for Analog Devices K Band
+	  Downconverter with integrated Fractional-N PLL and VCO.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called admv4420.
+
 config ADRF6780
         tristate "Analog Devices ADRF6780 Microwave Upconverter"
         depends on SPI
diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
index ae6899856c99..782e5baa1630 100644
--- a/drivers/iio/frequency/Makefile
+++ b/drivers/iio/frequency/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_AD9523) += ad9523.o
 obj-$(CONFIG_ADF4350) += adf4350.o
 obj-$(CONFIG_ADF4371) += adf4371.o
 obj-$(CONFIG_ADMV1013) += admv1013.o
+obj-$(CONFIG_ADMV4420) += admv4420.o
 obj-$(CONFIG_ADRF6780) += adrf6780.o
diff --git a/drivers/iio/frequency/admv4420.c b/drivers/iio/frequency/admv4420.c
new file mode 100644
index 000000000000..f32039ab2cd3
--- /dev/null
+++ b/drivers/iio/frequency/admv4420.c
@@ -0,0 +1,412 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * ADMV4420
+ *
+ * Copyright 2021 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+/* ADMV4420 Register Map */
+#define ADMV4420_SPI_CONFIG_1			0x00
+#define ADMV4420_SPI_CONFIG_2			0x01
+#define ADMV4420_CHIPTYPE			0x03
+#define ADMV4420_PRODUCT_ID_L			0x04
+#define ADMV4420_PRODUCT_ID_H			0x05
+#define ADMV4420_SCRATCHPAD			0x0A
+#define ADMV4420_SPI_REV			0x0B
+#define ADMV4420_ENABLES			0x103
+#define ADMV4420_SDO_LEVEL			0x108
+#define ADMV4420_INT_L				0x200
+#define ADMV4420_INT_H				0x201
+#define ADMV4420_FRAC_L				0x202
+#define ADMV4420_FRAC_M				0x203
+#define ADMV4420_FRAC_H				0x204
+#define ADMV4420_MOD_L				0x208
+#define ADMV4420_MOD_M				0x209
+#define ADMV4420_MOD_H				0x20A
+#define ADMV4420_R_DIV_L			0x20C
+#define ADMV4420_R_DIV_H			0x20D
+#define ADMV4420_REFERENCE			0x20E
+#define ADMV4420_VCO_DATA_READBACK1		0x211
+#define ADMV4420_VCO_DATA_READBACK2		0x212
+#define ADMV4420_PLL_MUX_SEL			0x213
+#define ADMV4420_LOCK_DETECT			0x214
+#define ADMV4420_BAND_SELECT			0x215
+#define ADMV4420_VCO_ALC_TIMEOUT		0x216
+#define ADMV4420_VCO_MANUAL			0x217
+#define ADMV4420_ALC				0x219
+#define ADMV4420_VCO_TIMEOUT1			0x21C
+#define ADMV4420_VCO_TIMEOUT2			0x21D
+#define ADMV4420_VCO_BAND_DIV			0x21E
+#define ADMV4420_VCO_READBACK_SEL		0x21F
+#define ADMV4420_AUTOCAL			0x226
+#define ADMV4420_CP_STATE			0x22C
+#define ADMV4420_CP_BLEED_EN			0x22D
+#define ADMV4420_CP_CURRENT			0x22E
+#define ADMV4420_CP_BLEED			0x22F
+
+
+#define ADMV4420_SPI_CONFIG_1_SOFTRESET_	BIT(7)
+#define ADMV4420_SPI_CONFIG_1_SDOACTIVE_	BIT(4)
+#define ADMV4420_SPI_CONFIG_1_SDOACTIVE		BIT(3)
+#define ADMV4420_SPI_CONFIG_1_SOFTRESET		BIT(1)
+
+#define ADMV4420_REFERENCE_IN_MODE(x)		(x << 1)
+#define ADMV4420_REFERENCE_DOUBLER(x)		(x << 2)
+#define ADMV4420_REFERENCE_DIVIDE_BY_2_MASK	BIT(0)
+#define ADMV4420_REFERENCE_MODE_MASK		BIT(1)
+#define ADMV4420_REFERENCE_DOUBLER_MASK		BIT(2)
+#define ADMV4420_REF_DIVIDER_MAX_VAL		GENMASK(9, 0)
+#define ADMV4420_N_COUNTER_INT_MAX		GENMASK(15, 0)
+#define ADMV4420_N_COUNTER_FRAC_MAX		GENMASK(23, 0)
+#define ADMV4420_N_COUNTER_MOD_MAX		GENMASK(23, 0)
+
+#define ADMV4420_INT_L_MASK			GENMASK(7, 0)
+#define ADMV4420_INT_H_MASK			GENMASK(15, 8)
+#define ADMV4420_FRAC_L_MASK			GENMASK(7, 0)
+#define ADMV4420_FRAC_M_MASK			GENMASK(15, 8)
+#define ADMV4420_FRAC_H_MASK			GENMASK(23, 16)
+#define ADMV4420_MOD_L_MASK			GENMASK(7, 0)
+#define ADMV4420_MOD_M_MASK			GENMASK(15, 8)
+#define ADMV4420_MOD_H_MASK			GENMASK(23, 16)
+
+#define ENABLE_PLL				BIT(6)
+#define ENABLE_LO				BIT(5)
+#define ENABLE_VCO				BIT(3)
+#define ENABLE_IFAMP				BIT(2)
+#define ENABLE_MIXER				BIT(1)
+#define ENABLE_LNA				BIT(0)
+
+#define ADMV4420_SCRATCH_PAD_VAL_1              0xAD
+#define ADMV4420_SCRATCH_PAD_VAL_2              0xEA
+
+#define ADMV4420_REF_FREQ_HZ                    50000000
+#define MAX_N_COUNTER                           655360UL
+#define MAX_R_DIVIDER                           1024
+#define ADMV4420_DEFAULT_LO_FREQ_HZ		16750000000ULL
+
+enum admv4420_mux_sel {
+	ADMV4420_LOW = 0,
+	ADMV4420_LOCK_DTCT = 1,
+	ADMV4420_R_COUNTER_PER_2 = 4,
+	ADMV4420_N_CONUTER_PER_2 = 5,
+	ADMV4420_HIGH = 8,
+};
+
+struct admv4420_reference_block {
+	bool doubler_en;
+	bool divide_by_2_en;
+	bool ref_single_ended;
+	u32 divider;
+};
+
+struct admv4420_n_counter {
+	u32 int_val;
+	u32 frac_val;
+	u32 mod_val;
+	u32 n_counter;
+};
+
+struct admv4420_state {
+	struct spi_device		*spi;
+	struct regmap			*regmap;
+	u64				pfd_freq_hz;
+	u64				vco_freq_hz;
+	u64				lo_freq_hz;
+	struct admv4420_reference_block ref_block;
+	struct admv4420_n_counter	n_counter;
+	enum admv4420_mux_sel		mux_sel;
+	struct mutex			lock;
+};
+
+static const struct regmap_config admv4420_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = BIT(7),
+};
+
+static int admv4420_reg_access(struct iio_dev *indio_dev,
+			       u32 reg, u32 writeval,
+			       u32 *readval)
+{
+	struct admv4420_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+	else
+		return regmap_write(st->regmap, reg, writeval);
+}
+
+static int admv4420_set_n_counter(struct admv4420_state *st, u32 int_val, u32 frac_val, u32 mod_val)
+{
+	int ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_FRAC_H, FIELD_GET(ADMV4420_FRAC_H_MASK, frac_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_FRAC_M, FIELD_GET(ADMV4420_FRAC_M_MASK, frac_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_FRAC_L, FIELD_GET(ADMV4420_FRAC_L_MASK, frac_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_MOD_H, FIELD_GET(ADMV4420_MOD_H_MASK, mod_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_MOD_M, FIELD_GET(ADMV4420_MOD_M_MASK, mod_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_MOD_L, FIELD_GET(ADMV4420_MOD_L_MASK, mod_val));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_INT_H, FIELD_GET(ADMV4420_INT_H_MASK, int_val));
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, ADMV4420_INT_L, FIELD_GET(ADMV4420_INT_L_MASK, int_val));
+}
+
+static int admv4420_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	struct admv4420_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_FREQUENCY:
+		*val = div_u64(st->lo_freq_hz, 1000000);
+		div_u64_rem(st->lo_freq_hz, 1000000, val2);
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info admv4420_info = {
+	.read_raw = admv4420_read_raw,
+	.debugfs_reg_access = &admv4420_reg_access,
+};
+
+static const struct iio_chan_spec admv4420_channels[] = {
+	{
+		.type = IIO_ALTVOLTAGE,
+		.output = 0,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY),
+	},
+};
+
+static void admv4420_dt_parse(struct admv4420_state *st)
+{
+	struct spi_device *spi = st->spi;
+
+	device_property_read_u64(&spi->dev, "adi,lo_freq_hz", &st->lo_freq_hz);
+	st->ref_block.ref_single_ended = of_property_read_bool(spi->dev.of_node,
+							       "adi,ref_ext_single_ended_en");
+}
+
+static inline uint64_t admv4420_calc_pfd_vco(struct admv4420_state *st)
+{
+	return div_u64(st->vco_freq_hz * 10, st->n_counter.n_counter);
+}
+
+static inline uint64_t admv4420_calc_pfd_ref(struct admv4420_state *st)
+{
+	uint64_t tmp;
+
+	tmp = ADMV4420_REF_FREQ_HZ * (st->ref_block.doubler_en ? 2 : 1);
+	return div_u64(tmp, st->ref_block.divider * (st->ref_block.divide_by_2_en ? 2 : 1));
+}
+
+static int admv4420_calc_parameters(struct admv4420_state *st)
+{
+	u64 pfd_ref, pfd_vco;
+	bool sol_found = false;
+
+	st->ref_block.doubler_en = false;
+	st->ref_block.divide_by_2_en = false;
+	st->vco_freq_hz = div_u64(st->lo_freq_hz, 2);
+
+	for (st->ref_block.divider = 1; st->ref_block.divider < MAX_R_DIVIDER;
+	    st->ref_block.divider++) {
+		pfd_ref = admv4420_calc_pfd_ref(st);
+		for (st->n_counter.n_counter = 1; st->n_counter.n_counter < MAX_N_COUNTER;
+		    st->n_counter.n_counter++) {
+			pfd_vco = admv4420_calc_pfd_vco(st);
+			if (pfd_ref == pfd_vco) {
+				sol_found = true;
+				break;
+			}
+		}
+
+		if (sol_found)
+			break;
+
+		st->n_counter.n_counter = 1;
+	}
+	if (!sol_found)
+		return -1;
+
+	st->n_counter.int_val = div_u64_rem(st->n_counter.n_counter, 10, &st->n_counter.frac_val);
+	st->n_counter.mod_val = 1;
+
+	return 0;
+}
+
+static int admv4420_setup(struct iio_dev *indio_dev)
+{
+	struct admv4420_state *st = iio_priv(indio_dev);
+	u32 val = 0;
+	int ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_SPI_CONFIG_1,
+			   ADMV4420_SPI_CONFIG_1_SOFTRESET_ | ADMV4420_SPI_CONFIG_1_SOFTRESET);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_SPI_CONFIG_1,
+			   ADMV4420_SPI_CONFIG_1_SDOACTIVE_ | ADMV4420_SPI_CONFIG_1_SDOACTIVE);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADMV4420_SCRATCH_PAD_VAL_1);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
+	if (ret)
+		return ret;
+
+	if (val != ADMV4420_SCRATCH_PAD_VAL_1) {
+		dev_err(indio_dev->dev.parent, "Failed ADMV4420 to read/write scratchpad %x ", val);
+		return -EIO;
+	}
+
+	ret = regmap_write(st->regmap, ADMV4420_SCRATCHPAD, ADMV4420_SCRATCH_PAD_VAL_2);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, ADMV4420_SCRATCHPAD, &val);
+	if (ret)
+		return ret;
+
+	if (val != ADMV4420_SCRATCH_PAD_VAL_2) {
+		dev_err(indio_dev->dev.parent, "Failed to read/write scratchpad %x ", val);
+		return -EIO;
+	}
+
+	st->mux_sel = ADMV4420_LOCK_DTCT;
+	st->lo_freq_hz = ADMV4420_DEFAULT_LO_FREQ_HZ;
+
+	admv4420_dt_parse(st);
+
+	ret = admv4420_calc_parameters(st);
+	if (ret) {
+		dev_err(indio_dev->dev.parent, "Failed calc parameters for %lld ", st->vco_freq_hz);
+		return ret;
+	}
+
+	ret = regmap_write(st->regmap, ADMV4420_R_DIV_L,
+			   FIELD_GET(0xFF, st->ref_block.divider));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_R_DIV_H,
+			   FIELD_GET(0xFF00, st->ref_block.divider));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_REFERENCE,
+			   st->ref_block.divide_by_2_en |
+			   ADMV4420_REFERENCE_IN_MODE(st->ref_block.ref_single_ended) |
+			   ADMV4420_REFERENCE_DOUBLER(st->ref_block.doubler_en));
+	if (ret)
+		return ret;
+
+	ret = admv4420_set_n_counter(st, st->n_counter.int_val, st->n_counter.frac_val, st->n_counter.mod_val);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_PLL_MUX_SEL, st->mux_sel);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADMV4420_ENABLES, ENABLE_PLL | ENABLE_LO | ENABLE_VCO |
+			   ENABLE_IFAMP | ENABLE_MIXER | ENABLE_LNA);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int admv4420_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct admv4420_state *st;
+	struct regmap *regmap;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_spi(spi, &admv4420_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "Error  ADMV4420 initializing spi regmap: %ld\n",
+			PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->regmap = regmap;
+	mutex_init(&st->lock);
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = "admv4420";
+	indio_dev->info = &admv4420_info;
+	indio_dev->channels = admv4420_channels;
+	indio_dev->num_channels = ARRAY_SIZE(admv4420_channels);
+
+	ret = admv4420_setup(indio_dev);
+	if (ret) {
+		dev_err(&spi->dev, "Setup ADMV4420 failed (%d)\n", ret);
+		return ret;
+	}
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id admv4420_of_match[] = {
+	{ .compatible = "adi,admv4420" },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, admv4420_of_match);
+
+static struct spi_driver admv4420_driver = {
+	.driver = {
+		.name	= "admv4420",
+		.of_match_table = admv4420_of_match,
+	},
+	.probe		= admv4420_probe,
+};
+module_spi_driver(admv4420_driver);
+
+MODULE_AUTHOR("Cristian Pop <cristian.pop@analog.com>");
+MODULE_DESCRIPTION("Analog Devices ADMV44200 K Band Downconverter");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.17.1