[PATCH v2 1/4] iio: add filter subfolder

[PATCH v2 1/4] iio: add filter subfolder

[Antoniu Miclaus]

Add filter subfolder for IIO devices that handle filter functionality.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/Kconfig         | 1 +
 drivers/iio/Makefile        | 1 +
 drivers/iio/filter/Kconfig  | 8 ++++++++
 drivers/iio/filter/Makefile | 6 ++++++
 4 files changed, 16 insertions(+)
 create mode 100644 drivers/iio/filter/Kconfig
 create mode 100644 drivers/iio/filter/Makefile

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 2334ad249b46..3a496a28bad4 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -77,6 +77,7 @@ source "drivers/iio/chemical/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/dac/Kconfig"
 source "drivers/iio/dummy/Kconfig"
+source "drivers/iio/filter/Kconfig"
 source "drivers/iio/frequency/Kconfig"
 source "drivers/iio/gyro/Kconfig"
 source "drivers/iio/health/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 65e39bd4f934..97d2fbcf0950 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -24,6 +24,7 @@ obj-y += common/
 obj-y += dac/
 obj-y += dummy/
 obj-y += gyro/
+obj-y += filter/
 obj-y += frequency/
 obj-y += health/
 obj-y += humidity/
diff --git a/drivers/iio/filter/Kconfig b/drivers/iio/filter/Kconfig
new file mode 100644
index 000000000000..e268bba43852
--- /dev/null
+++ b/drivers/iio/filter/Kconfig
@@ -0,0 +1,8 @@
+#
+# Filter drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Filters"
+
+endmenu
diff --git a/drivers/iio/filter/Makefile b/drivers/iio/filter/Makefile
new file mode 100644
index 000000000000..cc0892c01142
--- /dev/null
+++ b/drivers/iio/filter/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for industrial I/O Filter drivers
+#
+
+# When adding new entries keep the list in alphabetical order
-- 
2.34.0

[PATCH v2 2/4] iio:filter:admv8818: add support for ADMV8818

[Antoniu Miclaus]

The ADMV8818-EP is a fully monolithic microwave integrated
circuit (MMIC) that features a digitally selectable frequency of
operation. The device features four independently controlled high-
pass filters (HPFs) and four independently controlled low-pass
filters (LPFs) that span the 2 GHz to 18 GHz frequency range.

Datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/admv8818-ep.pdf
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - use enum for filter modes
 - remove bandwidth/center frequency related implementations
 drivers/iio/filter/Kconfig    |  10 +
 drivers/iio/filter/Makefile   |   1 +
 drivers/iio/filter/admv8818.c | 666 ++++++++++++++++++++++++++++++++++
 3 files changed, 677 insertions(+)
 create mode 100644 drivers/iio/filter/admv8818.c

diff --git a/drivers/iio/filter/Kconfig b/drivers/iio/filter/Kconfig
index e268bba43852..3ae35817ad82 100644
--- a/drivers/iio/filter/Kconfig
+++ b/drivers/iio/filter/Kconfig
@@ -5,4 +5,14 @@
 
 menu "Filters"
 
+config ADMV8818
+	tristate "Analog Devices ADMV8818 High-Pass and Low-Pass Filter"
+	depends on SPI && COMMON_CLK && 64BIT
+	help
+	  Say yes here to build support for Analog Devices ADMV8818
+	  2 GHz to 18 GHz, Digitally Tunable, High-Pass and Low-Pass Filter.
+
+	  To compile this driver as a module, choose M here: the
+	  modiule will be called admv8818.
+
 endmenu
