[PATCH 0/2] Add support for AD7293 Power Amplifier

[PATCH 0/2] Add support for AD7293 Power Amplifier

[Antoniu Miclaus]

The AD7293 is a Power Amplifier drain current controller containing 
functionality for general-purpose monitoring and control of 
current, voltage, and temperature, integrated into a single chip 
solution with an SPI-compatible interface.

Datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7293.pdf

This series of patches provide an initial implementation of the
driver with access from the userspace to the:
 - ADC channels (read raw values, set range, set offset)
 - DAC channels (set code, set offset)
 - Temperature sensing (read raw values, set offset)
 - Current sensing (read raw, set offset, set gain)

Antoniu Miclaus (2):
  iio:amplifiers:ad7293: add support for AD7293
  dt-bindings:iio:amplifiers: add ad7293 doc

 .../bindings/iio/amplifiers/adi,ad7293.yaml   |  49 ++
 drivers/iio/amplifiers/Kconfig                |  11 +
 drivers/iio/amplifiers/Makefile               |   1 +
 drivers/iio/amplifiers/ad7293.c               | 794 ++++++++++++++++++
 4 files changed, 855 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/amplifiers/adi,ad7293.yaml
 create mode 100644 drivers/iio/amplifiers/ad7293.c

-- 
2.33.1

[PATCH v4 1/3] iio: frequency: admv1013: add support for ADMV1013

[Antoniu Miclaus]

The ADMV1013 is a wideband, microwave upconverter optimized
for point to point microwave radio designs operating in the
24 GHz to 44 GHz radio frequency (RF) range.

Datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/ADMV1013.pdf

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v4:
 - add extended info for the LO feedthrough offset calibration
 drivers/iio/frequency/Kconfig    |  11 +
 drivers/iio/frequency/Makefile   |   1 +
 drivers/iio/frequency/admv1013.c | 631 +++++++++++++++++++++++++++++++
 3 files changed, 643 insertions(+)
 create mode 100644 drivers/iio/frequency/admv1013.c

diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
index 240b81502512..411b3b961e46 100644
--- a/drivers/iio/frequency/Kconfig
+++ b/drivers/iio/frequency/Kconfig
@@ -49,5 +49,16 @@ config ADF4371
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called adf4371.
+
+config ADMV1013
+	tristate "Analog Devices ADMV1013 Microwave Upconverter"
+	depends on SPI && COMMON_CLK
+	help
+	  Say yes here to build support for Analog Devices ADMV1013
+	  24 GHz to 44 GHz, Wideband, Microwave Upconverter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called admv1013.
+
 endmenu
 endmenu
diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
index 518b1e50caef..559922a8196e 100644
--- a/drivers/iio/frequency/Makefile
+++ b/drivers/iio/frequency/Makefile
@@ -7,3 +7,4 @@
 obj-$(CONFIG_AD9523) += ad9523.o
 obj-$(CONFIG_ADF4350) += adf4350.o
 obj-$(CONFIG_ADF4371) += adf4371.o
