On Fri, 18 Feb 2022 17:07:38 +0200 Cristian Pop <cristian.pop@xxxxxxxxxx> wrote: > Add support for K Band Downconverter with Integrated > Fractional-N PLL and VCO. > More info: > https://www.analog.com/en/products/admv4420.html > > Signed-off-by: Cristian Pop <cristian.pop@xxxxxxxxxx> Just some really minor comment from a final read through. Mainly we need to leave time for futher DT review. Thanks, Jonathan > --- > drivers/iio/frequency/Kconfig | 10 + > drivers/iio/frequency/Makefile | 1 + > drivers/iio/frequency/admv4420.c | 400 +++++++++++++++++++++++++++++++ > 3 files changed, 411 insertions(+) > create mode 100644 drivers/iio/frequency/admv4420.c > > diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig > index b44036f843af..d905214a0be6 100644 > --- a/drivers/iio/frequency/Kconfig > +++ b/drivers/iio/frequency/Kconfig > @@ -60,6 +60,16 @@ config ADMV1013 > To compile this driver as a module, choose M here: the > module will be called admv1013. > > +config ADMV4420 > + tristate "Analog Devices ADMV4420 K Band Downconverter" > + depends on SPI > + help > + Say yes here to build support for Analog Devices K Band > + Downconverter with integrated Fractional-N PLL and VCO. > + > + To compile this driver as a module, choose M here: the > + module will be called admv4420. > + > config ADRF6780 > tristate "Analog Devices ADRF6780 Microwave Upconverter" > depends on SPI > diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile > index ae6899856c99..782e5baa1630 100644 > --- a/drivers/iio/frequency/Makefile > +++ b/drivers/iio/frequency/Makefile > @@ -8,4 +8,5 @@ obj-$(CONFIG_AD9523) += ad9523.o > obj-$(CONFIG_ADF4350) += adf4350.o > obj-$(CONFIG_ADF4371) += adf4371.o > obj-$(CONFIG_ADMV1013) += admv1013.o > +obj-$(CONFIG_ADMV4420) += admv4420.o > obj-$(CONFIG_ADRF6780) += adrf6780.o > diff --git a/drivers/iio/frequency/admv4420.c b/drivers/iio/frequency/admv4420.c > new file mode 100644 > index 000000000000..b4579505372e > --- /dev/null > +++ b/drivers/iio/frequency/admv4420.c > @@ -0,0 +1,400 @@ > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause > +/* > + * ADMV4420 > + * > + * Copyright 2021 Analog Devices Inc. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/spi/spi.h> > +#include <linux/units.h> > + > +#include <asm/unaligned.h> > + > +/* ADMV4420 Register Map */ > +#define ADMV4420_SPI_CONFIG_1 0x00 > +#define ADMV4420_SPI_CONFIG_2 0x01 > +#define ADMV4420_CHIPTYPE 0x03 > +#define ADMV4420_PRODUCT_ID_L 0x04 > +#define ADMV4420_PRODUCT_ID_H 0x05 > +#define ADMV4420_SCRATCHPAD 0x0A > +#define ADMV4420_SPI_REV 0x0B > +#define ADMV4420_ENABLES 0x103 > +#define ADMV4420_SDO_LEVEL 0x108 > +#define ADMV4420_INT_L 0x200 > +#define ADMV4420_INT_H 0x201 > +#define ADMV4420_FRAC_L 0x202 > +#define ADMV4420_FRAC_M 0x203 > +#define ADMV4420_FRAC_H 0x204 > +#define ADMV4420_MOD_L 0x208 > +#define ADMV4420_MOD_M 0x209 > +#define ADMV4420_MOD_H 0x20A > +#define ADMV4420_R_DIV_L 0x20C > +#define ADMV4420_R_DIV_H 0x20D > +#define ADMV4420_REFERENCE 0x20E > +#define ADMV4420_VCO_DATA_READBACK1 0x211 > +#define ADMV4420_VCO_DATA_READBACK2 0x212 > +#define ADMV4420_PLL_MUX_SEL 0x213 > +#define ADMV4420_LOCK_DETECT 0x214 > +#define ADMV4420_BAND_SELECT 0x215 > +#define ADMV4420_VCO_ALC_TIMEOUT 0x216 > +#define ADMV4420_VCO_MANUAL 0x217 > +#define ADMV4420_ALC 0x219 > +#define ADMV4420_VCO_TIMEOUT1 0x21C > +#define ADMV4420_VCO_TIMEOUT2 0x21D > +#define ADMV4420_VCO_BAND_DIV 0x21E > +#define ADMV4420_VCO_READBACK_SEL 0x21F > +#define ADMV4420_AUTOCAL 0x226 > +#define ADMV4420_CP_STATE 0x22C > +#define ADMV4420_CP_BLEED_EN 0x22D > +#define ADMV4420_CP_CURRENT 0x22E > +#define ADMV4420_CP_BLEED 0x22F > + > +#define ADMV4420_SPI_CONFIG_1_SDOACTIVE (BIT(4) | BIT(3)) > +#define ADMV4420_SPI_CONFIG_1_ENDIAN (BIT(5) | BIT(2)) > +#define ADMV4420_SPI_CONFIG_1_SOFTRESET (BIT(7) | BIT(1)) > + > +#define ADMV4420_REFERENCE_DIVIDE_BY_2_MASK BIT(0) > +#define ADMV4420_REFERENCE_MODE_MASK BIT(1) > +#define ADMV4420_REFERENCE_IN_MODE(x) FIELD_PREP(ADMV4420_REFERENCE_MODE_MASK, x) > +#define ADMV4420_REFERENCE_DOUBLER_MASK BIT(2) > +#define ADMV4420_REFERENCE_DOUBLER(x) FIELD_PREP(ADMV4420_REFERENCE_DOUBLER_MASK, x) I'd have preferred that you only defined the MASK then used FIELD_PREP inline as that tends to be easier to review than hiding macros behind macros. > + > + > +struct admv4420_state { > + struct spi_device *spi; > + struct regmap *regmap; > + u64 vco_freq_hz; > + u64 lo_freq_hz; > + struct admv4420_reference_block ref_block; > + struct admv4420_n_counter n_counter; > + enum admv4420_mux_sel mux_sel; > + struct mutex lock; > + u8 transf_buf[4] ____cacheline_aligned; > +}; ... > +static int admv4420_set_n_counter(struct admv4420_state *st, u32 int_val, > + u32 frac_val, u32 mod_val) > +{ > + int ret; > + > + put_unaligned_le32(frac_val, st->transf_buf); Comment rather than suggestion that you change this, but we actually know transf_buf is aligned, so could have used cpu_to_le32() as it is __cacheline_aligned which is way more than 32 bits. > + ret = regmap_bulk_write(st->regmap, ADMV4420_FRAC_L, st->transf_buf, 3); > + if (ret) > + return ret; > + > + put_unaligned_le32(mod_val, st->transf_buf); > + ret = regmap_bulk_write(st->regmap, ADMV4420_MOD_L, st->transf_buf, 3); > + if (ret) > + return ret; > + > + put_unaligned_le32(int_val, st->transf_buf); > + return regmap_bulk_write(st->regmap, ADMV4420_INT_L, st->transf_buf, 2); > +} > + ... > +static void admv4420_fw_parse(struct admv4420_state *st) > +{ > + struct fwnode_handle *fwnode = dev_fwnode(&st->spi->dev); > + u32 tmp; > + int ret; > + > + ret = fwnode_property_read_u32(fwnode, "adi,lo-freq-khz", &tmp); Why not device_property_read_u32? > + if (!ret) > + st->lo_freq_hz = (u64)tmp * KILO; > + > + st->ref_block.ref_single_ended = fwnode_property_read_bool(fwnode, > + "adi,ref-ext-single-ended-en"); device_property_read_bool()? > +} > +