diff --git a/drivers/iio/filter/Makefile b/drivers/iio/filter/Makefile
index cc0892c01142..55e228c0dd20 100644
--- a/drivers/iio/filter/Makefile
+++ b/drivers/iio/filter/Makefile
@@ -4,3 +4,4 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_ADMV8818) += admv8818.o
diff --git a/drivers/iio/filter/admv8818.c b/drivers/iio/filter/admv8818.c
new file mode 100644
index 000000000000..6ad0327ea2fb
--- /dev/null
+++ b/drivers/iio/filter/admv8818.c
@@ -0,0 +1,666 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADMV8818 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/mutex.h>
+#include <linux/notifier.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/sysfs.h>
+#include <linux/units.h>
+
+/* ADMV8818 Register Map */
+#define ADMV8818_REG_SPI_CONFIG_A		0x0
+#define ADMV8818_REG_SPI_CONFIG_B		0x1
+#define ADMV8818_REG_CHIPTYPE			0x3
+#define ADMV8818_REG_PRODUCT_ID_L		0x4
+#define ADMV8818_REG_PRODUCT_ID_H		0x5
+#define ADMV8818_REG_FAST_LATCH_POINTER		0x10
+#define ADMV8818_REG_FAST_LATCH_STOP		0x11
+#define ADMV8818_REG_FAST_LATCH_START		0x12
+#define ADMV8818_REG_FAST_LATCH_DIRECTION	0x13
+#define ADMV8818_REG_FAST_LATCH_STATE		0x14
+#define ADMV8818_REG_WR0_SW			0x20
+#define ADMV8818_REG_WR0_FILTER			0x21
+#define ADMV8818_REG_WR1_SW			0x22
+#define ADMV8818_REG_WR1_FILTER			0x23
+#define ADMV8818_REG_WR2_SW			0x24
+#define ADMV8818_REG_WR2_FILTER			0x25
+#define ADMV8818_REG_WR3_SW			0x26
+#define ADMV8818_REG_WR3_FILTER			0x27
+#define ADMV8818_REG_WR4_SW			0x28
+#define ADMV8818_REG_WR4_FILTER			0x29
+#define ADMV8818_REG_LUT0_SW			0x100
+#define ADMV8818_REG_LUT0_FILTER		0x101
+#define ADMV8818_REG_LUT127_SW			0x1FE
+#define ADMV8818_REG_LUT127_FILTER		0x1FF
+
+/* ADMV8818_REG_SPI_CONFIG_A Map */
+#define ADMV8818_SOFTRESET_N_MSK		BIT(7)
+#define ADMV8818_LSB_FIRST_N_MSK		BIT(6)
+#define ADMV8818_ENDIAN_N_MSK			BIT(5)
+#define ADMV8818_SDOACTIVE_N_MSK		BIT(4)
+#define ADMV8818_SDOACTIVE_MSK			BIT(3)
+#define ADMV8818_ENDIAN_MSK			BIT(2)
+#define ADMV8818_LSBFIRST_MSK			BIT(1)
+#define ADMV8818_SOFTRESET_MSK			BIT(0)
+
+/* ADMV8818_REG_SPI_CONFIG_B Map */
+#define ADMV8818_SINGLE_INSTRUCTION_MSK		BIT(7)
+#define ADMV8818_CSB_STALL_MSK			BIT(6)
+#define ADMV8818_MASTER_SLAVE_RB_MSK		BIT(5)
+#define ADMV8818_MASTER_SLAVE_TRANSFER_MSK	BIT(0)
+
+/* ADMV8818_REG_WR0_SW Map */
+#define ADMV8818_SW_IN_SET_WR0_MSK		BIT(7)
+#define ADMV8818_SW_OUT_SET_WR0_MSK		BIT(6)
+#define ADMV8818_SW_IN_WR0_MSK			GENMASK(5, 3)
+#define ADMV8818_SW_OUT_WR0_MSK			GENMASK(2, 0)
+
+/* ADMV8818_REG_WR0_FILTER Map */
+#define ADMV8818_HPF_WR0_MSK			GENMASK(7, 4)
+#define ADMV8818_LPF_WR0_MSK			GENMASK(3, 0)
+
+enum {
+	ADMV8818_BW_FREQ,
+	ADMV8818_CENTER_FREQ
+};
+
+enum {
+	ADMV8818_AUTO_MODE,
+	ADMV8818_MANUAL_MODE,
+};
+
+struct admv8818_state {
+	struct spi_device	*spi;
+	struct regmap		*regmap;
+	struct clk		*clkin;
+	struct notifier_block	nb;
+	/* Protect against concurrent accesses to the device and data content*/
+	struct mutex		lock;
+	unsigned int		filter_mode;
+	unsigned int		freq_scale;
+	u64			cf_hz;
+};
+
+static const unsigned long long freq_range_hpf[4][2] = {
+	{1750000000ULL, 3550000000ULL},
+	{3400000000ULL, 7250000000ULL},
+	{6600000000, 12000000000},
+	{12500000000, 19900000000}
+};
+
+static const unsigned long long freq_range_lpf[4][2] = {
+	{2050000000ULL, 3850000000ULL},
+	{3350000000ULL, 7250000000ULL},
+	{7000000000, 13000000000},
+	{12550000000, 18500000000}
+};
+
+static const struct regmap_config admv8818_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = 0x80,
+	.max_register = 0x1FF,
+};
+
+static const char * const admv8818_modes[] = {
+	[0] = "auto",
+	[1] = "manual"
+};
+
+static int __admv8818_hpf_select(struct admv8818_state *st, u64 freq)
+{
+	unsigned int hpf_step = 0, hpf_band = 0, i, j;
+	u64 freq_step;
+	int ret;
+
+	if (freq < freq_range_hpf[0][0])
+		goto hpf_write;
+
+	if (freq > freq_range_hpf[3][1]) {
+		hpf_step = 15;
+		hpf_band = 4;
+
+		goto hpf_write;
+	}
+
+	for (i = 0; i < 4; i++) {
+		freq_step = div_u64((freq_range_hpf[i][1] -
+			freq_range_hpf[i][0]), 15);
+
+		if (freq > freq_range_hpf[i][0] &&
+		    (freq < freq_range_hpf[i][1] + freq_step)) {
+			hpf_band = i + 1;
+
+			for (j = 1; j <= 16; j++) {
+				if (freq < (freq_range_hpf[i][0] + (freq_step * j))) {
+					hpf_step = j - 1;
+					break;
+				}
+			}
+			break;
+		}
+	}
+
+	/* Close HPF frequency gap between 12 and 12.5 GHz */
+	if (freq >= 12000 * HZ_PER_MHZ && freq <= 12500 * HZ_PER_MHZ) {
+		hpf_band = 3;
+		hpf_step = 15;
+	}
+
+hpf_write:
+	ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_SW,
+				 ADMV8818_SW_IN_SET_WR0_MSK |
+				 ADMV8818_SW_IN_WR0_MSK,
+				 FIELD_PREP(ADMV8818_SW_IN_SET_WR0_MSK, 1) |
+				 FIELD_PREP(ADMV8818_SW_IN_WR0_MSK, hpf_band));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(st->regmap, ADMV8818_REG_WR0_FILTER,
+				  ADMV8818_HPF_WR0_MSK,
+				  FIELD_PREP(ADMV8818_HPF_WR0_MSK, hpf_step));
+}
+
+static int admv8818_hpf_select(struct admv8818_state *st, u64 freq)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv8818_hpf_select(st, freq);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int __admv8818_lpf_select(struct admv8818_state *st, u64 freq)
+{
+	unsigned int lpf_step = 0, lpf_band = 0, i, j;
+	u64 freq_step;
+	int ret;
+
+	if (freq > freq_range_lpf[3][1])
+		goto lpf_write;
+
+	if (freq < freq_range_lpf[0][0]) {
+		lpf_band = 1;
+
+		goto lpf_write;
+	}
+
+	for (i = 0; i < 4; i++) {
+		if (freq > freq_range_lpf[i][0] && freq < freq_range_lpf[i][1]) {
+			lpf_band = i + 1;
+			freq_step = div_u64((freq_range_lpf[i][1] - freq_range_lpf[i][0]), 15);
+
+			for (j = 0; j <= 15; j++) {
+				if (freq < (freq_range_lpf[i][0] + (freq_step * j))) {
+					lpf_step = j;
+					break;
+				}
+			}
+			break;
+		}
+	}
+
+lpf_write:
+	ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_SW,
+				 ADMV8818_SW_OUT_SET_WR0_MSK |
+				 ADMV8818_SW_OUT_WR0_MSK,
+				 FIELD_PREP(ADMV8818_SW_OUT_SET_WR0_MSK, 1) |
+				 FIELD_PREP(ADMV8818_SW_OUT_WR0_MSK, lpf_band));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(st->regmap, ADMV8818_REG_WR0_FILTER,
+				  ADMV8818_LPF_WR0_MSK,
+				  FIELD_PREP(ADMV8818_LPF_WR0_MSK, lpf_step));
+}
+
+static int admv8818_lpf_select(struct admv8818_state *st, u64 freq)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv8818_lpf_select(st, freq);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int admv8818_rfin_band_select(struct admv8818_state *st)
+{
+	int ret;
+
+	st->cf_hz = clk_get_rate(st->clkin);
+
+	mutex_lock(&st->lock);
+
+	ret = __admv8818_hpf_select(st, st->cf_hz);
+	if (ret)
+		goto exit;
+
+	ret = __admv8818_lpf_select(st, st->cf_hz);
+exit:
+	mutex_unlock(&st->lock);
+	return ret;
+}
+
+static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
+{
+	unsigned int data, hpf_band, hpf_state;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_SW, &data);
+	if (ret)
+		return ret;
+
+	hpf_band = FIELD_GET(ADMV8818_SW_IN_WR0_MSK, data);
+	if (!hpf_band) {
+		*hpf_freq = 0;
+		return ret;
+	}
+
+	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_FILTER, &data);
+	if (ret)
+		return ret;
+
+	hpf_state = FIELD_GET(ADMV8818_HPF_WR0_MSK, data);
+
+	*hpf_freq = div_u64(freq_range_hpf[hpf_band - 1][1] - freq_range_hpf[hpf_band - 1][0], 15);
+	*hpf_freq = freq_range_hpf[hpf_band - 1][0] + (*hpf_freq * hpf_state);
+
+	return ret;
+}
+
+static int admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv8818_read_hpf_freq(st, hpf_freq);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int __admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
+{
+	unsigned int data, lpf_band, lpf_state;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_SW, &data);
+	if (ret)
+		return ret;
+
+	lpf_band = FIELD_GET(ADMV8818_SW_OUT_WR0_MSK, data);
+	if (!lpf_band) {
+		*lpf_freq = 0;
+		return ret;
+	}
+
+	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_FILTER, &data);
+	if (ret)
+		return ret;
+
+	lpf_state = FIELD_GET(ADMV8818_LPF_WR0_MSK, data);
+
+	*lpf_freq = div_u64(freq_range_lpf[lpf_band - 1][1] - freq_range_lpf[lpf_band - 1][0], 15);
+	*lpf_freq = freq_range_lpf[lpf_band - 1][0] + (*lpf_freq * lpf_state);
+
+	return ret;
+}
+
+static int admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
+{
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __admv8818_read_lpf_freq(st, lpf_freq);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int admv8818_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long info)
+{
+	struct admv8818_state *st = iio_priv(indio_dev);
+
+	u64 freq = (u64)val * st->freq_scale;
+
+	switch (info) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		return admv8818_lpf_select(st, freq);
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		return admv8818_hpf_select(st, freq);
+	case IIO_CHAN_INFO_SCALE:
+		st->freq_scale = val;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int admv8818_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	struct admv8818_state *st = iio_priv(indio_dev);
+	int ret;
+	u64 freq;
+
+	switch (info) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		ret = admv8818_read_lpf_freq(st, &freq);
+		if (ret)
+			return ret;
+
+		*val = div_u64(freq, st->freq_scale);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		ret = admv8818_read_hpf_freq(st, &freq);
+		if (ret)
+			return ret;
+
+		*val = div_u64(freq, st->freq_scale);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->freq_scale;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int admv8818_reg_access(struct iio_dev *indio_dev,
+			       unsigned int reg,
+			       unsigned int write_val,
+			       unsigned int *read_val)
+{
+	struct admv8818_state *st = iio_priv(indio_dev);
+
+	if (read_val)
+		return regmap_read(st->regmap, reg, read_val);
+	else
+		return regmap_write(st->regmap, reg, write_val);
+}
+
+static int admv8818_get_mode(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan)
+{
+	struct admv8818_state *st = iio_priv(indio_dev);
+
+	return st->filter_mode;
+}
+
+static int admv8818_set_mode(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan,
+			     unsigned int mode)
+{
+	struct admv8818_state *st = iio_priv(indio_dev);
+	int ret = 0;
+
+	switch (mode) {
+	case ADMV8818_AUTO_MODE:
+		if (st->filter_mode && st->clkin) {
+			ret = clk_prepare_enable(st->clkin);
+			if (ret)
+				return ret;
+
+			ret = clk_notifier_register(st->clkin, &st->nb);
+			if (ret)
+				return ret;
+		} else {
+			return ret;
+		}
+
+		break;
+	case ADMV8818_MANUAL_MODE:
+		if (st->filter_mode == 0 && st->clkin) {
+			clk_disable_unprepare(st->clkin);
+
+			ret = clk_notifier_unregister(st->clkin, &st->nb);
+			if (ret)
+				return ret;
+		} else {
+			return ret;
+		}
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	st->filter_mode = mode;
+
+	return ret;
+}
+
+static const struct iio_info admv8818_info = {
+	.write_raw = admv8818_write_raw,
+	.read_raw = admv8818_read_raw,
+	.debugfs_reg_access = &admv8818_reg_access,
+};
+
+static const struct iio_enum admv8818_mode_enum = {
+	.items = admv8818_modes,
+	.num_items = ARRAY_SIZE(admv8818_modes),
+	.get = admv8818_get_mode,
+	.set = admv8818_set_mode,
+};
+
+static const struct iio_chan_spec_ext_info admv8818_ext_info[] = {
+	IIO_ENUM("filter_mode", IIO_SHARED_BY_ALL, &admv8818_mode_enum),
+	IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL, &admv8818_mode_enum),
+	{ },
+};
+
+#define ADMV8818_CHAN(_channel) {				\
+	.type = IIO_ALTVOLTAGE,					\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+	.info_mask_separate =					\
+		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
+		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY) | \
+		BIT(IIO_CHAN_INFO_SCALE) \
+}
+
+#define ADMV8818_CHAN_BW_CF(_channel, _admv8818_ext_info) {	\
+	.type = IIO_ALTVOLTAGE,					\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+	.ext_info = _admv8818_ext_info,				\
+}
+
+static const struct iio_chan_spec admv8818_channels[] = {
+	ADMV8818_CHAN(0),
+	ADMV8818_CHAN_BW_CF(0, admv8818_ext_info),
+};
+
+static int admv8818_freq_change(struct notifier_block *nb, unsigned long action, void *data)
+{
+	struct admv8818_state *st = container_of(nb, struct admv8818_state, nb);
+
+	if (action == POST_RATE_CHANGE)
+		return notifier_from_errno(admv8818_rfin_band_select(st));
+
+	return NOTIFY_OK;
+}
+
+static void admv8818_clk_notifier_unreg(void *data)
+{
+	struct admv8818_state *st = data;
+
+	if (st->filter_mode == 0)
+		clk_notifier_unregister(st->clkin, &st->nb);
+}
+
+static void admv8818_clk_disable(void *data)
+{
+	struct admv8818_state *st = data;
+
+	if (st->filter_mode == 0)
+		clk_disable_unprepare(st->clkin);
+}
+
+static int admv8818_init(struct admv8818_state *st)
+{
+	int ret;
+	struct spi_device *spi = st->spi;
+	unsigned int chip_id;
+
+	ret = regmap_update_bits(st->regmap, ADMV8818_REG_SPI_CONFIG_A,
+				 ADMV8818_SOFTRESET_N_MSK |
+				 ADMV8818_SOFTRESET_MSK,
+				 FIELD_PREP(ADMV8818_SOFTRESET_N_MSK, 1) |
+				 FIELD_PREP(ADMV8818_SOFTRESET_MSK, 1));
+	if (ret) {
+		dev_err(&spi->dev, "ADMV8818 Soft Reset failed.\n");
+		return ret;
+	}
+
+	ret = regmap_update_bits(st->regmap, ADMV8818_REG_SPI_CONFIG_A,
+				 ADMV8818_SDOACTIVE_N_MSK |
+				 ADMV8818_SDOACTIVE_MSK,
+				 FIELD_PREP(ADMV8818_SDOACTIVE_N_MSK, 1) |
+				 FIELD_PREP(ADMV8818_SDOACTIVE_MSK, 1));
+	if (ret) {
+		dev_err(&spi->dev, "ADMV8818 SDO Enable failed.\n");
+		return ret;
+	}
+
+	ret = regmap_read(st->regmap, ADMV8818_REG_CHIPTYPE, &chip_id);
+	if (ret) {
+		dev_err(&spi->dev, "ADMV8818 Chip ID read failed.\n");
+		return ret;
+	}
+
+	if (chip_id != 0x1) {
+		dev_err(&spi->dev, "ADMV8818 Invalid Chip ID.\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(st->regmap, ADMV8818_REG_SPI_CONFIG_B,
+				 ADMV8818_SINGLE_INSTRUCTION_MSK,
+				 FIELD_PREP(ADMV8818_SINGLE_INSTRUCTION_MSK, 1));
+	if (ret) {
+		dev_err(&spi->dev, "ADMV8818 Single Instruction failed.\n");
+		return ret;
+	}
+
+	st->freq_scale = HZ_PER_MHZ;
+
+	if (st->clkin)
+		return admv8818_rfin_band_select(st);
+	else
+		return 0;
+}
+
+static int admv8818_clk_setup(struct admv8818_state *st)
+{
+	struct spi_device *spi = st->spi;
+	int ret;
+
+	st->clkin = devm_clk_get_optional(&spi->dev, "rf_in");
+	if (IS_ERR(st->clkin))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
+				     "failed to get the input clock\n");
+	else if (!st->clkin)
+		return 0;
+
+	ret = clk_prepare_enable(st->clkin);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&spi->dev, admv8818_clk_disable, st);
+	if (ret)
+		return ret;
+
+	st->nb.notifier_call = admv8818_freq_change;
+	ret = clk_notifier_register(st->clkin, &st->nb);
+	if (ret < 0)
+		return ret;
+
+	return devm_add_action_or_reset(&spi->dev, admv8818_clk_notifier_unreg, st);
+}
+
+static int admv8818_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	struct admv8818_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_spi(spi, &admv8818_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	st = iio_priv(indio_dev);
+	st->regmap = regmap;
+
+	indio_dev->info = &admv8818_info;
+	indio_dev->name = "admv8818";
+	indio_dev->channels = admv8818_channels;
+	indio_dev->num_channels = ARRAY_SIZE(admv8818_channels);
+
+	st->spi = spi;
+
+	ret = admv8818_clk_setup(st);
+	if (ret)
+		return ret;
+
+	mutex_init(&st->lock);
+
+	ret = admv8818_init(st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id admv8818_id[] = {
+	{ "admv8818", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, admv8818_id);
+
+static const struct of_device_id admv8818_of_match[] = {
+	{ .compatible = "adi,admv8818" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, admv8818_of_match);
+
+static struct spi_driver admv8818_driver = {
+	.driver = {
+		.name = "admv8818",
+		.of_match_table = admv8818_of_match,
+	},
+	.probe = admv8818_probe,
+	.id_table = admv8818_id,
+};
+module_spi_driver(admv8818_driver);
+
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com");
+MODULE_DESCRIPTION("Analog Devices ADMV8818");
+MODULE_LICENSE("GPL v2");
-- 
2.34.0

[PATCH v2 3/4] dt-bindings:iio:filter: add admv8818 doc

[Antoniu Miclaus]

Add device tree bindings for the ADMV8818 Filter.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - remove `bw-hz` dt property
 .../bindings/iio/filter/adi,admv8818.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml

diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
new file mode 100644
index 000000000000..93e08bcd8cb4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/filter/adi,admv8818.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADMV8818 Digitally Tunable, High-Pass and Low-Pass Filter
+
+maintainers:
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+    Fully monolithic microwave integrated circuit (MMIC) that
+    features a digitally selectable frequency of operation.
+    The device features four independently controlled high-pass
+    filters (HPFs) and four independently controlled low-pass filters
+    (LPFs) that span the 2 GHz to 18 GHz frequency range.
+
+    https://www.analog.com/en/products/admv8818.html
+
+properties:
+  compatible:
+    enum:
+      - adi,admv8818
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 10000000
+
+  clocks:
+    description:
+      Definition of the external clock.
+    minItems: 1
+
+  clock-names:
+    items:
+      - const: rf_in
+
+  clock-output-names:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      admv8818@0 {
+        compatible = "adi,admv8818";
+        reg = <0>;
+        spi-max-frequency = <10000000>;
+        clocks = <&admv8818_rfin>;
+        clock-names = "rf_in";
+      };
+    };
+...
+
-- 
2.34.0

[PATCH v2 4/4] iio:filter:admv8818: Add sysfs ABI documentation

[Antoniu Miclaus]

Add initial ABI documentation for admv8818 filter sysfs interfaces.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - remove bandwidth/center frequency related custom device attributes
 - remove bypass filter mode
 .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818
new file mode 100644
index 000000000000..7211b5d0daa0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818
@@ -0,0 +1,44 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_high_pass_3db_frequency
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		The cut-off frequency of the ADMV8818 high pass filter. The value is scaled using
+		the `out_altvoltageY_scale` attribute so that GHz frequencies are valid inputs,
+		The accepted range of values for the frequencies is between 1.75GHz and 19.9GHz.
+
+		The default value for the scale is 1000000, therefore MHz frequency values are
+		passed as input.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_low_pass_3db_frequency
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		The cut-off frequency of the ADMV8818 low pass filter. The value is scaled using
+		the `out_altvoltageY_scale` attribute so that GHz frequencies are valid inputs,
+		The accepted range of values for the frequencies is between 2.05GHz and 18.85GHz.
+
+		The default value for the scale is 1000000, therefore MHz frequency values are
+		passed as input.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_scale
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Scale high pass and lowpass filter frequency values to Hz.
+
+What:		/sys/bus/iio/devices/iio:deviceX/filter_mode_available
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Reading this returns the valid values that can be written to the
+		on_altvoltage0_mode attribute:
+
+		- auto -> Adjust bandpass filter to track changes in input clock rate.
+		- manual -> disable/unregister the clock rate notifier / input clock tracking.
+
+What:		/sys/bus/iio/devices/iio:deviceX/filter_mode
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute configures the filter mode.
+		Reading returns the actual mode.
-- 
2.34.0

Re: [PATCH v2 2/4] iio:filter:admv8818: add support for ADMV8818

[kernel test robot]

Hi Antoniu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on v5.16-rc2 next-20211123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Antoniu-Miclaus/iio-add-filter-subfolder/20211123-214053
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20211124/202111241151.J9kfWWOf-lkp@intel.com/config.gz)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/5785226a8e5139d7275a8213a19c4e8479eca28b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Antoniu-Miclaus/iio-add-filter-subfolder/20211123-214053
        git checkout 5785226a8e5139d7275a8213a19c4e8479eca28b
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/filter/admv8818.c:469:74: error: macro "IIO_ENUM_AVAILABLE" passed 3 arguments, but takes just 2
     469 |  IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL, &admv8818_mode_enum),
         |                                                                          ^
   In file included from drivers/iio/filter/admv8818.c:14:
   include/linux/iio/iio.h:111: note: macro "IIO_ENUM_AVAILABLE" defined here
     111 | #define IIO_ENUM_AVAILABLE(_name, _e) \
         | 
>> drivers/iio/filter/admv8818.c:469:2: error: 'IIO_ENUM_AVAILABLE' undeclared here (not in a function)
     469 |  IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL, &admv8818_mode_enum),
         |  ^~~~~~~~~~~~~~~~~~