+obj-$(CONFIG_ADMV1013) += admv1013.o
diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
new file mode 100644
index 000000000000..e3d99afe5ecc
--- /dev/null
+++ b/drivers/iio/frequency/admv1013.c
@@ -0,0 +1,631 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADMV1013 driver
+ *
+ * Copyright 2021 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/notifier.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+
+#include <asm/unaligned.h>
+
+/* ADMV1013 Register Map */
+#define ADMV1013_REG_SPI_CONTROL		0x00
+#define ADMV1013_REG_ALARM			0x01
+#define ADMV1013_REG_ALARM_MASKS		0x02
+#define ADMV1013_REG_ENABLE			0x03
+#define ADMV1013_REG_LO_AMP_I			0x05
+#define ADMV1013_REG_LO_AMP_Q			0x06
+#define ADMV1013_REG_OFFSET_ADJUST_I		0x07
+#define ADMV1013_REG_OFFSET_ADJUST_Q		0x08
+#define ADMV1013_REG_QUAD			0x09
+#define ADMV1013_REG_VVA_TEMP_COMP		0x0A
+
+/* ADMV1013_REG_SPI_CONTROL Map */
+#define ADMV1013_PARITY_EN_MSK			BIT(15)
+#define ADMV1013_SPI_SOFT_RESET_MSK		BIT(14)
+#define ADMV1013_CHIP_ID_MSK			GENMASK(11, 4)
+#define ADMV1013_CHIP_ID			0xA
+#define ADMV1013_REVISION_ID_MSK		GENMASK(3, 0)
+
+/* ADMV1013_REG_ALARM Map */
+#define ADMV1013_PARITY_ERROR_MSK		BIT(15)
+#define ADMV1013_TOO_FEW_ERRORS_MSK		BIT(14)
+#define ADMV1013_TOO_MANY_ERRORS_MSK		BIT(13)
+#define ADMV1013_ADDRESS_RANGE_ERROR_MSK	BIT(12)
+
+/* ADMV1013_REG_ENABLE Map */
+#define ADMV1013_VGA_PD_MSK			BIT(15)
+#define ADMV1013_MIXER_PD_MSK			BIT(14)
+#define ADMV1013_QUAD_PD_MSK			GENMASK(13, 11)
+#define ADMV1013_BG_PD_MSK			BIT(10)
+#define ADMV1013_MIXER_IF_EN_MSK		BIT(7)
+#define ADMV1013_DET_EN_MSK			BIT(5)
+
+/* ADMV1013_REG_LO_AMP Map */
+#define ADMV1013_LOAMP_PH_ADJ_FINE_MSK		GENMASK(13, 7)
+#define ADMV1013_MIXER_VGATE_MSK		GENMASK(6, 0)
+
+/* ADMV1013_REG_OFFSET_ADJUST Map */
+#define ADMV1013_MIXER_OFF_ADJ_P_MSK		GENMASK(15, 9)
+#define ADMV1013_MIXER_OFF_ADJ_N_MSK		GENMASK(8, 2)
+
+/* ADMV1013_REG_QUAD Map */
+#define ADMV1013_QUAD_SE_MODE_MSK		GENMASK(9, 6)
+#define ADMV1013_QUAD_FILTERS_MSK		GENMASK(3, 0)
+
+/* ADMV1013_REG_VVA_TEMP_COMP Map */
+#define ADMV1013_VVA_TEMP_COMP_MSK		GENMASK(15, 0)
+
+enum {
+	ADMV1013_LO_FEED_OFFSET_CALIB_P,
+	ADMV1013_LO_FEED_OFFSET_CALIB_N
+};
+
+struct admv1013_state {
+	struct spi_device	*spi;
+	struct clk		*clkin;
+	/* Protect against concurrent accesses to the device and to data */
+	struct mutex		lock;
+	struct regulator	*reg;
+	struct notifier_block	nb;
+	unsigned int		quad_se_mode;
+	bool			vga_pd;
+	bool			mixer_pd;
+	bool			quad_pd;
+	bool			bg_pd;
+	bool			mixer_if_en;
+	bool			det_en;
+	u8			data[3] ____cacheline_aligned;
+};
+
+static int __admv1013_spi_read(struct admv1013_state *st, unsigned int reg,
+			       unsigned int *val)
+{
+	int ret;
+	struct spi_transfer t = {0};
+
+	st->data[0] = 0x80 | (reg << 1);
+	st->data[1] = 0x0;
+	st->data[2] = 0x0;
+
+	t.rx_buf = &st->data[0];
+	t.tx_buf = &st->data[0];
+	t.len = 3;
+
+	ret = spi_sync_transfer(st->spi, &t, 1);
+	if (ret)
+		return ret;
+
+	*val = (get_unaligned_be24(&st->data[0]) >> 1) & GENMASK(15, 0);
+
+	return ret;
+}
+
+static int admv1013_spi_read(struct admv1013_state *st, unsigned int reg,
+			     unsigned int *val)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv1013_spi_read(st, reg, val);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int __admv1013_spi_write(struct admv1013_state *st,
+				unsigned int reg,
+				unsigned int val)
+{
+	put_unaligned_be24((val << 1) | (reg << 17), &st->data[0]);
+
+	return spi_write(st->spi, &st->data[0], 3);
+}
+
+static int admv1013_spi_write(struct admv1013_state *st, unsigned int reg,
+			      unsigned int val)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv1013_spi_write(st, reg, val);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int __admv1013_spi_update_bits(struct admv1013_state *st, unsigned int reg,
+				      unsigned int mask, unsigned int val)
+{
+	int ret;
+	unsigned int data, temp;
+
+	ret = __admv1013_spi_read(st, reg, &data);
+	if (ret)
+		return ret;
+
+	temp = (data & ~mask) | (val & mask);
+
+	return __admv1013_spi_write(st, reg, temp);
+}
+
+static int admv1013_spi_update_bits(struct admv1013_state *st, unsigned int reg,
+				    unsigned int mask, unsigned int val)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv1013_spi_update_bits(st, reg, mask, val);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static ssize_t admv1013_read(struct iio_dev *indio_dev,
+			     uintptr_t private,
+			     const struct iio_chan_spec *chan,
+			     char *buf)
+{
+	struct admv1013_state *st = iio_priv(indio_dev);
+	unsigned int data, addr;
+	int ret;
+
+	switch (chan->channel2) {
+	case IIO_MOD_I:
+		addr = ADMV1013_REG_OFFSET_ADJUST_I;
+		break;
+	case IIO_MOD_Q:
+		addr = ADMV1013_REG_OFFSET_ADJUST_Q;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = admv1013_spi_read(st, addr, &data);
+	if (ret)
+		return ret;
+
+	switch ((u32)private) {
+	case ADMV1013_LO_FEED_OFFSET_CALIB_P:
+		data = FIELD_GET(ADMV1013_MIXER_OFF_ADJ_P_MSK, data);
+		break;
+	case ADMV1013_LO_FEED_OFFSET_CALIB_N:
+		data = FIELD_GET(ADMV1013_MIXER_OFF_ADJ_N_MSK, data);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return sysfs_emit(buf, "%u\n", data);
+}
+
+static int admv1013_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	struct admv1013_state *st = iio_priv(indio_dev);
+	unsigned int data, addr;
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_PHASE:
+		if (chan->channel2 == IIO_MOD_I)
+			addr = ADMV1013_REG_LO_AMP_I;
+		else
+			addr = ADMV1013_REG_LO_AMP_Q;
+
+		ret = admv1013_spi_read(st, addr, &data);
+		if (ret)
+			return ret;
+
+		*val = FIELD_GET(ADMV1013_LOAMP_PH_ADJ_FINE_MSK, data);
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t admv1013_write(struct iio_dev *indio_dev,
+			      uintptr_t private,
+			      const struct iio_chan_spec *chan,
+			      const char *buf, size_t len)
+{
+	struct admv1013_state *st = iio_priv(indio_dev);
+	unsigned int data, addr, msk;
+	int ret;
+
+	ret = kstrtou32(buf, 10, &data);
+	if (ret)
+		return ret;
+
+	switch (chan->channel2) {
+	case IIO_MOD_I:
+		addr = ADMV1013_REG_OFFSET_ADJUST_I;
+		break;
+	case IIO_MOD_Q:
+		addr = ADMV1013_REG_OFFSET_ADJUST_Q;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch ((u32)private) {
+	case ADMV1013_LO_FEED_OFFSET_CALIB_P:
+		msk = ADMV1013_MIXER_OFF_ADJ_P_MSK;
+		data = FIELD_PREP(ADMV1013_MIXER_OFF_ADJ_P_MSK, data);
+		break;
+	case ADMV1013_LO_FEED_OFFSET_CALIB_N:
+		msk = ADMV1013_MIXER_OFF_ADJ_N_MSK;
+		data = FIELD_PREP(ADMV1013_MIXER_OFF_ADJ_N_MSK, data);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = admv1013_spi_update_bits(st, addr, msk, data);
+
+	return ret ? ret : len;
+}
+
+static int admv1013_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long info)
+{
+	struct admv1013_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_PHASE:
+		if (chan->channel2 == IIO_MOD_I)
+			return admv1013_spi_update_bits(st, ADMV1013_REG_LO_AMP_I,
+							ADMV1013_LOAMP_PH_ADJ_FINE_MSK,
+							FIELD_PREP(ADMV1013_LOAMP_PH_ADJ_FINE_MSK, val));
+		else
+			return admv1013_spi_update_bits(st, ADMV1013_REG_LO_AMP_Q,
+							ADMV1013_LOAMP_PH_ADJ_FINE_MSK,
+							FIELD_PREP(ADMV1013_LOAMP_PH_ADJ_FINE_MSK, val));
+	default:
+		return -EINVAL;
+	}
+}
+
+static int admv1013_update_quad_filters(struct admv1013_state *st)
+{
+	unsigned int filt_raw;
+	u64 rate = clk_get_rate(st->clkin);
+
+	if (rate >= (5400 * HZ_PER_MHZ) && rate <= (7000 * HZ_PER_MHZ))
+		filt_raw = 15;
+	else if (rate >= (5400 * HZ_PER_MHZ) && rate <= (8000 * HZ_PER_MHZ))
+		filt_raw = 10;
+	else if (rate >= (6600 * HZ_PER_MHZ) && rate <= (9200 * HZ_PER_MHZ))
+		filt_raw = 5;
+	else
+		filt_raw = 0;
+
+	return __admv1013_spi_update_bits(st, ADMV1013_REG_QUAD,
+					ADMV1013_QUAD_FILTERS_MSK,
+					FIELD_PREP(ADMV1013_QUAD_FILTERS_MSK, filt_raw));
+}
+
+static int admv1013_update_mixer_vgate(struct admv1013_state *st)
+{
+	unsigned int vcm, mixer_vgate;
+
+	vcm = regulator_get_voltage(st->reg);
+
+	if (vcm >= 0 && vcm < 1800000)
+		mixer_vgate = (2389 * vcm / 1000000 + 8100) / 100;
+	else if (vcm > 1800000 && vcm < 2600000)
+		mixer_vgate = (2375 * vcm / 1000000 + 125) / 100;
+	else
+		return -EINVAL;
+
+	return __admv1013_spi_update_bits(st, ADMV1013_REG_LO_AMP_I,
+				 ADMV1013_MIXER_VGATE_MSK,
+				 FIELD_PREP(ADMV1013_MIXER_VGATE_MSK, mixer_vgate));
+}
+
+static int admv1013_reg_access(struct iio_dev *indio_dev,
+			       unsigned int reg,
+			       unsigned int write_val,
+			       unsigned int *read_val)
+{
+	struct admv1013_state *st = iio_priv(indio_dev);
+
+	if (read_val)
+		return admv1013_spi_read(st, reg, read_val);
+	else
+		return admv1013_spi_write(st, reg, write_val);
+}
+
+static const struct iio_info admv1013_info = {
+	.read_raw = admv1013_read_raw,
+	.write_raw = admv1013_write_raw,
+	.debugfs_reg_access = &admv1013_reg_access,
+};
+
+static int admv1013_freq_change(struct notifier_block *nb, unsigned long action, void *data)
+{
+	struct admv1013_state *st = container_of(nb, struct admv1013_state, nb);
+	int ret;
+
+	if (action == POST_RATE_CHANGE) {
+		mutex_lock(&st->lock);
+		ret = notifier_from_errno(admv1013_update_quad_filters(st));
+		mutex_unlock(&st->lock);
+		return ret;
+	}
+
+	return NOTIFY_OK;
+}
+
+#define _ADMV1013_EXT_INFO(_name, _shared, _ident) { \
+		.name = _name, \
+		.read = admv1013_read, \
+		.write = admv1013_write, \
+		.private = _ident, \
+		.shared = _shared, \
+}
+
+static const struct iio_chan_spec_ext_info admv1013_ext_info[] = {
+	_ADMV1013_EXT_INFO("lo_feedthrough_offset_calib_positive", IIO_SEPARATE,
+			   ADMV1013_LO_FEED_OFFSET_CALIB_P),
+	_ADMV1013_EXT_INFO("lo_feedthrough_offset_calib_negative", IIO_SEPARATE,
+			   ADMV1013_LO_FEED_OFFSET_CALIB_N),
+	{ },
+};
+
+#define ADMV1013_CHAN_PHASE(_channel, rf_comp) {		\
+	.type = IIO_ALTVOLTAGE,					\
+	.modified = 1,						\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel2 = IIO_MOD_##rf_comp,				\
+	.channel = _channel,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PHASE)		\
+	}
+
+#define ADMV1013_FEED_LO_CALIB(_channel, rf_comp,  _admv1013_ext_info) {\
+	.type = IIO_ALTVOLTAGE,					\
+	.modified = 1,						\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel2 = IIO_MOD_##rf_comp,				\
+	.channel = _channel,					\
+	.ext_info = _admv1013_ext_info,				\
+	}
+
+static const struct iio_chan_spec admv1013_channels[] = {
+	ADMV1013_CHAN_PHASE(0, I),
+	ADMV1013_CHAN_PHASE(0, Q),
+	ADMV1013_FEED_LO_CALIB(0, I, admv1013_ext_info),
+	ADMV1013_FEED_LO_CALIB(0, Q, admv1013_ext_info),
+};
+
+static int admv1013_init(struct admv1013_state *st)
+{
+	int ret;
+	unsigned int chip_id, enable_reg, enable_reg_msk;
+	struct spi_device *spi = st->spi;
+
+	/* Perform a software reset */
+	ret = __admv1013_spi_update_bits(st, ADMV1013_REG_SPI_CONTROL,
+					 ADMV1013_SPI_SOFT_RESET_MSK,
+					 FIELD_PREP(ADMV1013_SPI_SOFT_RESET_MSK, 1));
+	if (ret)
+		return ret;
+
+	ret = __admv1013_spi_update_bits(st, ADMV1013_REG_SPI_CONTROL,
+					 ADMV1013_SPI_SOFT_RESET_MSK,
+					 FIELD_PREP(ADMV1013_SPI_SOFT_RESET_MSK, 0));
+	if (ret)
+		return ret;
+
+	ret = __admv1013_spi_read(st, ADMV1013_REG_SPI_CONTROL, &chip_id);
+	if (ret)
+		return ret;
+
+	chip_id = FIELD_GET(ADMV1013_CHIP_ID_MSK, chip_id);
+	if (chip_id != ADMV1013_CHIP_ID) {
+		dev_err(&spi->dev, "Invalid Chip ID.\n");
+		return -EINVAL;
+	}
+
+	ret = __admv1013_spi_write(st, ADMV1013_REG_VVA_TEMP_COMP, 0xE700);
+	if (ret)
+		return ret;
+
+	ret = __admv1013_spi_update_bits(st, ADMV1013_REG_QUAD,
+					 ADMV1013_QUAD_SE_MODE_MSK,
+					 FIELD_PREP(ADMV1013_QUAD_SE_MODE_MSK, st->quad_se_mode));
+	if (ret)
+		return ret;
+
+	ret = admv1013_update_mixer_vgate(st);
+	if (ret)
+		return ret;
+
+	ret = admv1013_update_quad_filters(st);
+	if (ret)
+		return ret;
+
+	enable_reg_msk = ADMV1013_VGA_PD_MSK |
+			ADMV1013_MIXER_PD_MSK |
+			ADMV1013_QUAD_PD_MSK |
+			ADMV1013_BG_PD_MSK |
+			ADMV1013_MIXER_IF_EN_MSK |
+			ADMV1013_DET_EN_MSK;
+
+	enable_reg = FIELD_PREP(ADMV1013_VGA_PD_MSK, st->vga_pd) |
+			FIELD_PREP(ADMV1013_MIXER_PD_MSK, st->mixer_pd) |
+			FIELD_PREP(ADMV1013_QUAD_PD_MSK, st->quad_pd ? 7 : 0) |
+			FIELD_PREP(ADMV1013_BG_PD_MSK, st->bg_pd) |
+			FIELD_PREP(ADMV1013_MIXER_IF_EN_MSK, st->mixer_if_en) |
+			FIELD_PREP(ADMV1013_DET_EN_MSK, st->det_en);
+
+	return __admv1013_spi_update_bits(st, ADMV1013_REG_ENABLE, enable_reg_msk, enable_reg);
+}
+
+static void admv1013_clk_disable(void *data)
+{
+	clk_disable_unprepare(data);
+}
+
+static void admv1013_reg_disable(void *data)
+{
+	regulator_disable(data);
+}
+
+static void admv1013_powerdown(void *data)
+{
+	unsigned int enable_reg, enable_reg_msk;
+
+	/* Disable all components in the Enable Register */
+	enable_reg_msk = ADMV1013_VGA_PD_MSK |
+			ADMV1013_MIXER_PD_MSK |
+			ADMV1013_QUAD_PD_MSK |
+			ADMV1013_BG_PD_MSK |
+			ADMV1013_MIXER_IF_EN_MSK |
+			ADMV1013_DET_EN_MSK;
+
+	enable_reg = FIELD_PREP(ADMV1013_VGA_PD_MSK, 1) |
+			FIELD_PREP(ADMV1013_MIXER_PD_MSK, 1) |
+			FIELD_PREP(ADMV1013_QUAD_PD_MSK, 7) |
+			FIELD_PREP(ADMV1013_BG_PD_MSK, 1) |
+			FIELD_PREP(ADMV1013_MIXER_IF_EN_MSK, 0) |
+			FIELD_PREP(ADMV1013_DET_EN_MSK, 0);
+
+	admv1013_spi_update_bits(data, ADMV1013_REG_ENABLE, enable_reg_msk, enable_reg);
+}
+
+static int admv1013_properties_parse(struct admv1013_state *st)
+{
+	int ret;
+	struct spi_device *spi = st->spi;
+
+	st->vga_pd = device_property_read_bool(&spi->dev, "adi,vga-powerdown");
+	st->mixer_pd = device_property_read_bool(&spi->dev, "adi,mixer-powerdown");
+	st->quad_pd = device_property_read_bool(&spi->dev, "adi,quad-powerdown");
+	st->bg_pd = device_property_read_bool(&spi->dev, "adi,bg-powerdown");
+	st->mixer_if_en = device_property_read_bool(&spi->dev, "adi,mixer-if-enable");
+	st->det_en = device_property_read_bool(&spi->dev, "adi,detector-enable");
+
+	ret = device_property_read_u32(&spi->dev, "adi,quad-se-mode", &st->quad_se_mode);
+	if (ret)
+		st->quad_se_mode = 12;
+
+	st->reg = devm_regulator_get(&spi->dev, "vcm");
+	if (IS_ERR(st->reg))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
+				     "failed to get the common-mode voltage\n");
+
+	st->clkin = devm_clk_get(&spi->dev, "lo_in");
+	if (IS_ERR(st->clkin))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
+				     "failed to get the LO input clock\n");
+
+	return 0;
+}
+
+static int admv1013_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct admv1013_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	indio_dev->info = &admv1013_info;
+	indio_dev->name = "admv1013";
+	indio_dev->channels = admv1013_channels;
+	indio_dev->num_channels = ARRAY_SIZE(admv1013_channels);
+
+	st->spi = spi;
+
+	ret = admv1013_properties_parse(st);
+	if (ret)
+		return ret;
+
+	ret = regulator_enable(st->reg);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified Common-Mode Voltage!\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(&spi->dev, admv1013_reg_disable,
+				       st->reg);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(st->clkin);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&spi->dev, admv1013_clk_disable, st->clkin);
+	if (ret)
+		return ret;
+
+	st->nb.notifier_call = admv1013_freq_change;
+	ret = devm_clk_notifier_register(&spi->dev, st->clkin, &st->nb);
+	if (ret)
+		return ret;
+
+	mutex_init(&st->lock);
+
+	ret = admv1013_init(st);
+	if (ret) {
+		dev_err(&spi->dev, "admv1013 init failed\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(&spi->dev, admv1013_powerdown, st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id admv1013_id[] = {
+	{ "admv1013", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, admv1013_id);
+
+static const struct of_device_id admv1013_of_match[] = {
+	{ .compatible = "adi,admv1013" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, admv1013_of_match);
+
+static struct spi_driver admv1013_driver = {
+	.driver = {
+		.name = "admv1013",
+		.of_match_table = admv1013_of_match,
+	},
+	.probe = admv1013_probe,
+	.id_table = admv1013_id,
+};
+module_spi_driver(admv1013_driver);
+
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com");
+MODULE_DESCRIPTION("Analog Devices ADMV1013");
+MODULE_LICENSE("GPL v2");
-- 
2.33.1

Re: [PATCH v4 1/3] iio: frequency: admv1013: add support for ADMV1013

[Jonathan Cameron]

On Fri, 5 Nov 2021 13:29:28 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> The ADMV1013 is a wideband, microwave upconverter optimized
> for point to point microwave radio designs operating in the
> 24 GHz to 44 GHz radio frequency (RF) range.
> 
> Datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/ADMV1013.pdf
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

Hmm. Representing the LO signal as a clock seems a little interesting
but I guess that does let us use the clock framework to at least understand
it's frequency etc.

A few fairly trivial comments inline, but most of the discussion around this
is going to focus on ABI + DT bindings in the other patches.

Jonathan

> ---
> changes in v4:
>  - add extended info for the LO feedthrough offset calibration
>  drivers/iio/frequency/Kconfig    |  11 +
>  drivers/iio/frequency/Makefile   |   1 +
>  drivers/iio/frequency/admv1013.c | 631 +++++++++++++++++++++++++++++++
>  3 files changed, 643 insertions(+)
>  create mode 100644 drivers/iio/frequency/admv1013.c
> 
> diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
> index 240b81502512..411b3b961e46 100644
> --- a/drivers/iio/frequency/Kconfig
> +++ b/drivers/iio/frequency/Kconfig
> @@ -49,5 +49,16 @@ config ADF4371
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called adf4371.
> +
> +config ADMV1013
> +	tristate "Analog Devices ADMV1013 Microwave Upconverter"
> +	depends on SPI && COMMON_CLK
> +	help
> +	  Say yes here to build support for Analog Devices ADMV1013
> +	  24 GHz to 44 GHz, Wideband, Microwave Upconverter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called admv1013.
> +
>  endmenu
>  endmenu
> diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
> index 518b1e50caef..559922a8196e 100644
> --- a/drivers/iio/frequency/Makefile
> +++ b/drivers/iio/frequency/Makefile
> @@ -7,3 +7,4 @@
>  obj-$(CONFIG_AD9523) += ad9523.o
>  obj-$(CONFIG_ADF4350) += adf4350.o
>  obj-$(CONFIG_ADF4371) += adf4371.o
> +obj-$(CONFIG_ADMV1013) += admv1013.o
> diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
> new file mode 100644
> index 000000000000..e3d99afe5ecc
> --- /dev/null
> +++ b/drivers/iio/frequency/admv1013.c
> @@ -0,0 +1,631 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADMV1013 driver
> + *
> + * Copyright 2021 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/notifier.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/units.h>
> +
> +#include <asm/unaligned.h>
> +

> +
> +struct admv1013_state {
> +	struct spi_device	*spi;
> +	struct clk		*clkin;
> +	/* Protect against concurrent accesses to the device and to data */
> +	struct mutex		lock;
> +	struct regulator	*reg;
> +	struct notifier_block	nb;
> +	unsigned int		quad_se_mode;
> +	bool			vga_pd;
> +	bool			mixer_pd;
> +	bool			quad_pd;
> +	bool			bg_pd;
> +	bool			mixer_if_en;
> +	bool			det_en;
> +	u8			data[3] ____cacheline_aligned;
> +};
> +
> +static int __admv1013_spi_read(struct admv1013_state *st, unsigned int reg,
> +			       unsigned int *val)
> +{
> +	int ret;
> +	struct spi_transfer t = {0};
> +
> +	st->data[0] = 0x80 | (reg << 1);

FIELD_PREP()

> +	st->data[1] = 0x0;
> +	st->data[2] = 0x0;
> +
> +	t.rx_buf = &st->data[0];
> +	t.tx_buf = &st->data[0];
> +	t.len = 3;
> +
> +	ret = spi_sync_transfer(st->spi, &t, 1);
> +	if (ret)
> +		return ret;
> +
> +	*val = (get_unaligned_be24(&st->data[0]) >> 1) & GENMASK(15, 0);

FIELD_GET and GENMASK(16, 1) is slightly more readable.

> +
> +	return ret;
> +}
> +
> +
> +static int __admv1013_spi_write(struct admv1013_state *st,
> +				unsigned int reg,
> +				unsigned int val)
> +{
> +	put_unaligned_be24((val << 1) | (reg << 17), &st->data[0]);

Looks like a place where FIELD_PREP and appropriate masks would be useful.

> +
> +	return spi_write(st->spi, &st->data[0], 3);
> +}

blank line.

...

> +
> +static int admv1013_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long info)
> +{
> +	struct admv1013_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_PHASE:
> +		if (chan->channel2 == IIO_MOD_I)
> +			return admv1013_spi_update_bits(st, ADMV1013_REG_LO_AMP_I,
> +							ADMV1013_LOAMP_PH_ADJ_FINE_MSK,

line too long.  Uses some local variables for the various parameters to reduce the length.

> +							FIELD_PREP(ADMV1013_LOAMP_PH_ADJ_FINE_MSK, val));
> +		else
> +			return admv1013_spi_update_bits(st, ADMV1013_REG_LO_AMP_Q,
> +							ADMV1013_LOAMP_PH_ADJ_FINE_MSK,
> +							FIELD_PREP(ADMV1013_LOAMP_PH_ADJ_FINE_MSK, val));
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int admv1013_update_quad_filters(struct admv1013_state *st)
> +{
> +	unsigned int filt_raw;
> +	u64 rate = clk_get_rate(st->clkin);
> +
> +	if (rate >= (5400 * HZ_PER_MHZ) && rate <= (7000 * HZ_PER_MHZ))
> +		filt_raw = 15;
> +	else if (rate >= (5400 * HZ_PER_MHZ) && rate <= (8000 * HZ_PER_MHZ))
> +		filt_raw = 10;
> +	else if (rate >= (6600 * HZ_PER_MHZ) && rate <= (9200 * HZ_PER_MHZ))
> +		filt_raw = 5;
> +	else
> +		filt_raw = 0;
> +
> +	return __admv1013_spi_update_bits(st, ADMV1013_REG_QUAD,
> +					ADMV1013_QUAD_FILTERS_MSK,
> +					FIELD_PREP(ADMV1013_QUAD_FILTERS_MSK, filt_raw));
> +}
> +
> +static int admv1013_update_mixer_vgate(struct admv1013_state *st)
> +{
> +	unsigned int vcm, mixer_vgate;
> +
> +	vcm = regulator_get_voltage(st->reg);
> +
> +	if (vcm >= 0 && vcm < 1800000)
> +		mixer_vgate = (2389 * vcm / 1000000 + 8100) / 100;
> +	else if (vcm > 1800000 && vcm < 2600000)
> +		mixer_vgate = (2375 * vcm / 1000000 + 125) / 100;
> +	else
> +		return -EINVAL;
> +
> +	return __admv1013_spi_update_bits(st, ADMV1013_REG_LO_AMP_I,
> +				 ADMV1013_MIXER_VGATE_MSK,
> +				 FIELD_PREP(ADMV1013_MIXER_VGATE_MSK, mixer_vgate));
> +}
> +
> +static int admv1013_reg_access(struct iio_dev *indio_dev,
> +			       unsigned int reg,
> +			       unsigned int write_val,
> +			       unsigned int *read_val)
> +{
> +	struct admv1013_state *st = iio_priv(indio_dev);
> +
> +	if (read_val)
> +		return admv1013_spi_read(st, reg, read_val);
> +	else
> +		return admv1013_spi_write(st, reg, write_val);
> +}
> +
> +static const struct iio_info admv1013_info = {
> +	.read_raw = admv1013_read_raw,
> +	.write_raw = admv1013_write_raw,
> +	.debugfs_reg_access = &admv1013_reg_access,
> +};
> +
> +static int admv1013_freq_change(struct notifier_block *nb, unsigned long action, void *data)
> +{
> +	struct admv1013_state *st = container_of(nb, struct admv1013_state, nb);
> +	int ret;
> +
> +	if (action == POST_RATE_CHANGE) {
> +		mutex_lock(&st->lock);
> +		ret = notifier_from_errno(admv1013_update_quad_filters(st));
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +#define _ADMV1013_EXT_INFO(_name, _shared, _ident) { \
> +		.name = _name, \
> +		.read = admv1013_read, \
> +		.write = admv1013_write, \
> +		.private = _ident, \
> +		.shared = _shared, \
> +}
> +
> +static const struct iio_chan_spec_ext_info admv1013_ext_info[] = {
> +	_ADMV1013_EXT_INFO("lo_feedthrough_offset_calib_positive", IIO_SEPARATE,
> +			   ADMV1013_LO_FEED_OFFSET_CALIB_P),
> +	_ADMV1013_EXT_INFO("lo_feedthrough_offset_calib_negative", IIO_SEPARATE,
> +			   ADMV1013_LO_FEED_OFFSET_CALIB_N),
> +	{ },
> +};
> +
> +#define ADMV1013_CHAN_PHASE(_channel, rf_comp) {		\
> +	.type = IIO_ALTVOLTAGE,					\
> +	.modified = 1,						\
> +	.output = 1,						\
> +	.indexed = 1,						\
> +	.channel2 = IIO_MOD_##rf_comp,				\
> +	.channel = _channel,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PHASE)		\
> +	}
> +
> +#define ADMV1013_FEED_LO_CALIB(_channel, rf_comp,  _admv1013_ext_info) {\
> +	.type = IIO_ALTVOLTAGE,					\
> +	.modified = 1,						\
> +	.output = 1,						\
> +	.indexed = 1,						\
> +	.channel2 = IIO_MOD_##rf_comp,				\
> +	.channel = _channel,					\
> +	.ext_info = _admv1013_ext_info,				\
> +	}
> +
> +static const struct iio_chan_spec admv1013_channels[] = {
> +	ADMV1013_CHAN_PHASE(0, I),
> +	ADMV1013_CHAN_PHASE(0, Q),
> +	ADMV1013_FEED_LO_CALIB(0, I, admv1013_ext_info),
> +	ADMV1013_FEED_LO_CALIB(0, Q, admv1013_ext_info),
> +};
> +
> +static int admv1013_init(struct admv1013_state *st)
> +{
> +	int ret;
> +	unsigned int chip_id, enable_reg, enable_reg_msk;
> +	struct spi_device *spi = st->spi;
> +
> +	/* Perform a software reset */
> +	ret = __admv1013_spi_update_bits(st, ADMV1013_REG_SPI_CONTROL,
> +					 ADMV1013_SPI_SOFT_RESET_MSK,
> +					 FIELD_PREP(ADMV1013_SPI_SOFT_RESET_MSK, 1));
> +	if (ret)
> +		return ret;
> +
> +	ret = __admv1013_spi_update_bits(st, ADMV1013_REG_SPI_CONTROL,
> +					 ADMV1013_SPI_SOFT_RESET_MSK,
> +					 FIELD_PREP(ADMV1013_SPI_SOFT_RESET_MSK, 0));
> +	if (ret)
> +		return ret;
> +
> +	ret = __admv1013_spi_read(st, ADMV1013_REG_SPI_CONTROL, &chip_id);
> +	if (ret)
> +		return ret;
> +
> +	chip_id = FIELD_GET(ADMV1013_CHIP_ID_MSK, chip_id);
> +	if (chip_id != ADMV1013_CHIP_ID) {
> +		dev_err(&spi->dev, "Invalid Chip ID.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = __admv1013_spi_write(st, ADMV1013_REG_VVA_TEMP_COMP, 0xE700);
> +	if (ret)
> +		return ret;
> +
> +	ret = __admv1013_spi_update_bits(st, ADMV1013_REG_QUAD,
> +					 ADMV1013_QUAD_SE_MODE_MSK,
> +					 FIELD_PREP(ADMV1013_QUAD_SE_MODE_MSK, st->quad_se_mode));

Try to avoid very long lines where it doesn't affect readability.

> +	if (ret)
> +		return ret;
> +
> +	ret = admv1013_update_mixer_vgate(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = admv1013_update_quad_filters(st);
> +	if (ret)
> +		return ret;
> +
> +	enable_reg_msk = ADMV1013_VGA_PD_MSK |
> +			ADMV1013_MIXER_PD_MSK |
> +			ADMV1013_QUAD_PD_MSK |
> +			ADMV1013_BG_PD_MSK |
> +			ADMV1013_MIXER_IF_EN_MSK |
> +			ADMV1013_DET_EN_MSK;
> +
> +	enable_reg = FIELD_PREP(ADMV1013_VGA_PD_MSK, st->vga_pd) |
> +			FIELD_PREP(ADMV1013_MIXER_PD_MSK, st->mixer_pd) |
> +			FIELD_PREP(ADMV1013_QUAD_PD_MSK, st->quad_pd ? 7 : 0) |
> +			FIELD_PREP(ADMV1013_BG_PD_MSK, st->bg_pd) |
> +			FIELD_PREP(ADMV1013_MIXER_IF_EN_MSK, st->mixer_if_en) |
> +			FIELD_PREP(ADMV1013_DET_EN_MSK, st->det_en);
> +
> +	return __admv1013_spi_update_bits(st, ADMV1013_REG_ENABLE, enable_reg_msk, enable_reg);
> +}
...

> +
> +static int admv1013_properties_parse(struct admv1013_state *st)
> +{
> +	int ret;
> +	struct spi_device *spi = st->spi;
> +
> +	st->vga_pd = device_property_read_bool(&spi->dev, "adi,vga-powerdown");
> +	st->mixer_pd = device_property_read_bool(&spi->dev, "adi,mixer-powerdown");
> +	st->quad_pd = device_property_read_bool(&spi->dev, "adi,quad-powerdown");
> +	st->bg_pd = device_property_read_bool(&spi->dev, "adi,bg-powerdown");
> +	st->mixer_if_en = device_property_read_bool(&spi->dev, "adi,mixer-if-enable");
> +	st->det_en = device_property_read_bool(&spi->dev, "adi,detector-enable");

Comments on these in the binding doc review.



> +
> +	ret = device_property_read_u32(&spi->dev, "adi,quad-se-mode", &st->quad_se_mode);
> +	if (ret)
> +		st->quad_se_mode = 12;
> +
> +	st->reg = devm_regulator_get(&spi->dev, "vcm");
> +	if (IS_ERR(st->reg))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
> +				     "failed to get the common-mode voltage\n");
> +
> +	st->clkin = devm_clk_get(&spi->dev, "lo_in");
> +	if (IS_ERR(st->clkin))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
> +				     "failed to get the LO input clock\n");
> +
> +	return 0;
> +}
> +

[PATCH v4 2/3] dt-bindings: iio: frequency: add admv1013 doc

[Antoniu Miclaus]

Add device tree bindings for the ADMV1013 Upconverter.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../bindings/iio/frequency/adi,admv1013.yaml  | 119 ++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
new file mode 100644
index 000000000000..47993253a586
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,admv1013.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADMV1013 Microwave Upconverter
+
+maintainers:
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+   Wideband, microwave upconverter optimized for point to point microwave
+   radio designs operating in the 24 GHz to 44 GHz frequency range.
+
+   https://www.analog.com/en/products/admv1013.html
+
+properties:
+  compatible:
+    enum:
+      - adi,admv1013
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 1000000
+
+  clocks:
+    description:
+      Definition of the external clock.
+    minItems: 1
+
+  clock-names:
+    items:
+      - const: lo_in
+
+  clock-output-names:
+    maxItems: 1
+
+  vcm-supply:
+    description:
+      Analog voltage regulator.
+
+  adi,vga-powerdown:
+    description:
+      Power Down the Voltage Gain Amplifier Circuit available at
+      BG_RBIAS2 pin.
+    type: boolean
+
+  adi,mixer-powerdown:
+    description:
+      Power Down the Mixer Circuit. Enable to put the block in
+      a power down state.
+    type: boolean
+
+  adi,quad-powerdown:
+    description:
+      Power Down the Quadrupler. Enable to put the block in
+      a power down state.
+    type: boolean
+
+  adi,bg-powerdown:
+    description:
+      Power Down the Transmitter Band Gap. Enable to put the part in
+      a power down state.
+    type: boolean
+
+  adi,mixer-if-enable:
+    description:
+      Enable the Intermediate Frequency Mode. Either IF Mode or I/Q Mode
+      can be enabled at a time.
+    type: boolean
+
+  adi,detector-enable:
+    description:
+      Enable the Envelope Detector available at output pins VENV_P and
+      VENV_N. Disable to reduce power consumption.
+    type: boolean
+
+  adi,quad-se-mode:
+    description:
+      Switch the LO path from differential to single-ended operation.
+      Set value 6 for Single-Ended Mode, Negative Side Disabled.
+      Set value 9 for Single-Ended Mode, Positive Side Disabled.
+      Set value 12 for Differential Mode.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [6, 9, 12]
+
+  '#clock-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - vcm-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      admv1013@0{
+        compatible = "adi,admv1013";
+        reg = <0>;
+        spi-max-frequency = <1000000>;
+        clocks = <&admv1013_lo>;
+        clock-names = "lo_in";
+        vcm-supply = <&vcm>;
+        adi,quad-se-mode = <12>;
+        adi,mixer-if-enable;
+        adi,detector-enable;
+      };
+    };
+...
-- 
2.33.1

