| Message ID | 20210716114210.141560-1-antoniu.miclaus@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jonathan Cameron |
| Headers | show |
| Series | [v6,1/2] iio: frequency: adrf6780: add support for ADRF6780 | expand |
On Fri, Jul 16, 2021 at 2:43 PM Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > The ADRF6780 is a silicon germanium (SiGe) design, wideband, > microwave upconverter optimized for point to point microwave > radio designs operating in the 5.9 GHz to 23.6 GHz frequency > range. > Datasheet: > https://www.analog.com/media/en/technical-documentation/data-sheets/ADRF6780.pdf Is it one line? If not, please put on one line and drop below the blank line so it will go as a tag. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> First question is why not to use the regmap API (I have heard it has gained support of 17 bit)? ... > + depends on COMMON_CLK Is it mandatory for any function inside the device? ... > +static int adrf6780_spi_read(struct adrf6780_dev *dev, unsigned int reg, > + unsigned int *val) > +{ > + int ret; > + struct spi_transfer t = {0}; > + dev->data[0] = 0x80 | (reg << 1); This 0x80 I guess is pretty much standard and regmap SPI supports it. > + dev->data[1] = 0x0; > + dev->data[2] = 0x0; > + > + t.rx_buf = &dev->data[0]; > + t.tx_buf = &dev->data[0]; > + t.len = 3; > + > + ret = spi_sync_transfer(dev->spi, &t, 1); > + if (ret) > + return ret; > + > + *val = (get_unaligned_be24(&dev->data[0]) >> 1) & GENMASK(15, 0); > + > + return ret; > +} ... > + usleep_range(200, 250); Needs a comment.
On Fri, 16 Jul 2021 14:42:09 +0300 Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > The ADRF6780 is a silicon germanium (SiGe) design, wideband, > microwave upconverter optimized for point to point microwave > radio designs operating in the 5.9 GHz to 23.6 GHz frequency > range. > > Datasheet: > https://www.analog.com/media/en/technical-documentation/data-sheets/ADRF6780.pdf > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> Hi Antoniu As it looks like we'll have a v7, a few trivial comments from me. ... > + > +static int adrf6780_init(struct adrf6780_dev *dev) > +{ > + int ret; > + unsigned int chip_id, enable_reg, enable_reg_msk; > + struct spi_device *spi = dev->spi; > + > + /* Perform a software reset */ > + ret = adrf6780_reset(dev); > + if (ret) > + return ret; > + > + ret = adrf6780_spi_read(dev, ADRF6780_REG_CONTROL, &chip_id); > + if (ret) > + return ret; > + > + chip_id = FIELD_GET(ADRF6780_CHIP_ID_MSK, chip_id); > + if (chip_id != ADRF6780_CHIP_ID) { > + dev_err(&spi->dev, "ADRF6780 Invalid Chip ID.\n"); > + return -EINVAL; > + } > + > + enable_reg_msk = ADRF6780_VGA_BUFFER_EN_MSK | > + ADRF6780_DETECTOR_EN_MSK | > + ADRF6780_LO_BUFFER_EN_MSK | > + ADRF6780_IF_MODE_EN_MSK | > + ADRF6780_IQ_MODE_EN_MSK | > + ADRF6780_LO_X2_EN_MSK | > + ADRF6780_LO_PPF_EN_MSK | > + ADRF6780_LO_EN_MSK | > + ADRF6780_UC_BIAS_EN_MSK; > + > + enable_reg = FIELD_PREP(ADRF6780_VGA_BUFFER_EN_MSK, dev->vga_buff_en) | > + FIELD_PREP(ADRF6780_DETECTOR_EN_MSK, 1) | > + FIELD_PREP(ADRF6780_LO_BUFFER_EN_MSK, dev->lo_buff_en) | > + FIELD_PREP(ADRF6780_IF_MODE_EN_MSK, dev->if_mode_en) | > + FIELD_PREP(ADRF6780_IQ_MODE_EN_MSK, dev->iq_mode_en) | > + FIELD_PREP(ADRF6780_LO_X2_EN_MSK, dev->lo_x2_en) | > + FIELD_PREP(ADRF6780_LO_PPF_EN_MSK, dev->lo_ppf_en) | > + FIELD_PREP(ADRF6780_LO_EN_MSK, dev->lo_en) | > + FIELD_PREP(ADRF6780_UC_BIAS_EN_MSK, dev->uc_bias_en); Here we are probably turning on a bunch of stuff which will result in power usage. Would it be sensible to turn it off again in remove path? (devm managed should be fine). > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ENABLE, enable_reg_msk, enable_reg); > + if (ret) > + return ret; > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, > + ADRF6780_LO_SIDEBAND_MSK, > + FIELD_PREP(ADRF6780_LO_SIDEBAND_MSK, dev->lo_sideband)); > + if (ret) > + return ret; > + > + return adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, > + ADRF6780_VDET_OUTPUT_SELECT_MSK, > + FIELD_PREP(ADRF6780_VDET_OUTPUT_SELECT_MSK, dev->vdet_out_en)); > +} > + > +static void adrf6780_dt_parse(struct adrf6780_dev *dev) Trivial nitpick, but given this is all device property based (great) the dt naming is now misleading. Perhaps _properties_parse() ? > +{ > + struct spi_device *spi = dev->spi; > + > + dev->vga_buff_en = device_property_read_bool(&spi->dev, "adi,vga-buff-en"); > + dev->lo_buff_en = device_property_read_bool(&spi->dev, "adi,lo-buff-en"); > + dev->if_mode_en = device_property_read_bool(&spi->dev, "adi,if-mode-en"); > + dev->iq_mode_en = device_property_read_bool(&spi->dev, "adi,iq-mode-en"); > + dev->lo_x2_en = device_property_read_bool(&spi->dev, "adi,lo-x2-en"); > + dev->lo_ppf_en = device_property_read_bool(&spi->dev, "adi,lo-ppf-en"); > + dev->lo_en = device_property_read_bool(&spi->dev, "adi,lo-en"); > + dev->uc_bias_en = device_property_read_bool(&spi->dev, "adi,uc-bias-en"); > + dev->lo_sideband = device_property_read_bool(&spi->dev, "adi,lo-sideband"); > + dev->vdet_out_en = device_property_read_bool(&spi->dev, "adi,vdet-out-en"); > +}
On Fri, 16 Jul 2021 14:42:09 +0300 Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > The ADRF6780 is a silicon germanium (SiGe) design, wideband, > microwave upconverter optimized for point to point microwave > radio designs operating in the 5.9 GHz to 23.6 GHz frequency > range. > > Datasheet: > https://www.analog.com/media/en/technical-documentation/data-sheets/ADRF6780.pdf > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> Hmm. I didn't really take my time to read the datasheet properly until now so wasn't totally sure what this device is doing. Sorry about that! Anyhow, I'd definitely likes some descriptive comments or stuff in the patch description to explain what the channels are actually mapping to. This is a rather different device than most of the frequency related devices we've seen before. I 'think' there is effectively only one output channel for this device and the things about it we can control are "phase" indirectly by tweaking the phase of the LO path. The linearize control doesn't seem to be directly related the scale of any channel, so not sure that's appropriate to map that way. The ADC is not reading a voltage as is specified here, but rather something that is almost linearly related to RF output power in dBm. Perhaps if you give a listing of the resulting sysfs ABI and what it controls here in the patch description of v7 we can see if we can figure out a more appropriate mapping to existing ABI (or extension of the defined ABI if necessary). I'm not totally convinced all the stuff in device tree belongs there either - the LO x2 for example feels like it might be more appropriate exposed to userspace in some fashion (it was this that got me spending the time to work out what the device actually does!) Jonathan > --- > changes in v6: > - fix warning detected by kernel test robot > drivers/iio/frequency/Kconfig | 12 + > drivers/iio/frequency/Makefile | 1 + > drivers/iio/frequency/adrf6780.c | 474 +++++++++++++++++++++++++++++++ > 3 files changed, 487 insertions(+) > create mode 100644 drivers/iio/frequency/adrf6780.c > > diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig > index 240b81502512..2c9e0559e8a4 100644 > --- a/drivers/iio/frequency/Kconfig > +++ b/drivers/iio/frequency/Kconfig > @@ -49,5 +49,17 @@ config ADF4371 > > To compile this driver as a module, choose M here: the > module will be called adf4371. > + > +config ADRF6780 > + tristate "Analog Devices ADRF6780 Microwave Upconverter" > + depends on SPI > + depends on COMMON_CLK > + help > + Say yes here to build support for Analog Devices ADRF6780 > + 5.9 GHz to 23.6 GHz, Wideband, Microwave Upconverter. > + > + To compile this driver as a module, choose M here: the > + module will be called adrf6780. > + > endmenu > endmenu > diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile > index 518b1e50caef..ae3136c79202 100644 > --- a/drivers/iio/frequency/Makefile > +++ b/drivers/iio/frequency/Makefile > @@ -7,3 +7,4 @@ > obj-$(CONFIG_AD9523) += ad9523.o > obj-$(CONFIG_ADF4350) += adf4350.o > obj-$(CONFIG_ADF4371) += adf4371.o > +obj-$(CONFIG_ADRF6780) += adrf6780.o > diff --git a/drivers/iio/frequency/adrf6780.c b/drivers/iio/frequency/adrf6780.c > new file mode 100644 > index 000000000000..6643dbc541c7 > --- /dev/null > +++ b/drivers/iio/frequency/adrf6780.c > @@ -0,0 +1,474 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ADRF6780 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/delay.h> > +#include <linux/device.h> > +#include <linux/iio/iio.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/spi/spi.h> > + > +#include <asm/unaligned.h> > + > +/* ADRF6780 Register Map */ > +#define ADRF6780_REG_CONTROL 0x00 > +#define ADRF6780_REG_ALARM_READBACK 0x01 > +#define ADRF6780_REG_ALARM_MASKS 0x02 > +#define ADRF6780_REG_ENABLE 0x03 > +#define ADRF6780_REG_LINEARIZE 0x04 > +#define ADRF6780_REG_LO_PATH 0x05 > +#define ADRF6780_REG_ADC_CONTROL 0x06 > +#define ADRF6780_REG_ADC_OUTPUT 0x0C > + > +/* ADRF6780_REG_CONTROL Map */ > +#define ADRF6780_PARITY_EN_MSK BIT(15) > +#define ADRF6780_SOFT_RESET_MSK BIT(14) > +#define ADRF6780_CHIP_ID_MSK GENMASK(11, 4) > +#define ADRF6780_CHIP_ID 0xA > +#define ADRF6780_CHIP_REVISION_MSK GENMASK(3, 0) > + > +/* ADRF6780_REG_ALARM_READBACK Map */ > +#define ADRF6780_PARITY_ERROR_MSK BIT(15) > +#define ADRF6780_TOO_FEW_ERRORS_MSK BIT(14) > +#define ADRF6780_TOO_MANY_ERRORS_MSK BIT(13) > +#define ADRF6780_ADDRESS_RANGE_ERROR_MSK BIT(12) > + > +/* ADRF6780_REG_ENABLE Map */ > +#define ADRF6780_VGA_BUFFER_EN_MSK BIT(8) > +#define ADRF6780_DETECTOR_EN_MSK BIT(7) > +#define ADRF6780_LO_BUFFER_EN_MSK BIT(6) > +#define ADRF6780_IF_MODE_EN_MSK BIT(5) > +#define ADRF6780_IQ_MODE_EN_MSK BIT(4) > +#define ADRF6780_LO_X2_EN_MSK BIT(3) > +#define ADRF6780_LO_PPF_EN_MSK BIT(2) > +#define ADRF6780_LO_EN_MSK BIT(1) > +#define ADRF6780_UC_BIAS_EN_MSK BIT(0) > + > +/* ADRF6780_REG_LINEARIZE Map */ > +#define ADRF6780_RDAC_LINEARIZE_MSK GENMASK(7, 0) > + > +/* ADRF6780_REG_LO_PATH Map */ > +#define ADRF6780_LO_SIDEBAND_MSK BIT(10) > +#define ADRF6780_Q_PATH_PHASE_ACCURACY_MSK GENMASK(7, 4) > +#define ADRF6780_I_PATH_PHASE_ACCURACY_MSK GENMASK(3, 0) > + > +/* ADRF6780_REG_ADC_CONTROL Map */ > +#define ADRF6780_VDET_OUTPUT_SELECT_MSK BIT(3) > +#define ADRF6780_ADC_START_MSK BIT(2) > +#define ADRF6780_ADC_EN_MSK BIT(1) > +#define ADRF6780_ADC_CLOCK_EN_MSK BIT(0) > + > +/* ADRF6780_REG_ADC_OUTPUT Map */ > +#define ADRF6780_ADC_STATUS_MSK BIT(8) > +#define ADRF6780_ADC_VALUE_MSK GENMASK(7, 0) > + > +struct adrf6780_dev { > + struct spi_device *spi; > + struct clk *clkin; > + /* Protect against concurrent accesses to the device */ > + struct mutex lock; > + bool vga_buff_en; > + bool lo_buff_en; > + bool if_mode_en; > + bool iq_mode_en; > + bool lo_x2_en; > + bool lo_ppf_en; > + bool lo_en; > + bool uc_bias_en; > + bool lo_sideband; > + bool vdet_out_en; > + u8 data[3] ____cacheline_aligned; > +}; > + > +static int adrf6780_spi_read(struct adrf6780_dev *dev, unsigned int reg, > + unsigned int *val) > +{ > + int ret; > + struct spi_transfer t = {0}; > + > + dev->data[0] = 0x80 | (reg << 1); > + dev->data[1] = 0x0; > + dev->data[2] = 0x0; > + > + t.rx_buf = &dev->data[0]; > + t.tx_buf = &dev->data[0]; > + t.len = 3; > + > + ret = spi_sync_transfer(dev->spi, &t, 1); > + if (ret) > + return ret; > + > + *val = (get_unaligned_be24(&dev->data[0]) >> 1) & GENMASK(15, 0); > + > + return ret; > +} > + > +static int adrf6780_spi_write(struct adrf6780_dev *dev, > + unsigned int reg, > + unsigned int val) > +{ > + put_unaligned_be24((val << 1) | (reg << 17), &dev->data[0]); > + > + return spi_write(dev->spi, &dev->data[0], 3); > +} > + > +static int adrf6780_spi_update_bits(struct adrf6780_dev *dev, unsigned int reg, > + unsigned int mask, unsigned int val) > +{ > + int ret; > + unsigned int data, temp; > + > + ret = adrf6780_spi_read(dev, reg, &data); > + if (ret) > + return ret; > + > + temp = (data & ~mask) | (val & mask); > + > + return adrf6780_spi_write(dev, reg, temp); > +} > + > +static int adrf6780_read_voltage_raw(struct adrf6780_dev *dev, unsigned int *read_val) > +{ > + int ret; > + > + mutex_lock(&dev->lock); > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, > + ADRF6780_ADC_EN_MSK | > + ADRF6780_ADC_CLOCK_EN_MSK | > + ADRF6780_ADC_START_MSK, > + FIELD_PREP(ADRF6780_ADC_EN_MSK, 1) | > + FIELD_PREP(ADRF6780_ADC_CLOCK_EN_MSK, 1) | > + FIELD_PREP(ADRF6780_ADC_START_MSK, 1)); > + if (ret) > + goto exit; > + > + usleep_range(200, 250); > + > + ret = adrf6780_spi_read(dev, ADRF6780_REG_ADC_OUTPUT, read_val); > + if (ret) > + goto exit; > + > + if (!(read_val & ADRF6780_ADC_STATUS_MSK)) { > + ret = -EINVAL; > + goto exit; > + } > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, > + ADRF6780_ADC_START_MSK, > + FIELD_PREP(ADRF6780_ADC_START_MSK, 0)); > + if (ret) > + goto exit; > + > + ret = adrf6780_spi_read(dev, ADRF6780_REG_ADC_OUTPUT, read_val); > + > +exit: > + mutex_unlock(&dev->lock); > + return ret; > +} > + > +static int adrf6780_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long info) > +{ > + struct adrf6780_dev *dev = iio_priv(indio_dev); > + unsigned int data; > + int ret; > + > + switch (info) { > + case IIO_CHAN_INFO_RAW: > + ret = adrf6780_read_voltage_raw(dev, &data); > + if (ret) > + return ret; > + > + *val = data & ADRF6780_ADC_VALUE_MSK; > + > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_SCALE: > + ret = adrf6780_spi_read(dev, ADRF6780_REG_LINEARIZE, &data); > + if (ret) > + return ret; > + > + *val = data & ADRF6780_RDAC_LINEARIZE_MSK; > + > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_PHASE: > + ret = adrf6780_spi_read(dev, ADRF6780_REG_LO_PATH, &data); > + if (ret) > + return ret; > + > + switch (chan->channel2) { > + case IIO_MOD_I: > + *val = data & ADRF6780_I_PATH_PHASE_ACCURACY_MSK; > + > + return IIO_VAL_INT; > + case IIO_MOD_Q: > + *val = FIELD_GET(ADRF6780_Q_PATH_PHASE_ACCURACY_MSK, data); > + > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + default: > + return -EINVAL; > + } > +} > + > +static int adrf6780_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long info) > +{ > + struct adrf6780_dev *dev = iio_priv(indio_dev); > + int ret; > + > + switch (info) { > + case IIO_CHAN_INFO_SCALE: > + return adrf6780_spi_write(dev, ADRF6780_REG_LINEARIZE, val); > + case IIO_CHAN_INFO_PHASE: > + switch (chan->channel2) { > + case IIO_MOD_I: > + mutex_lock(&dev->lock); > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, > + ADRF6780_I_PATH_PHASE_ACCURACY_MSK, > + FIELD_PREP(ADRF6780_I_PATH_PHASE_ACCURACY_MSK, val)); > + mutex_unlock(&dev->lock); > + > + return ret; > + case IIO_MOD_Q: > + mutex_lock(&dev->lock); > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, > + ADRF6780_Q_PATH_PHASE_ACCURACY_MSK, > + FIELD_PREP(ADRF6780_Q_PATH_PHASE_ACCURACY_MSK, val)); > + mutex_unlock(&dev->lock); > + > + return ret; > + default: > + return -EINVAL; > + } > + default: > + return -EINVAL; > + } > +} > + > +static int adrf6780_reg_access(struct iio_dev *indio_dev, > + unsigned int reg, > + unsigned int write_val, > + unsigned int *read_val) > +{ > + struct adrf6780_dev *dev = iio_priv(indio_dev); > + > + if (read_val) > + return adrf6780_spi_read(dev, reg, read_val); > + else > + return adrf6780_spi_write(dev, reg, write_val); > +} > + > +static const struct iio_info adrf6780_info = { > + .read_raw = adrf6780_read_raw, > + .write_raw = adrf6780_write_raw, > + .debugfs_reg_access = &adrf6780_reg_access, > +}; > + > +#define ADRF6780_CHAN(_channel) { \ > + .type = IIO_VOLTAGE, \ > + .output = 1, \ > + .indexed = 1, \ > + .channel = _channel, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_SCALE) \ > +} > + > +#define ADRF6780_CHAN_IQ(_channel, rf_comp) { \ > + .type = IIO_ALTVOLTAGE, \ > + .modified = 1, \ > + .output = 1, \ > + .indexed = 1, \ > + .channel2 = IIO_MOD_##rf_comp, \ > + .channel = _channel, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_PHASE) \ > +} > + > +static const struct iio_chan_spec adrf6780_channels[] = { > + ADRF6780_CHAN(0), > + ADRF6780_CHAN_IQ(0, I), > + ADRF6780_CHAN_IQ(0, Q), > +}; > + > +static int adrf6780_reset(struct adrf6780_dev *dev) > +{ > + int ret; > + struct spi_device *spi = dev->spi; > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_CONTROL, > + ADRF6780_SOFT_RESET_MSK, > + FIELD_PREP(ADRF6780_SOFT_RESET_MSK, 1)); > + if (ret) { > + dev_err(&spi->dev, "ADRF6780 SPI software reset failed.\n"); > + return ret; > + } > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_CONTROL, > + ADRF6780_SOFT_RESET_MSK, > + FIELD_PREP(ADRF6780_SOFT_RESET_MSK, 0)); > + if (ret) { > + dev_err(&spi->dev, "ADRF6780 SPI software reset disable failed.\n"); > + return ret; > + } > + > + return 0; > +} > + > +static int adrf6780_init(struct adrf6780_dev *dev) > +{ > + int ret; > + unsigned int chip_id, enable_reg, enable_reg_msk; > + struct spi_device *spi = dev->spi; > + > + /* Perform a software reset */ > + ret = adrf6780_reset(dev); > + if (ret) > + return ret; > + > + ret = adrf6780_spi_read(dev, ADRF6780_REG_CONTROL, &chip_id); > + if (ret) > + return ret; > + > + chip_id = FIELD_GET(ADRF6780_CHIP_ID_MSK, chip_id); > + if (chip_id != ADRF6780_CHIP_ID) { > + dev_err(&spi->dev, "ADRF6780 Invalid Chip ID.\n"); > + return -EINVAL; > + } > + > + enable_reg_msk = ADRF6780_VGA_BUFFER_EN_MSK | > + ADRF6780_DETECTOR_EN_MSK | > + ADRF6780_LO_BUFFER_EN_MSK | > + ADRF6780_IF_MODE_EN_MSK | > + ADRF6780_IQ_MODE_EN_MSK | > + ADRF6780_LO_X2_EN_MSK | > + ADRF6780_LO_PPF_EN_MSK | > + ADRF6780_LO_EN_MSK | > + ADRF6780_UC_BIAS_EN_MSK; > + > + enable_reg = FIELD_PREP(ADRF6780_VGA_BUFFER_EN_MSK, dev->vga_buff_en) | > + FIELD_PREP(ADRF6780_DETECTOR_EN_MSK, 1) | > + FIELD_PREP(ADRF6780_LO_BUFFER_EN_MSK, dev->lo_buff_en) | > + FIELD_PREP(ADRF6780_IF_MODE_EN_MSK, dev->if_mode_en) | > + FIELD_PREP(ADRF6780_IQ_MODE_EN_MSK, dev->iq_mode_en) | > + FIELD_PREP(ADRF6780_LO_X2_EN_MSK, dev->lo_x2_en) | > + FIELD_PREP(ADRF6780_LO_PPF_EN_MSK, dev->lo_ppf_en) | > + FIELD_PREP(ADRF6780_LO_EN_MSK, dev->lo_en) | > + FIELD_PREP(ADRF6780_UC_BIAS_EN_MSK, dev->uc_bias_en); > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ENABLE, enable_reg_msk, enable_reg); > + if (ret) > + return ret; > + > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, > + ADRF6780_LO_SIDEBAND_MSK, > + FIELD_PREP(ADRF6780_LO_SIDEBAND_MSK, dev->lo_sideband)); > + if (ret) > + return ret; > + > + return adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, > + ADRF6780_VDET_OUTPUT_SELECT_MSK, > + FIELD_PREP(ADRF6780_VDET_OUTPUT_SELECT_MSK, dev->vdet_out_en)); > +} > + > +static void adrf6780_clk_disable(void *data) > +{ > + clk_disable_unprepare(data); > +} > + > +static void adrf6780_dt_parse(struct adrf6780_dev *dev) > +{ > + struct spi_device *spi = dev->spi; > + > + dev->vga_buff_en = device_property_read_bool(&spi->dev, "adi,vga-buff-en"); > + dev->lo_buff_en = device_property_read_bool(&spi->dev, "adi,lo-buff-en"); > + dev->if_mode_en = device_property_read_bool(&spi->dev, "adi,if-mode-en"); > + dev->iq_mode_en = device_property_read_bool(&spi->dev, "adi,iq-mode-en"); > + dev->lo_x2_en = device_property_read_bool(&spi->dev, "adi,lo-x2-en"); > + dev->lo_ppf_en = device_property_read_bool(&spi->dev, "adi,lo-ppf-en"); > + dev->lo_en = device_property_read_bool(&spi->dev, "adi,lo-en"); > + dev->uc_bias_en = device_property_read_bool(&spi->dev, "adi,uc-bias-en"); > + dev->lo_sideband = device_property_read_bool(&spi->dev, "adi,lo-sideband"); > + dev->vdet_out_en = device_property_read_bool(&spi->dev, "adi,vdet-out-en"); > +} > + > +static int adrf6780_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct adrf6780_dev *dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*dev)); > + if (!indio_dev) > + return -ENOMEM; > + > + dev = iio_priv(indio_dev); > + > + indio_dev->info = &adrf6780_info; > + indio_dev->name = "adrf6780"; > + indio_dev->channels = adrf6780_channels; > + indio_dev->num_channels = ARRAY_SIZE(adrf6780_channels); > + > + dev->spi = spi; > + > + adrf6780_dt_parse(dev); > + > + dev->clkin = devm_clk_get(&spi->dev, "lo_in"); > + if (IS_ERR(dev->clkin)) > + return PTR_ERR(dev->clkin); > + > + ret = clk_prepare_enable(dev->clkin); > + if (ret < 0) > + return ret; > + > + ret = devm_add_action_or_reset(&spi->dev, adrf6780_clk_disable, dev->clkin); > + if (ret < 0) > + return ret; > + > + mutex_init(&dev->lock); > + > + ret = adrf6780_init(dev); > + if (ret) > + return ret; > + > + return devm_iio_device_register(&spi->dev, indio_dev); > +} > + > +static const struct spi_device_id adrf6780_id[] = { > + { "adrf6780", 0 }, > + {} > +}; > +MODULE_DEVICE_TABLE(spi, adrf6780_id); > + > +static const struct of_device_id adrf6780_of_match[] = { > + { .compatible = "adi,adrf6780" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, adrf6780_of_match); > + > +static struct spi_driver adrf6780_driver = { > + .driver = { > + .name = "adrf6780", > + .of_match_table = adrf6780_of_match, > + }, > + .probe = adrf6780_probe, > + .id_table = adrf6780_id, > +}; > +module_spi_driver(adrf6780_driver); > + > +MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com"); > +MODULE_DESCRIPTION("Analog Devices ADRF6780"); > +MODULE_LICENSE("GPL v2");
Hi Antoniu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on iio/togreg]
[also build test ERROR on linux/master linus/master v5.14-rc1 next-20210716]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Antoniu-Miclaus/iio-frequency-adrf6780-add-support-for-ADRF6780/20210718-104320
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/59f8f05a22397c9b919c512d1e28cbad5caf78a7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Antoniu-Miclaus/iio-frequency-adrf6780-add-support-for-ADRF6780/20210718-104320
git checkout 59f8f05a22397c9b919c512d1e28cbad5caf78a7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/iio/frequency/adrf6780.c: In function 'adrf6780_read_voltage_raw':
>> drivers/iio/frequency/adrf6780.c:161:17: error: invalid operands to binary & (have 'unsigned int *' and 'long unsigned int')
161 | if (!(read_val & ADRF6780_ADC_STATUS_MSK)) {
| ^
vim +161 drivers/iio/frequency/adrf6780.c
138
139 static int adrf6780_read_voltage_raw(struct adrf6780_dev *dev, unsigned int *read_val)
140 {
141 int ret;
142
143 mutex_lock(&dev->lock);
144
145 ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL,
146 ADRF6780_ADC_EN_MSK |
147 ADRF6780_ADC_CLOCK_EN_MSK |
148 ADRF6780_ADC_START_MSK,
149 FIELD_PREP(ADRF6780_ADC_EN_MSK, 1) |
150 FIELD_PREP(ADRF6780_ADC_CLOCK_EN_MSK, 1) |
151 FIELD_PREP(ADRF6780_ADC_START_MSK, 1));
152 if (ret)
153 goto exit;
154
155 usleep_range(200, 250);
156
157 ret = adrf6780_spi_read(dev, ADRF6780_REG_ADC_OUTPUT, read_val);
158 if (ret)
159 goto exit;
160
> 161 if (!(read_val & ADRF6780_ADC_STATUS_MSK)) {
162 ret = -EINVAL;
163 goto exit;
164 }
165
166 ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL,
167 ADRF6780_ADC_START_MSK,
168 FIELD_PREP(ADRF6780_ADC_START_MSK, 0));
169 if (ret)
170 goto exit;
171
172 ret = adrf6780_spi_read(dev, ADRF6780_REG_ADC_OUTPUT, read_val);
173
174 exit:
175 mutex_unlock(&dev->lock);
176 return ret;
177 }
178
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hello Andy, Thanks for the review: > -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@gmail.com> > Sent: Friday, July 16, 2021 5:53 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; > jic23@kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH v6 1/2] iio: frequency: adrf6780: add support for > ADRF6780 > > [External] > > On Fri, Jul 16, 2021 at 2:43 PM Antoniu Miclaus > <antoniu.miclaus@analog.com> wrote: > > > > The ADRF6780 is a silicon germanium (SiGe) design, wideband, > > microwave upconverter optimized for point to point microwave > > radio designs operating in the 5.9 GHz to 23.6 GHz frequency > > range. > > > Datasheet: > > https://www.analog.com/media/en/technical-documentation/data- > sheets/ADRF6780.pdf > > Is it one line? If not, please put on one line and drop below the > blank line so it will go as a tag. Yes, it is one line. > > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > First question is why not to use the regmap API (I have heard it has > gained support of 17 bit)? Initially that was the plan, but after this patch: https://github.com/torvalds/linux/commit/4191f19792bf91267835eb090d970e9cd6277a65 the custom write formats for regmap allow the read only via cached registers. Therefore, I preferred using spi transfers for write/read to/from the device. > ... > > > + depends on COMMON_CLK > > Is it mandatory for any function inside the device? Yes. It will serve as LO input to the device. > > ... > > > +static int adrf6780_spi_read(struct adrf6780_dev *dev, unsigned int reg, > > + unsigned int *val) > > +{ > > + int ret; > > + struct spi_transfer t = {0}; > > > + dev->data[0] = 0x80 | (reg << 1); > > This 0x80 I guess is pretty much standard and regmap SPI supports it. > > > + dev->data[1] = 0x0; > > + dev->data[2] = 0x0; > > + > > + t.rx_buf = &dev->data[0]; > > + t.tx_buf = &dev->data[0]; > > + t.len = 3; > > + > > + ret = spi_sync_transfer(dev->spi, &t, 1); > > + if (ret) > > + return ret; > > + > > + *val = (get_unaligned_be24(&dev->data[0]) >> 1) & GENMASK(15, 0); > > + > > + return ret; > > +} > > ... > > > + usleep_range(200, 250); > > Needs a comment. Will add in next patch version. > > -- > With Best Regards, > Andy Shevchenko Regards, Antoniu
Hello Jonathan, > -----Original Message----- > From: Jonathan Cameron <jic23@kernel.org> > Sent: Saturday, July 17, 2021 4:26 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v6 1/2] iio: frequency: adrf6780: add support for > ADRF6780 > > [External] > > On Fri, 16 Jul 2021 14:42:09 +0300 > Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > > The ADRF6780 is a silicon germanium (SiGe) design, wideband, > > microwave upconverter optimized for point to point microwave > > radio designs operating in the 5.9 GHz to 23.6 GHz frequency > > range. > > > > Datasheet: > > https://www.analog.com/media/en/technical-documentation/data- > sheets/ADRF6780.pdf > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > Hi Antoniu > > As it looks like we'll have a v7, a few trivial comments from me. > > ... > > > + > > +static int adrf6780_init(struct adrf6780_dev *dev) > > +{ > > + int ret; > > + unsigned int chip_id, enable_reg, enable_reg_msk; > > + struct spi_device *spi = dev->spi; > > + > > + /* Perform a software reset */ > > + ret = adrf6780_reset(dev); > > + if (ret) > > + return ret; > > + > > + ret = adrf6780_spi_read(dev, ADRF6780_REG_CONTROL, &chip_id); > > + if (ret) > > + return ret; > > + > > + chip_id = FIELD_GET(ADRF6780_CHIP_ID_MSK, chip_id); > > + if (chip_id != ADRF6780_CHIP_ID) { > > + dev_err(&spi->dev, "ADRF6780 Invalid Chip ID.\n"); > > + return -EINVAL; > > + } > > + > > + enable_reg_msk = ADRF6780_VGA_BUFFER_EN_MSK | > > + ADRF6780_DETECTOR_EN_MSK | > > + ADRF6780_LO_BUFFER_EN_MSK | > > + ADRF6780_IF_MODE_EN_MSK | > > + ADRF6780_IQ_MODE_EN_MSK | > > + ADRF6780_LO_X2_EN_MSK | > > + ADRF6780_LO_PPF_EN_MSK | > > + ADRF6780_LO_EN_MSK | > > + ADRF6780_UC_BIAS_EN_MSK; > > + > > + enable_reg = FIELD_PREP(ADRF6780_VGA_BUFFER_EN_MSK, dev- > >vga_buff_en) | > > + FIELD_PREP(ADRF6780_DETECTOR_EN_MSK, 1) | > > + FIELD_PREP(ADRF6780_LO_BUFFER_EN_MSK, dev- > >lo_buff_en) | > > + FIELD_PREP(ADRF6780_IF_MODE_EN_MSK, dev- > >if_mode_en) | > > + FIELD_PREP(ADRF6780_IQ_MODE_EN_MSK, dev- > >iq_mode_en) | > > + FIELD_PREP(ADRF6780_LO_X2_EN_MSK, dev- > >lo_x2_en) | > > + FIELD_PREP(ADRF6780_LO_PPF_EN_MSK, dev- > >lo_ppf_en) | > > + FIELD_PREP(ADRF6780_LO_EN_MSK, dev->lo_en) | > > + FIELD_PREP(ADRF6780_UC_BIAS_EN_MSK, dev- > >uc_bias_en); > > Here we are probably turning on a bunch of stuff which will result in power > usage. > Would it be sensible to turn it off again in remove path? (devm managed > should be fine). Most of these attributes are enabled by default after device reset. Taking into account this statement, is it still worth adding a 'custom' remove path? > > > > + > > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ENABLE, > enable_reg_msk, enable_reg); > > + if (ret) > > + return ret; > > + > > + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, > > + > ADRF6780_LO_SIDEBAND_MSK, > > + > FIELD_PREP(ADRF6780_LO_SIDEBAND_MSK, dev->lo_sideband)); > > + if (ret) > > + return ret; > > + > > + return adrf6780_spi_update_bits(dev, > ADRF6780_REG_ADC_CONTROL, > > + > ADRF6780_VDET_OUTPUT_SELECT_MSK, > > + > FIELD_PREP(ADRF6780_VDET_OUTPUT_SELECT_MSK, dev- > >vdet_out_en)); > > +} > > + > > > +static void adrf6780_dt_parse(struct adrf6780_dev *dev) > > Trivial nitpick, but given this is all device property based (great) > the dt naming is now misleading. Perhaps _properties_parse() ? > > > +{ > > + struct spi_device *spi = dev->spi; > > + > > + dev->vga_buff_en = device_property_read_bool(&spi->dev, > "adi,vga-buff-en"); > > + dev->lo_buff_en = device_property_read_bool(&spi->dev, "adi,lo- > buff-en"); > > + dev->if_mode_en = device_property_read_bool(&spi->dev, "adi,if- > mode-en"); > > + dev->iq_mode_en = device_property_read_bool(&spi->dev, "adi,iq- > mode-en"); > > + dev->lo_x2_en = device_property_read_bool(&spi->dev, "adi,lo-x2- > en"); > > + dev->lo_ppf_en = device_property_read_bool(&spi->dev, "adi,lo- > ppf-en"); > > + dev->lo_en = device_property_read_bool(&spi->dev, "adi,lo-en"); > > + dev->uc_bias_en = device_property_read_bool(&spi->dev, "adi,uc- > bias-en"); > > + dev->lo_sideband = device_property_read_bool(&spi->dev, "adi,lo- > sideband"); > > + dev->vdet_out_en = device_property_read_bool(&spi->dev, > "adi,vdet-out-en"); > > +} -- Antoniu Miclăuş
On Tue, Jul 20, 2021 at 4:17 PM Miclaus, Antoniu <Antoniu.Miclaus@analog.com> wrote: > > From: Andy Shevchenko <andy.shevchenko@gmail.com> > > Sent: Friday, July 16, 2021 5:53 PM > > On Fri, Jul 16, 2021 at 2:43 PM Antoniu Miclaus > > <antoniu.miclaus@analog.com> wrote: ... > > First question is why not to use the regmap API (I have heard it has > > gained support of 17 bit)? > > Initially that was the plan, but after this patch: > https://github.com/torvalds/linux/commit/4191f19792bf91267835eb090d970e9cd6277a65 > the custom write formats for regmap allow the read only via cached registers. > > Therefore, I preferred using spi transfers for write/read to/from the device. Not sure I follow you. That patch is upstream. Does it prevent you from switching to regmap SPI API? ... > > > + depends on COMMON_CLK > > > > Is it mandatory for any function inside the device? > > Yes. It will serve as LO input to the device. But can the device work without it (with limited functionality)? -- With Best Regards, Andy Shevchenko
> -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@gmail.com> > Sent: Tuesday, July 20, 2021 5:08 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; > jic23@kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH v6 1/2] iio: frequency: adrf6780: add support for > ADRF6780 > > [External] > > On Tue, Jul 20, 2021 at 4:17 PM Miclaus, Antoniu > <Antoniu.Miclaus@analog.com> wrote: > > > From: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Sent: Friday, July 16, 2021 5:53 PM > > > On Fri, Jul 16, 2021 at 2:43 PM Antoniu Miclaus > > > <antoniu.miclaus@analog.com> wrote: > > ... > > > > First question is why not to use the regmap API (I have heard it has > > > gained support of 17 bit)? > > > > Initially that was the plan, but after this patch: > > > https://urldefense.com/v3/__https://github.com/torvalds/linux/commit/41 > 91f19792bf91267835eb090d970e9cd6277a65__;!!A3Ni8CS0y2Y!ptUV0YC2nfD > 6AdH_y5U0ziyELl4B9pDL0ubkdpFHFtrFNE_NqUS_TWm_gE-SlHV315Ak$ > > the custom write formats for regmap allow the read only via cached > registers. > > > > Therefore, I preferred using spi transfers for write/read to/from the > device. > > Not sure I follow you. That patch is upstream. Does it prevent you > from switching to regmap SPI API? It does not prevent me from switching to regmap SPI API. It will prevent me from using regmap_read to read directly from device. https://github.com/torvalds/linux/commit/4191f19792bf91267835eb090d970e9cd6277a65#diff-3e0b7d2f0a55adb6573693a514cb8024a81a55da848cb22632ab5fd4dd6dd4e4R39 > > ... > > > > > + depends on COMMON_CLK > > > > > > Is it mandatory for any function inside the device? > > > > Yes. It will serve as LO input to the device. > > But can the device work without it (with limited functionality)? This aspect is not clearly stated in the datasheet, but since the Enable register has the LO enabled by default, I considered this aspect as being mandatory. I will try to find out more information from the hardware guys who developed the part. > > -- > With Best Regards, > Andy Shevchenko
On Tue, Jul 20, 2021 at 5:33 PM Miclaus, Antoniu <Antoniu.Miclaus@analog.com> wrote: > > From: Andy Shevchenko <andy.shevchenko@gmail.com> > > Sent: Tuesday, July 20, 2021 5:08 PM > > On Tue, Jul 20, 2021 at 4:17 PM Miclaus, Antoniu > > <Antoniu.Miclaus@analog.com> wrote: > > > > From: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > Sent: Friday, July 16, 2021 5:53 PM > > > > On Fri, Jul 16, 2021 at 2:43 PM Antoniu Miclaus > > > > <antoniu.miclaus@analog.com> wrote: ... > > > > First question is why not to use the regmap API (I have heard it has > > > > gained support of 17 bit)? > > > > > > Initially that was the plan, but after this patch: > > > > > https://urldefense.com/v3/__https://github.com/torvalds/linux/commit/41 > > 91f19792bf91267835eb090d970e9cd6277a65__;!!A3Ni8CS0y2Y!ptUV0YC2nfD > > 6AdH_y5U0ziyELl4B9pDL0ubkdpFHFtrFNE_NqUS_TWm_gE-SlHV315Ak$ > > > the custom write formats for regmap allow the read only via cached > > registers. > > > > > > Therefore, I preferred using spi transfers for write/read to/from the > > device. > > > > Not sure I follow you. That patch is upstream. Does it prevent you > > from switching to regmap SPI API? > > It does not prevent me from switching to regmap SPI API. > It will prevent me from using regmap_read to read directly from device. > https://github.com/torvalds/linux/commit/4191f19792bf91267835eb090d970e9cd6277a65#diff-3e0b7d2f0a55adb6573693a514cb8024a81a55da848cb22632ab5fd4dd6dd4e4R39 How? If it's the case it's a huge regression in the API and must be fixed ASAP.
On Tue, 20 Jul 2021 13:34:08 +0000 "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote: > Hello Jonathan, ... > > > + > > > + enable_reg = FIELD_PREP(ADRF6780_VGA_BUFFER_EN_MSK, dev- > > >vga_buff_en) | > > > + FIELD_PREP(ADRF6780_DETECTOR_EN_MSK, 1) | > > > + FIELD_PREP(ADRF6780_LO_BUFFER_EN_MSK, dev- > > >lo_buff_en) | > > > + FIELD_PREP(ADRF6780_IF_MODE_EN_MSK, dev- > > >if_mode_en) | > > > + FIELD_PREP(ADRF6780_IQ_MODE_EN_MSK, dev- > > >iq_mode_en) | > > > + FIELD_PREP(ADRF6780_LO_X2_EN_MSK, dev- > > >lo_x2_en) | > > > + FIELD_PREP(ADRF6780_LO_PPF_EN_MSK, dev- > > >lo_ppf_en) | > > > + FIELD_PREP(ADRF6780_LO_EN_MSK, dev->lo_en) | > > > + FIELD_PREP(ADRF6780_UC_BIAS_EN_MSK, dev- > > >uc_bias_en); > > > > Here we are probably turning on a bunch of stuff which will result in power > > usage. > > Would it be sensible to turn it off again in remove path? (devm managed > > should be fine). > > Most of these attributes are enabled by default after device reset. > Taking into account this statement, is it still worth adding a 'custom' remove path? Perhaps a nice to have if they save power. Got to love hardware that eats power until you load a driver! Ah well, Jonathan
diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig index 240b81502512..2c9e0559e8a4 100644 --- a/drivers/iio/frequency/Kconfig +++ b/drivers/iio/frequency/Kconfig @@ -49,5 +49,17 @@ config ADF4371 To compile this driver as a module, choose M here: the module will be called adf4371. + +config ADRF6780 + tristate "Analog Devices ADRF6780 Microwave Upconverter" + depends on SPI + depends on COMMON_CLK + help + Say yes here to build support for Analog Devices ADRF6780 + 5.9 GHz to 23.6 GHz, Wideband, Microwave Upconverter. + + To compile this driver as a module, choose M here: the + module will be called adrf6780. + endmenu endmenu diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile index 518b1e50caef..ae3136c79202 100644 --- a/drivers/iio/frequency/Makefile +++ b/drivers/iio/frequency/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_AD9523) += ad9523.o obj-$(CONFIG_ADF4350) += adf4350.o obj-$(CONFIG_ADF4371) += adf4371.o +obj-$(CONFIG_ADRF6780) += adrf6780.o diff --git a/drivers/iio/frequency/adrf6780.c b/drivers/iio/frequency/adrf6780.c new file mode 100644 index 000000000000..6643dbc541c7 --- /dev/null +++ b/drivers/iio/frequency/adrf6780.c @@ -0,0 +1,474 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ADRF6780 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/delay.h> +#include <linux/device.h> +#include <linux/iio/iio.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/spi/spi.h> + +#include <asm/unaligned.h> + +/* ADRF6780 Register Map */ +#define ADRF6780_REG_CONTROL 0x00 +#define ADRF6780_REG_ALARM_READBACK 0x01 +#define ADRF6780_REG_ALARM_MASKS 0x02 +#define ADRF6780_REG_ENABLE 0x03 +#define ADRF6780_REG_LINEARIZE 0x04 +#define ADRF6780_REG_LO_PATH 0x05 +#define ADRF6780_REG_ADC_CONTROL 0x06 +#define ADRF6780_REG_ADC_OUTPUT 0x0C + +/* ADRF6780_REG_CONTROL Map */ +#define ADRF6780_PARITY_EN_MSK BIT(15) +#define ADRF6780_SOFT_RESET_MSK BIT(14) +#define ADRF6780_CHIP_ID_MSK GENMASK(11, 4) +#define ADRF6780_CHIP_ID 0xA +#define ADRF6780_CHIP_REVISION_MSK GENMASK(3, 0) + +/* ADRF6780_REG_ALARM_READBACK Map */ +#define ADRF6780_PARITY_ERROR_MSK BIT(15) +#define ADRF6780_TOO_FEW_ERRORS_MSK BIT(14) +#define ADRF6780_TOO_MANY_ERRORS_MSK BIT(13) +#define ADRF6780_ADDRESS_RANGE_ERROR_MSK BIT(12) + +/* ADRF6780_REG_ENABLE Map */ +#define ADRF6780_VGA_BUFFER_EN_MSK BIT(8) +#define ADRF6780_DETECTOR_EN_MSK BIT(7) +#define ADRF6780_LO_BUFFER_EN_MSK BIT(6) +#define ADRF6780_IF_MODE_EN_MSK BIT(5) +#define ADRF6780_IQ_MODE_EN_MSK BIT(4) +#define ADRF6780_LO_X2_EN_MSK BIT(3) +#define ADRF6780_LO_PPF_EN_MSK BIT(2) +#define ADRF6780_LO_EN_MSK BIT(1) +#define ADRF6780_UC_BIAS_EN_MSK BIT(0) + +/* ADRF6780_REG_LINEARIZE Map */ +#define ADRF6780_RDAC_LINEARIZE_MSK GENMASK(7, 0) + +/* ADRF6780_REG_LO_PATH Map */ +#define ADRF6780_LO_SIDEBAND_MSK BIT(10) +#define ADRF6780_Q_PATH_PHASE_ACCURACY_MSK GENMASK(7, 4) +#define ADRF6780_I_PATH_PHASE_ACCURACY_MSK GENMASK(3, 0) + +/* ADRF6780_REG_ADC_CONTROL Map */ +#define ADRF6780_VDET_OUTPUT_SELECT_MSK BIT(3) +#define ADRF6780_ADC_START_MSK BIT(2) +#define ADRF6780_ADC_EN_MSK BIT(1) +#define ADRF6780_ADC_CLOCK_EN_MSK BIT(0) + +/* ADRF6780_REG_ADC_OUTPUT Map */ +#define ADRF6780_ADC_STATUS_MSK BIT(8) +#define ADRF6780_ADC_VALUE_MSK GENMASK(7, 0) + +struct adrf6780_dev { + struct spi_device *spi; + struct clk *clkin; + /* Protect against concurrent accesses to the device */ + struct mutex lock; + bool vga_buff_en; + bool lo_buff_en; + bool if_mode_en; + bool iq_mode_en; + bool lo_x2_en; + bool lo_ppf_en; + bool lo_en; + bool uc_bias_en; + bool lo_sideband; + bool vdet_out_en; + u8 data[3] ____cacheline_aligned; +}; + +static int adrf6780_spi_read(struct adrf6780_dev *dev, unsigned int reg, + unsigned int *val) +{ + int ret; + struct spi_transfer t = {0}; + + dev->data[0] = 0x80 | (reg << 1); + dev->data[1] = 0x0; + dev->data[2] = 0x0; + + t.rx_buf = &dev->data[0]; + t.tx_buf = &dev->data[0]; + t.len = 3; + + ret = spi_sync_transfer(dev->spi, &t, 1); + if (ret) + return ret; + + *val = (get_unaligned_be24(&dev->data[0]) >> 1) & GENMASK(15, 0); + + return ret; +} + +static int adrf6780_spi_write(struct adrf6780_dev *dev, + unsigned int reg, + unsigned int val) +{ + put_unaligned_be24((val << 1) | (reg << 17), &dev->data[0]); + + return spi_write(dev->spi, &dev->data[0], 3); +} + +static int adrf6780_spi_update_bits(struct adrf6780_dev *dev, unsigned int reg, + unsigned int mask, unsigned int val) +{ + int ret; + unsigned int data, temp; + + ret = adrf6780_spi_read(dev, reg, &data); + if (ret) + return ret; + + temp = (data & ~mask) | (val & mask); + + return adrf6780_spi_write(dev, reg, temp); +} + +static int adrf6780_read_voltage_raw(struct adrf6780_dev *dev, unsigned int *read_val) +{ + int ret; + + mutex_lock(&dev->lock); + + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, + ADRF6780_ADC_EN_MSK | + ADRF6780_ADC_CLOCK_EN_MSK | + ADRF6780_ADC_START_MSK, + FIELD_PREP(ADRF6780_ADC_EN_MSK, 1) | + FIELD_PREP(ADRF6780_ADC_CLOCK_EN_MSK, 1) | + FIELD_PREP(ADRF6780_ADC_START_MSK, 1)); + if (ret) + goto exit; + + usleep_range(200, 250); + + ret = adrf6780_spi_read(dev, ADRF6780_REG_ADC_OUTPUT, read_val); + if (ret) + goto exit; + + if (!(read_val & ADRF6780_ADC_STATUS_MSK)) { + ret = -EINVAL; + goto exit; + } + + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, + ADRF6780_ADC_START_MSK, + FIELD_PREP(ADRF6780_ADC_START_MSK, 0)); + if (ret) + goto exit; + + ret = adrf6780_spi_read(dev, ADRF6780_REG_ADC_OUTPUT, read_val); + +exit: + mutex_unlock(&dev->lock); + return ret; +} + +static int adrf6780_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long info) +{ + struct adrf6780_dev *dev = iio_priv(indio_dev); + unsigned int data; + int ret; + + switch (info) { + case IIO_CHAN_INFO_RAW: + ret = adrf6780_read_voltage_raw(dev, &data); + if (ret) + return ret; + + *val = data & ADRF6780_ADC_VALUE_MSK; + + return IIO_VAL_INT; + + case IIO_CHAN_INFO_SCALE: + ret = adrf6780_spi_read(dev, ADRF6780_REG_LINEARIZE, &data); + if (ret) + return ret; + + *val = data & ADRF6780_RDAC_LINEARIZE_MSK; + + return IIO_VAL_INT; + case IIO_CHAN_INFO_PHASE: + ret = adrf6780_spi_read(dev, ADRF6780_REG_LO_PATH, &data); + if (ret) + return ret; + + switch (chan->channel2) { + case IIO_MOD_I: + *val = data & ADRF6780_I_PATH_PHASE_ACCURACY_MSK; + + return IIO_VAL_INT; + case IIO_MOD_Q: + *val = FIELD_GET(ADRF6780_Q_PATH_PHASE_ACCURACY_MSK, data); + + return IIO_VAL_INT; + default: + return -EINVAL; + } + default: + return -EINVAL; + } +} + +static int adrf6780_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long info) +{ + struct adrf6780_dev *dev = iio_priv(indio_dev); + int ret; + + switch (info) { + case IIO_CHAN_INFO_SCALE: + return adrf6780_spi_write(dev, ADRF6780_REG_LINEARIZE, val); + case IIO_CHAN_INFO_PHASE: + switch (chan->channel2) { + case IIO_MOD_I: + mutex_lock(&dev->lock); + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, + ADRF6780_I_PATH_PHASE_ACCURACY_MSK, + FIELD_PREP(ADRF6780_I_PATH_PHASE_ACCURACY_MSK, val)); + mutex_unlock(&dev->lock); + + return ret; + case IIO_MOD_Q: + mutex_lock(&dev->lock); + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, + ADRF6780_Q_PATH_PHASE_ACCURACY_MSK, + FIELD_PREP(ADRF6780_Q_PATH_PHASE_ACCURACY_MSK, val)); + mutex_unlock(&dev->lock); + + return ret; + default: + return -EINVAL; + } + default: + return -EINVAL; + } +} + +static int adrf6780_reg_access(struct iio_dev *indio_dev, + unsigned int reg, + unsigned int write_val, + unsigned int *read_val) +{ + struct adrf6780_dev *dev = iio_priv(indio_dev); + + if (read_val) + return adrf6780_spi_read(dev, reg, read_val); + else + return adrf6780_spi_write(dev, reg, write_val); +} + +static const struct iio_info adrf6780_info = { + .read_raw = adrf6780_read_raw, + .write_raw = adrf6780_write_raw, + .debugfs_reg_access = &adrf6780_reg_access, +}; + +#define ADRF6780_CHAN(_channel) { \ + .type = IIO_VOLTAGE, \ + .output = 1, \ + .indexed = 1, \ + .channel = _channel, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE) \ +} + +#define ADRF6780_CHAN_IQ(_channel, rf_comp) { \ + .type = IIO_ALTVOLTAGE, \ + .modified = 1, \ + .output = 1, \ + .indexed = 1, \ + .channel2 = IIO_MOD_##rf_comp, \ + .channel = _channel, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_PHASE) \ +} + +static const struct iio_chan_spec adrf6780_channels[] = { + ADRF6780_CHAN(0), + ADRF6780_CHAN_IQ(0, I), + ADRF6780_CHAN_IQ(0, Q), +}; + +static int adrf6780_reset(struct adrf6780_dev *dev) +{ + int ret; + struct spi_device *spi = dev->spi; + + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_CONTROL, + ADRF6780_SOFT_RESET_MSK, + FIELD_PREP(ADRF6780_SOFT_RESET_MSK, 1)); + if (ret) { + dev_err(&spi->dev, "ADRF6780 SPI software reset failed.\n"); + return ret; + } + + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_CONTROL, + ADRF6780_SOFT_RESET_MSK, + FIELD_PREP(ADRF6780_SOFT_RESET_MSK, 0)); + if (ret) { + dev_err(&spi->dev, "ADRF6780 SPI software reset disable failed.\n"); + return ret; + } + + return 0; +} + +static int adrf6780_init(struct adrf6780_dev *dev) +{ + int ret; + unsigned int chip_id, enable_reg, enable_reg_msk; + struct spi_device *spi = dev->spi; + + /* Perform a software reset */ + ret = adrf6780_reset(dev); + if (ret) + return ret; + + ret = adrf6780_spi_read(dev, ADRF6780_REG_CONTROL, &chip_id); + if (ret) + return ret; + + chip_id = FIELD_GET(ADRF6780_CHIP_ID_MSK, chip_id); + if (chip_id != ADRF6780_CHIP_ID) { + dev_err(&spi->dev, "ADRF6780 Invalid Chip ID.\n"); + return -EINVAL; + } + + enable_reg_msk = ADRF6780_VGA_BUFFER_EN_MSK | + ADRF6780_DETECTOR_EN_MSK | + ADRF6780_LO_BUFFER_EN_MSK | + ADRF6780_IF_MODE_EN_MSK | + ADRF6780_IQ_MODE_EN_MSK | + ADRF6780_LO_X2_EN_MSK | + ADRF6780_LO_PPF_EN_MSK | + ADRF6780_LO_EN_MSK | + ADRF6780_UC_BIAS_EN_MSK; + + enable_reg = FIELD_PREP(ADRF6780_VGA_BUFFER_EN_MSK, dev->vga_buff_en) | + FIELD_PREP(ADRF6780_DETECTOR_EN_MSK, 1) | + FIELD_PREP(ADRF6780_LO_BUFFER_EN_MSK, dev->lo_buff_en) | + FIELD_PREP(ADRF6780_IF_MODE_EN_MSK, dev->if_mode_en) | + FIELD_PREP(ADRF6780_IQ_MODE_EN_MSK, dev->iq_mode_en) | + FIELD_PREP(ADRF6780_LO_X2_EN_MSK, dev->lo_x2_en) | + FIELD_PREP(ADRF6780_LO_PPF_EN_MSK, dev->lo_ppf_en) | + FIELD_PREP(ADRF6780_LO_EN_MSK, dev->lo_en) | + FIELD_PREP(ADRF6780_UC_BIAS_EN_MSK, dev->uc_bias_en); + + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_ENABLE, enable_reg_msk, enable_reg); + if (ret) + return ret; + + ret = adrf6780_spi_update_bits(dev, ADRF6780_REG_LO_PATH, + ADRF6780_LO_SIDEBAND_MSK, + FIELD_PREP(ADRF6780_LO_SIDEBAND_MSK, dev->lo_sideband)); + if (ret) + return ret; + + return adrf6780_spi_update_bits(dev, ADRF6780_REG_ADC_CONTROL, + ADRF6780_VDET_OUTPUT_SELECT_MSK, + FIELD_PREP(ADRF6780_VDET_OUTPUT_SELECT_MSK, dev->vdet_out_en)); +} + +static void adrf6780_clk_disable(void *data) +{ + clk_disable_unprepare(data); +} + +static void adrf6780_dt_parse(struct adrf6780_dev *dev) +{ + struct spi_device *spi = dev->spi; + + dev->vga_buff_en = device_property_read_bool(&spi->dev, "adi,vga-buff-en"); + dev->lo_buff_en = device_property_read_bool(&spi->dev, "adi,lo-buff-en"); + dev->if_mode_en = device_property_read_bool(&spi->dev, "adi,if-mode-en"); + dev->iq_mode_en = device_property_read_bool(&spi->dev, "adi,iq-mode-en"); + dev->lo_x2_en = device_property_read_bool(&spi->dev, "adi,lo-x2-en"); + dev->lo_ppf_en = device_property_read_bool(&spi->dev, "adi,lo-ppf-en"); + dev->lo_en = device_property_read_bool(&spi->dev, "adi,lo-en"); + dev->uc_bias_en = device_property_read_bool(&spi->dev, "adi,uc-bias-en"); + dev->lo_sideband = device_property_read_bool(&spi->dev, "adi,lo-sideband"); + dev->vdet_out_en = device_property_read_bool(&spi->dev, "adi,vdet-out-en"); +} + +static int adrf6780_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + struct adrf6780_dev *dev; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*dev)); + if (!indio_dev) + return -ENOMEM; + + dev = iio_priv(indio_dev); + + indio_dev->info = &adrf6780_info; + indio_dev->name = "adrf6780"; + indio_dev->channels = adrf6780_channels; + indio_dev->num_channels = ARRAY_SIZE(adrf6780_channels); + + dev->spi = spi; + + adrf6780_dt_parse(dev); + + dev->clkin = devm_clk_get(&spi->dev, "lo_in"); + if (IS_ERR(dev->clkin)) + return PTR_ERR(dev->clkin); + + ret = clk_prepare_enable(dev->clkin); + if (ret < 0) + return ret; + + ret = devm_add_action_or_reset(&spi->dev, adrf6780_clk_disable, dev->clkin); + if (ret < 0) + return ret; + + mutex_init(&dev->lock); + + ret = adrf6780_init(dev); + if (ret) + return ret; + + return devm_iio_device_register(&spi->dev, indio_dev); +} + +static const struct spi_device_id adrf6780_id[] = { + { "adrf6780", 0 }, + {} +}; +MODULE_DEVICE_TABLE(spi, adrf6780_id); + +static const struct of_device_id adrf6780_of_match[] = { + { .compatible = "adi,adrf6780" }, + {} +}; +MODULE_DEVICE_TABLE(of, adrf6780_of_match); + +static struct spi_driver adrf6780_driver = { + .driver = { + .name = "adrf6780", + .of_match_table = adrf6780_of_match, + }, + .probe = adrf6780_probe, + .id_table = adrf6780_id, +}; +module_spi_driver(adrf6780_driver); + +MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com"); +MODULE_DESCRIPTION("Analog Devices ADRF6780"); +MODULE_LICENSE("GPL v2");
The ADRF6780 is a silicon germanium (SiGe) design, wideband, microwave upconverter optimized for point to point microwave radio designs operating in the 5.9 GHz to 23.6 GHz frequency range. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADRF6780.pdf Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- changes in v6: - fix warning detected by kernel test robot drivers/iio/frequency/Kconfig | 12 + drivers/iio/frequency/Makefile | 1 + drivers/iio/frequency/adrf6780.c | 474 +++++++++++++++++++++++++++++++ 3 files changed, 487 insertions(+) create mode 100644 drivers/iio/frequency/adrf6780.c