vim +/IIO_ENUM_AVAILABLE +469 drivers/iio/filter/admv8818.c

   466	
   467	static const struct iio_chan_spec_ext_info admv8818_ext_info[] = {
   468		IIO_ENUM("filter_mode", IIO_SHARED_BY_ALL, &admv8818_mode_enum),
 > 469		IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL, &admv8818_mode_enum),
   470		{ },
   471	};
   472	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

RE: [PATCH v2 2/4] iio:filter:admv8818: add support for ADMV8818

[Miclaus, Antoniu]

--
Antoniu Miclăuş

> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Sent: Wednesday, November 24, 2021 5:18 AM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; jic23@kernel.org;
> robh+dt@kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: kbuild-all@lists.01.org; Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> Subject: Re: [PATCH v2 2/4] iio:filter:admv8818: add support for ADMV8818
> 
> [External]
> 
> Hi Antoniu,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on jic23-iio/togreg]
> [also build test ERROR on v5.16-rc2 next-20211123]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://urldefense.com/v3/__https://git-scm.com/docs/git-format-
> patch__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmtpaNcqkCI
> zWdlzlwyoRyKLHRxkS_1UDWEO$ ]
> 
> url:    https://urldefense.com/v3/__https://github.com/0day-
> ci/linux/commits/Antoniu-Miclaus/iio-add-filter-subfolder/20211123-
> 214053__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmtpaNcqk
> CIzWdlzlwyoRyKLHRxkS3HtIozj$
> base:
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/g
> it/jic23/iio.git__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmtp
> aNcqkCIzWdlzlwyoRyKLHRxkS8fYcXl7$  togreg
> config: x86_64-allyesconfig
> (https://urldefense.com/v3/__https://download.01.org/0day-
> ci/archive/20211124/202111241151.J9kfWWOf-
> lkp@intel.com/config.gz__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQ
> OzXhLKmtpaNcqkCIzWdlzlwyoRyKLHRxkS7LldO4I$ )
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         # https://urldefense.com/v3/__https://github.com/0day-
> ci/linux/commit/5785226a8e5139d7275a8213a19c4e8479eca28b__;!!A3Ni8CS
> 0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmtpaNcqkCIzWdlzlwyoRyKLHR
> xkSyLuvece$
>         git remote add linux-review
> https://urldefense.com/v3/__https://github.com/0day-
> ci/linux__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmtpaNcqk
> CIzWdlzlwyoRyKLHRxkS5pcjrk6$
>         git fetch --no-tags linux-review Antoniu-Miclaus/iio-add-filter-
> subfolder/20211123-214053
>         git checkout 5785226a8e5139d7275a8213a19c4e8479eca28b
>         # save the config file to linux build tree
>         mkdir build_dir
>         make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/iio/filter/admv8818.c:469:74: error: macro
> "IIO_ENUM_AVAILABLE" passed 3 arguments, but takes just 2
>      469 |  IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL,
> &admv8818_mode_enum),
>          |                                                                          ^
>    In file included from drivers/iio/filter/admv8818.c:14:
>    include/linux/iio/iio.h:111: note: macro "IIO_ENUM_AVAILABLE" defined
> here
>      111 | #define IIO_ENUM_AVAILABLE(_name, _e) \
>          |
> >> drivers/iio/filter/admv8818.c:469:2: error: 'IIO_ENUM_AVAILABLE'
> undeclared here (not in a function)
>      469 |  IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL,
> &admv8818_mode_enum),
>          |  ^~~~~~~~~~~~~~~~~~
> 
> 
> vim +/IIO_ENUM_AVAILABLE +469 drivers/iio/filter/admv8818.c
> 
>    466
>    467	static const struct iio_chan_spec_ext_info admv8818_ext_info[] = {
>    468		IIO_ENUM("filter_mode", IIO_SHARED_BY_ALL,
> &admv8818_mode_enum),
>  > 469		IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_ALL,
> &admv8818_mode_enum),
>    470		{ },
>    471	};
>    472
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://urldefense.com/v3/__https://lists.01.org/hyperkitty/list/kbuild-
> all@lists.01.org__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmt
> paNcqkCIzWdlzlwyoRyKLHRxkSyoFmbJO$