Re: [PATCH v4 2/3] dt-bindings: iio: frequency: add admv1013 doc

[Rob Herring]

On Fri, 05 Nov 2021 13:29:29 +0200, Antoniu Miclaus wrote:
> Add device tree bindings for the ADMV1013 Upconverter.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  .../bindings/iio/frequency/adi,admv1013.yaml  | 119 ++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 2/3] dt-bindings: iio: frequency: add admv1013 doc

[Jonathan Cameron]

On Fri, 5 Nov 2021 13:29:29 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add device tree bindings for the ADMV1013 Upconverter.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

My high frequency theory etc is rather rusty being nearly 20 years since I last
looked at this stuff so I may have some interpretations below wrong...

My main concern here is that most of the powerdown entries prevent the device
doing anything useful so as far as I can tell would make no sense to ever set.

Jonathan

> ---
>  .../bindings/iio/frequency/adi,admv1013.yaml  | 119 ++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
> new file mode 100644
> index 000000000000..47993253a586
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,admv1013.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADMV1013 Microwave Upconverter
> +
> +maintainers:
> +  - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +description: |
> +   Wideband, microwave upconverter optimized for point to point microwave
> +   radio designs operating in the 24 GHz to 44 GHz frequency range.
> +
> +   https://www.analog.com/en/products/admv1013.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,admv1013
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 1000000
> +
> +  clocks:
> +    description:
> +      Definition of the external clock.

