Add filter subfolder for IIO devices that handle filter functionality.
Signed-off-by: Antoniu Miclaus <[email protected]>
---
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
Add device tree bindings for the ADMV8818 Filter.
Signed-off-by: Antoniu Miclaus <[email protected]>
---
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 <[email protected]>
+
+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
Add initial ABI documentation for admv8818 filter sysfs interfaces.
Signed-off-by: Antoniu Miclaus <[email protected]>
---
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: [email protected]
+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: [email protected]
+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: [email protected]
+Description:
+ Scale high pass and lowpass filter frequency values to Hz.
+
+What: /sys/bus/iio/devices/iio:deviceX/filter_mode_available
+KernelVersion:
+Contact: [email protected]
+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: [email protected]
+Description:
+ This attribute configures the filter mode.
+ Reading returns the actual mode.
--
2.34.0
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 <[email protected]>
---
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 <[email protected]");
+MODULE_DESCRIPTION("Analog Devices ADMV8818");
+MODULE_LICENSE("GPL v2");
--
2.34.0
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/[email protected]/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 <[email protected]>
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/[email protected]
--
Antoniu Micl?u?
> -----Original Message-----
> From: kernel test robot <[email protected]>
> Sent: Wednesday, November 24, 2021 5:18 AM
> To: Miclaus, Antoniu <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; Miclaus, Antoniu <[email protected]>
> 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-
> [email protected]/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 <[email protected]>
>
> 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-
> [email protected]__;!!A3Ni8CS0y2Y!qezv4zARrMS9nlUF2pyY55iNqZQOzXhLKmt
> paNcqkCIzWdlzlwyoRyKLHRxkSyoFmbJO$
Similar to ADMV1013, this patch depends on:
https://patchwork.kernel.org/project/linux-iio/patch/[email protected]/
On Tue, 23 Nov 2021 15:39:00 +0200
Antoniu Miclaus <[email protected]> wrote:
> Add initial ABI documentation for admv8818 filter sysfs interfaces.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
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: [email protected]
> +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: [email protected]
> +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: [email protected]
> +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: [email protected]
> +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: [email protected]
> +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
On Tue, 23 Nov 2021 15:38:59 +0200
Antoniu Miclaus <[email protected]> wrote:
> Add device tree bindings for the ADMV8818 Filter.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
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 <[email protected]>
> +
> +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";
> + };
> + };
> +...
> +
On Tue, 23 Nov 2021 15:38:58 +0200
Antoniu Miclaus <[email protected]> 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 <[email protected]>
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);
> +}
...
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 <[email protected]>
> ---
> 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 <[email protected]>