Similar to ADMV1013, this patch depends on:
https://patchwork.kernel.org/project/linux-iio/patch/20211119085627.6348-1-antoniu.miclaus@analog.com/

Re: [PATCH v2 4/4] iio:filter:admv8818: Add sysfs ABI documentation

[Jonathan Cameron]

On Tue, 23 Nov 2021 15:39:00 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add initial ABI documentation for admv8818 filter sysfs interfaces.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

Hi Anntoniu,

I'd focused on the other attributes previously and as a result missed
the issue with scaling for the 3db_frequency attrs.

See below,

Jonathan

> ---
> changes in v2:
>  - remove bandwidth/center frequency related custom device attributes
>  - remove bypass filter mode
>  .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818
> new file mode 100644
> index 000000000000..7211b5d0daa0
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818
> @@ -0,0 +1,44 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_high_pass_3db_frequency
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		The cut-off frequency of the ADMV8818 high pass filter. The value is scaled using
> +		the `out_altvoltageY_scale` attribute so that GHz frequencies are valid inputs,

out_altvoltage_Y_scale applies to out_altvoltageY_raw (sure that doesn't actually exist here, but
we can't change it's meaning to something entirely different like this).

> +		The accepted range of values for the frequencies is between 1.75GHz and 19.9GHz.
> +
> +		The default value for the scale is 1000000, therefore MHz frequency values are
> +		passed as input.