I mention elsewhere that this isn't exactly a clock in the normal sense
given we are modulating a signal onto it's frequency multiplied form but
meh I guess it might be fed by something looking clock like...

> +    minItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: lo_in
> +
> +  clock-output-names:
> +    maxItems: 1

Does it actually make sense to handle the output as a clock? It has a multiple
GHz bandwidth.

> +
> +  vcm-supply:
> +    description:
> +      Analog voltage regulator.

> +
> +  adi,vga-powerdown:
> +    description:
> +      Power Down the Voltage Gain Amplifier Circuit available at
> +      BG_RBIAS2 pin.

Is that all it does?  Datasheet is a bit vague, but I'm guessing this
also leaves the device not doing anything useful so makes little sense
to expose.

> +    type: boolean
> +
> +  adi,mixer-powerdown:
> +    description:
> +      Power Down the Mixer Circuit. Enable to put the block in
> +      a power down state.
As below. Device doesn't do anything useful with the mixer disabled
so I don't think this is something for DT.
> +    type: boolean
> +
> +  adi,quad-powerdown:
> +    description:
> +      Power Down the Quadrupler. Enable to put the block in
> +      a power down state.

I'm struggling a bit with the affect of this when reading the datasheet, but
I think it means the device is pointless as we have no LO signal and hence no
output.  Hence I don't think we should have this in DT.

