* [PATCH 0/4] Add support for ADMV8818
@ 2021-11-09 12:31 Antoniu Miclaus
2021-11-09 12:31 ` [PATCH 1/4] iio: add filter subfolder Antoniu Miclaus
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Antoniu Miclaus @ 2021-11-09 12:31 UTC (permalink / raw)
To: jic23, robh+dt, linux-iio, devicetree, linux-kernel; +Cc: 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.
This patch series includes a proposal to add a subsection in the
IIO subsystem for filter drivers.
NOTE:
Currently depends on 64-bit architecture since the input
clock that server as RFIN should support values
in the range 2 GHz to 18 GHz.
We might need some scaling implementation in the clock
framework so that u64 types are supported when using 32-bit
architectures.
Antoniu Miclaus (4):
iio: add filter subfolder
iio:filter:admv8818: add support for ADMV8818
dt-bindings:iio:filter: add admv8818 doc
iio:filter:admv8818: Add sysfs ABI documentation
.../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 ++
.../bindings/iio/filter/adi,admv8818.yaml | 78 ++
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
drivers/iio/filter/Kconfig | 18 +
drivers/iio/filter/Makefile | 7 +
drivers/iio/filter/admv8818.c | 819 ++++++++++++++++++
7 files changed, 984 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818
create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
create mode 100644 drivers/iio/filter/Kconfig
create mode 100644 drivers/iio/filter/Makefile
create mode 100644 drivers/iio/filter/admv8818.c
--
2.33.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/4] iio: add filter subfolder 2021-11-09 12:31 [PATCH 0/4] Add support for ADMV8818 Antoniu Miclaus @ 2021-11-09 12:31 ` Antoniu Miclaus 2021-11-12 17:36 ` Jonathan Cameron 2021-11-09 12:31 ` [PATCH 2/4] iio:filter:admv8818: add support for ADMV8818 Antoniu Miclaus ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Antoniu Miclaus @ 2021-11-09 12:31 UTC (permalink / raw) To: jic23, robh+dt, linux-iio, devicetree, linux-kernel; +Cc: Antoniu Miclaus Add filter subfolder for IIO devices that handle filter functionality. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- 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.33.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] iio: add filter subfolder 2021-11-09 12:31 ` [PATCH 1/4] iio: add filter subfolder Antoniu Miclaus @ 2021-11-12 17:36 ` Jonathan Cameron 2021-11-12 17:44 ` Jonathan Cameron 0 siblings, 1 reply; 17+ messages in thread From: Jonathan Cameron @ 2021-11-12 17:36 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Tue, 9 Nov 2021 14:31:24 +0200 Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > Add filter subfolder for IIO devices that handle filter functionality. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> Hi Antoniu, Are we likely to see many filter drivers? If not we could classify them as analog front ends and put them in the AFE directory? If there are going to be lots then I'm fine with a new directory. Jonathan > --- > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] iio: add filter subfolder 2021-11-12 17:36 ` Jonathan Cameron @ 2021-11-12 17:44 ` Jonathan Cameron 0 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2021-11-12 17:44 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Fri, 12 Nov 2021 17:36:21 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 9 Nov 2021 14:31:24 +0200 > Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > > Add filter subfolder for IIO devices that handle filter functionality. > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > Hi Antoniu, > > Are we likely to see many filter drivers? If not we could classify them > as analog front ends and put them in the AFE directory? On actually reading what device was I'm fine with filters directory. AFE is more appropriate if we are talking about a front end for something we are going to feed to an ADC rather that filtering a microwave frequency signal. Jonathan > > If there are going to be lots then I'm fine with a new directory. > > Jonathan > > > --- > > 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] iio:filter:admv8818: add support for ADMV8818 2021-11-09 12:31 [PATCH 0/4] Add support for ADMV8818 Antoniu Miclaus 2021-11-09 12:31 ` [PATCH 1/4] iio: add filter subfolder Antoniu Miclaus @ 2021-11-09 12:31 ` Antoniu Miclaus 2021-11-12 18:06 ` Jonathan Cameron 2021-11-09 12:31 ` [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc Antoniu Miclaus 2021-11-09 12:31 ` [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation Antoniu Miclaus 3 siblings, 1 reply; 17+ messages in thread From: Antoniu Miclaus @ 2021-11-09 12:31 UTC (permalink / raw) To: jic23, robh+dt, linux-iio, devicetree, linux-kernel; +Cc: 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> --- drivers/iio/filter/Kconfig | 10 + drivers/iio/filter/Makefile | 1 + drivers/iio/filter/admv8818.c | 819 ++++++++++++++++++++++++++++++++++ 3 files changed, 830 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..c31f4e53573e --- /dev/null +++ b/drivers/iio/filter/admv8818.c @@ -0,0 +1,819 @@ +// 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 +}; + +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; + u64 bw_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] = "bypass", + [2] = "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_filter_bypass(struct admv8818_state *st) +{ + int ret; + + mutex_lock(&st->lock); + + ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_SW, + ADMV8818_SW_IN_SET_WR0_MSK | + ADMV8818_SW_IN_WR0_MSK | + ADMV8818_SW_OUT_SET_WR0_MSK | + ADMV8818_SW_OUT_WR0_MSK, + FIELD_PREP(ADMV8818_SW_IN_SET_WR0_MSK, 1) | + FIELD_PREP(ADMV8818_SW_IN_WR0_MSK, 0) | + FIELD_PREP(ADMV8818_SW_OUT_SET_WR0_MSK, 1) | + FIELD_PREP(ADMV8818_SW_OUT_WR0_MSK, 0)); + if (ret) + goto exit; + + ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_FILTER, + ADMV8818_HPF_WR0_MSK | + ADMV8818_LPF_WR0_MSK, + FIELD_PREP(ADMV8818_HPF_WR0_MSK, 0) | + FIELD_PREP(ADMV8818_LPF_WR0_MSK, 0)); + +exit: + mutex_unlock(&st->lock); + + return ret; +} + +static int admv8818_rfin_band_select(struct admv8818_state *st) +{ + int ret; + u64 lpf, hpf; + + st->cf_hz = clk_get_rate(st->clkin); + + if (!st->bw_hz) { + lpf = st->cf_hz; + hpf = st->cf_hz; + } else { + lpf = st->cf_hz + div_u64(st->bw_hz, 2); + hpf = st->cf_hz - div_u64(st->bw_hz, 2); + } + + mutex_lock(&st->lock); + + ret = __admv8818_hpf_select(st, hpf); + if (ret) + goto exit; + + ret = __admv8818_lpf_select(st, lpf); +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 ssize_t admv8818_read(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + char *buf) +{ + struct admv8818_state *st = iio_priv(indio_dev); + u64 val, lpf_freq, hpf_freq; + int ret; + + mutex_lock(&st->lock); + + ret = __admv8818_read_lpf_freq(st, &lpf_freq); + if (ret) + goto exit; + + ret = __admv8818_read_hpf_freq(st, &hpf_freq); + +exit: + mutex_unlock(&st->lock); + + if (ret) + return ret; + + switch ((u32)private) { + case ADMV8818_BW_FREQ: + val = lpf_freq - hpf_freq; + break; + case ADMV8818_CENTER_FREQ: + val = hpf_freq + (lpf_freq - hpf_freq) / 2; + break; + default: + return -EINVAL; + } + + return sysfs_emit(buf, "%llu\n", val); +} + +static ssize_t admv8818_write(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len) +{ + struct admv8818_state *st = iio_priv(indio_dev); + unsigned long long freq, lpf_freq, hpf_freq; + int ret; + + ret = kstrtoull(buf, 10, &freq); + if (ret) + return ret; + + switch ((u32)private) { + case ADMV8818_BW_FREQ: + st->bw_hz = freq; + hpf_freq = st->cf_hz - div_u64(freq, 2); + lpf_freq = hpf_freq + freq; + + break; + case ADMV8818_CENTER_FREQ: + st->cf_hz = freq; + hpf_freq = freq - div_u64(st->bw_hz, 2); + lpf_freq = hpf_freq + st->bw_hz; + + break; + default: + return -EINVAL; + } + + mutex_lock(&st->lock); + + ret = __admv8818_lpf_select(st, lpf_freq); + if (ret) + goto exit; + + ret = __admv8818_hpf_select(st, hpf_freq); + +exit: + mutex_unlock(&st->lock); + + return ret ? ret : len; +} + +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 0: + 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 1: + if (st->filter_mode == 0 && st->clkin) { + clk_disable_unprepare(st->clkin); + + ret = clk_notifier_unregister(st->clkin, &st->nb); + if (ret) + return ret; + } + + ret = admv8818_filter_bypass(st); + if (ret) + return ret; + + break; + case 2: + 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, +}; + +#define _ADMV8818_EXT_INFO(_name, _shared, _ident) { \ + .name = _name, \ + .read = admv8818_read, \ + .write = admv8818_write, \ + .private = _ident, \ + .shared = _shared, \ +} + +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, +}; + +#define IIO_ENUM_AVAILABLE_SHARED(_name, _shared, _e) \ +{ \ + .name = (_name "_available"), \ + .shared = _shared, \ + .read = iio_enum_available_read, \ + .private = (uintptr_t)(_e), \ +} + +static const struct iio_chan_spec_ext_info admv8818_ext_info[] = { + _ADMV8818_EXT_INFO("filter_band_pass_bandwidth_3db_frequency", IIO_SEPARATE, + ADMV8818_BW_FREQ), + _ADMV8818_EXT_INFO("filter_band_pass_center_frequency", IIO_SEPARATE, + ADMV8818_CENTER_FREQ), + IIO_ENUM("mode", IIO_SEPARATE, &admv8818_mode_enum), + IIO_ENUM_AVAILABLE_SHARED("mode", IIO_SEPARATE, &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; + + device_property_read_u64(&spi->dev, "adi,bw-hz", &st->bw_hz); + + 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.33.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] iio:filter:admv8818: add support for ADMV8818 2021-11-09 12:31 ` [PATCH 2/4] iio:filter:admv8818: add support for ADMV8818 Antoniu Miclaus @ 2021-11-12 18:06 ` Jonathan Cameron 0 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2021-11-12 18:06 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Tue, 9 Nov 2021 14:31:25 +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> No significant comments in the actual driver from me. Nice and clean, we just need to figure out the ABI questions etc. Thanks, Jonathan > --- > drivers/iio/filter/Kconfig | 10 + > drivers/iio/filter/Makefile | 1 + > drivers/iio/filter/admv8818.c | 819 ++++++++++++++++++++++++++++++++++ > 3 files changed, 830 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..c31f4e53573e > --- /dev/null > +++ b/drivers/iio/filter/admv8818.c > @@ -0,0 +1,819 @@ > +// 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 > +}; > + > +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; > + u64 bw_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] = "bypass", > + [2] = "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_filter_bypass(struct admv8818_state *st) > +{ > + int ret; > + > + mutex_lock(&st->lock); > + > + ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_SW, > + ADMV8818_SW_IN_SET_WR0_MSK | > + ADMV8818_SW_IN_WR0_MSK | > + ADMV8818_SW_OUT_SET_WR0_MSK | > + ADMV8818_SW_OUT_WR0_MSK, > + FIELD_PREP(ADMV8818_SW_IN_SET_WR0_MSK, 1) | > + FIELD_PREP(ADMV8818_SW_IN_WR0_MSK, 0) | > + FIELD_PREP(ADMV8818_SW_OUT_SET_WR0_MSK, 1) | > + FIELD_PREP(ADMV8818_SW_OUT_WR0_MSK, 0)); > + if (ret) > + goto exit; > + > + ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_FILTER, > + ADMV8818_HPF_WR0_MSK | > + ADMV8818_LPF_WR0_MSK, > + FIELD_PREP(ADMV8818_HPF_WR0_MSK, 0) | > + FIELD_PREP(ADMV8818_LPF_WR0_MSK, 0)); > + > +exit: > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > + > +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 0: If you do end up going this way then use an enum. > + 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 1: > + if (st->filter_mode == 0 && st->clkin) { > + clk_disable_unprepare(st->clkin); > + > + ret = clk_notifier_unregister(st->clkin, &st->nb); > + if (ret) > + return ret; > + } > + > + ret = admv8818_filter_bypass(st); > + if (ret) > + return ret; > + > + break; > + case 2: > + 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; > +} ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc 2021-11-09 12:31 [PATCH 0/4] Add support for ADMV8818 Antoniu Miclaus 2021-11-09 12:31 ` [PATCH 1/4] iio: add filter subfolder Antoniu Miclaus 2021-11-09 12:31 ` [PATCH 2/4] iio:filter:admv8818: add support for ADMV8818 Antoniu Miclaus @ 2021-11-09 12:31 ` Antoniu Miclaus 2021-11-10 1:07 ` Rob Herring 2021-11-12 17:46 ` Jonathan Cameron 2021-11-09 12:31 ` [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation Antoniu Miclaus 3 siblings, 2 replies; 17+ messages in thread From: Antoniu Miclaus @ 2021-11-09 12:31 UTC (permalink / raw) To: jic23, robh+dt, linux-iio, devicetree, linux-kernel; +Cc: Antoniu Miclaus Add device tree bindings for the ADMV8818 Filter. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ 1 file changed, 78 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..d581e236dbdc --- /dev/null +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml @@ -0,0 +1,78 @@ +# 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 + + adi,bw-hz: + description: + Allows the user to increase the Bandpass Filter (BPF) bandwidth + in Hz. Normally when invoked by the clk notifier, the driver + sets the HPF cutoff close below the frequency and the LPF cutoff + close above the frequency, and thus creating a BPF. + $ref: /schemas/types.yaml#/definitions/uint64 + + '#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"; + adi,bw-hz = /bits/ 64 <600000000>; + }; + }; +... + -- 2.33.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc 2021-11-09 12:31 ` [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc Antoniu Miclaus @ 2021-11-10 1:07 ` Rob Herring 2021-11-10 9:39 ` Miclaus, Antoniu 2021-11-12 17:46 ` Jonathan Cameron 1 sibling, 1 reply; 17+ messages in thread From: Rob Herring @ 2021-11-10 1:07 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: jic23, linux-iio, linux-kernel, devicetree, robh+dt On Tue, 09 Nov 2021 14:31:26 +0200, Antoniu Miclaus wrote: > Add device tree bindings for the ADMV8818 Filter. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > --- > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ > 1 file changed, 78 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml: properties:adi,bw-hz: '$ref' should not be valid under {'const': '$ref'} hint: Standard unit suffix properties don't need a type $ref from schema $id: http://devicetree.org/meta-schemas/core.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml: ignoring, error in schema: properties: adi,bw-hz warning: no schema found in file: ./Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml Documentation/devicetree/bindings/iio/filter/adi,admv8818.example.dt.yaml:0:0: /example-0/spi/admv8818@0: failed to match any schema with compatible: ['adi,admv8818'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1552959 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc 2021-11-10 1:07 ` Rob Herring @ 2021-11-10 9:39 ` Miclaus, Antoniu 0 siblings, 0 replies; 17+ messages in thread From: Miclaus, Antoniu @ 2021-11-10 9:39 UTC (permalink / raw) To: Rob Herring; +Cc: jic23, linux-iio, linux-kernel, devicetree, robh+dt -- Antoniu Miclăuş > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Wednesday, November 10, 2021 3:07 AM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: jic23@kernel.org; linux-iio@vger.kernel.org; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; robh+dt@kernel.org > Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc > > [External] > > On Tue, 09 Nov 2021 14:31:26 +0200, Antoniu Miclaus wrote: > > Add device tree bindings for the ADMV8818 Filter. > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > --- > > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ > > 1 file changed, 78 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m > dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt- > review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml: > properties:adi,bw-hz: '$ref' should not be valid under {'const': '$ref'} > hint: Standard unit suffix properties don't need a type $ref > from schema $id: > https://urldefense.com/v3/__http://devicetree.org/meta- > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!tVywX0fQFR2vYZq- > YTIJiJ0AXF2WapK4hXuLoFCYYlg19vxsZrLtIe7gWW7NfqCnlwuk$ > /builds/robherring/linux-dt- > review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml: > ignoring, error in schema: properties: adi,bw-hz > warning: no schema found in file: > ./Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > Documentation/devicetree/bindings/iio/filter/adi,admv8818.example.dt.ya > ml:0:0: /example-0/spi/admv8818@0: failed to match any schema with > compatible: ['adi,admv8818'] > > doc reference errors (make refcheckdocs): > > See > https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1552959_ > _;!!A3Ni8CS0y2Y!tVywX0fQFR2vYZq- > YTIJiJ0AXF2WapK4hXuLoFCYYlg19vxsZrLtIe7gWW7NfqkOdOfT$ > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. Thanks! I will resubmit this patch in v2 after the driver is reviewed. Regards, Antoniu ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc 2021-11-09 12:31 ` [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc Antoniu Miclaus 2021-11-10 1:07 ` Rob Herring @ 2021-11-12 17:46 ` Jonathan Cameron 2021-11-16 14:43 ` Miclaus, Antoniu 1 sibling, 1 reply; 17+ messages in thread From: Jonathan Cameron @ 2021-11-12 17:46 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Tue, 9 Nov 2021 14:31:26 +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> > --- > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ > 1 file changed, 78 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..d581e236dbdc > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > @@ -0,0 +1,78 @@ > +# 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" Is this what we'd normally think of as a clock signal? I'd not expect a nice squarewave on that pin for example so this seems an odd way to define it. > + > + clock-output-names: > + maxItems: 1 > + > + adi,bw-hz: > + description: > + Allows the user to increase the Bandpass Filter (BPF) bandwidth > + in Hz. Normally when invoked by the clk notifier, the driver > + sets the HPF cutoff close below the frequency and the LPF cutoff > + close above the frequency, and thus creating a BPF. I don't understand this item at all. Why do we need a control to basically change how the other filter parameters are expressed? > + $ref: /schemas/types.yaml#/definitions/uint64 > + > + '#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"; > + adi,bw-hz = /bits/ 64 <600000000>; > + }; > + }; > +... > + ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc 2021-11-12 17:46 ` Jonathan Cameron @ 2021-11-16 14:43 ` Miclaus, Antoniu 2021-11-21 12:24 ` Jonathan Cameron 0 siblings, 1 reply; 17+ messages in thread From: Miclaus, Antoniu @ 2021-11-16 14:43 UTC (permalink / raw) To: Jonathan Cameron; +Cc: robh+dt, linux-iio, devicetree, linux-kernel Hello Jonathan, -- Antoniu Miclăuş > -----Original Message----- > From: Jonathan Cameron <jic23@kernel.org> > Sent: Friday, November 12, 2021 7:46 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: robh+dt@kernel.org; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc > > [External] > > On Tue, 9 Nov 2021 14:31:26 +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> > > --- > > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ > > 1 file changed, 78 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..d581e236dbdc > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > > @@ -0,0 +1,78 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/filter/adi,a > dmv8818.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3yE_r > Ny91yIkDRTXFe54x-cHq_BtsyzDOedLohB5D$ > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta- > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3 > yE_rNy91yIkDRTXFe54x-cHq_BtsyzDOeYdHtx0a$ > > + > > +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" > > Is this what we'd normally think of as a clock signal? I'd not expect > a nice squarewave on that pin for example so this seems an odd way to > define it. > The only actual use of this part, until now, was to filter the output of the following part: https://www.analog.com/en/products/adf5610.html This is the reason of using the clock framework in the driver. Moreover, the clock input is optional inside the driver. > > + > > + clock-output-names: > > + maxItems: 1 > > + > > + adi,bw-hz: > > + description: > > + Allows the user to increase the Bandpass Filter (BPF) bandwidth > > + in Hz. Normally when invoked by the clk notifier, the driver > > + sets the HPF cutoff close below the frequency and the LPF cutoff > > + close above the frequency, and thus creating a BPF. > > I don't understand this item at all. Why do we need a control to > basically change how the other filter parameters are expressed? > Indeed, this property was requested by the users of the application in which this part was involved. Same goes for the filter modes and the bandwidth in the ABI documentation. If you think these attributes/properties are way too custom, I can drop them. Let me know your thoughts. > > + $ref: /schemas/types.yaml#/definitions/uint64 > > + > > + '#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"; > > + adi,bw-hz = /bits/ 64 <600000000>; > > + }; > > + }; > > +... > > + ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc 2021-11-16 14:43 ` Miclaus, Antoniu @ 2021-11-21 12:24 ` Jonathan Cameron 0 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2021-11-21 12:24 UTC (permalink / raw) To: Miclaus, Antoniu; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Tue, 16 Nov 2021 14:43:14 +0000 "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote: > Hello Jonathan, > > -- > Antoniu Miclăuş > > > -----Original Message----- > > From: Jonathan Cameron <jic23@kernel.org> > > Sent: Friday, November 12, 2021 7:46 PM > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > > Cc: robh+dt@kernel.org; linux-iio@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc > > > > [External] > > > > On Tue, 9 Nov 2021 14:31:26 +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> > > > --- > > > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ > > > 1 file changed, 78 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..d581e236dbdc > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > > > @@ -0,0 +1,78 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: > > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/filter/adi,a > > dmv8818.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3yE_r > > Ny91yIkDRTXFe54x-cHq_BtsyzDOedLohB5D$ > > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta- > > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3 > > yE_rNy91yIkDRTXFe54x-cHq_BtsyzDOeYdHtx0a$ > > > + > > > +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" > > > > Is this what we'd normally think of as a clock signal? I'd not expect > > a nice squarewave on that pin for example so this seems an odd way to > > define it. > > > The only actual use of this part, until now, was to filter the output of the following part: > https://www.analog.com/en/products/adf5610.html > This is the reason of using the clock framework in the driver. Moreover, the clock input is > optional inside the driver. OK, so in theory that part is generating a sinusoid. I guess the clk framework works for handling such devices, even if it's not typically what people expect from a clk. > > > + > > > + clock-output-names: > > > + maxItems: 1 > > > + > > > + adi,bw-hz: > > > + description: > > > + Allows the user to increase the Bandpass Filter (BPF) bandwidth > > > + in Hz. Normally when invoked by the clk notifier, the driver > > > + sets the HPF cutoff close below the frequency and the LPF cutoff > > > + close above the frequency, and thus creating a BPF. > > > > I don't understand this item at all. Why do we need a control to > > basically change how the other filter parameters are expressed? > > > > Indeed, this property was requested by the users of the application in which this part was involved. > Same goes for the filter modes and the bandwidth in the ABI documentation. > > If you think these attributes/properties are way too custom, I can drop them. It's interesting. I guess the point here is people want a nice autonomous system to keep the filter set appropriately for cleaning up a generated sine wave. It could be argued that is a hardware related thing so makes sense in DT. We are sort of 'emulating' a bandpass filter in the driver if we use this, but I guess if that's the main use case then this is perhaps a reasonable decision. > > Let me know your thoughts. > > > + $ref: /schemas/types.yaml#/definitions/uint64 > > > + > > > + '#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"; > > > + adi,bw-hz = /bits/ 64 <600000000>; > > > + }; > > > + }; > > > +... > > > + > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation 2021-11-09 12:31 [PATCH 0/4] Add support for ADMV8818 Antoniu Miclaus ` (2 preceding siblings ...) 2021-11-09 12:31 ` [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc Antoniu Miclaus @ 2021-11-09 12:31 ` Antoniu Miclaus 2021-11-12 17:56 ` Jonathan Cameron 3 siblings, 1 reply; 17+ messages in thread From: Antoniu Miclaus @ 2021-11-09 12:31 UTC (permalink / raw) To: jic23, robh+dt, linux-iio, devicetree, linux-kernel; +Cc: Antoniu Miclaus Add initial ABI documentation for admv8818 filter sysfs interfaces. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 +++++++++++++++++++ 1 file changed, 60 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..7fa5b0819055 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 @@ -0,0 +1,60 @@ +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/out_altvoltageY_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 -> enable/register the clock rate notifier + - manual -> disable/unregister the clock rate notifier + - bypass -> bypass LPF/HPF and disable/unregister the clock rate notifier + +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode +KernelVersion: +Contact: linux-iio@vger.kernel.org +Description: + This attribute configures the filter mode. + Reading returns the actual mode. + +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_bandwidth_3db_frequency +KernelVersion: +Contact: linux-iio@vger.kernel.org +Description: + Store the band pass bandwidth frequency value applied. + Reading returns the bandwidth frequency scaled. + + +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_center_frequency +KernelVersion: +Contact: linux-iio@vger.kernel.org +Description: + Store the band pass center frequency value applied. + Reading returns the center frequency scaled. -- 2.33.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation 2021-11-09 12:31 ` [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation Antoniu Miclaus @ 2021-11-12 17:56 ` Jonathan Cameron 2021-11-12 18:05 ` Jonathan Cameron 2021-11-16 14:43 ` Miclaus, Antoniu 0 siblings, 2 replies; 17+ messages in thread From: Jonathan Cameron @ 2021-11-12 17:56 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Tue, 9 Nov 2021 14:31:27 +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> > --- > .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 +++++++++++++++++++ > 1 file changed, 60 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..7fa5b0819055 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > @@ -0,0 +1,60 @@ > +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. I don't think this ABI really works unfortunately. What we are talking here is a bunch of selectable filters and one high pass + one low pass filter max can be enabled at a time. So two options, we either have simply a single out_altvoltage_filter_low_pass_3db_frequency out_altvoltage_filter_high_pass_3db_frequency Probably both with index 0 and index free channels are a silly idea given it's fine to just have one with index 0. or if there is sufficient reason to setup a selectable set of options then we could look at indexed filters and a _symbol type selection which may seem odd but generalises fairly well from Phase Shift Keying type symbol stuff we have had before (though still in staging because no one has cleaned the drivers up yet). > + > +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/out_altvoltageY_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 -> enable/register the clock rate notifier Hmm I'm wondering about the usecases of this. If this is being used with a clk device, then I think only the notifier option makes much sense. If it's not a clk that linux is aware of then manual makes more sense. > + - manual -> disable/unregister the clock rate notifier > + - bypass -> bypass LPF/HPF and disable/unregister the clock rate notifier This should be separate enable for the two filters though I think we've use the value 0 to mean this in the past. The bypasses look to be per filter anyway, so a single mode is insufficiently flexible. In the vast majority of cases, mode attributes are not used because they are always device specific and hence generic code has no idea what to do with them. > + > +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode > +KernelVersion: > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute configures the filter mode. > + Reading returns the actual mode. > + > +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_bandwidth_3db_frequency > +KernelVersion: > +Contact: linux-iio@vger.kernel.org > +Description: > + Store the band pass bandwidth frequency value applied. > + Reading returns the bandwidth frequency scaled. The device has no concept of bandpass that I can find so why are we introducing it? Let the user set the two filters to achieve this result. Userspace can do the maths for us :) > + > + > +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_center_frequency > +KernelVersion: > +Contact: linux-iio@vger.kernel.org > +Description: > + Store the band pass center frequency value applied. > + Reading returns the center frequency scaled. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation 2021-11-12 17:56 ` Jonathan Cameron @ 2021-11-12 18:05 ` Jonathan Cameron 2021-11-16 14:43 ` Miclaus, Antoniu 1 sibling, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2021-11-12 18:05 UTC (permalink / raw) To: Antoniu Miclaus; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Fri, 12 Nov 2021 17:56:25 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 9 Nov 2021 14:31:27 +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> > > --- > > .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 +++++++++++++++++++ > > 1 file changed, 60 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..7fa5b0819055 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > > @@ -0,0 +1,60 @@ > > +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. > > I don't think this ABI really works unfortunately. What we are talking here is a bunch of > selectable filters and one high pass + one low pass filter max can be enabled at a time. > > So two options, we either have simply a single > out_altvoltage_filter_low_pass_3db_frequency > out_altvoltage_filter_high_pass_3db_frequency > Probably both with index 0 and index free channels are a silly idea given it's fine to just have > one with index 0. > > or if there is sufficient reason to setup a selectable set of options then > we could look at indexed filters and a _symbol type selection which may seem > odd but generalises fairly well from Phase Shift Keying type symbol stuff we > have had before (though still in staging because no one has cleaned the drivers > up yet). Ignore this comment. You have already done what I'm suggesting and I didn't read you docs closely enough. Sorry about that! However, this is generic, move it to the main sysfs-bus-iio docs rather than in here. Snag is that we have to provide the values in Hz as that's what the ABI already has defined. If we have to define ways to deal with all the zeros as they don't fit in existing path then we can add those to the core. Jonathan > > > > + > > +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/out_altvoltageY_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 -> enable/register the clock rate notifier > > Hmm I'm wondering about the usecases of this. > > If this is being used with a clk device, then I think only the notifier option makes much > sense. If it's not a clk that linux is aware of then manual makes more sense. > > > + - manual -> disable/unregister the clock rate notifier > > + - bypass -> bypass LPF/HPF and disable/unregister the clock rate notifier > > This should be separate enable for the two filters though I think we've use the value 0 > to mean this in the past. The bypasses look to be per filter anyway, so a single > mode is insufficiently flexible. > > In the vast majority of cases, mode attributes are not used because they are always device > specific and hence generic code has no idea what to do with them. > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode > > +KernelVersion: > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + This attribute configures the filter mode. > > + Reading returns the actual mode. > > + > > +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_bandwidth_3db_frequency > > +KernelVersion: > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Store the band pass bandwidth frequency value applied. > > + Reading returns the bandwidth frequency scaled. > > The device has no concept of bandpass that I can find so why are we introducing it? > Let the user set the two filters to achieve this result. Userspace can do the maths for us :) > > > + > > + > > +What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_center_frequency > > +KernelVersion: > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Store the band pass center frequency value applied. > > + Reading returns the center frequency scaled. > ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation 2021-11-12 17:56 ` Jonathan Cameron 2021-11-12 18:05 ` Jonathan Cameron @ 2021-11-16 14:43 ` Miclaus, Antoniu 2021-11-21 12:32 ` Jonathan Cameron 1 sibling, 1 reply; 17+ messages in thread From: Miclaus, Antoniu @ 2021-11-16 14:43 UTC (permalink / raw) To: Jonathan Cameron; +Cc: robh+dt, linux-iio, devicetree, linux-kernel Hello Jonathan, -- Antoniu Miclăuş > -----Original Message----- > From: Jonathan Cameron <jic23@kernel.org> > Sent: Friday, November 12, 2021 7:56 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: robh+dt@kernel.org; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation > > [External] > > On Tue, 9 Nov 2021 14:31:27 +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> > > --- > > .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 +++++++++++++++++++ > > 1 file changed, 60 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..7fa5b0819055 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > > @@ -0,0 +1,60 @@ > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_high_pass_3 > db_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. > > I don't think this ABI really works unfortunately. What we are talking here is > a bunch of > selectable filters and one high pass + one low pass filter max can be enabled > at a time. > > So two options, we either have simply a single > out_altvoltage_filter_low_pass_3db_frequency > out_altvoltage_filter_high_pass_3db_frequency > Probably both with index 0 and index free channels are a silly idea given it's > fine to just have > one with index 0. > > or if there is sufficient reason to setup a selectable set of options then > we could look at indexed filters and a _symbol type selection which may > seem > odd but generalises fairly well from Phase Shift Keying type symbol stuff we > have had before (though still in staging because no one has cleaned the > drivers > up yet). > > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_low_pass_3 > db_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/out_altvoltageY_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 -> enable/register the clock rate notifier > > Hmm I'm wondering about the usecases of this. > > If this is being used with a clk device, then I think only the notifier option > makes much > sense. If it's not a clk that linux is aware of then manual makes more sense. > > > + - manual -> disable/unregister the clock rate notifier > > + - bypass -> bypass LPF/HPF and disable/unregister the clock > rate notifier > > This should be separate enable for the two filters though I think we've use > the value 0 > to mean this in the past. The bypasses look to be per filter anyway, so a > single > mode is insufficiently flexible. > > In the vast majority of cases, mode attributes are not used because they are > always device > specific and hence generic code has no idea what to do with them. > As I mentioned also in the dt-bindings comments, these attributes were added because they were requested by the users of the application in which this part was involved. If you think these attributes/properties are way too custom, I can drop them. Same goes for the bandwidth attribute. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode > > +KernelVersion: > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + This attribute configures the filter mode. > > + Reading returns the actual mode. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_ > bandwidth_3db_frequency > > +KernelVersion: > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Store the band pass bandwidth frequency value applied. > > + Reading returns the bandwidth frequency scaled. > > The device has no concept of bandpass that I can find so why are we > introducing it? > Let the user set the two filters to achieve this result. Userspace can do the > maths for us :) > > > + > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_ > center_frequency > > +KernelVersion: > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Store the band pass center frequency value applied. > > + Reading returns the center frequency scaled. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation 2021-11-16 14:43 ` Miclaus, Antoniu @ 2021-11-21 12:32 ` Jonathan Cameron 0 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2021-11-21 12:32 UTC (permalink / raw) To: Miclaus, Antoniu; +Cc: robh+dt, linux-iio, devicetree, linux-kernel On Tue, 16 Nov 2021 14:43:16 +0000 "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote: > Hello Jonathan, Hi Antoniu > > -- > Antoniu Miclăuş > > > -----Original Message----- > > From: Jonathan Cameron <jic23@kernel.org> > > Sent: Friday, November 12, 2021 7:56 PM > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > > Cc: robh+dt@kernel.org; linux-iio@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation > > > > [External] > > > > On Tue, 9 Nov 2021 14:31:27 +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> > > > --- > > > .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 +++++++++++++++++++ > > > 1 file changed, 60 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..7fa5b0819055 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > > > @@ -0,0 +1,60 @@ > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_high_pass_3 > > db_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. > > > > I don't think this ABI really works unfortunately. What we are talking here is > > a bunch of > > selectable filters and one high pass + one low pass filter max can be enabled > > at a time. > > > > So two options, we either have simply a single > > out_altvoltage_filter_low_pass_3db_frequency > > out_altvoltage_filter_high_pass_3db_frequency > > Probably both with index 0 and index free channels are a silly idea given it's > > fine to just have > > one with index 0. > > > > or if there is sufficient reason to setup a selectable set of options then > > we could look at indexed filters and a _symbol type selection which may > > seem > > odd but generalises fairly well from Phase Shift Keying type symbol stuff we > > have had before (though still in staging because no one has cleaned the > > drivers > > up yet). > > > > > > > + > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_low_pass_3 > > db_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/out_altvoltageY_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 -> enable/register the clock rate notifier Probably want to separate this description from the 'how' So "auto -> Adjust bandpass filter to track changes in input clock rate." or something along those lines. > > > > Hmm I'm wondering about the usecases of this. > > > > If this is being used with a clk device, then I think only the notifier option > > makes much > > sense. If it's not a clk that linux is aware of then manual makes more sense. > > > > > + - manual -> disable/unregister the clock rate notifier > > > + - bypass -> bypass LPF/HPF and disable/unregister the clock > > rate notifier > > > > This should be separate enable for the two filters though I think we've use > > the value 0 > > to mean this in the past. The bypasses look to be per filter anyway, so a > > single > > mode is insufficiently flexible. > > > > In the vast majority of cases, mode attributes are not used because they are > > always device > > specific and hence generic code has no idea what to do with them. > > > > As I mentioned also in the dt-bindings comments, these attributes were added > because they were requested by the users of the application in which this part > was involved. > > If you think these attributes/properties are way too custom, I can drop them. > > Same goes for the bandwidth attribute. If that's the most common use case then it's fine to keep them in my view. Bypass is disabling a particular filter though so we should express it like that. out_altvotage0_low_pass_filter_en etc Also I think a single 1/0 attribute called something like out_altvoltage0_filter_auto_en which, when set makes the control parameters read only. > > > > + > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode > > > +KernelVersion: > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + This attribute configures the filter mode. > > > + Reading returns the actual mode. > > > + > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_ > > bandwidth_3db_frequency > > > +KernelVersion: > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Store the band pass bandwidth frequency value applied. > > > + Reading returns the bandwidth frequency scaled. > > > > The device has no concept of bandpass that I can find so why are we > > introducing it? > > Let the user set the two filters to achieve this result. Userspace can do the > > maths for us :) Definitely expose the two filters separately. The auto path can control them appropriately but if things have moved to userspace control then I think exposing each filter is a better bet. Given we should be able to disable them independently it's more than possible a user will want just a low pass or just a high pass filter depending on their application. > > > > > + > > > + > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_ > > center_frequency > > > +KernelVersion: > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Store the band pass center frequency value applied. > > > + Reading returns the center frequency scaled. > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-11-21 12:27 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-09 12:31 [PATCH 0/4] Add support for ADMV8818 Antoniu Miclaus 2021-11-09 12:31 ` [PATCH 1/4] iio: add filter subfolder Antoniu Miclaus 2021-11-12 17:36 ` Jonathan Cameron 2021-11-12 17:44 ` Jonathan Cameron 2021-11-09 12:31 ` [PATCH 2/4] iio:filter:admv8818: add support for ADMV8818 Antoniu Miclaus 2021-11-12 18:06 ` Jonathan Cameron 2021-11-09 12:31 ` [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc Antoniu Miclaus 2021-11-10 1:07 ` Rob Herring 2021-11-10 9:39 ` Miclaus, Antoniu 2021-11-12 17:46 ` Jonathan Cameron 2021-11-16 14:43 ` Miclaus, Antoniu 2021-11-21 12:24 ` Jonathan Cameron 2021-11-09 12:31 ` [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation Antoniu Miclaus 2021-11-12 17:56 ` Jonathan Cameron 2021-11-12 18:05 ` Jonathan Cameron 2021-11-16 14:43 ` Miclaus, Antoniu 2021-11-21 12:32 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).