Hmm. The scaling things is not how the other instances of 3db_frequency work and we need to be consistent.
So even though it's a lot of zeros this needs to be in HZ.

Note we ran into a requirement for 64 bit values recently so now have IIO_VAL_INT_64 which will
probably work for you here.

once that is tidied up these two 3db_frequency attributes are standard ABI (fit with the same for
other channels).  As such, please put them in the relevant existing entries for similar 3db_frequency.

Note that, if you want to express ranges that belongs in the sysfs attribute
*3db_frequency_available, not the documentation.



> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_low_pass_3db_frequency
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		The cut-off frequency of the ADMV8818 low pass filter. The value is scaled using
> +		the `out_altvoltageY_scale` attribute so that GHz frequencies are valid inputs,
> +		The accepted range of values for the frequencies is between 2.05GHz and 18.85GHz.
> +
> +		The default value for the scale is 1000000, therefore MHz frequency values are
> +		passed as input.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_scale
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Scale high pass and lowpass filter frequency values to Hz.

As above, this needs to go because it doesn't logically mean this when considered alongside the
existing ABI.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/filter_mode_available
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Reading this returns the valid values that can be written to the
> +		on_altvoltage0_mode attribute:
> +
> +		- auto -> Adjust bandpass filter to track changes in input clock rate.
> +		- manual -> disable/unregister the clock rate notifier / input clock tracking.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/filter_mode
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This attribute configures the filter mode.
> +		Reading returns the actual mode.
This bit works for me.  There is a risk we will run into different definitions of filter_mode
in the future and the docs builder doesn't let you have multiple definitions. If that happens
we may need to move this.   Can do it when needed however as this is the first such definition.