> +    type: boolean
> +
> +  adi,bg-powerdown:
> +    description:
> +      Power Down the Transmitter Band Gap. Enable to put the part in
> +      a power down state.
> +    type: boolean
Another one where I think disabling it in DT makes the device pointless.
These are absolutely things you might want to wire up in power management
callbacks, but not have in DT.

> +
> +  adi,mixer-if-enable:
> +    description:
> +      Enable the Intermediate Frequency Mode. Either IF Mode or I/Q Mode
> +      can be enabled at a time.
How do we enable IQ mode?   Is this better described as a choice of mode?
IQ vs IF?  If so make it an appropriate enum rather than this which kind
of suggests it's enabling the mixer which is not hte case.

> +    type: boolean
> +
> +  adi,detector-enable:
> +    description:
> +      Enable the Envelope Detector available at output pins VENV_P and
> +      VENV_N. Disable to reduce power consumption.

So the reason you'd turn this off is it isn't wired to anything.  What might
it be wired to?  My guess is it would be to an ADC, probably also wired to the
host processor?   If so this should be represented as a consumer node of the
ADC so that we can provide the envelope reading if it is wired up.  If it is
not wired up then we turn it off.

> +    type: boolean
> +
> +  adi,quad-se-mode:
> +    description:
> +      Switch the LO path from differential to single-ended operation.
> +      Set value 6 for Single-Ended Mode, Negative Side Disabled.
> +      Set value 9 for Single-Ended Mode, Positive Side Disabled.
> +      Set value 12 for Differential Mode.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [6, 9, 12]