Thanks,

Jonathan

Re: [PATCH v2 3/4] dt-bindings:iio:filter: add admv8818 doc

[Jonathan Cameron]

On Tue, 23 Nov 2021 15:38:59 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

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

This looks good to me.  The clocks things does leave me a little
nervous though, so let's see what Rob thinks.

Key thing here is that these aren't typically clocks being used or
generated in an SoC, but rather high microwave signals that
we are filtering...  It's useful to treat them as clocks to get
the filters to automatically adjust if the input frequency changes.

Jonathan

> ---
> changes in v2:
>  - remove `bw-hz` dt property
>  .../bindings/iio/filter/adi,admv8818.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> new file mode 100644
> index 000000000000..93e08bcd8cb4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/filter/adi,admv8818.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADMV8818 Digitally Tunable, High-Pass and Low-Pass Filter
> +
> +maintainers:
> +  - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +description: |
> +    Fully monolithic microwave integrated circuit (MMIC) that
> +    features a digitally selectable frequency of operation.
> +    The device features four independently controlled high-pass
> +    filters (HPFs) and four independently controlled low-pass filters
> +    (LPFs) that span the 2 GHz to 18 GHz frequency range.
> +
> +    https://www.analog.com/en/products/admv8818.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,admv8818
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 10000000
> +
> +  clocks:
> +    description:
> +      Definition of the external clock.
> +    minItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: rf_in
> +
> +  clock-output-names:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      admv8818@0 {
> +        compatible = "adi,admv8818";
> +        reg = <0>;
> +        spi-max-frequency = <10000000>;
> +        clocks = <&admv8818_rfin>;
> +        clock-names = "rf_in";
> +      };
> +    };
> +...
> +

Re: [PATCH v2 2/4] iio:filter:admv8818: add support for ADMV8818

[Jonathan Cameron]

On Tue, 23 Nov 2021 15:38:58 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> The ADMV8818-EP is a fully monolithic microwave integrated
> circuit (MMIC) that features a digitally selectable frequency of
> operation. The device features four independently controlled high-
> pass filters (HPFs) and four independently controlled low-pass
> filters (LPFs) that span the 2 GHz to 18 GHz frequency range.
> 
> Datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/admv8818-ep.pdf
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

I think the ABI needs tweaking a little if the clk isn't present
+ dt-binding currently says the clk is required but the driver has it optional.
Should definitely be optional...

...

> +
> +static int admv8818_set_mode(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     unsigned int mode)
> +{
> +	struct admv8818_state *st = iio_priv(indio_dev);
> +	int ret = 0;
> +
> +	switch (mode) {
> +	case ADMV8818_AUTO_MODE:
> +		if (st->filter_mode && st->clkin) {
> +			ret = clk_prepare_enable(st->clkin);
> +			if (ret)
> +				return ret;
> +
> +			ret = clk_notifier_register(st->clkin, &st->nb);
> +			if (ret)
> +				return ret;
> +		} else {
> +			return ret;

If you have a path that will always return 0, then just make it return 0
and don't set ret to a default value.  More readable to make that explicit
as I don't need to look at what value ret has.

I wondered about suggesting that you don't have a this attribute if not
clkin registered, but it's better to have it and just have it always
set to manual.  However that changes the available values, so you will
still need to have two seperate chan_spec arrays and pick between them
depending on whether the clock is supplied.

I'd express the logic here differently as well

	if (!st->clkin)
		if (mode == ADV8818_MANUAL_MODE)
			return 0; //you could just skip this case, but maybe it's nice to have.

		return -EINVAL;
	}
	//Now we know it might actually makes sense to do something

	switch(mode) {
	case ADMV8818_AUTO_MODE:
		if (!st->filter_mode)
			return 0;

		ret = clk_prepare_enable(st->clk_in);
		if (ret)
			return ret;

		ret = clk_notifier_register(st->clkin, &st->nb);
		if (ret) {
			clk_disable_unprepare(st->clk_in);

			return ret;
		}

	}

> +		}
> +
> +		break;
> +	case ADMV8818_MANUAL_MODE:
> +		if (st->filter_mode == 0 && st->clkin) {
> +			clk_disable_unprepare(st->clkin);
> +
> +			ret = clk_notifier_unregister(st->clkin, &st->nb);
> +			if (ret)
> +				return ret;
> +		} else {
> +			return ret;
> +		}
> +
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	st->filter_mode = mode;
> +
> +	return ret;
> +}
> +