Datasheet is rather unclear on this to my mind.  From the point of view
of how it is used it is about the LO inputs, not the quadrupler at all,
that just happens to be the first unit to care about this.
I'm also not keen on magic numbers where they can be avoided and these
are particularly 'magic'.

So maybe
adi,local-oscilator-mode
enum [diff, se-pos, se-neg] 

?
> +
> +  '#clock-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - vcm-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      admv1013@0{
> +        compatible = "adi,admv1013";
> +        reg = <0>;
> +        spi-max-frequency = <1000000>;
> +        clocks = <&admv1013_lo>;
> +        clock-names = "lo_in";
> +        vcm-supply = <&vcm>;
> +        adi,quad-se-mode = <12>;
> +        adi,mixer-if-enable;
> +        adi,detector-enable;
> +      };
> +    };
> +...

[PATCH v4 3/3] Documentation:ABI:testing:admv1013: add ABI docs

[Antoniu Miclaus]

Add documentation for the use of the Local Oscillator Feedthrough Offset
calibration.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../testing/sysfs-bus-iio-frequency-admv1013  | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013
new file mode 100644
index 000000000000..f52cd55a66c6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013
@@ -0,0 +1,27 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_i_lo_feedthrough_offset_calib_positive
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration I Positive
+		in the Intermediate Frequency mode.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_i_lo_feedthrough_offset_calib_negative
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration I Negative
+		in the Intermediate Frequency mode.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_q_lo_feedthrough_offset_calib_positive
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration Q Positive
+		in the Intermediate Frequency mode.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_q_lo_feedthrough_offset_calib_negative
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration Q Negative
+		in the Intermediate Frequency mode.
-- 
2.33.1

Re: [PATCH v4 3/3] Documentation:ABI:testing:admv1013: add ABI docs

[Jonathan Cameron]

On Fri, 5 Nov 2021 13:29:30 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add documentation for the use of the Local Oscillator Feedthrough Offset
> calibration.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  .../testing/sysfs-bus-iio-frequency-admv1013  | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013
> new file mode 100644
> index 000000000000..f52cd55a66c6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1013
> @@ -0,0 +1,27 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_i_lo_feedthrough_offset_calib_positive
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration I Positive
> +		in the Intermediate Frequency mode.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_i_lo_feedthrough_offset_calib_negative
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration I Negative
> +		in the Intermediate Frequency mode.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_q_lo_feedthrough_offset_calib_positive
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration Q Positive
> +		in the Intermediate Frequency mode.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_q_lo_feedthrough_offset_calib_negative
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Read/write raw value for the Local Oscillatior Feedthrough Offset Calibration Q Negative
> +		in the Intermediate Frequency mode.

Hmm. 
Unfortuantely we never came to a clear conclusion on ABI in previous versions. 
However whatever we do should remain in line with the current ABI and I'm not sure this does.
We tend to call calibration offsets calibbias (though the reasons are probably lost the mists
of time). 

Let's see if I can work out what these actually are.