...

> +
> +static int admv8818_init(struct admv8818_state *st)
> +{
> +	int ret;
> +	struct spi_device *spi = st->spi;
> +	unsigned int chip_id;
> +
> +	ret = regmap_update_bits(st->regmap, ADMV8818_REG_SPI_CONFIG_A,
> +				 ADMV8818_SOFTRESET_N_MSK |
> +				 ADMV8818_SOFTRESET_MSK,
> +				 FIELD_PREP(ADMV8818_SOFTRESET_N_MSK, 1) |
> +				 FIELD_PREP(ADMV8818_SOFTRESET_MSK, 1));
> +	if (ret) {
> +		dev_err(&spi->dev, "ADMV8818 Soft Reset failed.\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_update_bits(st->regmap, ADMV8818_REG_SPI_CONFIG_A,
> +				 ADMV8818_SDOACTIVE_N_MSK |
> +				 ADMV8818_SDOACTIVE_MSK,
> +				 FIELD_PREP(ADMV8818_SDOACTIVE_N_MSK, 1) |
> +				 FIELD_PREP(ADMV8818_SDOACTIVE_MSK, 1));
> +	if (ret) {
> +		dev_err(&spi->dev, "ADMV8818 SDO Enable failed.\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_read(st->regmap, ADMV8818_REG_CHIPTYPE, &chip_id);
> +	if (ret) {
> +		dev_err(&spi->dev, "ADMV8818 Chip ID read failed.\n");
> +		return ret;
> +	}
> +
> +	if (chip_id != 0x1) {
> +		dev_err(&spi->dev, "ADMV8818 Invalid Chip ID.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_update_bits(st->regmap, ADMV8818_REG_SPI_CONFIG_B,
> +				 ADMV8818_SINGLE_INSTRUCTION_MSK,
> +				 FIELD_PREP(ADMV8818_SINGLE_INSTRUCTION_MSK, 1));
> +	if (ret) {
> +		dev_err(&spi->dev, "ADMV8818 Single Instruction failed.\n");
> +		return ret;
> +	}
> +
> +	st->freq_scale = HZ_PER_MHZ;
> +
> +	if (st->clkin)
> +		return admv8818_rfin_band_select(st);
> +	else
> +		return 0;
> +}
> +
> +static int admv8818_clk_setup(struct admv8818_state *st)
> +{
> +	struct spi_device *spi = st->spi;
> +	int ret;
> +
> +	st->clkin = devm_clk_get_optional(&spi->dev, "rf_in");
> +	if (IS_ERR(st->clkin))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
> +				     "failed to get the input clock\n");
> +	else if (!st->clkin)

You have this as required in the dt-binding which you should relax.
(note I didn't raise that for the dt-bindings as reviewed them before
the driver)

> +		return 0;
> +
> +	ret = clk_prepare_enable(st->clkin);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(&spi->dev, admv8818_clk_disable, st);
> +	if (ret)
> +		return ret;
> +
> +	st->nb.notifier_call = admv8818_freq_change;
> +	ret = clk_notifier_register(st->clkin, &st->nb);
> +	if (ret < 0)
> +		return ret;
> +
> +	return devm_add_action_or_reset(&spi->dev, admv8818_clk_notifier_unreg, st);
> +}

...

Re: [PATCH v2 3/4] dt-bindings:iio:filter: add admv8818 doc

[Rob Herring]

On Tue, 23 Nov 2021 15:38:59 +0200, Antoniu Miclaus wrote:
> Add device tree bindings for the ADMV8818 Filter.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - remove `bw-hz` dt property
>  .../bindings/iio/filter/adi,admv8818.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> 

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