LO stands for Local Oscillator and is effectively one of the inputs to the device (mixed
with the baseband to produce the output).  Device doing either single side band, or Quadrature
modulation.

This particular tweak is a trick to remove dc offsets in the internal mixer or coupling
across different signals. (CARRIER FEEDTHROUGH NULLING) page 26 of the datasheet.

So what do we call these.  Question 1, are they on the input or the output?  Kind of in the middle
so even that part isn't obvious.  I'd argue they are effectively on the input signal but could
be persuaded that wasn't true.  They also only apply in Single side band (IF) mode...

So if we define the device as having LO band baseband inputs, then we'd have the following.
(some of these we won't actually expose as there aren't any controls on them)

in_altvoltage0 -.. positive LO input.
in_altvoltage1 -.. negative LO input.
in_altvoltage0-1 .. LO input
/* For quadrature modulation */
in_altvoltage2 - .. I_P positive baseband input to be modulated as I
in_altvoltage3 - .. I_N negative baseband input to be modulated as I
in_altvoltage2-3 -  The differential baseband input to be modulated as I.

in_altvoltage4 - .. Q_P positive baseband input to be modulated as Q
in_altvoltage5 - .. Q_N negative baseband input to be modulated as Q
in_altvoltage4-5 - The differential baseband input to be modulated as I.

/* In Single Side Band IF Mode */
in_altvoltage2 - .. IF_I Single Side band signal modulated as I
in_altvoltage3 - .. IF_Q Single Side band signal modulated as Q

In all cases output would be only one as there is only one pin.
 
out_voltage0

Now we can't have differential channels with I and Q without cheating
and using ext_info instead of the auto created elements, however the intent
is to keep the naming convention inline with what we will have should
we ever extend IIO to separate channel2 uses.  The LO vs IF vs I can be sensibly
handled with channel labels I think rather than adding even more info to the
naming.

So my reading of the datasheet suggests we should have something like

This is messy because we have two uses of the single input - one for I and one for Q.
in_altvoltage0_q_calibbias - LO feedthrough Q offset on positive side. 
in_altvotlage1_q_calibbias - LO feedthrough Q offset on negative side.
in_altvoltage0_n_calibbias - LO feedthrough N offset on positive side.
in_altvoltage1_n_calibbias - LO feedhtrough N offset on negative side.

These next two are awkwards as they are phase shifts of a differential channel after
using it to generate the internal LO_Q and LO_N signals.
in_altvoltage0-1_q_phase - LO path quadrature Q phase shift
in_altvoltate0-1_n_phase - LO path quadrature N phase shift

Slightly amusingly there aren't any obvious things to hang on an output channel
except for the envelope detector if that's wired somewhere useful and maybe the
VVA temperature compensation or the two Voltage Control Variable Attenuators
if you expose those as being wired up to regulators so we can know their voltage
and hence the output scaling.

Jonathan

Re: [PATCH 0/2] Add support for AD7293 Power Amplifier

[Jonathan Cameron]

On Fri, 5 Nov 2021 13:29:27 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> The AD7293 is a Power Amplifier drain current controller containing 
> functionality for general-purpose monitoring and control of 
> current, voltage, and temperature, integrated into a single chip 
> solution with an SPI-compatible interface.
> 
> Datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7293.pdf

Wrong cover letter.  This had me rather confused for a few moments as I had
two copies of this in my inbox with different patches underneath!
> 
> This series of patches provide an initial implementation of the
> driver with access from the userspace to the:
>  - ADC channels (read raw values, set range, set offset)
>  - DAC channels (set code, set offset)
>  - Temperature sensing (read raw values, set offset)
>  - Current sensing (read raw, set offset, set gain)
> 
> Antoniu Miclaus (2):
>   iio:amplifiers:ad7293: add support for AD7293
>   dt-bindings:iio:amplifiers: add ad7293 doc
> 
>  .../bindings/iio/amplifiers/adi,ad7293.yaml   |  49 ++
>  drivers/iio/amplifiers/Kconfig                |  11 +
>  drivers/iio/amplifiers/Makefile               |   1 +
>  drivers/iio/amplifiers/ad7293.c               | 794 ++++++++++++++++++
>  4 files changed, 855 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/amplifiers/adi,ad7293.yaml
>  create mode 100644 drivers/iio/amplifiers/ad7293.c
> 

[PATCH 0/2] Add support for AD7293 Power Amplifier

[Antoniu Miclaus]

The AD7293 is a Power Amplifier drain current controller containing 
functionality for general-purpose monitoring and control of 
current, voltage, and temperature, integrated into a single chip 
solution with an SPI-compatible interface.

Datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7293.pdf

This series of patches provide an initial implementation of the
driver with access from the userspace to the:
 - ADC channels (read raw values, set range, set offset)
 - DAC channels (set code, set offset)
 - Temperature sensing (read raw values, set offset)
 - Current sensing (read raw, set offset, set gain)

Antoniu Miclaus (2):
  iio:amplifiers:ad7293: add support for AD7293
  dt-bindings:iio:amplifiers: add ad7293 doc

 .../bindings/iio/amplifiers/adi,ad7293.yaml   |  49 ++
 drivers/iio/amplifiers/Kconfig                |  11 +
 drivers/iio/amplifiers/Makefile               |   1 +
 drivers/iio/amplifiers/ad7293.c               | 794 ++++++++++++++++++
 4 files changed, 855 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/amplifiers/adi,ad7293.yaml
 create mode 100644 drivers/iio/amplifiers/ad7293.c

-- 
2.33.1