| Message ID | 20200128111302.24359-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs | expand |
On Tue, 28 Jan 2020 13:13:00 +0200 Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > From: Edward Kigwana <ekigwana@gmail.com> > > The ADF4360-N (N is 0..9) are a family of integrated integer-N synthesizer > and voltage controlled oscillator (VCO). > > Control of all the on-chip registers is through a simple 3-wire SPI > interface. The device operates with a power supply ranging from 3.0 V to > 3.6 V and can be powered down when not in use. > > The initial draft-work was done by Lars-Peter Clausen <lars@metafoo.de> > The current/latest/complete version was implemented by > Edward Kigwana <ekigwana@gmail.com>. > > The ADF4360-9 is also present on the Analog Devices 'Advanced Active > Learning Module 2000' (ADALM-2000). > > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-0.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-1.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-2.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-3.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-4.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-5.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-6.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-7.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-8.pdf > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-9.pdf > Link: https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/ADALM2000.html > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > Signed-off-by: Edward Kigwana <ekigwana@gmail.com> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> Superficially this looks like you really have a clock source driver. You then wrap it in IIO in order to provide convenient userspace interfaces. I'm not sure we want to do that and I definitely need agreement from those responsible for clock source drivers before I take anything that does do this sort of combination of the two types of driver. I can see that, for these high speed devices, the intent is normally to provide an input signal to some external circuitry rather than some internal clock signal, but given this is registered as a clock source the argument that it is somehow special doesn't seem to hold. A few more specific comments inline. Thanks, Jonathan Jonathan > --- > > Changelog v2 -> v3: > * dropped patch about adf4371.yaml; added by accident since it was used to > compare against > * addressed Rob's review comments for DT schema > > Changelog v1 -> v2: > * validate dt-bindings file with > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml > > drivers/iio/frequency/Kconfig | 11 + > drivers/iio/frequency/Makefile | 1 + > drivers/iio/frequency/adf4360.c | 1263 +++++++++++++++++++++++++++++++ > 3 files changed, 1275 insertions(+) > create mode 100644 drivers/iio/frequency/adf4360.c > > diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig > index 240b81502512..781236496661 100644 > --- a/drivers/iio/frequency/Kconfig > +++ b/drivers/iio/frequency/Kconfig > @@ -39,6 +39,17 @@ config ADF4350 > To compile this driver as a module, choose M here: the > module will be called adf4350. > > +config ADF4360 > + tristate "Analog Devices ADF4360 Wideband Synthesizers" > + depends on SPI > + depends on COMMON_CLK > + help > + Say yes here to build support for Analog Devices ADF4360 > + Wideband Synthesizers. The driver provides direct access via sysfs. > + > + To compile this driver as a module, choose M here: the > + module will be called adf4360. > + > config ADF4371 > tristate "Analog Devices ADF4371/ADF4372 Wideband Synthesizers" > depends on SPI > diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile > index 518b1e50caef..85f2f81da662 100644 > --- a/drivers/iio/frequency/Makefile > +++ b/drivers/iio/frequency/Makefile > @@ -6,4 +6,5 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_AD9523) += ad9523.o > obj-$(CONFIG_ADF4350) += adf4350.o > +obj-$(CONFIG_ADF4360) += adf4360.o > obj-$(CONFIG_ADF4371) += adf4371.o > diff --git a/drivers/iio/frequency/adf4360.c b/drivers/iio/frequency/adf4360.c > new file mode 100644 > index 000000000000..49ad58092171 > --- /dev/null > +++ b/drivers/iio/frequency/adf4360.c > @@ -0,0 +1,1263 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ADF4360 PLL with Integrated Synthesizer and VCO > + * > + * Copyright 2014-2019 Analog Devices Inc. > + * Copyright 2019-2020 Edward Kigwana. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <linux/util_macros.h> > + > +#include <linux/iio/iio.h> > + > +/* Register address macro */ > +#define ADF4360_REG(x) (x) > + > +/* ADF4360_CTRL */ > +#define ADF4360_ADDR_CPL_MSK GENMASK(3, 2) > +#define ADF4360_CPL(x) FIELD_PREP(ADF4360_ADDR_CPL_MSK, x) > +#define ADF4360_ADDR_MUXOUT_MSK GENMASK(7, 5) > +#define ADF4360_MUXOUT(x) FIELD_PREP(ADF4360_ADDR_MUXOUT_MSK, x) > +#define ADF4360_ADDR_PDP_MSK BIT(8) > +#define ADF4360_PDP(x) FIELD_PREP(ADF4360_ADDR_PDP_MSK, x) > +#define ADF4360_ADDR_MTLD_MSK BIT(11) > +#define ADF4360_MTLD(x) FIELD_PREP(ADF4360_ADDR_MTLD_MSK, x) > +#define ADF4360_ADDR_OPL_MSK GENMASK(13, 12) > +#define ADF4360_OPL(x) FIELD_PREP(ADF4360_ADDR_OPL_MSK, x) > +#define ADF4360_ADDR_CPI1_MSK GENMASK(16, 14) > +#define ADF4360_CPI1(x) FIELD_PREP(ADF4360_ADDR_CPI1_MSK, x) > +#define ADF4360_ADDR_CPI2_MSK GENMASK(19, 17) > +#define ADF4360_CPI2(x) FIELD_PREP(ADF4360_ADDR_CPI2_MSK, x) > +#define ADF4360_ADDR_PWR_DWN_MSK GENMASK(21, 20) > +#define ADF4360_POWERDOWN(x) FIELD_PREP(ADF4360_ADDR_PWR_DWN_MSK, x) > +#define ADF4360_ADDR_PRESCALER_MSK GENMASK(23, 22) > +#define ADF4360_PRESCALER(x) FIELD_PREP(ADF4360_ADDR_PRESCALER_MSK, x) > + > +/* ADF4360_NDIV */ > +#define ADF4360_ADDR_A_CNTR_MSK GENMASK(6, 2) > +#define ADF4360_A_COUNTER(x) FIELD_PREP(ADF4360_ADDR_A_CNTR_MSK, x) > +#define ADF4360_ADDR_B_CNTR_MSK GENMASK(20, 8) > +#define ADF4360_B_COUNTER(x) FIELD_PREP(ADF4360_ADDR_B_CNTR_MSK, x) > +#define ADF4360_ADDR_OUT_DIV2_MSK BIT(22) > +#define ADF4360_OUT_DIV2(x) FIELD_PREP(ADF4360_ADDR_OUT_DIV2_MSK, x) > +#define ADF4360_ADDR_DIV2_SEL_MSK BIT(23) > +#define ADF4360_PRESCALER_DIV2(x) FIELD_PREP(ADF4360_ADDR_DIV2_SEL_MSK, x) > + > +/* ADF4360_RDIV */ > +#define ADF4360_ADDR_R_CNTR_MSK GENMASK(15, 2) > +#define ADF4360_R_COUNTER(x) FIELD_PREP(ADF4360_ADDR_R_CNTR_MSK, x) > +#define ADF4360_ADDR_ABP_MSK GENMASK(17, 16) > +#define ADF4360_ABP(x) FIELD_PREP(ADF4360_ADDR_ABP_MSK, x) > +#define ADF4360_ADDR_BSC_MSK GENMASK(21, 20) > +#define ADF4360_BSC(x) FIELD_PREP(ADF4360_ADDR_BSC_MSK, x) > + > +/* Specifications */ > +#define ADF4360_MAX_PFD_RATE 8000000 /* 8 MHz */ > +#define ADF4360_MAX_COUNTER_RATE 300000000 /* 300 MHz */ > +#define ADF4360_MAX_REFIN_RATE 250000000 /* 250 MHz */ > + > +enum { > + ADF4360_CTRL, > + ADF4360_RDIV, > + ADF4360_NDIV, > + ADF4360_REG_NUM, > +}; > + > +enum { > + ADF4360_GEN1_PC_5, > + ADF4360_GEN1_PC_10, > + ADF4360_GEN1_PC_15, > + ADF4360_GEN1_PC_20, > +}; > + > +enum { > + ADF4360_GEN2_PC_2_5, > + ADF4360_GEN2_PC_5, > + ADF4360_GEN2_PC_7_5, > + ADF4360_GEN2_PC_10, > +}; > + > +enum { > + ADF4360_MUXOUT_THREE_STATE, > + ADF4360_MUXOUT_LOCK_DETECT, > + ADF4360_MUXOUT_NDIV, > + ADF4360_MUXOUT_DVDD, > + ADF4360_MUXOUT_RDIV, > + ADF4360_MUXOUT_OD_LD, > + ADF4360_MUXOUT_SDO, > + ADF4360_MUXOUT_GND, > +}; > + > +enum { > + ADF4360_PL_3_5, > + ADF4360_PL_5, > + ADF4360_PL_7_5, > + ADF4360_PL_11, > +}; > + > +enum { > + ADF4360_CPI_0_31, > + ADF4360_CPI_0_62, > + ADF4360_CPI_0_93, > + ADF4360_CPI_1_25, > + ADF4360_CPI_1_56, > + ADF4360_CPI_1_87, > + ADF4360_CPI_2_18, > + ADF4360_CPI_2_50, > +}; > + > +enum { > + ADF4360_POWER_DOWN_NORMAL, > + ADF4360_POWER_DOWN_SOFT_ASYNC, > + ADF4360_POWER_DOWN_CE, > + ADF4360_POWER_DOWN_SOFT_SYNC, > + ADF4360_POWER_DOWN_REGULATOR, > +}; > + > +enum { > + ADF4360_PRESCALER_8, > + ADF4360_PRESCALER_16, > + ADF4360_PRESCALER_32, > +}; > + > +enum { > + ADF4360_ABP_3_0NS, > + ADF4360_ABP_1_3NS, > + ADF4360_ABP_6_0NS, > +}; > + > +enum { > + ADF4360_BSC_1, > + ADF4360_BSC_2, > + ADF4360_BSC_4, > + ADF4360_BSC_8, > +}; > + > +enum { > + ID_ADF4360_0, > + ID_ADF4360_1, > + ID_ADF4360_2, > + ID_ADF4360_3, > + ID_ADF4360_4, > + ID_ADF4360_5, > + ID_ADF4360_6, > + ID_ADF4360_7, > + ID_ADF4360_8, > + ID_ADF4360_9, > +}; > + > +enum { > + ADF4360_FREQ_REFIN, > + ADF4360_MTLD, > + ADF4360_FREQ_PFD, > +}; > + > +static const char * const adf4360_power_level_modes[] = { This isn't an enum. These are real values in sensible units not magic strings. Handle them as integers. We may need to define additional ABI for this but it should be possible to 'fit' it in the normal structure of ABI. > + [ADF4360_PL_3_5] = "3500-uA", > + [ADF4360_PL_5] = "5000-uA", > + [ADF4360_PL_7_5] = "7500-uA", > + [ADF4360_PL_11] = "11000-uA", > +}; > + > +static const unsigned int adf4360_power_lvl_microamp[] = { > + [ADF4360_PL_3_5] = 3500, > + [ADF4360_PL_5] = 5000, > + [ADF4360_PL_7_5] = 7500, > + [ADF4360_PL_11] = 11000, > +}; > + > +static const unsigned int adf4360_cpi_modes[] = { > + [ADF4360_CPI_0_31] = 310, > + [ADF4360_CPI_0_62] = 620, > + [ADF4360_CPI_0_93] = 930, > + [ADF4360_CPI_1_25] = 1250, > + [ADF4360_CPI_1_56] = 1560, > + [ADF4360_CPI_1_87] = 1870, > + [ADF4360_CPI_2_18] = 2180, > + [ADF4360_CPI_2_50] = 2500, > +}; > + > +static const char * const adf4360_muxout_modes[] = { Superficially from glancing at the datasheet I thing this is debug functionality? Perhaps move it to debugfs so as to avoid creating complex new ABI in sysfs. > + [ADF4360_MUXOUT_THREE_STATE] = "three-state", > + [ADF4360_MUXOUT_LOCK_DETECT] = "lock-detect", > + [ADF4360_MUXOUT_NDIV] = "ndiv", > + [ADF4360_MUXOUT_DVDD] = "dvdd", > + [ADF4360_MUXOUT_RDIV] = "rdiv", > + [ADF4360_MUXOUT_OD_LD] = "od-ld", > + [ADF4360_MUXOUT_SDO] = "sdo", > + [ADF4360_MUXOUT_GND] = "gnd", > +}; > + > +static const char * const adf4360_power_down_modes[] = { > + [ADF4360_POWER_DOWN_NORMAL] = "normal", > + [ADF4360_POWER_DOWN_SOFT_ASYNC] = "soft-async", > + [ADF4360_POWER_DOWN_CE] = "ce", > + [ADF4360_POWER_DOWN_SOFT_SYNC] = "soft-sync", > + [ADF4360_POWER_DOWN_REGULATOR] = "regulator", This seems to map rather oddly to the datasheet. Perhaps some comments to explain what is going on here would help/ > +}; > + > +struct adf4360_output { > + struct clk_hw hw; > + struct iio_dev *indio_dev; > +}; > + > +#define to_output(_hw) container_of(_hw, struct adf4360_output, hw) > + > +struct adf4360_chip_info { > + unsigned int vco_min; > + unsigned int vco_max; > + unsigned int default_cpl; > +}; > + > +struct adf4360_state { > + struct spi_device *spi; > + const struct adf4360_chip_info *info; > + struct adf4360_output output; > + struct clk *clkin; > + struct gpio_desc *muxout_gpio; > + struct gpio_desc *chip_en_gpio; > + struct regulator *vdd_reg; > + struct mutex lock; /* Protect PLL state. */ > + unsigned int part_id; > + unsigned long clkin_freq; > + unsigned long freq_req; > + unsigned long r; > + unsigned long n; > + unsigned int vco_min; > + unsigned int vco_max; > + unsigned int pfd_freq; > + unsigned int cpi; > + bool pdp; > + bool mtld; > + unsigned int power_level; > + unsigned int muxout_mode; > + unsigned int power_down_mode; > + bool initial_reg_seq; > + const char *clk_out_name; > + unsigned int regs[ADF4360_REG_NUM]; > + u8 spi_data[3] ____cacheline_aligned; > +}; > + > +static const struct adf4360_chip_info adf4360_chip_info_tbl[] = { > + { /* ADF4360-0 */ > + .vco_min = 2400000000U, > + .vco_max = 2725000000U, > + .default_cpl = ADF4360_GEN1_PC_10, > + }, { /* ADF4360-1 */ > + .vco_min = 2050000000U, > + .vco_max = 2450000000U, > + .default_cpl = ADF4360_GEN1_PC_15, > + }, { /* ADF4360-2 */ > + .vco_min = 1850000000U, > + .vco_max = 2170000000U, > + .default_cpl = ADF4360_GEN1_PC_15, > + }, { /* ADF4360-3 */ > + .vco_min = 1600000000U, > + .vco_max = 1950000000U, > + .default_cpl = ADF4360_GEN1_PC_15, > + }, { /* ADF4360-4 */ > + .vco_min = 1450000000U, > + .vco_max = 1750000000U, > + .default_cpl = ADF4360_GEN1_PC_15, > + }, { /* ADF4360-5 */ > + .vco_min = 1200000000U, > + .vco_max = 1400000000U, > + .default_cpl = ADF4360_GEN1_PC_10, > + }, { /* ADF4360-6 */ > + .vco_min = 1050000000U, > + .vco_max = 1250000000U, > + .default_cpl = ADF4360_GEN1_PC_10, > + }, { /* ADF4360-7 */ > + .vco_min = 350000000U, > + .vco_max = 1800000000U, > + .default_cpl = ADF4360_GEN1_PC_5, > + }, { /* ADF4360-8 */ > + .vco_min = 65000000U, > + .vco_max = 400000000U, > + .default_cpl = ADF4360_GEN2_PC_5, > + }, { /* ADF4360-9 */ > + .vco_min = 65000000U, > + .vco_max = 400000000U, > + .default_cpl = ADF4360_GEN2_PC_5, > + } > +}; > + > +static int adf4360_write_reg(struct adf4360_state *st, unsigned int reg, > + unsigned int val) > +{ > + int ret; > + > + val |= reg; > + > + st->spi_data[0] = (val >> 16) & 0xff; > + st->spi_data[1] = (val >> 8) & 0xff; > + st->spi_data[2] = val & 0xff; > + > + ret = spi_write(st->spi, st->spi_data, ARRAY_SIZE(st->spi_data)); > + if (ret == 0) > + st->regs[reg] = val; > + > + return ret; > +} > + > +/* fVCO = B * fREFIN / R */ > + > +static unsigned long adf4360_clk_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + > + if (st->r == 0) > + return 0; > + > + /* > + * The result is guaranteed to fit in 32-bit, but the intermediate > + * result might require 64-bit. > + */ > + return DIV_ROUND_CLOSEST_ULL((uint64_t)parent_rate * st->n, st->r); > +} > + > +static unsigned int adf4360_calc_prescaler(unsigned int pfd_freq, > + unsigned int n, > + unsigned int *out_p, > + unsigned int *out_a, > + unsigned int *out_b) > +{ > + unsigned int rate = pfd_freq * n; > + unsigned int p, a, b; > + > + /* Make sure divider counter input frequency is low enough */ > + p = 8; > + while (p < 32 && rate / p > ADF4360_MAX_COUNTER_RATE) > + p *= 2; > + > + /* > + * The range of dividers that can be produced using the dual-modulus > + * pre-scaler is not continuous for values of n < p*(p-1). If we end up > + * with a non supported divider value, pick the next closest one. > + */ > + a = n % p; > + b = n / p; > + > + if (b < 3) { > + b = 3; > + a = 0; > + } else if (a > b) { > + if (a - b < p - a) { > + a = b; > + } else { > + a = 0; > + b++; > + } > + } > + > + if (out_p) > + *out_p = p; > + if (out_a) > + *out_a = a; > + if (out_b) > + *out_b = b; > + > + return p * b + a; > +} > + > +static long adf4360_clk_round_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + unsigned int r, n; > + unsigned int pfd_freq; > + > + if (*parent_rate == 0) > + return 0; > + > + if (st->part_id == ID_ADF4360_9) > + return *parent_rate * st->n / st->r; > + > + if (rate > st->vco_max) > + return st->vco_max; > + > + /* ADF4360-0 to AD4370-7 have an optional by two divider */ > + if (st->part_id <= ID_ADF4360_7) { > + if (rate < st->vco_min / 2) > + return st->vco_min / 2; > + if (rate < st->vco_min && rate > st->vco_max / 2) { > + if (st->vco_min - rate < rate - st->vco_max / 2) > + return st->vco_min; > + else > + return st->vco_max / 2; > + } > + } else { > + if (rate < st->vco_min) > + return st->vco_min; > + } > + > + r = DIV_ROUND_CLOSEST(*parent_rate, st->pfd_freq); > + pfd_freq = *parent_rate / r; > + n = DIV_ROUND_CLOSEST(rate, pfd_freq); > + > + if (st->part_id <= ID_ADF4360_7) > + n = adf4360_calc_prescaler(pfd_freq, n, NULL, NULL, NULL); > + > + return pfd_freq * n; > +} > + > +static inline bool adf4360_is_powerdown(struct adf4360_state *st) > +{ > + return (st->power_down_mode != ADF4360_POWER_DOWN_NORMAL); > +} > + > +static int adf4360_set_freq(struct adf4360_state *st, unsigned long rate) > +{ > + unsigned int val_r, val_n, val_ctrl; > + unsigned int pfd_freq; > + unsigned long r, n; > + int ret; > + > + if (!st->clkin_freq || (st->clkin_freq > ADF4360_MAX_REFIN_RATE)) > + return -EINVAL; > + > + if ((rate < st->vco_min) || (rate > st->vco_max)) > + return -EINVAL; > + > + if (adf4360_is_powerdown(st)) > + ret = -EBUSY; > + > + r = DIV_ROUND_CLOSEST(st->clkin_freq, st->pfd_freq); > + pfd_freq = st->clkin_freq / r; > + n = DIV_ROUND_CLOSEST(rate, pfd_freq); > + > + val_ctrl = ADF4360_CPL(st->info->default_cpl) | > + ADF4360_MUXOUT(st->muxout_mode) | > + ADF4360_PDP(!st->pdp) | > + ADF4360_MTLD(st->mtld) | > + ADF4360_OPL(st->power_level) | > + ADF4360_CPI1(st->cpi) | > + ADF4360_CPI2(st->cpi) | > + ADF4360_POWERDOWN(st->power_down_mode); > + > + /* ADF4360-0 to ADF4360-7 have a dual-modulous prescaler */ > + if (st->part_id <= ID_ADF4360_7) { > + unsigned int p, a, b; > + > + n = adf4360_calc_prescaler(pfd_freq, n, &p, &a, &b); > + > + switch (p) { > + case 8: > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_8); > + break; > + case 16: > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_16); > + break; > + default: > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_32); > + break; > + } > + > + val_n = ADF4360_A_COUNTER(a) | > + ADF4360_B_COUNTER(b); > + > + if (rate < st->vco_min) > + val_n |= ADF4360_OUT_DIV2(true) | > + ADF4360_PRESCALER_DIV2(true); > + } else { > + val_n = ADF4360_B_COUNTER(n); > + } > + > + /* > + * Always use BSC divider of 8, see Analog Devices AN-1347. > + * http://www.analog.com/media/en/technical-documentation/application-notes/AN-1347.pdf > + */ > + val_r = ADF4360_R_COUNTER(r) | > + ADF4360_ABP(ADF4360_ABP_3_0NS) | > + ADF4360_BSC(ADF4360_BSC_8); > + > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_RDIV), val_r); > + if (ret) > + return ret; > + > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val_ctrl); > + if (ret) > + return ret; > + > + /* > + * Allow the transient behavior of the ADF4360-7 during initial > + * power-up to settle. > + */ > + if (st->initial_reg_seq) { > + usleep_range(15000, 20000); > + st->initial_reg_seq = false; > + } > + > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_NDIV), val_n); > + if (ret) > + return ret; > + > + st->freq_req = rate; > + st->n = n; > + st->r = r; > + > + return 0; > +} > + > +static int adf4360_clk_set_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long parent_rate) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + int ret; > + > + if ((parent_rate == 0) || (parent_rate > ADF4360_MAX_REFIN_RATE)) > + return -EINVAL; > + > + mutex_lock(&st->lock); > + if (st->clkin_freq != parent_rate) > + st->clkin_freq = parent_rate; > + > + ret = adf4360_set_freq(st, rate); > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static int __adf4360_power_down(struct adf4360_state *st, unsigned int mode) > +{ > + struct device *dev = &st->spi->dev; > + unsigned int val; > + int ret = 0; > + > + switch (mode) { > + case ADF4360_POWER_DOWN_NORMAL: > + if (st->vdd_reg) { > + ret = regulator_enable(st->vdd_reg); > + if (ret) { > + dev_err(dev, "Supply enable error: %d\n", ret); > + return ret; > + } > + } > + > + st->initial_reg_seq = true; > + st->power_down_mode = mode; > + ret = adf4360_set_freq(st, st->freq_req); > + break; > + case ADF4360_POWER_DOWN_SOFT_ASYNC: /* fallthrough */ > + case ADF4360_POWER_DOWN_SOFT_SYNC: > + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > + val |= ADF4360_POWERDOWN(mode); > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > + break; > + case ADF4360_POWER_DOWN_CE: > + if (st->chip_en_gpio) > + gpiod_set_value(st->chip_en_gpio, 0x0); > + else > + return -ENODEV; > + break; > + case ADF4360_POWER_DOWN_REGULATOR: > + if (!st->vdd_reg) > + return -ENODEV; > + > + if (st->chip_en_gpio) > + ret = __adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > + else > + ret = __adf4360_power_down(st, > + ADF4360_POWER_DOWN_SOFT_ASYNC); > + > + ret = regulator_disable(st->vdd_reg); > + if (ret) > + dev_err(dev, "Supply disable error: %d\n", ret); > + break; > + } > + if (ret == 0) > + st->power_down_mode = mode; > + > + return 0; > +} > + > +static int adf4360_power_down(struct adf4360_state *st, unsigned int mode) > +{ > + int ret; > + > + mutex_lock(&st->lock); > + ret = __adf4360_power_down(st, mode); > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static int adf4360_clk_prepare(struct clk_hw *hw) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + > + return adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); > +} > + > +static void adf4360_clk_unprepare(struct clk_hw *hw) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + > + adf4360_power_down(st, ADF4360_POWER_DOWN_SOFT_ASYNC); > +} > + > +static int adf4360_clk_enable(struct clk_hw *hw) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + > + if (st->chip_en_gpio) > + gpiod_set_value(st->chip_en_gpio, 0x1); > + > + return 0; > +} > + > +static void adf4360_clk_disable(struct clk_hw *hw) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + > + if (st->chip_en_gpio) > + adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > +} > + > +static int adf4360_clk_is_enabled(struct clk_hw *hw) > +{ > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > + struct adf4360_state *st = iio_priv(indio_dev); > + > + return adf4360_is_powerdown(st); > +} > + > +static const struct clk_ops adf4360_clk_ops = { > + .recalc_rate = adf4360_clk_recalc_rate, > + .round_rate = adf4360_clk_round_rate, > + .set_rate = adf4360_clk_set_rate, > + .prepare = adf4360_clk_prepare, > + .unprepare = adf4360_clk_unprepare, > + .enable = adf4360_clk_enable, > + .disable = adf4360_clk_disable, > + .is_enabled = adf4360_clk_is_enabled, > +}; > + > +static ssize_t adf4360_read(struct iio_dev *indio_dev, > + uintptr_t private, > + const struct iio_chan_spec *chan, > + char *buf) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + unsigned long val; > + int ret = 0; > + > + switch ((u32)private) { > + case ADF4360_FREQ_REFIN: > + val = st->clkin_freq; > + break; > + case ADF4360_MTLD: > + val = st->mtld; > + break; > + case ADF4360_FREQ_PFD: > + val = st->pfd_freq; > + break; > + default: > + ret = -EINVAL; > + val = 0; > + } > + > + return ret < 0 ? ret : sprintf(buf, "%lu\n", val); > +} > + > +static ssize_t adf4360_write(struct iio_dev *indio_dev, > + uintptr_t private, > + const struct iio_chan_spec *chan, > + const char *buf, size_t len) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + unsigned long readin, tmp; > + bool mtld; > + int ret = 0; > + > + mutex_lock(&st->lock); > + switch ((u32)private) { > + case ADF4360_FREQ_REFIN: > + ret = kstrtoul(buf, 10, &readin); > + if (ret) > + break; > + > + if ((readin > ADF4360_MAX_REFIN_RATE) || (readin == 0)) { > + ret = -EINVAL; > + break; > + } > + > + if (st->clkin) { > + tmp = clk_round_rate(st->clkin, readin); > + if (tmp != readin) { > + ret = -EINVAL; > + break; > + } > + > + ret = clk_set_rate(st->clkin, tmp); > + if (ret) > + break; A bit odd to directly provide an interface to control and entirely different bit of hardware. If there are specific demands on the input clock as a result of something to do with the outputs, then fair enough. Direct tweaking like this seems like a very odd interface. > + } > + > + st->clkin_freq = readin; > + break; > + case ADF4360_MTLD: > + ret = kstrtobool(buf, &mtld); > + if (ret) > + break; > + > + st->mtld = mtld; > + break; > + case ADF4360_FREQ_PFD: > + ret = kstrtoul(buf, 10, &readin); > + if (ret) > + break; > + > + if ((readin > ADF4360_MAX_PFD_RATE) || (readin == 0)) { > + ret = -EINVAL; > + break; > + } > + > + st->pfd_freq = readin; > + break; > + default: > + ret = -EINVAL; > + } > + > + if (ret == 0) > + ret = adf4360_set_freq(st, st->freq_req); > + mutex_unlock(&st->lock); > + > + return ret ? ret : len; > +} > + > +static int adf4360_get_muxout_mode(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + > + return st->muxout_mode; > +} > + > +static int adf4360_set_muxout_mode(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + unsigned int mode) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + unsigned int writeval; > + int ret = 0; > + > + mutex_lock(&st->lock); > + writeval = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > + writeval |= ADF4360_MUXOUT(mode & 0x7); > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), writeval); > + if (ret == 0) > + st->muxout_mode = mode & 0x7; > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static const struct iio_enum adf4360_muxout_modes_available = { > + .items = adf4360_muxout_modes, > + .num_items = ARRAY_SIZE(adf4360_muxout_modes), > + .get = adf4360_get_muxout_mode, > + .set = adf4360_set_muxout_mode, > +}; > + > +static int adf4360_get_power_down(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + > + return st->power_down_mode; > +} > + > +static int adf4360_set_power_down(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + unsigned int mode) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + > + return adf4360_power_down(st, mode); > +} > + > +static const struct iio_enum adf4360_pwr_dwn_modes_available = { > + .items = adf4360_power_down_modes, > + .num_items = ARRAY_SIZE(adf4360_power_down_modes), > + .get = adf4360_get_power_down, > + .set = adf4360_set_power_down, > +}; > + > +static int adf4360_get_power_level(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + > + return st->power_level; > +} > + > +static int adf4360_set_power_level(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + unsigned int level) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + unsigned int val; > + int ret; > + > + if (adf4360_is_powerdown(st)) > + return -EBUSY; > + > + mutex_lock(&st->lock); > + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_OPL_MSK; > + val |= ADF4360_OPL(level); > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > + st->power_level = level; > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static const struct iio_enum adf4360_pwr_lvl_modes_available = { > + .items = adf4360_power_level_modes, > + .num_items = ARRAY_SIZE(adf4360_power_level_modes), > + .get = adf4360_get_power_level, > + .set = adf4360_set_power_level, > +}; > + > +#define _ADF4360_EXT_INFO(_name, _ident) { \ > + .name = _name, \ > + .read = adf4360_read, \ > + .write = adf4360_write, \ > + .private = _ident, \ > + .shared = IIO_SEPARATE, \ > +} > + > +static const struct iio_chan_spec_ext_info adf4360_ext_info[] = { This is a wide range of new ABI. These all need documentation in Documentation/ABI/testing/sysfs-bus-iio-* Without docs, it's hard to discuss if these are appropriate but a few initial comments... > + _ADF4360_EXT_INFO("refin_frequency", ADF4360_FREQ_REFIN), Looks like a control that should be matched to some clock source and not change at runtime? > + _ADF4360_EXT_INFO("mute_till_lock_detect", ADF4360_MTLD), > + _ADF4360_EXT_INFO("pfd_frequency", ADF4360_FREQ_PFD), > + IIO_ENUM_AVAILABLE("muxout_mode", &adf4360_muxout_modes_available), > + IIO_ENUM("muxout_mode", false, &adf4360_muxout_modes_available), > + IIO_ENUM_AVAILABLE("power_down", &adf4360_pwr_dwn_modes_available), > + IIO_ENUM("power_down", false, &adf4360_pwr_dwn_modes_available), > + IIO_ENUM_AVAILABLE("power_level", &adf4360_pwr_lvl_modes_available), > + IIO_ENUM("power_level", false, &adf4360_pwr_lvl_modes_available), > + { }, > +}; > + > +static const struct iio_chan_spec adf4360_chan = { > + .type = IIO_ALTVOLTAGE, > + .indexed = 1, > + .output = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY), > + .ext_info = adf4360_ext_info, > +}; > + > +static int adf4360_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, > + int *val2, > + long mask) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + bool lk_det; > + > + switch (mask) { > + case IIO_CHAN_INFO_FREQUENCY: > + if (adf4360_is_powerdown(st)) > + return -EBUSY; > + > + lk_det = (ADF4360_MUXOUT_LOCK_DETECT | ADF4360_MUXOUT_OD_LD) & > + st->muxout_mode; > + if (lk_det && st->muxout_gpio) { > + if (!gpiod_get_value(st->muxout_gpio)) { > + dev_dbg(&st->spi->dev, "PLL un-locked\n"); > + return -EBUSY; > + } > + } > + > + *val = st->freq_req; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + > + return 0; > +}; > + > +static int adf4360_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, > + int val2, > + long mask) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + int ret; > + > + mutex_lock(&st->lock); > + switch (mask) { > + case IIO_CHAN_INFO_FREQUENCY: > + ret = adf4360_set_freq(st, val); > + break; > + default: > + ret = -EINVAL; > + } > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static int adf4360_reg_access(struct iio_dev *indio_dev, > + unsigned int reg, > + unsigned int writeval, > + unsigned int *readval) > +{ > + struct adf4360_state *st = iio_priv(indio_dev); > + int ret = 0; > + > + if (reg >= ADF4360_REG_NUM) > + return -EFAULT; > + > + mutex_lock(&st->lock); > + if (readval) { > + *readval = st->regs[reg]; > + } else { > + writeval &= 0xFFFFFC; > + ret = adf4360_write_reg(st, reg, writeval); > + } > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static const struct iio_info adf4360_iio_info = { > + .read_raw = &adf4360_read_raw, > + .write_raw = &adf4360_write_raw, > + .debugfs_reg_access = &adf4360_reg_access, > +}; > + > +static int adf4360_get_gpio(struct adf4360_state *st) > +{ > + struct device *dev = &st->spi->dev; > + unsigned int val; > + int ret, i; > + > + st->chip_en_gpio = devm_gpiod_get_optional(dev, "enable", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->chip_en_gpio)) { > + dev_err(dev, "Chip enable GPIO error\n"); Put handling in here to prevent an error message of DEFER. Same for the other routes where this might happen. > + return PTR_ERR(st->chip_en_gpio); > + } > + > + if (st->chip_en_gpio) > + st->power_down_mode = ADF4360_POWER_DOWN_CE; > + > + st->muxout_gpio = devm_gpiod_get_optional(dev, "adi,muxout", GPIOD_IN); > + if (IS_ERR(st->muxout_gpio)) { > + dev_err(dev, "Muxout GPIO error\n"); > + return PTR_ERR(st->muxout_gpio); > + } > + > + if (!st->muxout_gpio) > + return 0; > + > + /* ADF4360 PLLs are write only devices, try to probe using GPIO. */ > + for (i = 0; i < 4; i++) { > + if (i & 1) > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_DVDD); > + else > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_GND); > + > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > + if (ret) > + return ret; > + > + ret = gpiod_get_value(st->muxout_gpio); > + if (ret ^ (i & 1)) { > + dev_err(dev, "Probe failed (muxout)"); > + return -ENODEV; > + } > + } > + > + return 0; > +} > + > +static void adf4360_clkin_disable(void *data) > +{ > + struct adf4360_state *st = data; > + > + clk_disable_unprepare(st->clkin); > +} > + > +static int adf4360_get_clkin(struct adf4360_state *st) > +{ > + struct device *dev = &st->spi->dev; > + struct clk *clk; > + int ret; > + > + clk = devm_clk_get(dev, "clkin"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + ret = clk_prepare_enable(clk); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(dev, adf4360_clkin_disable, st); > + if (ret) > + return ret; > + > + st->clkin = clk; > + st->clkin_freq = clk_get_rate(clk); > + > + return 0; > +} > + > +static void adf4360_clk_del_provider(void *data) > +{ > + struct adf4360_state *st = data; > + > + of_clk_del_provider(st->spi->dev.of_node); > +} > + > +static int adf4360_clk_register(struct adf4360_state *st) > +{ Hmm. This makes me wonder why this is an IIO driver rather than a clk driver? Definitely needs some more information in the patch description or a cover letter. > + struct spi_device *spi = st->spi; > + struct clk_init_data init; > + struct clk *clk; > + const char *parent_name; > + int ret; > + > + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0); > + if (!parent_name) > + return -EINVAL; > + > + init.name = st->clk_out_name; > + init.ops = &adf4360_clk_ops; > + init.flags = CLK_SET_RATE_GATE; > + init.parent_names = &parent_name; > + init.num_parents = 1; > + > + st->output.hw.init = &init; > + > + clk = devm_clk_register(&spi->dev, &st->output.hw); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + ret = of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get, clk); > + if (ret) > + return ret; > + > + return devm_add_action_or_reset(&spi->dev, adf4360_clk_del_provider, st); > +} > + > +static int adf4360_parse_dt(struct adf4360_state *st) > +{ > + struct device *dev = &st->spi->dev; > + u32 tmp; > + int ret; > + > + ret = device_property_read_string(dev, "clock-output-names", > + &st->clk_out_name); > + if ((ret < 0) && dev->of_node) > + st->clk_out_name = dev->of_node->name; > + > + if (st->part_id >= ID_ADF4360_7) { > + /* > + * ADF4360-7 to ADF4360-9 have a VCO that is tuned to a specific > + * range using an external inductor. These properties describe > + * the range selected by the external inductor. > + */ > + ret = device_property_read_u32(dev, > + "adi,vco-minimum-frequency-hz", > + &tmp); > + if (ret == 0) > + st->vco_min = max(st->info->vco_min, tmp); > + else > + st->vco_min = st->info->vco_min; > + > + ret = device_property_read_u32(dev, > + "adi,vco-maximum-frequency-hz", > + &tmp); > + if (ret == 0) > + st->vco_max = min(st->info->vco_max, tmp); > + else > + st->vco_max = st->info->vco_max; > + } else { > + st->vco_min = st->info->vco_min; > + st->vco_max = st->info->vco_max; > + } > + > + st->pdp = device_property_read_bool(dev, "adi,loop-filter-inverting"); > + > + ret = device_property_read_u32(dev, > + "adi,loop-filter-pfd-frequency-hz", > + &tmp); > + if (ret == 0) { > + st->pfd_freq = tmp; > + } else { > + dev_err(dev, "PFD frequency property missing\n"); > + return ret; > + } > + > + ret = device_property_read_u32(dev, > + "adi,loop-filter-charge-pump-current-microamp", > + &tmp); > + if (ret == 0) { > + st->cpi = find_closest(tmp, adf4360_cpi_modes, > + ARRAY_SIZE(adf4360_cpi_modes)); > + } else { > + dev_err(dev, "CPI property missing\n"); > + return ret; > + } > + > + ret = device_property_read_u32(dev, "adi,power-up-frequency-hz", &tmp); > + if (ret == 0) > + st->freq_req = tmp; > + else > + st->freq_req = st->vco_min; > + > + ret = device_property_read_u32(dev, "adi,power-out-level-microamp", > + &tmp); > + if (ret == 0) > + st->power_level = find_closest(tmp, adf4360_power_lvl_microamp, > + ARRAY_SIZE(adf4360_power_lvl_microamp)); > + else > + st->power_level = ADF4360_PL_5; > + > + return 0; > +} > + > +static int adf4360_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + const struct spi_device_id *id = spi_get_device_id(spi); Given we require various dt parameters to be present, might as well associate the id with the of_ structures instead and use the dt calls throughout. Even better if you use the firmware type independent versions. > + struct adf4360_state *st; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + > + mutex_init(&st->lock); > + > + spi_set_drvdata(spi, indio_dev); > + > + st->spi = spi; > + st->info = &adf4360_chip_info_tbl[id->driver_data]; > + st->part_id = id->driver_data; > + st->muxout_mode = ADF4360_MUXOUT_LOCK_DETECT; > + st->mtld = true; > + > + ret = adf4360_parse_dt(st); > + if (ret) { > + dev_err(&spi->dev, "Parsing properties failed (%d)\n", ret); > + return -ENODEV; > + } > + > + indio_dev->dev.parent = &spi->dev; > + > + if (spi->dev.of_node) > + indio_dev->name = spi->dev.of_node->name; > + else > + indio_dev->name = spi_get_device_id(spi)->name; > + > + indio_dev->info = &adf4360_iio_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = &adf4360_chan; > + indio_dev->num_channels = 1; > + st->output.indio_dev = indio_dev; > + > + ret = adf4360_get_gpio(st); > + if (ret) > + return ret; > + > + ret = adf4360_get_clkin(st); > + if (ret) > + return ret; > + > + st->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd"); > + if (IS_ERR(st->vdd_reg)) { > + if (PTR_ERR(st->vdd_reg) != -ENODEV) { > + dev_err(&spi->dev, "Regulator error\n"); > + return PTR_ERR(st->vdd_reg); > + } > + > + st->vdd_reg = NULL; > + } > + > + ret = adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); > + if (ret) > + return ret; > + > + ret = adf4360_clk_register(st); > + if (ret) > + return ret; > + > + return devm_iio_device_register(&spi->dev, indio_dev); > +} > + > +static const struct of_device_id adf4360_of_match[] = { > + { .compatible = "adi,adf4360-0", }, > + { .compatible = "adi,adf4360-1", }, > + { .compatible = "adi,adf4360-2", }, > + { .compatible = "adi,adf4360-3", }, > + { .compatible = "adi,adf4360-4", }, > + { .compatible = "adi,adf4360-5", }, > + { .compatible = "adi,adf4360-6", }, > + { .compatible = "adi,adf4360-7", }, > + { .compatible = "adi,adf4360-8", }, > + { .compatible = "adi,adf4360-9", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, adf4360_of_match); > + > +static const struct spi_device_id adf4360_id[] = { As mentioned above, you can't actually probe this device without a pile of dt stuff. So this fallback doesn't make much sense. Use the data field in the of table above and get rid of this table entirely. > + {"adf4360-0", ID_ADF4360_0}, > + {"adf4360-1", ID_ADF4360_1}, > + {"adf4360-2", ID_ADF4360_2}, > + {"adf4360-3", ID_ADF4360_3}, > + {"adf4360-4", ID_ADF4360_4}, > + {"adf4360-5", ID_ADF4360_5}, > + {"adf4360-6", ID_ADF4360_6}, > + {"adf4360-7", ID_ADF4360_7}, > + {"adf4360-8", ID_ADF4360_8}, > + {"adf4360-9", ID_ADF4360_9}, > + {} > +}; > + > +static struct spi_driver adf4360_driver = { > + .driver = { > + .name = "adf4360", > + .of_match_table = adf4360_of_match, > + .owner = THIS_MODULE, It's a long time since we had to set the .owner field for each driver. Follow through what happens in module_spi_driver, spi_register_driver etc and you'll find it's set automatically during driver registration. > + }, > + .probe = adf4360_probe, > + .id_table = adf4360_id, > +}; > +module_spi_driver(adf4360_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); > +MODULE_AUTHOR("Edward Kigwana <ekigwana@gmail.com>"); > +MODULE_DESCRIPTION("Analog Devices ADF4360 PLL");
On Sun, 2020-02-02 at 14:45 +0000, Jonathan Cameron wrote: > [External] > > On Tue, 28 Jan 2020 13:13:00 +0200 > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > From: Edward Kigwana <ekigwana@gmail.com> > > > > The ADF4360-N (N is 0..9) are a family of integrated integer-N synthesizer > > and voltage controlled oscillator (VCO). > > > > Control of all the on-chip registers is through a simple 3-wire SPI > > interface. The device operates with a power supply ranging from 3.0 V to > > 3.6 V and can be powered down when not in use. > > > > The initial draft-work was done by Lars-Peter Clausen <lars@metafoo.de> > > The current/latest/complete version was implemented by > > Edward Kigwana <ekigwana@gmail.com>. > > > > The ADF4360-9 is also present on the Analog Devices 'Advanced Active > > Learning Module 2000' (ADALM-2000). > > > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-0.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-1.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-2.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-3.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-4.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-5.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-6.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-7.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-8.pdf > > Link: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-9.pdf > > Link: > > https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/ADALM2000.html > > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > Signed-off-by: Edward Kigwana <ekigwana@gmail.com> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > Superficially this looks like you really have a clock source driver. > You then wrap it in IIO in order to provide convenient userspace interfaces. > > I'm not sure we want to do that and I definitely need agreement from those > responsible for clock source drivers before I take anything that does do > this sort of combination of the two types of driver. > I'll admit that I am a bit fuzzy about these frequency-generators/clock- sources/synthesizer chips. In the sense, I don't know where to draw the line between when to consider it [primarly] an IIO device [for the IIO subsystem] or when to consider it a clock- device [primarily, for the clk subsystem]. Obviously there's an inertia [for me at least] to go IIO, as I know it a bit better. We've had some quick/short discussions [internally] about this driver, and also about the LTC6952. We didn't have a bigger one, where we try to discuss them more in-detail; but we do have a plan to do it. So, then maybe [until then] a question: how do we decide this? [generally speaking, not just adf4360 & ltc6952] i.e. when to consider it clk-first or iio-first; I'm not sure if there is a clear-cut rule, but maybe some guidelines/thoughts? Obviously, I'll have to read-up more on the clk framework code [as well] to get a feel for it. We've done some things internally [up until now] with some of these clock-chips that's been mostly IIO-centric. Now, much of it may not be correct, but it is what we use as template when writing new driver, which [of course] is not good. And, I also don't want to push/force our mistakes upstream, because that is [well...] bad. Hence this/these question(s). Thanks Alex > I can see that, for these high speed devices, the intent is normally to > provide > an input signal to some external circuitry rather than some internal clock > signal, but given this is registered as a clock source the argument that it is > somehow special doesn't seem to hold. > > A few more specific comments inline. > > Thanks, > > Jonathan > > Jonathan > > > --- > > > > Changelog v2 -> v3: > > * dropped patch about adf4371.yaml; added by accident since it was used to > > compare against > > * addressed Rob's review comments for DT schema > > > > Changelog v1 -> v2: > > * validate dt-bindings file with > > make dt_binding_check > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/frequency/adi,adf4360. > > yaml > > > > drivers/iio/frequency/Kconfig | 11 + > > drivers/iio/frequency/Makefile | 1 + > > drivers/iio/frequency/adf4360.c | 1263 +++++++++++++++++++++++++++++++ > > 3 files changed, 1275 insertions(+) > > create mode 100644 drivers/iio/frequency/adf4360.c > > > > diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig > > index 240b81502512..781236496661 100644 > > --- a/drivers/iio/frequency/Kconfig > > +++ b/drivers/iio/frequency/Kconfig > > @@ -39,6 +39,17 @@ config ADF4350 > > To compile this driver as a module, choose M here: the > > module will be called adf4350. > > > > +config ADF4360 > > + tristate "Analog Devices ADF4360 Wideband Synthesizers" > > + depends on SPI > > + depends on COMMON_CLK > > + help > > + Say yes here to build support for Analog Devices ADF4360 > > + Wideband Synthesizers. The driver provides direct access via sysfs. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called adf4360. > > + > > config ADF4371 > > tristate "Analog Devices ADF4371/ADF4372 Wideband Synthesizers" > > depends on SPI > > diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile > > index 518b1e50caef..85f2f81da662 100644 > > --- a/drivers/iio/frequency/Makefile > > +++ b/drivers/iio/frequency/Makefile > > @@ -6,4 +6,5 @@ > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_AD9523) += ad9523.o > > obj-$(CONFIG_ADF4350) += adf4350.o > > +obj-$(CONFIG_ADF4360) += adf4360.o > > obj-$(CONFIG_ADF4371) += adf4371.o > > diff --git a/drivers/iio/frequency/adf4360.c > > b/drivers/iio/frequency/adf4360.c > > new file mode 100644 > > index 000000000000..49ad58092171 > > --- /dev/null > > +++ b/drivers/iio/frequency/adf4360.c > > @@ -0,0 +1,1263 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * ADF4360 PLL with Integrated Synthesizer and VCO > > + * > > + * Copyright 2014-2019 Analog Devices Inc. > > + * Copyright 2019-2020 Edward Kigwana. > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/clk-provider.h> > > +#include <linux/delay.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/regulator/consumer.h> > > +#include <linux/spi/spi.h> > > +#include <linux/util_macros.h> > > + > > +#include <linux/iio/iio.h> > > + > > +/* Register address macro */ > > +#define ADF4360_REG(x) (x) > > + > > +/* ADF4360_CTRL */ > > +#define ADF4360_ADDR_CPL_MSK GENMASK(3, 2) > > +#define ADF4360_CPL(x) FIELD_PREP(ADF4360_ADDR_CPL_MSK, > > x) > > +#define ADF4360_ADDR_MUXOUT_MSK GENMASK(7, 5) > > +#define ADF4360_MUXOUT(x) FIELD_PREP(ADF4360_ADDR_MUXOUT_MSK, x) > > +#define ADF4360_ADDR_PDP_MSK BIT(8) > > +#define ADF4360_PDP(x) FIELD_PREP(ADF4360_ADDR_PDP_MSK, > > x) > > +#define ADF4360_ADDR_MTLD_MSK BIT(11) > > +#define ADF4360_MTLD(x) FIELD_PREP(ADF4360_ADDR_MTLD_MSK > > , x) > > +#define ADF4360_ADDR_OPL_MSK GENMASK(13, 12) > > +#define ADF4360_OPL(x) FIELD_PREP(ADF4360_ADDR_OPL_MSK, > > x) > > +#define ADF4360_ADDR_CPI1_MSK GENMASK(16, 14) > > +#define ADF4360_CPI1(x) FIELD_PREP(ADF4360_ADDR_CPI1_MSK > > , x) > > +#define ADF4360_ADDR_CPI2_MSK GENMASK(19, 17) > > +#define ADF4360_CPI2(x) FIELD_PREP(ADF4360_ADDR_CPI2_MSK > > , x) > > +#define ADF4360_ADDR_PWR_DWN_MSK GENMASK(21, 20) > > +#define ADF4360_POWERDOWN(x) FIELD_PREP(ADF4360_ADDR_PWR_DWN_ > > MSK, x) > > +#define ADF4360_ADDR_PRESCALER_MSK GENMASK(23, 22) > > +#define ADF4360_PRESCALER(x) FIELD_PREP(ADF4360_ADDR_PRESCALE > > R_MSK, x) > > + > > +/* ADF4360_NDIV */ > > +#define ADF4360_ADDR_A_CNTR_MSK GENMASK(6, 2) > > +#define ADF4360_A_COUNTER(x) FIELD_PREP(ADF4360_ADDR_A_CNTR_M > > SK, x) > > +#define ADF4360_ADDR_B_CNTR_MSK GENMASK(20, 8) > > +#define ADF4360_B_COUNTER(x) FIELD_PREP(ADF4360_ADDR_B_CNTR_M > > SK, x) > > +#define ADF4360_ADDR_OUT_DIV2_MSK BIT(22) > > +#define ADF4360_OUT_DIV2(x) FIELD_PREP(ADF4360_ADDR_OUT_DIV2 > > _MSK, x) > > +#define ADF4360_ADDR_DIV2_SEL_MSK BIT(23) > > +#define ADF4360_PRESCALER_DIV2(x) FIELD_PREP(ADF4360_ADDR_DIV2_SEL_MSK, x) > > + > > +/* ADF4360_RDIV */ > > +#define ADF4360_ADDR_R_CNTR_MSK GENMASK(15, 2) > > +#define ADF4360_R_COUNTER(x) FIELD_PREP(ADF4360_ADDR_R_CNTR_M > > SK, x) > > +#define ADF4360_ADDR_ABP_MSK GENMASK(17, 16) > > +#define ADF4360_ABP(x) FIELD_PREP(ADF4360_ADDR_ABP_MSK, > > x) > > +#define ADF4360_ADDR_BSC_MSK GENMASK(21, 20) > > +#define ADF4360_BSC(x) FIELD_PREP(ADF4360_ADDR_BSC_MSK, > > x) > > + > > +/* Specifications */ > > +#define ADF4360_MAX_PFD_RATE 8000000 /* 8 MHz */ > > +#define ADF4360_MAX_COUNTER_RATE 300000000 /* 300 MHz */ > > +#define ADF4360_MAX_REFIN_RATE 250000000 /* 250 MHz */ > > + > > +enum { > > + ADF4360_CTRL, > > + ADF4360_RDIV, > > + ADF4360_NDIV, > > + ADF4360_REG_NUM, > > +}; > > + > > +enum { > > + ADF4360_GEN1_PC_5, > > + ADF4360_GEN1_PC_10, > > + ADF4360_GEN1_PC_15, > > + ADF4360_GEN1_PC_20, > > +}; > > + > > +enum { > > + ADF4360_GEN2_PC_2_5, > > + ADF4360_GEN2_PC_5, > > + ADF4360_GEN2_PC_7_5, > > + ADF4360_GEN2_PC_10, > > +}; > > + > > +enum { > > + ADF4360_MUXOUT_THREE_STATE, > > + ADF4360_MUXOUT_LOCK_DETECT, > > + ADF4360_MUXOUT_NDIV, > > + ADF4360_MUXOUT_DVDD, > > + ADF4360_MUXOUT_RDIV, > > + ADF4360_MUXOUT_OD_LD, > > + ADF4360_MUXOUT_SDO, > > + ADF4360_MUXOUT_GND, > > +}; > > + > > +enum { > > + ADF4360_PL_3_5, > > + ADF4360_PL_5, > > + ADF4360_PL_7_5, > > + ADF4360_PL_11, > > +}; > > + > > +enum { > > + ADF4360_CPI_0_31, > > + ADF4360_CPI_0_62, > > + ADF4360_CPI_0_93, > > + ADF4360_CPI_1_25, > > + ADF4360_CPI_1_56, > > + ADF4360_CPI_1_87, > > + ADF4360_CPI_2_18, > > + ADF4360_CPI_2_50, > > +}; > > + > > +enum { > > + ADF4360_POWER_DOWN_NORMAL, > > + ADF4360_POWER_DOWN_SOFT_ASYNC, > > + ADF4360_POWER_DOWN_CE, > > + ADF4360_POWER_DOWN_SOFT_SYNC, > > + ADF4360_POWER_DOWN_REGULATOR, > > +}; > > + > > +enum { > > + ADF4360_PRESCALER_8, > > + ADF4360_PRESCALER_16, > > + ADF4360_PRESCALER_32, > > +}; > > + > > +enum { > > + ADF4360_ABP_3_0NS, > > + ADF4360_ABP_1_3NS, > > + ADF4360_ABP_6_0NS, > > +}; > > + > > +enum { > > + ADF4360_BSC_1, > > + ADF4360_BSC_2, > > + ADF4360_BSC_4, > > + ADF4360_BSC_8, > > +}; > > + > > +enum { > > + ID_ADF4360_0, > > + ID_ADF4360_1, > > + ID_ADF4360_2, > > + ID_ADF4360_3, > > + ID_ADF4360_4, > > + ID_ADF4360_5, > > + ID_ADF4360_6, > > + ID_ADF4360_7, > > + ID_ADF4360_8, > > + ID_ADF4360_9, > > +}; > > + > > +enum { > > + ADF4360_FREQ_REFIN, > > + ADF4360_MTLD, > > + ADF4360_FREQ_PFD, > > +}; > > + > > +static const char * const adf4360_power_level_modes[] = { > This isn't an enum. These are real values in sensible units not > magic strings. Handle them as integers. > > We may need to define additional ABI for this but it should be > possible to 'fit' it in the normal structure of ABI. > > > + [ADF4360_PL_3_5] = "3500-uA", > > + [ADF4360_PL_5] = "5000-uA", > > + [ADF4360_PL_7_5] = "7500-uA", > > + [ADF4360_PL_11] = "11000-uA", > > +}; > > + > > +static const unsigned int adf4360_power_lvl_microamp[] = { > > + [ADF4360_PL_3_5] = 3500, > > + [ADF4360_PL_5] = 5000, > > + [ADF4360_PL_7_5] = 7500, > > + [ADF4360_PL_11] = 11000, > > +}; > > + > > +static const unsigned int adf4360_cpi_modes[] = { > > + [ADF4360_CPI_0_31] = 310, > > + [ADF4360_CPI_0_62] = 620, > > + [ADF4360_CPI_0_93] = 930, > > + [ADF4360_CPI_1_25] = 1250, > > + [ADF4360_CPI_1_56] = 1560, > > + [ADF4360_CPI_1_87] = 1870, > > + [ADF4360_CPI_2_18] = 2180, > > + [ADF4360_CPI_2_50] = 2500, > > +}; > > + > > +static const char * const adf4360_muxout_modes[] = { > Superficially from glancing at the datasheet I thing this is debug > functionality? Perhaps move it to debugfs so as to avoid creating > complex new ABI in sysfs. > > > + [ADF4360_MUXOUT_THREE_STATE] = "three-state", > > + [ADF4360_MUXOUT_LOCK_DETECT] = "lock-detect", > > + [ADF4360_MUXOUT_NDIV] = "ndiv", > > + [ADF4360_MUXOUT_DVDD] = "dvdd", > > + [ADF4360_MUXOUT_RDIV] = "rdiv", > > + [ADF4360_MUXOUT_OD_LD] = "od-ld", > > + [ADF4360_MUXOUT_SDO] = "sdo", > > + [ADF4360_MUXOUT_GND] = "gnd", > > +}; > > + > > +static const char * const adf4360_power_down_modes[] = { > > + [ADF4360_POWER_DOWN_NORMAL] = "normal", > > + [ADF4360_POWER_DOWN_SOFT_ASYNC] = "soft-async", > > + [ADF4360_POWER_DOWN_CE] = "ce", > > + [ADF4360_POWER_DOWN_SOFT_SYNC] = "soft-sync", > > + [ADF4360_POWER_DOWN_REGULATOR] = "regulator", > This seems to map rather oddly to the datasheet. Perhaps some > comments to explain what is going on here would help/ > > +}; > > + > > +struct adf4360_output { > > + struct clk_hw hw; > > + struct iio_dev *indio_dev; > > +}; > > + > > +#define to_output(_hw) container_of(_hw, struct adf4360_output, hw) > > + > > +struct adf4360_chip_info { > > + unsigned int vco_min; > > + unsigned int vco_max; > > + unsigned int default_cpl; > > +}; > > + > > +struct adf4360_state { > > + struct spi_device *spi; > > + const struct adf4360_chip_info *info; > > + struct adf4360_output output; > > + struct clk *clkin; > > + struct gpio_desc *muxout_gpio; > > + struct gpio_desc *chip_en_gpio; > > + struct regulator *vdd_reg; > > + struct mutex lock; /* Protect PLL state. */ > > + unsigned int part_id; > > + unsigned long clkin_freq; > > + unsigned long freq_req; > > + unsigned long r; > > + unsigned long n; > > + unsigned int vco_min; > > + unsigned int vco_max; > > + unsigned int pfd_freq; > > + unsigned int cpi; > > + bool pdp; > > + bool mtld; > > + unsigned int power_level; > > + unsigned int muxout_mode; > > + unsigned int power_down_mode; > > + bool initial_reg_seq; > > + const char *clk_out_name; > > + unsigned int regs[ADF4360_REG_NUM]; > > + u8 spi_data[3] ____cacheline_aligned; > > +}; > > + > > +static const struct adf4360_chip_info adf4360_chip_info_tbl[] = { > > + { /* ADF4360-0 */ > > + .vco_min = 2400000000U, > > + .vco_max = 2725000000U, > > + .default_cpl = ADF4360_GEN1_PC_10, > > + }, { /* ADF4360-1 */ > > + .vco_min = 2050000000U, > > + .vco_max = 2450000000U, > > + .default_cpl = ADF4360_GEN1_PC_15, > > + }, { /* ADF4360-2 */ > > + .vco_min = 1850000000U, > > + .vco_max = 2170000000U, > > + .default_cpl = ADF4360_GEN1_PC_15, > > + }, { /* ADF4360-3 */ > > + .vco_min = 1600000000U, > > + .vco_max = 1950000000U, > > + .default_cpl = ADF4360_GEN1_PC_15, > > + }, { /* ADF4360-4 */ > > + .vco_min = 1450000000U, > > + .vco_max = 1750000000U, > > + .default_cpl = ADF4360_GEN1_PC_15, > > + }, { /* ADF4360-5 */ > > + .vco_min = 1200000000U, > > + .vco_max = 1400000000U, > > + .default_cpl = ADF4360_GEN1_PC_10, > > + }, { /* ADF4360-6 */ > > + .vco_min = 1050000000U, > > + .vco_max = 1250000000U, > > + .default_cpl = ADF4360_GEN1_PC_10, > > + }, { /* ADF4360-7 */ > > + .vco_min = 350000000U, > > + .vco_max = 1800000000U, > > + .default_cpl = ADF4360_GEN1_PC_5, > > + }, { /* ADF4360-8 */ > > + .vco_min = 65000000U, > > + .vco_max = 400000000U, > > + .default_cpl = ADF4360_GEN2_PC_5, > > + }, { /* ADF4360-9 */ > > + .vco_min = 65000000U, > > + .vco_max = 400000000U, > > + .default_cpl = ADF4360_GEN2_PC_5, > > + } > > +}; > > + > > +static int adf4360_write_reg(struct adf4360_state *st, unsigned int reg, > > + unsigned int val) > > +{ > > + int ret; > > + > > + val |= reg; > > + > > + st->spi_data[0] = (val >> 16) & 0xff; > > + st->spi_data[1] = (val >> 8) & 0xff; > > + st->spi_data[2] = val & 0xff; > > + > > + ret = spi_write(st->spi, st->spi_data, ARRAY_SIZE(st->spi_data)); > > + if (ret == 0) > > + st->regs[reg] = val; > > + > > + return ret; > > +} > > + > > +/* fVCO = B * fREFIN / R */ > > + > > +static unsigned long adf4360_clk_recalc_rate(struct clk_hw *hw, > > + unsigned long parent_rate) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + if (st->r == 0) > > + return 0; > > + > > + /* > > + * The result is guaranteed to fit in 32-bit, but the intermediate > > + * result might require 64-bit. > > + */ > > + return DIV_ROUND_CLOSEST_ULL((uint64_t)parent_rate * st->n, st->r); > > +} > > + > > +static unsigned int adf4360_calc_prescaler(unsigned int pfd_freq, > > + unsigned int n, > > + unsigned int *out_p, > > + unsigned int *out_a, > > + unsigned int *out_b) > > +{ > > + unsigned int rate = pfd_freq * n; > > + unsigned int p, a, b; > > + > > + /* Make sure divider counter input frequency is low enough */ > > + p = 8; > > + while (p < 32 && rate / p > ADF4360_MAX_COUNTER_RATE) > > + p *= 2; > > + > > + /* > > + * The range of dividers that can be produced using the dual-modulus > > + * pre-scaler is not continuous for values of n < p*(p-1). If we end up > > + * with a non supported divider value, pick the next closest one. > > + */ > > + a = n % p; > > + b = n / p; > > + > > + if (b < 3) { > > + b = 3; > > + a = 0; > > + } else if (a > b) { > > + if (a - b < p - a) { > > + a = b; > > + } else { > > + a = 0; > > + b++; > > + } > > + } > > + > > + if (out_p) > > + *out_p = p; > > + if (out_a) > > + *out_a = a; > > + if (out_b) > > + *out_b = b; > > + > > + return p * b + a; > > +} > > + > > +static long adf4360_clk_round_rate(struct clk_hw *hw, > > + unsigned long rate, > > + unsigned long *parent_rate) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + unsigned int r, n; > > + unsigned int pfd_freq; > > + > > + if (*parent_rate == 0) > > + return 0; > > + > > + if (st->part_id == ID_ADF4360_9) > > + return *parent_rate * st->n / st->r; > > + > > + if (rate > st->vco_max) > > + return st->vco_max; > > + > > + /* ADF4360-0 to AD4370-7 have an optional by two divider */ > > + if (st->part_id <= ID_ADF4360_7) { > > + if (rate < st->vco_min / 2) > > + return st->vco_min / 2; > > + if (rate < st->vco_min && rate > st->vco_max / 2) { > > + if (st->vco_min - rate < rate - st->vco_max / 2) > > + return st->vco_min; > > + else > > + return st->vco_max / 2; > > + } > > + } else { > > + if (rate < st->vco_min) > > + return st->vco_min; > > + } > > + > > + r = DIV_ROUND_CLOSEST(*parent_rate, st->pfd_freq); > > + pfd_freq = *parent_rate / r; > > + n = DIV_ROUND_CLOSEST(rate, pfd_freq); > > + > > + if (st->part_id <= ID_ADF4360_7) > > + n = adf4360_calc_prescaler(pfd_freq, n, NULL, NULL, NULL); > > + > > + return pfd_freq * n; > > +} > > + > > +static inline bool adf4360_is_powerdown(struct adf4360_state *st) > > +{ > > + return (st->power_down_mode != ADF4360_POWER_DOWN_NORMAL); > > +} > > + > > +static int adf4360_set_freq(struct adf4360_state *st, unsigned long rate) > > +{ > > + unsigned int val_r, val_n, val_ctrl; > > + unsigned int pfd_freq; > > + unsigned long r, n; > > + int ret; > > + > > + if (!st->clkin_freq || (st->clkin_freq > ADF4360_MAX_REFIN_RATE)) > > + return -EINVAL; > > + > > + if ((rate < st->vco_min) || (rate > st->vco_max)) > > + return -EINVAL; > > + > > + if (adf4360_is_powerdown(st)) > > + ret = -EBUSY; > > + > > + r = DIV_ROUND_CLOSEST(st->clkin_freq, st->pfd_freq); > > + pfd_freq = st->clkin_freq / r; > > + n = DIV_ROUND_CLOSEST(rate, pfd_freq); > > + > > + val_ctrl = ADF4360_CPL(st->info->default_cpl) | > > + ADF4360_MUXOUT(st->muxout_mode) | > > + ADF4360_PDP(!st->pdp) | > > + ADF4360_MTLD(st->mtld) | > > + ADF4360_OPL(st->power_level) | > > + ADF4360_CPI1(st->cpi) | > > + ADF4360_CPI2(st->cpi) | > > + ADF4360_POWERDOWN(st->power_down_mode); > > + > > + /* ADF4360-0 to ADF4360-7 have a dual-modulous prescaler */ > > + if (st->part_id <= ID_ADF4360_7) { > > + unsigned int p, a, b; > > + > > + n = adf4360_calc_prescaler(pfd_freq, n, &p, &a, &b); > > + > > + switch (p) { > > + case 8: > > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_8); > > + break; > > + case 16: > > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_16); > > + break; > > + default: > > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_32); > > + break; > > + } > > + > > + val_n = ADF4360_A_COUNTER(a) | > > + ADF4360_B_COUNTER(b); > > + > > + if (rate < st->vco_min) > > + val_n |= ADF4360_OUT_DIV2(true) | > > + ADF4360_PRESCALER_DIV2(true); > > + } else { > > + val_n = ADF4360_B_COUNTER(n); > > + } > > + > > + /* > > + * Always use BSC divider of 8, see Analog Devices AN-1347. > > + * > > http://www.analog.com/media/en/technical-documentation/application-notes/AN-1347.pdf > > + */ > > + val_r = ADF4360_R_COUNTER(r) | > > + ADF4360_ABP(ADF4360_ABP_3_0NS) | > > + ADF4360_BSC(ADF4360_BSC_8); > > + > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_RDIV), val_r); > > + if (ret) > > + return ret; > > + > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val_ctrl); > > + if (ret) > > + return ret; > > + > > + /* > > + * Allow the transient behavior of the ADF4360-7 during initial > > + * power-up to settle. > > + */ > > + if (st->initial_reg_seq) { > > + usleep_range(15000, 20000); > > + st->initial_reg_seq = false; > > + } > > + > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_NDIV), val_n); > > + if (ret) > > + return ret; > > + > > + st->freq_req = rate; > > + st->n = n; > > + st->r = r; > > + > > + return 0; > > +} > > + > > +static int adf4360_clk_set_rate(struct clk_hw *hw, > > + unsigned long rate, > > + unsigned long parent_rate) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + int ret; > > + > > + if ((parent_rate == 0) || (parent_rate > ADF4360_MAX_REFIN_RATE)) > > + return -EINVAL; > > + > > + mutex_lock(&st->lock); > > + if (st->clkin_freq != parent_rate) > > + st->clkin_freq = parent_rate; > > + > > + ret = adf4360_set_freq(st, rate); > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static int __adf4360_power_down(struct adf4360_state *st, unsigned int > > mode) > > +{ > > + struct device *dev = &st->spi->dev; > > + unsigned int val; > > + int ret = 0; > > + > > + switch (mode) { > > + case ADF4360_POWER_DOWN_NORMAL: > > + if (st->vdd_reg) { > > + ret = regulator_enable(st->vdd_reg); > > + if (ret) { > > + dev_err(dev, "Supply enable error: %d\n", ret); > > + return ret; > > + } > > + } > > + > > + st->initial_reg_seq = true; > > + st->power_down_mode = mode; > > + ret = adf4360_set_freq(st, st->freq_req); > > + break; > > + case ADF4360_POWER_DOWN_SOFT_ASYNC: /* fallthrough */ > > + case ADF4360_POWER_DOWN_SOFT_SYNC: > > + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > > + val |= ADF4360_POWERDOWN(mode); > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > + break; > > + case ADF4360_POWER_DOWN_CE: > > + if (st->chip_en_gpio) > > + gpiod_set_value(st->chip_en_gpio, 0x0); > > + else > > + return -ENODEV; > > + break; > > + case ADF4360_POWER_DOWN_REGULATOR: > > + if (!st->vdd_reg) > > + return -ENODEV; > > + > > + if (st->chip_en_gpio) > > + ret = __adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > > + else > > + ret = __adf4360_power_down(st, > > + ADF4360_POWER_DOWN_SOFT_ASYNC); > > + > > + ret = regulator_disable(st->vdd_reg); > > + if (ret) > > + dev_err(dev, "Supply disable error: %d\n", ret); > > + break; > > + } > > + if (ret == 0) > > + st->power_down_mode = mode; > > + > > + return 0; > > +} > > + > > +static int adf4360_power_down(struct adf4360_state *st, unsigned int mode) > > +{ > > + int ret; > > + > > + mutex_lock(&st->lock); > > + ret = __adf4360_power_down(st, mode); > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static int adf4360_clk_prepare(struct clk_hw *hw) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + return adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); > > +} > > + > > +static void adf4360_clk_unprepare(struct clk_hw *hw) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + adf4360_power_down(st, ADF4360_POWER_DOWN_SOFT_ASYNC); > > +} > > + > > +static int adf4360_clk_enable(struct clk_hw *hw) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + if (st->chip_en_gpio) > > + gpiod_set_value(st->chip_en_gpio, 0x1); > > + > > + return 0; > > +} > > + > > +static void adf4360_clk_disable(struct clk_hw *hw) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + if (st->chip_en_gpio) > > + adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > > +} > > + > > +static int adf4360_clk_is_enabled(struct clk_hw *hw) > > +{ > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + return adf4360_is_powerdown(st); > > +} > > + > > +static const struct clk_ops adf4360_clk_ops = { > > + .recalc_rate = adf4360_clk_recalc_rate, > > + .round_rate = adf4360_clk_round_rate, > > + .set_rate = adf4360_clk_set_rate, > > + .prepare = adf4360_clk_prepare, > > + .unprepare = adf4360_clk_unprepare, > > + .enable = adf4360_clk_enable, > > + .disable = adf4360_clk_disable, > > + .is_enabled = adf4360_clk_is_enabled, > > +}; > > + > > +static ssize_t adf4360_read(struct iio_dev *indio_dev, > > + uintptr_t private, > > + const struct iio_chan_spec *chan, > > + char *buf) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + unsigned long val; > > + int ret = 0; > > + > > + switch ((u32)private) { > > + case ADF4360_FREQ_REFIN: > > + val = st->clkin_freq; > > + break; > > + case ADF4360_MTLD: > > + val = st->mtld; > > + break; > > + case ADF4360_FREQ_PFD: > > + val = st->pfd_freq; > > + break; > > + default: > > + ret = -EINVAL; > > + val = 0; > > + } > > + > > + return ret < 0 ? ret : sprintf(buf, "%lu\n", val); > > +} > > + > > +static ssize_t adf4360_write(struct iio_dev *indio_dev, > > + uintptr_t private, > > + const struct iio_chan_spec *chan, > > + const char *buf, size_t len) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + unsigned long readin, tmp; > > + bool mtld; > > + int ret = 0; > > + > > + mutex_lock(&st->lock); > > + switch ((u32)private) { > > + case ADF4360_FREQ_REFIN: > > + ret = kstrtoul(buf, 10, &readin); > > + if (ret) > > + break; > > + > > + if ((readin > ADF4360_MAX_REFIN_RATE) || (readin == 0)) { > > + ret = -EINVAL; > > + break; > > + } > > + > > + if (st->clkin) { > > + tmp = clk_round_rate(st->clkin, readin); > > + if (tmp != readin) { > > + ret = -EINVAL; > > + break; > > + } > > + > > + ret = clk_set_rate(st->clkin, tmp); > > + if (ret) > > + break; > A bit odd to directly provide an interface to control and entirely different > bit of hardware. If there are specific demands on the input clock as a > result > of something to do with the outputs, then fair enough. Direct tweaking like > this seems like a very odd interface. > > > + } > > + > > + st->clkin_freq = readin; > > + break; > > + case ADF4360_MTLD: > > + ret = kstrtobool(buf, &mtld); > > + if (ret) > > + break; > > + > > + st->mtld = mtld; > > + break; > > + case ADF4360_FREQ_PFD: > > + ret = kstrtoul(buf, 10, &readin); > > + if (ret) > > + break; > > + > > + if ((readin > ADF4360_MAX_PFD_RATE) || (readin == 0)) { > > + ret = -EINVAL; > > + break; > > + } > > + > > + st->pfd_freq = readin; > > + break; > > + default: > > + ret = -EINVAL; > > + } > > + > > + if (ret == 0) > > + ret = adf4360_set_freq(st, st->freq_req); > > + mutex_unlock(&st->lock); > > + > > + return ret ? ret : len; > > +} > > + > > +static int adf4360_get_muxout_mode(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + return st->muxout_mode; > > +} > > + > > +static int adf4360_set_muxout_mode(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, > > + unsigned int mode) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + unsigned int writeval; > > + int ret = 0; > > + > > + mutex_lock(&st->lock); > > + writeval = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > > + writeval |= ADF4360_MUXOUT(mode & 0x7); > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), writeval); > > + if (ret == 0) > > + st->muxout_mode = mode & 0x7; > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static const struct iio_enum adf4360_muxout_modes_available = { > > + .items = adf4360_muxout_modes, > > + .num_items = ARRAY_SIZE(adf4360_muxout_modes), > > + .get = adf4360_get_muxout_mode, > > + .set = adf4360_set_muxout_mode, > > +}; > > + > > +static int adf4360_get_power_down(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + return st->power_down_mode; > > +} > > + > > +static int adf4360_set_power_down(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, > > + unsigned int mode) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + return adf4360_power_down(st, mode); > > +} > > + > > +static const struct iio_enum adf4360_pwr_dwn_modes_available = { > > + .items = adf4360_power_down_modes, > > + .num_items = ARRAY_SIZE(adf4360_power_down_modes), > > + .get = adf4360_get_power_down, > > + .set = adf4360_set_power_down, > > +}; > > + > > +static int adf4360_get_power_level(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + > > + return st->power_level; > > +} > > + > > +static int adf4360_set_power_level(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, > > + unsigned int level) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + unsigned int val; > > + int ret; > > + > > + if (adf4360_is_powerdown(st)) > > + return -EBUSY; > > + > > + mutex_lock(&st->lock); > > + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_OPL_MSK; > > + val |= ADF4360_OPL(level); > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > + st->power_level = level; > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static const struct iio_enum adf4360_pwr_lvl_modes_available = { > > + .items = adf4360_power_level_modes, > > + .num_items = ARRAY_SIZE(adf4360_power_level_modes), > > + .get = adf4360_get_power_level, > > + .set = adf4360_set_power_level, > > +}; > > + > > +#define _ADF4360_EXT_INFO(_name, _ident) { \ > > + .name = _name, \ > > + .read = adf4360_read, \ > > + .write = adf4360_write, \ > > + .private = _ident, \ > > + .shared = IIO_SEPARATE, \ > > +} > > + > > +static const struct iio_chan_spec_ext_info adf4360_ext_info[] = { > > This is a wide range of new ABI. These all need documentation > in Documentation/ABI/testing/sysfs-bus-iio-* > > Without docs, it's hard to discuss if these are appropriate but a few initial > comments... > > + _ADF4360_EXT_INFO("refin_frequency", ADF4360_FREQ_REFIN), > Looks like a control that should be matched to some clock source and > not change at runtime? > > > + _ADF4360_EXT_INFO("mute_till_lock_detect", ADF4360_MTLD), > > + _ADF4360_EXT_INFO("pfd_frequency", ADF4360_FREQ_PFD), > > + IIO_ENUM_AVAILABLE("muxout_mode", &adf4360_muxout_modes_available), > > + IIO_ENUM("muxout_mode", false, &adf4360_muxout_modes_available), > > + IIO_ENUM_AVAILABLE("power_down", &adf4360_pwr_dwn_modes_available), > > + IIO_ENUM("power_down", false, &adf4360_pwr_dwn_modes_available), > > + IIO_ENUM_AVAILABLE("power_level", &adf4360_pwr_lvl_modes_available), > > + IIO_ENUM("power_level", false, &adf4360_pwr_lvl_modes_available), > > + { }, > > +}; > > + > > +static const struct iio_chan_spec adf4360_chan = { > > + .type = IIO_ALTVOLTAGE, > > + .indexed = 1, > > + .output = 1, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY), > > + .ext_info = adf4360_ext_info, > > +}; > > + > > +static int adf4360_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, > > + int *val2, > > + long mask) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + bool lk_det; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_FREQUENCY: > > + if (adf4360_is_powerdown(st)) > > + return -EBUSY; > > + > > + lk_det = (ADF4360_MUXOUT_LOCK_DETECT | ADF4360_MUXOUT_OD_LD) & > > + st->muxout_mode; > > + if (lk_det && st->muxout_gpio) { > > + if (!gpiod_get_value(st->muxout_gpio)) { > > + dev_dbg(&st->spi->dev, "PLL un-locked\n"); > > + return -EBUSY; > > + } > > + } > > + > > + *val = st->freq_req; > > + return IIO_VAL_INT; > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +}; > > + > > +static int adf4360_write_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int val, > > + int val2, > > + long mask) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + int ret; > > + > > + mutex_lock(&st->lock); > > + switch (mask) { > > + case IIO_CHAN_INFO_FREQUENCY: > > + ret = adf4360_set_freq(st, val); > > + break; > > + default: > > + ret = -EINVAL; > > + } > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static int adf4360_reg_access(struct iio_dev *indio_dev, > > + unsigned int reg, > > + unsigned int writeval, > > + unsigned int *readval) > > +{ > > + struct adf4360_state *st = iio_priv(indio_dev); > > + int ret = 0; > > + > > + if (reg >= ADF4360_REG_NUM) > > + return -EFAULT; > > + > > + mutex_lock(&st->lock); > > + if (readval) { > > + *readval = st->regs[reg]; > > + } else { > > + writeval &= 0xFFFFFC; > > + ret = adf4360_write_reg(st, reg, writeval); > > + } > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static const struct iio_info adf4360_iio_info = { > > + .read_raw = &adf4360_read_raw, > > + .write_raw = &adf4360_write_raw, > > + .debugfs_reg_access = &adf4360_reg_access, > > +}; > > + > > +static int adf4360_get_gpio(struct adf4360_state *st) > > +{ > > + struct device *dev = &st->spi->dev; > > + unsigned int val; > > + int ret, i; > > + > > + st->chip_en_gpio = devm_gpiod_get_optional(dev, "enable", > > + GPIOD_OUT_HIGH); > > + if (IS_ERR(st->chip_en_gpio)) { > > + dev_err(dev, "Chip enable GPIO error\n"); > > Put handling in here to prevent an error message of DEFER. > Same for the other routes where this might happen. > > > + return PTR_ERR(st->chip_en_gpio); > > + } > > + > > + if (st->chip_en_gpio) > > + st->power_down_mode = ADF4360_POWER_DOWN_CE; > > + > > + st->muxout_gpio = devm_gpiod_get_optional(dev, "adi,muxout", GPIOD_IN); > > + if (IS_ERR(st->muxout_gpio)) { > > + dev_err(dev, "Muxout GPIO error\n"); > > + return PTR_ERR(st->muxout_gpio); > > + } > > + > > + if (!st->muxout_gpio) > > + return 0; > > + > > + /* ADF4360 PLLs are write only devices, try to probe using GPIO. */ > > + for (i = 0; i < 4; i++) { > > + if (i & 1) > > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_DVDD); > > + else > > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_GND); > > + > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > + if (ret) > > + return ret; > > + > > + ret = gpiod_get_value(st->muxout_gpio); > > + if (ret ^ (i & 1)) { > > + dev_err(dev, "Probe failed (muxout)"); > > + return -ENODEV; > > + } > > + } > > + > > + return 0; > > +} > > + > > +static void adf4360_clkin_disable(void *data) > > +{ > > + struct adf4360_state *st = data; > > + > > + clk_disable_unprepare(st->clkin); > > +} > > + > > +static int adf4360_get_clkin(struct adf4360_state *st) > > +{ > > + struct device *dev = &st->spi->dev; > > + struct clk *clk; > > + int ret; > > + > > + clk = devm_clk_get(dev, "clkin"); > > + if (IS_ERR(clk)) > > + return PTR_ERR(clk); > > + > > + ret = clk_prepare_enable(clk); > > + if (ret) > > + return ret; > > + > > + ret = devm_add_action_or_reset(dev, adf4360_clkin_disable, st); > > + if (ret) > > + return ret; > > + > > + st->clkin = clk; > > + st->clkin_freq = clk_get_rate(clk); > > + > > + return 0; > > +} > > + > > +static void adf4360_clk_del_provider(void *data) > > +{ > > + struct adf4360_state *st = data; > > + > > + of_clk_del_provider(st->spi->dev.of_node); > > +} > > + > > +static int adf4360_clk_register(struct adf4360_state *st) > > +{ > > Hmm. This makes me wonder why this is an IIO driver rather than a clk > driver? Definitely needs some more information in the patch description > or a cover letter. > > > + struct spi_device *spi = st->spi; > > + struct clk_init_data init; > > + struct clk *clk; > > + const char *parent_name; > > + int ret; > > + > > + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0); > > + if (!parent_name) > > + return -EINVAL; > > + > > + init.name = st->clk_out_name; > > + init.ops = &adf4360_clk_ops; > > + init.flags = CLK_SET_RATE_GATE; > > + init.parent_names = &parent_name; > > + init.num_parents = 1; > > + > > + st->output.hw.init = &init; > > + > > + clk = devm_clk_register(&spi->dev, &st->output.hw); > > + if (IS_ERR(clk)) > > + return PTR_ERR(clk); > > + > > + ret = of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get, clk); > > + if (ret) > > + return ret; > > + > > + return devm_add_action_or_reset(&spi->dev, adf4360_clk_del_provider, > > st); > > +} > > + > > +static int adf4360_parse_dt(struct adf4360_state *st) > > +{ > > + struct device *dev = &st->spi->dev; > > + u32 tmp; > > + int ret; > > + > > + ret = device_property_read_string(dev, "clock-output-names", > > + &st->clk_out_name); > > + if ((ret < 0) && dev->of_node) > > + st->clk_out_name = dev->of_node->name; > > + > > + if (st->part_id >= ID_ADF4360_7) { > > + /* > > + * ADF4360-7 to ADF4360-9 have a VCO that is tuned to a specific > > + * range using an external inductor. These properties describe > > + * the range selected by the external inductor. > > + */ > > + ret = device_property_read_u32(dev, > > + "adi,vco-minimum-frequency-hz", > > + &tmp); > > + if (ret == 0) > > + st->vco_min = max(st->info->vco_min, tmp); > > + else > > + st->vco_min = st->info->vco_min; > > + > > + ret = device_property_read_u32(dev, > > + "adi,vco-maximum-frequency-hz", > > + &tmp); > > + if (ret == 0) > > + st->vco_max = min(st->info->vco_max, tmp); > > + else > > + st->vco_max = st->info->vco_max; > > + } else { > > + st->vco_min = st->info->vco_min; > > + st->vco_max = st->info->vco_max; > > + } > > + > > + st->pdp = device_property_read_bool(dev, "adi,loop-filter-inverting"); > > + > > + ret = device_property_read_u32(dev, > > + "adi,loop-filter-pfd-frequency-hz", > > + &tmp); > > + if (ret == 0) { > > + st->pfd_freq = tmp; > > + } else { > > + dev_err(dev, "PFD frequency property missing\n"); > > + return ret; > > + } > > + > > + ret = device_property_read_u32(dev, > > + "adi,loop-filter-charge-pump-current-microamp", > > + &tmp); > > + if (ret == 0) { > > + st->cpi = find_closest(tmp, adf4360_cpi_modes, > > + ARRAY_SIZE(adf4360_cpi_modes)); > > + } else { > > + dev_err(dev, "CPI property missing\n"); > > + return ret; > > + } > > + > > + ret = device_property_read_u32(dev, "adi,power-up-frequency-hz", &tmp); > > + if (ret == 0) > > + st->freq_req = tmp; > > + else > > + st->freq_req = st->vco_min; > > + > > + ret = device_property_read_u32(dev, "adi,power-out-level-microamp", > > + &tmp); > > + if (ret == 0) > > + st->power_level = find_closest(tmp, adf4360_power_lvl_microamp, > > + ARRAY_SIZE(adf4360_power_lvl_microamp)); > > + else > > + st->power_level = ADF4360_PL_5; > > + > > + return 0; > > +} > > + > > +static int adf4360_probe(struct spi_device *spi) > > +{ > > + struct iio_dev *indio_dev; > > + const struct spi_device_id *id = spi_get_device_id(spi); > > Given we require various dt parameters to be present, might as well > associate the id with the of_ structures instead and use the dt > calls throughout. Even better if you use the firmware type independent > versions. > > > + struct adf4360_state *st; > > + int ret; > > + > > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + st = iio_priv(indio_dev); > > + > > + mutex_init(&st->lock); > > + > > + spi_set_drvdata(spi, indio_dev); > > + > > + st->spi = spi; > > + st->info = &adf4360_chip_info_tbl[id->driver_data]; > > + st->part_id = id->driver_data; > > + st->muxout_mode = ADF4360_MUXOUT_LOCK_DETECT; > > + st->mtld = true; > > + > > + ret = adf4360_parse_dt(st); > > + if (ret) { > > + dev_err(&spi->dev, "Parsing properties failed (%d)\n", ret); > > + return -ENODEV; > > + } > > + > > + indio_dev->dev.parent = &spi->dev; > > + > > + if (spi->dev.of_node) > > + indio_dev->name = spi->dev.of_node->name; > > + else > > + indio_dev->name = spi_get_device_id(spi)->name; > > + > > + indio_dev->info = &adf4360_iio_info; > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + indio_dev->channels = &adf4360_chan; > > + indio_dev->num_channels = 1; > > + st->output.indio_dev = indio_dev; > > + > > + ret = adf4360_get_gpio(st); > > + if (ret) > > + return ret; > > + > > + ret = adf4360_get_clkin(st); > > + if (ret) > > + return ret; > > + > > + st->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd"); > > + if (IS_ERR(st->vdd_reg)) { > > + if (PTR_ERR(st->vdd_reg) != -ENODEV) { > > + dev_err(&spi->dev, "Regulator error\n"); > > + return PTR_ERR(st->vdd_reg); > > + } > > + > > + st->vdd_reg = NULL; > > + } > > + > > + ret = adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); > > + if (ret) > > + return ret; > > + > > + ret = adf4360_clk_register(st); > > + if (ret) > > + return ret; > > + > > + return devm_iio_device_register(&spi->dev, indio_dev); > > +} > > + > > +static const struct of_device_id adf4360_of_match[] = { > > + { .compatible = "adi,adf4360-0", }, > > + { .compatible = "adi,adf4360-1", }, > > + { .compatible = "adi,adf4360-2", }, > > + { .compatible = "adi,adf4360-3", }, > > + { .compatible = "adi,adf4360-4", }, > > + { .compatible = "adi,adf4360-5", }, > > + { .compatible = "adi,adf4360-6", }, > > + { .compatible = "adi,adf4360-7", }, > > + { .compatible = "adi,adf4360-8", }, > > + { .compatible = "adi,adf4360-9", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, adf4360_of_match); > > + > > +static const struct spi_device_id adf4360_id[] = { > > As mentioned above, you can't actually probe this device > without a pile of dt stuff. So this fallback doesn't > make much sense. Use the data field in the of table > above and get rid of this table entirely. > > > + {"adf4360-0", ID_ADF4360_0}, > > + {"adf4360-1", ID_ADF4360_1}, > > + {"adf4360-2", ID_ADF4360_2}, > > + {"adf4360-3", ID_ADF4360_3}, > > + {"adf4360-4", ID_ADF4360_4}, > > + {"adf4360-5", ID_ADF4360_5}, > > + {"adf4360-6", ID_ADF4360_6}, > > + {"adf4360-7", ID_ADF4360_7}, > > + {"adf4360-8", ID_ADF4360_8}, > > + {"adf4360-9", ID_ADF4360_9}, > > + {} > > +}; > > + > > +static struct spi_driver adf4360_driver = { > > + .driver = { > > + .name = "adf4360", > > + .of_match_table = adf4360_of_match, > > + .owner = THIS_MODULE, > > It's a long time since we had to set the .owner field for each driver. > > Follow through what happens in module_spi_driver, spi_register_driver > etc and you'll find it's set automatically during driver registration. > > > + }, > > + .probe = adf4360_probe, > > + .id_table = adf4360_id, > > +}; > > +module_spi_driver(adf4360_driver); > > + > > +MODULE_LICENSE("GPL v2"); > > +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); > > +MODULE_AUTHOR("Edward Kigwana <ekigwana@gmail.com>"); > > +MODULE_DESCRIPTION("Analog Devices ADF4360 PLL");
On Mon, 3 Feb 2020 11:18:51 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Sun, 2020-02-02 at 14:45 +0000, Jonathan Cameron wrote: > > [External] > > > > On Tue, 28 Jan 2020 13:13:00 +0200 > > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > > > From: Edward Kigwana <ekigwana@gmail.com> > > > > > > The ADF4360-N (N is 0..9) are a family of integrated integer-N synthesizer > > > and voltage controlled oscillator (VCO). > > > > > > Control of all the on-chip registers is through a simple 3-wire SPI > > > interface. The device operates with a power supply ranging from 3.0 V to > > > 3.6 V and can be powered down when not in use. > > > > > > The initial draft-work was done by Lars-Peter Clausen <lars@metafoo.de> > > > The current/latest/complete version was implemented by > > > Edward Kigwana <ekigwana@gmail.com>. > > > > > > The ADF4360-9 is also present on the Analog Devices 'Advanced Active > > > Learning Module 2000' (ADALM-2000). > > > > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-0.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-1.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-2.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-3.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-4.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-5.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-6.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-7.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-8.pdf > > > Link: > > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-9.pdf > > > Link: > > > https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/ADALM2000.html > > > > > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > > Signed-off-by: Edward Kigwana <ekigwana@gmail.com> > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > > Superficially this looks like you really have a clock source driver. > > You then wrap it in IIO in order to provide convenient userspace interfaces. > > > > I'm not sure we want to do that and I definitely need agreement from those > > responsible for clock source drivers before I take anything that does do > > this sort of combination of the two types of driver. > > > > I'll admit that I am a bit fuzzy about these frequency-generators/clock- > sources/synthesizer chips. > In the sense, I don't know where to draw the line between when to consider it > [primarly] an IIO device [for the IIO subsystem] or when to consider it a clock- > device [primarily, for the clk subsystem]. > Obviously there's an inertia [for me at least] to go IIO, as I know it a bit > better. > > We've had some quick/short discussions [internally] about this driver, and also > about the LTC6952. We didn't have a bigger one, where we try to discuss them > more in-detail; but we do have a plan to do it. > > So, then maybe [until then] a question: how do we decide this? [generally > speaking, not just adf4360 & ltc6952] > i.e. when to consider it clk-first or iio-first; > I'm not sure if there is a clear-cut rule, but maybe some guidelines/thoughts? > Obviously, I'll have to read-up more on the clk framework code [as well] to get > a feel for it. > > We've done some things internally [up until now] with some of these clock-chips > that's been mostly IIO-centric. Now, much of it may not be correct, but it is > what we use as template when writing new driver, which [of course] is not good. > And, I also don't want to push/force our mistakes upstream, because that is > [well...] bad. Hence this/these question(s). Clocks aren't an area I know much about either. I'd suggest an RFC with a description of the types of devices and some examples. Send that to linux-iio and linux-clk and see what responses you get. Make sure to highlight the fuzzy boundary when we get into waveform generators etc rather than straight forward clocks. + include some of the devices that definitely aren't suitable for use clocking normal SoCs etc. Will be interesting to see where this one goes. Jonathan > > Thanks > Alex > > > I can see that, for these high speed devices, the intent is normally to > > provide > > an input signal to some external circuitry rather than some internal clock > > signal, but given this is registered as a clock source the argument that it is > > somehow special doesn't seem to hold. > > > > A few more specific comments inline. > > > > Thanks, > > > > Jonathan > > > > Jonathan > > > > > --- > > > > > > Changelog v2 -> v3: > > > * dropped patch about adf4371.yaml; added by accident since it was used to > > > compare against > > > * addressed Rob's review comments for DT schema > > > > > > Changelog v1 -> v2: > > > * validate dt-bindings file with > > > make dt_binding_check > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/frequency/adi,adf4360. > > > yaml > > > > > > drivers/iio/frequency/Kconfig | 11 + > > > drivers/iio/frequency/Makefile | 1 + > > > drivers/iio/frequency/adf4360.c | 1263 +++++++++++++++++++++++++++++++ > > > 3 files changed, 1275 insertions(+) > > > create mode 100644 drivers/iio/frequency/adf4360.c > > > > > > diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig > > > index 240b81502512..781236496661 100644 > > > --- a/drivers/iio/frequency/Kconfig > > > +++ b/drivers/iio/frequency/Kconfig > > > @@ -39,6 +39,17 @@ config ADF4350 > > > To compile this driver as a module, choose M here: the > > > module will be called adf4350. > > > > > > +config ADF4360 > > > + tristate "Analog Devices ADF4360 Wideband Synthesizers" > > > + depends on SPI > > > + depends on COMMON_CLK > > > + help > > > + Say yes here to build support for Analog Devices ADF4360 > > > + Wideband Synthesizers. The driver provides direct access via sysfs. > > > + > > > + To compile this driver as a module, choose M here: the > > > + module will be called adf4360. > > > + > > > config ADF4371 > > > tristate "Analog Devices ADF4371/ADF4372 Wideband Synthesizers" > > > depends on SPI > > > diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile > > > index 518b1e50caef..85f2f81da662 100644 > > > --- a/drivers/iio/frequency/Makefile > > > +++ b/drivers/iio/frequency/Makefile > > > @@ -6,4 +6,5 @@ > > > # When adding new entries keep the list in alphabetical order > > > obj-$(CONFIG_AD9523) += ad9523.o > > > obj-$(CONFIG_ADF4350) += adf4350.o > > > +obj-$(CONFIG_ADF4360) += adf4360.o > > > obj-$(CONFIG_ADF4371) += adf4371.o > > > diff --git a/drivers/iio/frequency/adf4360.c > > > b/drivers/iio/frequency/adf4360.c > > > new file mode 100644 > > > index 000000000000..49ad58092171 > > > --- /dev/null > > > +++ b/drivers/iio/frequency/adf4360.c > > > @@ -0,0 +1,1263 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * ADF4360 PLL with Integrated Synthesizer and VCO > > > + * > > > + * Copyright 2014-2019 Analog Devices Inc. > > > + * Copyright 2019-2020 Edward Kigwana. > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/clk.h> > > > +#include <linux/clk-provider.h> > > > +#include <linux/delay.h> > > > +#include <linux/device.h> > > > +#include <linux/err.h> > > > +#include <linux/gpio/consumer.h> > > > +#include <linux/kernel.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/regulator/consumer.h> > > > +#include <linux/spi/spi.h> > > > +#include <linux/util_macros.h> > > > + > > > +#include <linux/iio/iio.h> > > > + > > > +/* Register address macro */ > > > +#define ADF4360_REG(x) (x) > > > + > > > +/* ADF4360_CTRL */ > > > +#define ADF4360_ADDR_CPL_MSK GENMASK(3, 2) > > > +#define ADF4360_CPL(x) FIELD_PREP(ADF4360_ADDR_CPL_MSK, > > > x) > > > +#define ADF4360_ADDR_MUXOUT_MSK GENMASK(7, 5) > > > +#define ADF4360_MUXOUT(x) FIELD_PREP(ADF4360_ADDR_MUXOUT_MSK, x) > > > +#define ADF4360_ADDR_PDP_MSK BIT(8) > > > +#define ADF4360_PDP(x) FIELD_PREP(ADF4360_ADDR_PDP_MSK, > > > x) > > > +#define ADF4360_ADDR_MTLD_MSK BIT(11) > > > +#define ADF4360_MTLD(x) FIELD_PREP(ADF4360_ADDR_MTLD_MSK > > > , x) > > > +#define ADF4360_ADDR_OPL_MSK GENMASK(13, 12) > > > +#define ADF4360_OPL(x) FIELD_PREP(ADF4360_ADDR_OPL_MSK, > > > x) > > > +#define ADF4360_ADDR_CPI1_MSK GENMASK(16, 14) > > > +#define ADF4360_CPI1(x) FIELD_PREP(ADF4360_ADDR_CPI1_MSK > > > , x) > > > +#define ADF4360_ADDR_CPI2_MSK GENMASK(19, 17) > > > +/* ADF4360_RDIV */ > > > +#define ADF4360_CPI2(x) FIELD_PREP(ADF4360_ADDR_CPI2_MSK > > > , x) > > > +#define ADF4360_ADDR_PWR_DWN_MSK GENMASK(21, 20) > > > +#define ADF4360_POWERDOWN(x) FIELD_PREP(ADF4360_ADDR_PWR_DWN_ > > > MSK, x) > > > +#define ADF4360_ADDR_PRESCALER_MSK GENMASK(23, 22) > > > +#define ADF4360_PRESCALER(x) FIELD_PREP(ADF4360_ADDR_PRESCALE > > > R_MSK, x) > > > + > > > +/* ADF4360_NDIV */ > > > +#define ADF4360_ADDR_A_CNTR_MSK GENMASK(6, 2) > > > +#define ADF4360_A_COUNTER(x) FIELD_PREP(ADF4360_ADDR_A_CNTR_M > > > SK, x) > > > +#define ADF4360_ADDR_B_CNTR_MSK GENMASK(20, 8) > > > +#define ADF4360_B_COUNTER(x) FIELD_PREP(ADF4360_ADDR_B_CNTR_M > > > SK, x) > > > +#define ADF4360_ADDR_OUT_DIV2_MSK BIT(22) > > > +#define ADF4360_OUT_DIV2(x) FIELD_PREP(ADF4360_ADDR_OUT_DIV2 > > > _MSK, x) > > > +#define ADF4360_ADDR_DIV2_SEL_MSK BIT(23) > > > +#define ADF4360_PRESCALER_DIV2(x) FIELD_PREP(ADF4360_ADDR_DIV2_SEL_MSK, x) > > > + > > > +#define ADF4360_ADDR_R_CNTR_MSK GENMASK(15, 2) > > > +#define ADF4360_R_COUNTER(x) FIELD_PREP(ADF4360_ADDR_R_CNTR_M > > > SK, x) > > > +#define ADF4360_ADDR_ABP_MSK GENMASK(17, 16) > > > +#define ADF4360_ABP(x) FIELD_PREP(ADF4360_ADDR_ABP_MSK, > > > x) > > > +#define ADF4360_ADDR_BSC_MSK GENMASK(21, 20) > > > +#define ADF4360_BSC(x) FIELD_PREP(ADF4360_ADDR_BSC_MSK, > > > x) > > > + > > > +/* Specifications */ > > > +#define ADF4360_MAX_PFD_RATE 8000000 /* 8 MHz */ > > > +#define ADF4360_MAX_COUNTER_RATE 300000000 /* 300 MHz */ > > > +#define ADF4360_MAX_REFIN_RATE 250000000 /* 250 MHz */ > > > + > > > +enum { > > > + ADF4360_CTRL, > > > + ADF4360_RDIV, > > > + ADF4360_NDIV, > > > + ADF4360_REG_NUM, > > > +}; > > > + > > > +enum { > > > + ADF4360_GEN1_PC_5, > > > + ADF4360_GEN1_PC_10, > > > + ADF4360_GEN1_PC_15, > > > + ADF4360_GEN1_PC_20, > > > +}; > > > + > > > +enum { > > > + ADF4360_GEN2_PC_2_5, > > > + ADF4360_GEN2_PC_5, > > > + ADF4360_GEN2_PC_7_5, > > > + ADF4360_GEN2_PC_10, > > > +}; > > > + > > > +enum { > > > + ADF4360_MUXOUT_THREE_STATE, > > > + ADF4360_MUXOUT_LOCK_DETECT, > > > + ADF4360_MUXOUT_NDIV, > > > + ADF4360_MUXOUT_DVDD, > > > + ADF4360_MUXOUT_RDIV, > > > + ADF4360_MUXOUT_OD_LD, > > > + ADF4360_MUXOUT_SDO, > > > + ADF4360_MUXOUT_GND, > > > +}; > > > + > > > +enum { > > > + ADF4360_PL_3_5, > > > + ADF4360_PL_5, > > > + ADF4360_PL_7_5, > > > + ADF4360_PL_11, > > > +}; > > > + > > > +enum { > > > + ADF4360_CPI_0_31, > > > + ADF4360_CPI_0_62, > > > + ADF4360_CPI_0_93, > > > + ADF4360_CPI_1_25, > > > + ADF4360_CPI_1_56, > > > + ADF4360_CPI_1_87, > > > + ADF4360_CPI_2_18, > > > + ADF4360_CPI_2_50, > > > +}; > > > + > > > +enum { > > > + ADF4360_POWER_DOWN_NORMAL, > > > + ADF4360_POWER_DOWN_SOFT_ASYNC, > > > + ADF4360_POWER_DOWN_CE, > > > + ADF4360_POWER_DOWN_SOFT_SYNC, > > > + ADF4360_POWER_DOWN_REGULATOR, > > > +}; > > > + > > > +enum { > > > + ADF4360_PRESCALER_8, > > > + ADF4360_PRESCALER_16, > > > + ADF4360_PRESCALER_32, > > > +}; > > > + > > > +enum { > > > + ADF4360_ABP_3_0NS, > > > + ADF4360_ABP_1_3NS, > > > + ADF4360_ABP_6_0NS, > > > +}; > > > + > > > +enum { > > > + ADF4360_BSC_1, > > > + ADF4360_BSC_2, > > > + ADF4360_BSC_4, > > > + ADF4360_BSC_8, > > > +}; > > > + > > > +enum { > > > + ID_ADF4360_0, > > > + ID_ADF4360_1, > > > + ID_ADF4360_2, > > > + ID_ADF4360_3, > > > + ID_ADF4360_4, > > > + ID_ADF4360_5, > > > + ID_ADF4360_6, > > > + ID_ADF4360_7, > > > + ID_ADF4360_8, > > > + ID_ADF4360_9, > > > +}; > > > + > > > +enum { > > > + ADF4360_FREQ_REFIN, > > > + ADF4360_MTLD, > > > + ADF4360_FREQ_PFD, > > > +}; > > > + > > > +static const char * const adf4360_power_level_modes[] = { > > This isn't an enum. These are real values in sensible units not > > magic strings. Handle them as integers. > > > > We may need to define additional ABI for this but it should be > > possible to 'fit' it in the normal structure of ABI. > > > > > + [ADF4360_PL_3_5] = "3500-uA", > > > + [ADF4360_PL_5] = "5000-uA", > > > + [ADF4360_PL_7_5] = "7500-uA", > > > + [ADF4360_PL_11] = "11000-uA", > > > +}; > > > + > > > +static const unsigned int adf4360_power_lvl_microamp[] = { > > > + [ADF4360_PL_3_5] = 3500, > > > + [ADF4360_PL_5] = 5000, > > > + [ADF4360_PL_7_5] = 7500, > > > + [ADF4360_PL_11] = 11000, > > > +}; > > > + > > > +static const unsigned int adf4360_cpi_modes[] = { > > > + [ADF4360_CPI_0_31] = 310, > > > + [ADF4360_CPI_0_62] = 620, > > > + [ADF4360_CPI_0_93] = 930, > > > + [ADF4360_CPI_1_25] = 1250, > > > + [ADF4360_CPI_1_56] = 1560, > > > + [ADF4360_CPI_1_87] = 1870, > > > + [ADF4360_CPI_2_18] = 2180, > > > + [ADF4360_CPI_2_50] = 2500, > > > +}; > > > + > > > +static const char * const adf4360_muxout_modes[] = { > > Superficially from glancing at the datasheet I thing this is debug > > functionality? Perhaps move it to debugfs so as to avoid creating > > complex new ABI in sysfs. > > > > > + [ADF4360_MUXOUT_THREE_STATE] = "three-state", > > > + [ADF4360_MUXOUT_LOCK_DETECT] = "lock-detect", > > > + [ADF4360_MUXOUT_NDIV] = "ndiv", > > > + [ADF4360_MUXOUT_DVDD] = "dvdd", > > > + [ADF4360_MUXOUT_RDIV] = "rdiv", > > > + [ADF4360_MUXOUT_OD_LD] = "od-ld", > > > + [ADF4360_MUXOUT_SDO] = "sdo", > > > + [ADF4360_MUXOUT_GND] = "gnd", > > > +}; > > > + > > > +static const char * const adf4360_power_down_modes[] = { > > > + [ADF4360_POWER_DOWN_NORMAL] = "normal", > > > + [ADF4360_POWER_DOWN_SOFT_ASYNC] = "soft-async", > > > + [ADF4360_POWER_DOWN_CE] = "ce", > > > + [ADF4360_POWER_DOWN_SOFT_SYNC] = "soft-sync", > > > + [ADF4360_POWER_DOWN_REGULATOR] = "regulator", > > This seems to map rather oddly to the datasheet. Perhaps some > > comments to explain what is going on here would help/ > > > +}; > > > + > > > +struct adf4360_output { > > > + struct clk_hw hw; > > > + struct iio_dev *indio_dev; > > > +}; > > > + > > > +#define to_output(_hw) container_of(_hw, struct adf4360_output, hw) > > > + > > > +struct adf4360_chip_info { > > > + unsigned int vco_min; > > > + unsigned int vco_max; > > > + unsigned int default_cpl; > > > +}; > > > + > > > +struct adf4360_state { > > > + struct spi_device *spi; > > > + const struct adf4360_chip_info *info; > > > + struct adf4360_output output; > > > + struct clk *clkin; > > > + struct gpio_desc *muxout_gpio; > > > + struct gpio_desc *chip_en_gpio; > > > + struct regulator *vdd_reg; > > > + struct mutex lock; /* Protect PLL state. */ > > > + unsigned int part_id; > > > + unsigned long clkin_freq; > > > + unsigned long freq_req; > > > + unsigned long r; > > > + unsigned long n; > > > + unsigned int vco_min; > > > + unsigned int vco_max; > > > + unsigned int pfd_freq; > > > + unsigned int cpi; > > > + bool pdp; > > > + bool mtld; > > > + unsigned int power_level; > > > + unsigned int muxout_mode; > > > + unsigned int power_down_mode; > > > + bool initial_reg_seq; > > > + const char *clk_out_name; > > > + unsigned int regs[ADF4360_REG_NUM]; > > > + u8 spi_data[3] ____cacheline_aligned; > > > +}; > > > + > > > +static const struct adf4360_chip_info adf4360_chip_info_tbl[] = { > > > + { /* ADF4360-0 */ > > > + .vco_min = 2400000000U, > > > + .vco_max = 2725000000U, > > > + .default_cpl = ADF4360_GEN1_PC_10, > > > + }, { /* ADF4360-1 */ > > > + .vco_min = 2050000000U, > > > + .vco_max = 2450000000U, > > > + .default_cpl = ADF4360_GEN1_PC_15, > > > + }, { /* ADF4360-2 */ > > > + .vco_min = 1850000000U, > > > + .vco_max = 2170000000U, > > > + .default_cpl = ADF4360_GEN1_PC_15, > > > + }, { /* ADF4360-3 */ > > > + .vco_min = 1600000000U, > > > + .vco_max = 1950000000U, > > > + .default_cpl = ADF4360_GEN1_PC_15, > > > + }, { /* ADF4360-4 */ > > > + .vco_min = 1450000000U, > > > + .vco_max = 1750000000U, > > > + .default_cpl = ADF4360_GEN1_PC_15, > > > + }, { /* ADF4360-5 */ > > > + .vco_min = 1200000000U, > > > + .vco_max = 1400000000U, > > > + .default_cpl = ADF4360_GEN1_PC_10, > > > + }, { /* ADF4360-6 */ > > > + .vco_min = 1050000000U, > > > + .vco_max = 1250000000U, > > > + .default_cpl = ADF4360_GEN1_PC_10, > > > + }, { /* ADF4360-7 */ > > > + .vco_min = 350000000U, > > > + .vco_max = 1800000000U, > > > + .default_cpl = ADF4360_GEN1_PC_5, > > > + }, { /* ADF4360-8 */ > > > + .vco_min = 65000000U, > > > + .vco_max = 400000000U, > > > + .default_cpl = ADF4360_GEN2_PC_5, > > > + }, { /* ADF4360-9 */ > > > + .vco_min = 65000000U, > > > + .vco_max = 400000000U, > > > + .default_cpl = ADF4360_GEN2_PC_5, > > > + } > > > +}; > > > + > > > +static int adf4360_write_reg(struct adf4360_state *st, unsigned int reg, > > > + unsigned int val) > > > +{ > > > + int ret; > > > + > > > + val |= reg; > > > + > > > + st->spi_data[0] = (val >> 16) & 0xff; > > > + st->spi_data[1] = (val >> 8) & 0xff; > > > + st->spi_data[2] = val & 0xff; > > > + > > > + ret = spi_write(st->spi, st->spi_data, ARRAY_SIZE(st->spi_data)); > > > + if (ret == 0) > > > + st->regs[reg] = val; > > > + > > > + return ret; > > > +} > > > + > > > +/* fVCO = B * fREFIN / R */ > > > + > > > +static unsigned long adf4360_clk_recalc_rate(struct clk_hw *hw, > > > + unsigned long parent_rate) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + if (st->r == 0) > > > + return 0; > > > + > > > + /* > > > + * The result is guaranteed to fit in 32-bit, but the intermediate > > > + * result might require 64-bit. > > > + */ > > > + return DIV_ROUND_CLOSEST_ULL((uint64_t)parent_rate * st->n, st->r); > > > +} > > > + > > > +static unsigned int adf4360_calc_prescaler(unsigned int pfd_freq, > > > + unsigned int n, > > > + unsigned int *out_p, > > > + unsigned int *out_a, > > > + unsigned int *out_b) > > > +{ > > > + unsigned int rate = pfd_freq * n; > > > + unsigned int p, a, b; > > > + > > > + /* Make sure divider counter input frequency is low enough */ > > > + p = 8; > > > + while (p < 32 && rate / p > ADF4360_MAX_COUNTER_RATE) > > > + p *= 2; > > > + > > > + /* > > > + * The range of dividers that can be produced using the dual-modulus > > > + * pre-scaler is not continuous for values of n < p*(p-1). If we end up > > > + * with a non supported divider value, pick the next closest one. > > > + */ > > > + a = n % p; > > > + b = n / p; > > > + > > > + if (b < 3) { > > > + b = 3; > > > + a = 0; > > > + } else if (a > b) { > > > + if (a - b < p - a) { > > > + a = b; > > > + } else { > > > + a = 0; > > > + b++; > > > + } > > > + } > > > + > > > + if (out_p) > > > + *out_p = p; > > > + if (out_a) > > > + *out_a = a; > > > + if (out_b) > > > + *out_b = b; > > > + > > > + return p * b + a; > > > +} > > > + > > > +static long adf4360_clk_round_rate(struct clk_hw *hw, > > > + unsigned long rate, > > > + unsigned long *parent_rate) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned int r, n; > > > + unsigned int pfd_freq; > > > + > > > + if (*parent_rate == 0) > > > + return 0; > > > + > > > + if (st->part_id == ID_ADF4360_9) > > > + return *parent_rate * st->n / st->r; > > > + > > > + if (rate > st->vco_max) > > > + return st->vco_max; > > > + > > > + /* ADF4360-0 to AD4370-7 have an optional by two divider */ > > > + if (st->part_id <= ID_ADF4360_7) { > > > + if (rate < st->vco_min / 2) > > > + return st->vco_min / 2; > > > + if (rate < st->vco_min && rate > st->vco_max / 2) { > > > + if (st->vco_min - rate < rate - st->vco_max / 2) > > > + return st->vco_min; > > > + else > > > + return st->vco_max / 2; > > > + } > > > + } else { > > > + if (rate < st->vco_min) > > > + return st->vco_min; > > > + } > > > + > > > + r = DIV_ROUND_CLOSEST(*parent_rate, st->pfd_freq); > > > + pfd_freq = *parent_rate / r; > > > + n = DIV_ROUND_CLOSEST(rate, pfd_freq); > > > + > > > + if (st->part_id <= ID_ADF4360_7) > > > + n = adf4360_calc_prescaler(pfd_freq, n, NULL, NULL, NULL); > > > + > > > + return pfd_freq * n; > > > +} > > > + > > > +static inline bool adf4360_is_powerdown(struct adf4360_state *st) > > > +{ > > > + return (st->power_down_mode != ADF4360_POWER_DOWN_NORMAL); > > > +} > > > + > > > +static int adf4360_set_freq(struct adf4360_state *st, unsigned long rate) > > > +{ > > > + unsigned int val_r, val_n, val_ctrl; > > > + unsigned int pfd_freq; > > > + unsigned long r, n; > > > + int ret; > > > + > > > + if (!st->clkin_freq || (st->clkin_freq > ADF4360_MAX_REFIN_RATE)) > > > + return -EINVAL; > > > + > > > + if ((rate < st->vco_min) || (rate > st->vco_max)) > > > + return -EINVAL; > > > + > > > + if (adf4360_is_powerdown(st)) > > > + ret = -EBUSY; > > > + > > > + r = DIV_ROUND_CLOSEST(st->clkin_freq, st->pfd_freq); > > > + pfd_freq = st->clkin_freq / r; > > > + n = DIV_ROUND_CLOSEST(rate, pfd_freq); > > > + > > > + val_ctrl = ADF4360_CPL(st->info->default_cpl) | > > > + ADF4360_MUXOUT(st->muxout_mode) | > > > + ADF4360_PDP(!st->pdp) | > > > + ADF4360_MTLD(st->mtld) | > > > + ADF4360_OPL(st->power_level) | > > > + ADF4360_CPI1(st->cpi) | > > > + ADF4360_CPI2(st->cpi) | > > > + ADF4360_POWERDOWN(st->power_down_mode); > > > + > > > + /* ADF4360-0 to ADF4360-7 have a dual-modulous prescaler */ > > > + if (st->part_id <= ID_ADF4360_7) { > > > + unsigned int p, a, b; > > > + > > > + n = adf4360_calc_prescaler(pfd_freq, n, &p, &a, &b); > > > + > > > + switch (p) { > > > + case 8: > > > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_8); > > > + break; > > > + case 16: > > > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_16); > > > + break; > > > + default: > > > + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_32); > > > + break; > > > + } > > > + > > > + val_n = ADF4360_A_COUNTER(a) | > > > + ADF4360_B_COUNTER(b); > > > + > > > + if (rate < st->vco_min) > > > + val_n |= ADF4360_OUT_DIV2(true) | > > > + ADF4360_PRESCALER_DIV2(true); > > > + } else { > > > + val_n = ADF4360_B_COUNTER(n); > > > + } > > > + > > > + /* > > > + * Always use BSC divider of 8, see Analog Devices AN-1347. > > > + * > > > http://www.analog.com/media/en/technical-documentation/application-notes/AN-1347.pdf > > > + */ > > > + val_r = ADF4360_R_COUNTER(r) | > > > + ADF4360_ABP(ADF4360_ABP_3_0NS) | > > > + ADF4360_BSC(ADF4360_BSC_8); > > > + > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_RDIV), val_r); > > > + if (ret) > > > + return ret; > > > + > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val_ctrl); > > > + if (ret) > > > + return ret; > > > + > > > + /* > > > + * Allow the transient behavior of the ADF4360-7 during initial > > > + * power-up to settle. > > > + */ > > > + if (st->initial_reg_seq) { > > > + usleep_range(15000, 20000); > > > + st->initial_reg_seq = false; > > > + } > > > + > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_NDIV), val_n); > > > + if (ret) > > > + return ret; > > > + > > > + st->freq_req = rate; > > > + st->n = n; > > > + st->r = r; > > > + > > > + return 0; > > > +} > > > + > > > +static int adf4360_clk_set_rate(struct clk_hw *hw, > > > + unsigned long rate, > > > + unsigned long parent_rate) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + int ret; > > > + > > > + if ((parent_rate == 0) || (parent_rate > ADF4360_MAX_REFIN_RATE)) > > > + return -EINVAL; > > > + > > > + mutex_lock(&st->lock); > > > + if (st->clkin_freq != parent_rate) > > > + st->clkin_freq = parent_rate; > > > + > > > + ret = adf4360_set_freq(st, rate); > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static int __adf4360_power_down(struct adf4360_state *st, unsigned int > > > mode) > > > +{ > > > + struct device *dev = &st->spi->dev; > > > + unsigned int val; > > > + int ret = 0; > > > + > > > + switch (mode) { > > > + case ADF4360_POWER_DOWN_NORMAL: > > > + if (st->vdd_reg) { > > > + ret = regulator_enable(st->vdd_reg); > > > + if (ret) { > > > + dev_err(dev, "Supply enable error: %d\n", ret); > > > + return ret; > > > + } > > > + } > > > + > > > + st->initial_reg_seq = true; > > > + st->power_down_mode = mode; > > > + ret = adf4360_set_freq(st, st->freq_req); > > > + break; > > > + case ADF4360_POWER_DOWN_SOFT_ASYNC: /* fallthrough */ > > > + case ADF4360_POWER_DOWN_SOFT_SYNC: > > > + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > > > + val |= ADF4360_POWERDOWN(mode); > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > > + break; > > > + case ADF4360_POWER_DOWN_CE: > > > + if (st->chip_en_gpio) > > > + gpiod_set_value(st->chip_en_gpio, 0x0); > > > + else > > > + return -ENODEV; > > > + break; > > > + case ADF4360_POWER_DOWN_REGULATOR: > > > + if (!st->vdd_reg) > > > + return -ENODEV; > > > + > > > + if (st->chip_en_gpio) > > > + ret = __adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > > > + else > > > + ret = __adf4360_power_down(st, > > > + ADF4360_POWER_DOWN_SOFT_ASYNC); > > > + > > > + ret = regulator_disable(st->vdd_reg); > > > + if (ret) > > > + dev_err(dev, "Supply disable error: %d\n", ret); > > > + break; > > > + } > > > + if (ret == 0) > > > + st->power_down_mode = mode; > > > + > > > + return 0; > > > +} > > > + > > > +static int adf4360_power_down(struct adf4360_state *st, unsigned int mode) > > > +{ > > > + int ret; > > > + > > > + mutex_lock(&st->lock); > > > + ret = __adf4360_power_down(st, mode); > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static int adf4360_clk_prepare(struct clk_hw *hw) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + return adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); > > > +} > > > + > > > +static void adf4360_clk_unprepare(struct clk_hw *hw) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + adf4360_power_down(st, ADF4360_POWER_DOWN_SOFT_ASYNC); > > > +} > > > + > > > +static int adf4360_clk_enable(struct clk_hw *hw) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + if (st->chip_en_gpio) > > > + gpiod_set_value(st->chip_en_gpio, 0x1); > > > + > > > + return 0; > > > +} > > > + > > > +static void adf4360_clk_disable(struct clk_hw *hw) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + if (st->chip_en_gpio) > > > + adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > > > +} > > > + > > > +static int adf4360_clk_is_enabled(struct clk_hw *hw) > > > +{ > > > + struct iio_dev *indio_dev = to_output(hw)->indio_dev; > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + return adf4360_is_powerdown(st); > > > +} > > > + > > > +static const struct clk_ops adf4360_clk_ops = { > > > + .recalc_rate = adf4360_clk_recalc_rate, > > > + .round_rate = adf4360_clk_round_rate, > > > + .set_rate = adf4360_clk_set_rate, > > > + .prepare = adf4360_clk_prepare, > > > + .unprepare = adf4360_clk_unprepare, > > > + .enable = adf4360_clk_enable, > > > + .disable = adf4360_clk_disable, > > > + .is_enabled = adf4360_clk_is_enabled, > > > +}; > > > + > > > +static ssize_t adf4360_read(struct iio_dev *indio_dev, > > > + uintptr_t private, > > > + const struct iio_chan_spec *chan, > > > + char *buf) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned long val; > > > + int ret = 0; > > > + > > > + switch ((u32)private) { > > > + case ADF4360_FREQ_REFIN: > > > + val = st->clkin_freq; > > > + break; > > > + case ADF4360_MTLD: > > > + val = st->mtld; > > > + break; > > > + case ADF4360_FREQ_PFD: > > > + val = st->pfd_freq; > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + val = 0; > > > + } > > > + > > > + return ret < 0 ? ret : sprintf(buf, "%lu\n", val); > > > +} > > > + > > > +static ssize_t adf4360_write(struct iio_dev *indio_dev, > > > + uintptr_t private, > > > + const struct iio_chan_spec *chan, > > > + const char *buf, size_t len) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned long readin, tmp; > > > + bool mtld; > > > + int ret = 0; > > > + > > > + mutex_lock(&st->lock); > > > + switch ((u32)private) { > > > + case ADF4360_FREQ_REFIN: > > > + ret = kstrtoul(buf, 10, &readin); > > > + if (ret) > > > + break; > > > + > > > + if ((readin > ADF4360_MAX_REFIN_RATE) || (readin == 0)) { > > > + ret = -EINVAL; > > > + break; > > > + } > > > + > > > + if (st->clkin) { > > > + tmp = clk_round_rate(st->clkin, readin); > > > + if (tmp != readin) { > > > + ret = -EINVAL; > > > + break; > > > + } > > > + > > > + ret = clk_set_rate(st->clkin, tmp); > > > + if (ret) > > > + break; > > A bit odd to directly provide an interface to control and entirely different > > bit of hardware. If there are specific demands on the input clock as a > > result > > of something to do with the outputs, then fair enough. Direct tweaking like > > this seems like a very odd interface. > > > > > + } > > > + > > > + st->clkin_freq = readin; > > > + break; > > > + case ADF4360_MTLD: > > > + ret = kstrtobool(buf, &mtld); > > > + if (ret) > > > + break; > > > + > > > + st->mtld = mtld; > > > + break; > > > + case ADF4360_FREQ_PFD: > > > + ret = kstrtoul(buf, 10, &readin); > > > + if (ret) > > > + break; > > > + > > > + if ((readin > ADF4360_MAX_PFD_RATE) || (readin == 0)) { > > > + ret = -EINVAL; > > > + break; > > > + } > > > + > > > + st->pfd_freq = readin; > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + } > > > + > > > + if (ret == 0) > > > + ret = adf4360_set_freq(st, st->freq_req); > > > + mutex_unlock(&st->lock); > > > + > > > + return ret ? ret : len; > > > +} > > > + > > > +static int adf4360_get_muxout_mode(struct iio_dev *indio_dev, > > > + const struct iio_chan_spec *chan) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + return st->muxout_mode; > > > +} > > > + > > > +static int adf4360_set_muxout_mode(struct iio_dev *indio_dev, > > > + const struct iio_chan_spec *chan, > > > + unsigned int mode) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned int writeval; > > > + int ret = 0; > > > + > > > + mutex_lock(&st->lock); > > > + writeval = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > > > + writeval |= ADF4360_MUXOUT(mode & 0x7); > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), writeval); > > > + if (ret == 0) > > > + st->muxout_mode = mode & 0x7; > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static const struct iio_enum adf4360_muxout_modes_available = { > > > + .items = adf4360_muxout_modes, > > > + .num_items = ARRAY_SIZE(adf4360_muxout_modes), > > > + .get = adf4360_get_muxout_mode, > > > + .set = adf4360_set_muxout_mode, > > > +}; > > > + > > > +static int adf4360_get_power_down(struct iio_dev *indio_dev, > > > + const struct iio_chan_spec *chan) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + return st->power_down_mode; > > > +} > > > + > > > +static int adf4360_set_power_down(struct iio_dev *indio_dev, > > > + const struct iio_chan_spec *chan, > > > + unsigned int mode) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + return adf4360_power_down(st, mode); > > > +} > > > + > > > +static const struct iio_enum adf4360_pwr_dwn_modes_available = { > > > + .items = adf4360_power_down_modes, > > > + .num_items = ARRAY_SIZE(adf4360_power_down_modes), > > > + .get = adf4360_get_power_down, > > > + .set = adf4360_set_power_down, > > > +}; > > > + > > > +static int adf4360_get_power_level(struct iio_dev *indio_dev, > > > + const struct iio_chan_spec *chan) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + > > > + return st->power_level; > > > +} > > > + > > > +static int adf4360_set_power_level(struct iio_dev *indio_dev, > > > + const struct iio_chan_spec *chan, > > > + unsigned int level) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned int val; > > > + int ret; > > > + > > > + if (adf4360_is_powerdown(st)) > > > + return -EBUSY; > > > + > > > + mutex_lock(&st->lock); > > > + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_OPL_MSK; > > > + val |= ADF4360_OPL(level); > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > > + st->power_level = level; > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static const struct iio_enum adf4360_pwr_lvl_modes_available = { > > > + .items = adf4360_power_level_modes, > > > + .num_items = ARRAY_SIZE(adf4360_power_level_modes), > > > + .get = adf4360_get_power_level, > > > + .set = adf4360_set_power_level, > > > +}; > > > + > > > +#define _ADF4360_EXT_INFO(_name, _ident) { \ > > > + .name = _name, \ > > > + .read = adf4360_read, \ > > > + .write = adf4360_write, \ > > > + .private = _ident, \ > > > + .shared = IIO_SEPARATE, \ > > > +} > > > + > > > +static const struct iio_chan_spec_ext_info adf4360_ext_info[] = { > > > > This is a wide range of new ABI. These all need documentation > > in Documentation/ABI/testing/sysfs-bus-iio-* > > > > Without docs, it's hard to discuss if these are appropriate but a few initial > > comments... > > > + _ADF4360_EXT_INFO("refin_frequency", ADF4360_FREQ_REFIN), > > Looks like a control that should be matched to some clock source and > > not change at runtime? > > > > > + _ADF4360_EXT_INFO("mute_till_lock_detect", ADF4360_MTLD), > > > + _ADF4360_EXT_INFO("pfd_frequency", ADF4360_FREQ_PFD), > > > + IIO_ENUM_AVAILABLE("muxout_mode", &adf4360_muxout_modes_available), > > > + IIO_ENUM("muxout_mode", false, &adf4360_muxout_modes_available), > > > + IIO_ENUM_AVAILABLE("power_down", &adf4360_pwr_dwn_modes_available), > > > + IIO_ENUM("power_down", false, &adf4360_pwr_dwn_modes_available), > > > + IIO_ENUM_AVAILABLE("power_level", &adf4360_pwr_lvl_modes_available), > > > + IIO_ENUM("power_level", false, &adf4360_pwr_lvl_modes_available), > > > + { }, > > > +}; > > > + > > > +static const struct iio_chan_spec adf4360_chan = { > > > + .type = IIO_ALTVOLTAGE, > > > + .indexed = 1, > > > + .output = 1, > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY), > > > + .ext_info = adf4360_ext_info, > > > +}; > > > + > > > +static int adf4360_read_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int *val, > > > + int *val2, > > > + long mask) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + bool lk_det; > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_FREQUENCY: > > > + if (adf4360_is_powerdown(st)) > > > + return -EBUSY; > > > + > > > + lk_det = (ADF4360_MUXOUT_LOCK_DETECT | ADF4360_MUXOUT_OD_LD) & > > > + st->muxout_mode; > > > + if (lk_det && st->muxout_gpio) { > > > + if (!gpiod_get_value(st->muxout_gpio)) { > > > + dev_dbg(&st->spi->dev, "PLL un-locked\n"); > > > + return -EBUSY; > > > + } > > > + } > > > + > > > + *val = st->freq_req; > > > + return IIO_VAL_INT; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +}; > > > + > > > +static int adf4360_write_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int val, > > > + int val2, > > > + long mask) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + int ret; > > > + > > > + mutex_lock(&st->lock); > > > + switch (mask) { > > > + case IIO_CHAN_INFO_FREQUENCY: > > > + ret = adf4360_set_freq(st, val); > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + } > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static int adf4360_reg_access(struct iio_dev *indio_dev, > > > + unsigned int reg, > > > + unsigned int writeval, > > > + unsigned int *readval) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + int ret = 0; > > > + > > > + if (reg >= ADF4360_REG_NUM) > > > + return -EFAULT; > > > + > > > + mutex_lock(&st->lock); > > > + if (readval) { > > > + *readval = st->regs[reg]; > > > + } else { > > > + writeval &= 0xFFFFFC; > > > + ret = adf4360_write_reg(st, reg, writeval); > > > + } > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static const struct iio_info adf4360_iio_info = { > > > + .read_raw = &adf4360_read_raw, > > > + .write_raw = &adf4360_write_raw, > > > + .debugfs_reg_access = &adf4360_reg_access, > > > +}; > > > + > > > +static int adf4360_get_gpio(struct adf4360_state *st) > > > +{ > > > + struct device *dev = &st->spi->dev; > > > + unsigned int val; > > > + int ret, i; > > > + > > > + st->chip_en_gpio = devm_gpiod_get_optional(dev, "enable", > > > + GPIOD_OUT_HIGH); > > > + if (IS_ERR(st->chip_en_gpio)) { > > > + dev_err(dev, "Chip enable GPIO error\n"); > > > > Put handling in here to prevent an error message of DEFER. > > Same for the other routes where this might happen. > > > > > + return PTR_ERR(st->chip_en_gpio); > > > + } > > > + > > > + if (st->chip_en_gpio) > > > + st->power_down_mode = ADF4360_POWER_DOWN_CE; > > > + > > > + st->muxout_gpio = devm_gpiod_get_optional(dev, "adi,muxout", GPIOD_IN); > > > + if (IS_ERR(st->muxout_gpio)) { > > > + dev_err(dev, "Muxout GPIO error\n"); > > > + return PTR_ERR(st->muxout_gpio); > > > + } > > > + > > > + if (!st->muxout_gpio) > > > + return 0; > > > + > > > + /* ADF4360 PLLs are write only devices, try to probe using GPIO. */ > > > + for (i = 0; i < 4; i++) { > > > + if (i & 1) > > > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_DVDD); > > > + else > > > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_GND); > > > + > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > > + if (ret) > > > + return ret; > > > + > > > + ret = gpiod_get_value(st->muxout_gpio); > > > + if (ret ^ (i & 1)) { > > > + dev_err(dev, "Probe failed (muxout)"); > > > + return -ENODEV; > > > + } > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static void adf4360_clkin_disable(void *data) > > > +{ > > > + struct adf4360_state *st = data; > > > + > > > + clk_disable_unprepare(st->clkin); > > > +} > > > + > > > +static int adf4360_get_clkin(struct adf4360_state *st) > > > +{ > > > + struct device *dev = &st->spi->dev; > > > + struct clk *clk; > > > + int ret; > > > + > > > + clk = devm_clk_get(dev, "clkin"); > > > + if (IS_ERR(clk)) > > > + return PTR_ERR(clk); > > > + > > > + ret = clk_prepare_enable(clk); > > > + if (ret) > > > + return ret; > > > + > > > + ret = devm_add_action_or_reset(dev, adf4360_clkin_disable, st); > > > + if (ret) > > > + return ret; > > > + > > > + st->clkin = clk; > > > + st->clkin_freq = clk_get_rate(clk); > > > + > > > + return 0; > > > +} > > > + > > > +static void adf4360_clk_del_provider(void *data) > > > +{ > > > + struct adf4360_state *st = data; > > > + > > > + of_clk_del_provider(st->spi->dev.of_node); > > > +} > > > + > > > +static int adf4360_clk_register(struct adf4360_state *st) > > > +{ > > > > Hmm. This makes me wonder why this is an IIO driver rather than a clk > > driver? Definitely needs some more information in the patch description > > or a cover letter. > > > > > + struct spi_device *spi = st->spi; > > > + struct clk_init_data init; > > > + struct clk *clk; > > > + const char *parent_name; > > > + int ret; > > > + > > > + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0); > > > + if (!parent_name) > > > + return -EINVAL; > > > + > > > + init.name = st->clk_out_name; > > > + init.ops = &adf4360_clk_ops; > > > + init.flags = CLK_SET_RATE_GATE; > > > + init.parent_names = &parent_name; > > > + init.num_parents = 1; > > > + > > > + st->output.hw.init = &init; > > > + > > > + clk = devm_clk_register(&spi->dev, &st->output.hw); > > > + if (IS_ERR(clk)) > > > + return PTR_ERR(clk); > > > + > > > + ret = of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get, clk); > > > + if (ret) > > > + return ret; > > > + > > > + return devm_add_action_or_reset(&spi->dev, adf4360_clk_del_provider, > > > st); > > > +} > > > + > > > +static int adf4360_parse_dt(struct adf4360_state *st) > > > +{ > > > + struct device *dev = &st->spi->dev; > > > + u32 tmp; > > > + int ret; > > > + > > > + ret = device_property_read_string(dev, "clock-output-names", > > > + &st->clk_out_name); > > > + if ((ret < 0) && dev->of_node) > > > + st->clk_out_name = dev->of_node->name; > > > + > > > + if (st->part_id >= ID_ADF4360_7) { > > > + /* > > > + * ADF4360-7 to ADF4360-9 have a VCO that is tuned to a specific > > > + * range using an external inductor. These properties describe > > > + * the range selected by the external inductor. > > > + */ > > > + ret = device_property_read_u32(dev, > > > + "adi,vco-minimum-frequency-hz", > > > + &tmp); > > > + if (ret == 0) > > > + st->vco_min = max(st->info->vco_min, tmp); > > > + else > > > + st->vco_min = st->info->vco_min; > > > + > > > + ret = device_property_read_u32(dev, > > > + "adi,vco-maximum-frequency-hz", > > > + &tmp); > > > + if (ret == 0) > > > + st->vco_max = min(st->info->vco_max, tmp); > > > + else > > > + st->vco_max = st->info->vco_max; > > > + } else { > > > + st->vco_min = st->info->vco_min; > > > + st->vco_max = st->info->vco_max; > > > + } > > > + > > > + st->pdp = device_property_read_bool(dev, "adi,loop-filter-inverting"); > > > + > > > + ret = device_property_read_u32(dev, > > > + "adi,loop-filter-pfd-frequency-hz", > > > + &tmp); > > > + if (ret == 0) { > > > + st->pfd_freq = tmp; > > > + } else { > > > + dev_err(dev, "PFD frequency property missing\n"); > > > + return ret; > > > + } > > > + > > > + ret = device_property_read_u32(dev, > > > + "adi,loop-filter-charge-pump-current-microamp", > > > + &tmp); > > > + if (ret == 0) { > > > + st->cpi = find_closest(tmp, adf4360_cpi_modes, > > > + ARRAY_SIZE(adf4360_cpi_modes)); > > > + } else { > > > + dev_err(dev, "CPI property missing\n"); > > > + return ret; > > > + } > > > + > > > + ret = device_property_read_u32(dev, "adi,power-up-frequency-hz", &tmp); > > > + if (ret == 0) > > > + st->freq_req = tmp; > > > + else > > > + st->freq_req = st->vco_min; > > > + > > > + ret = device_property_read_u32(dev, "adi,power-out-level-microamp", > > > + &tmp); > > > + if (ret == 0) > > > + st->power_level = find_closest(tmp, adf4360_power_lvl_microamp, > > > + ARRAY_SIZE(adf4360_power_lvl_microamp)); > > > + else > > > + st->power_level = ADF4360_PL_5; > > > + > > > + return 0; > > > +} > > > + > > > +static int adf4360_probe(struct spi_device *spi) > > > +{ > > > + struct iio_dev *indio_dev; > > > + const struct spi_device_id *id = spi_get_device_id(spi); > > > > Given we require various dt parameters to be present, might as well > > associate the id with the of_ structures instead and use the dt > > calls throughout. Even better if you use the firmware type independent > > versions. > > > > > + struct adf4360_state *st; > > > + int ret; > > > + > > > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > > > + if (!indio_dev) > > > + return -ENOMEM; > > > + > > > + st = iio_priv(indio_dev); > > > + > > > + mutex_init(&st->lock); > > > + > > > + spi_set_drvdata(spi, indio_dev); > > > + > > > + st->spi = spi; > > > + st->info = &adf4360_chip_info_tbl[id->driver_data]; > > > + st->part_id = id->driver_data; > > > + st->muxout_mode = ADF4360_MUXOUT_LOCK_DETECT; > > > + st->mtld = true; > > > + > > > + ret = adf4360_parse_dt(st); > > > + if (ret) { > > > + dev_err(&spi->dev, "Parsing properties failed (%d)\n", ret); > > > + return -ENODEV; > > > + } > > > + > > > + indio_dev->dev.parent = &spi->dev; > > > + > > > + if (spi->dev.of_node) > > > + indio_dev->name = spi->dev.of_node->name; > > > + else > > > + indio_dev->name = spi_get_device_id(spi)->name; > > > + > > > + indio_dev->info = &adf4360_iio_info; > > > + indio_dev->modes = INDIO_DIRECT_MODE; > > > + indio_dev->channels = &adf4360_chan; > > > + indio_dev->num_channels = 1; > > > + st->output.indio_dev = indio_dev; > > > + > > > + ret = adf4360_get_gpio(st); > > > + if (ret) > > > + return ret; > > > + > > > + ret = adf4360_get_clkin(st); > > > + if (ret) > > > + return ret; > > > + > > > + st->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd"); > > > + if (IS_ERR(st->vdd_reg)) { > > > + if (PTR_ERR(st->vdd_reg) != -ENODEV) { > > > + dev_err(&spi->dev, "Regulator error\n"); > > > + return PTR_ERR(st->vdd_reg); > > > + } > > > + > > > + st->vdd_reg = NULL; > > > + } > > > + > > > + ret = adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); > > > + if (ret) > > > + return ret; > > > + > > > + ret = adf4360_clk_register(st); > > > + if (ret) > > > + return ret; > > > + > > > + return devm_iio_device_register(&spi->dev, indio_dev); > > > +} > > > + > > > +static const struct of_device_id adf4360_of_match[] = { > > > + { .compatible = "adi,adf4360-0", }, > > > + { .compatible = "adi,adf4360-1", }, > > > + { .compatible = "adi,adf4360-2", }, > > > + { .compatible = "adi,adf4360-3", }, > > > + { .compatible = "adi,adf4360-4", }, > > > + { .compatible = "adi,adf4360-5", }, > > > + { .compatible = "adi,adf4360-6", }, > > > + { .compatible = "adi,adf4360-7", }, > > > + { .compatible = "adi,adf4360-8", }, > > > + { .compatible = "adi,adf4360-9", }, > > > + {}, > > > +}; > > > +MODULE_DEVICE_TABLE(of, adf4360_of_match); > > > + > > > +static const struct spi_device_id adf4360_id[] = { > > > > As mentioned above, you can't actually probe this device > > without a pile of dt stuff. So this fallback doesn't > > make much sense. Use the data field in the of table > > above and get rid of this table entirely. > > > > > + {"adf4360-0", ID_ADF4360_0}, > > > + {"adf4360-1", ID_ADF4360_1}, > > > + {"adf4360-2", ID_ADF4360_2}, > > > + {"adf4360-3", ID_ADF4360_3}, > > > + {"adf4360-4", ID_ADF4360_4}, > > > + {"adf4360-5", ID_ADF4360_5}, > > > + {"adf4360-6", ID_ADF4360_6}, > > > + {"adf4360-7", ID_ADF4360_7}, > > > + {"adf4360-8", ID_ADF4360_8}, > > > + {"adf4360-9", ID_ADF4360_9}, > > > + {} > > > +}; > > > + > > > +static struct spi_driver adf4360_driver = { > > > + .driver = { > > > + .name = "adf4360", > > > + .of_match_table = adf4360_of_match, > > > + .owner = THIS_MODULE, > > > > It's a long time since we had to set the .owner field for each driver. > > > > Follow through what happens in module_spi_driver, spi_register_driver > > etc and you'll find it's set automatically during driver registration. > > > > > + }, > > > + .probe = adf4360_probe, > > > + .id_table = adf4360_id, > > > +}; > > > +module_spi_driver(adf4360_driver); > > > + > > > +MODULE_LICENSE("GPL v2"); > > > +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); > > > +MODULE_AUTHOR("Edward Kigwana <ekigwana@gmail.com>"); > > > +MODULE_DESCRIPTION("Analog Devices ADF4360 PLL");
On Mon, Feb 3, 2020 at 1:47 PM Ardelean, Alexandru <alexandru.Ardelean@analog.com> wrote: > On Sun, 2020-02-02 at 14:45 +0000, Jonathan Cameron wrote: > > On Tue, 28 Jan 2020 13:13:00 +0200 > > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: Just few comments on the code in case either this will go, or to teach an author about best practices in LK. > > > +#include <linux/err.h> > > > +#include <linux/gpio/consumer.h> > > > +#include <linux/kernel.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/regulator/consumer.h> > > > +#include <linux/spi/spi.h> > > > +#include <linux/util_macros.h> ... > > > +enum { > > > + ADF4360_CTRL, > > > + ADF4360_RDIV, > > > + ADF4360_NDIV, > > > + ADF4360_REG_NUM, Sounds line no need for comma (is it indeed a terminator line?). > > > +}; ... > > > +static int adf4360_write_reg(struct adf4360_state *st, unsigned int reg, > > > + unsigned int val) > > > +{ > > > + int ret; > > > + > > > + val |= reg; This is dangerous. Shouldn't be some mask applied? > > > + st->spi_data[0] = (val >> 16) & 0xff; > > > + st->spi_data[1] = (val >> 8) & 0xff; > > > + st->spi_data[2] = val & 0xff; All ' & 0xff' are redundant. > > > + ret = spi_write(st->spi, st->spi_data, ARRAY_SIZE(st->spi_data)); > > > + if (ret == 0) > > > + st->regs[reg] = val; > > > + > > > + return ret; Please, use pattern: if (ret) return ret; ... return 0; > > > +} ... > > > +static unsigned int adf4360_calc_prescaler(unsigned int pfd_freq, > > > + unsigned int n, > > > + unsigned int *out_p, > > > + unsigned int *out_a, > > > + unsigned int *out_b) > > > +{ > > > + unsigned int rate = pfd_freq * n; > > > + unsigned int p, a, b; > > > + > > > + /* Make sure divider counter input frequency is low enough */ > > > + p = 8; > > > + while (p < 32 && rate / p > ADF4360_MAX_COUNTER_RATE) > > > + p *= 2; > > > + > > > + /* > > > + * The range of dividers that can be produced using the dual-modulus > > > + * pre-scaler is not continuous for values of n < p*(p-1). If we end up > > > + * with a non supported divider value, pick the next closest one. > > > + */ > > > + a = n % p; > > > + b = n / p; You may avoid divisions if you use shifts. Currently it's a bit hard to compiler to prove that p is power of 2. > > > + if (b < 3) { > > > + b = 3; > > > + a = 0; > > > + } else if (a > b) { Does this guarantee p >= a? > > > + if (a - b < p - a) { > > > + a = b; > > > + } else { > > > + a = 0; > > > + b++; > > > + } > > > + } I guess above conditional tree can be a bit improved, although I don't see right now in what way. > > > + return p * b + a; > > > +} ... > > > + /* ADF4360-0 to AD4370-7 have an optional by two divider */ > > > + if (st->part_id <= ID_ADF4360_7) { > > > + if (rate < st->vco_min / 2) > > > + return st->vco_min / 2; > > > + if (rate < st->vco_min && rate > st->vco_max / 2) { > > > + if (st->vco_min - rate < rate - st->vco_max / 2) > > > + return st->vco_min; > > > + else Redundant. > > > + return st->vco_max / 2; > > > + } > > > + } else { > > > + if (rate < st->vco_min) } else if (...) { > > > + return st->vco_min; > > > + } ... > > > + case ADF4360_POWER_DOWN_REGULATOR: > > > + if (!st->vdd_reg) > > > + return -ENODEV; > > > + > > > + if (st->chip_en_gpio) > > > + ret = __adf4360_power_down(st, ADF4360_POWER_DOWN_CE); > > > + else > > > + ret = __adf4360_power_down(st, > > > + ADF4360_POWER_DOWN_SOFT_ASYNC); Missed error check. > > > + > > > + ret = regulator_disable(st->vdd_reg); > > > + if (ret) > > > + dev_err(dev, "Supply disable error: %d\n", ret); > > > + break; > > > + } ... > > > + if (ret == 0) > > > + st->power_down_mode = mode; > > > + > > > + return 0; Looks like 'return ret;' but see one comment at the beginning of the letter. ... > > > +static ssize_t adf4360_read(struct iio_dev *indio_dev, > > > + uintptr_t private, > > > + const struct iio_chan_spec *chan, > > > + char *buf) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned long val; > > > + int ret = 0; Redundant variable. > > > + > > > + switch ((u32)private) { > > > + case ADF4360_FREQ_REFIN: > > > + val = st->clkin_freq; > > > + break; > > > + case ADF4360_MTLD: > > > + val = st->mtld; > > > + break; > > > + case ADF4360_FREQ_PFD: > > > + val = st->pfd_freq; > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + val = 0; > > > + } > > > + > > > + return ret < 0 ? ret : sprintf(buf, "%lu\n", val); > > > +} ... > > > +static ssize_t adf4360_write(struct iio_dev *indio_dev, > > > + uintptr_t private, > > > + const struct iio_chan_spec *chan, > > > + const char *buf, size_t len) > > > +{ > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + unsigned long readin, tmp; > > > + bool mtld; > > > + int ret = 0; > > > + > > > + mutex_lock(&st->lock); > > > + switch ((u32)private) { Strange casting. Why? > > > + if ((readin > ADF4360_MAX_REFIN_RATE) || (readin == 0)) { Too many parentheses. > > > + ret = -EINVAL; > > > + break; > > > + } > > > + > > > + if (st->clkin) { > > > + tmp = clk_round_rate(st->clkin, readin); > > > + if (tmp != readin) { > > > + ret = -EINVAL; > > > + break; > > > + } > > > + > > > + ret = clk_set_rate(st->clkin, tmp); > > > + if (ret) > > > + break; > > A bit odd to directly provide an interface to control and entirely different > > bit of hardware. If there are specific demands on the input clock as a > > result > > of something to do with the outputs, then fair enough. Direct tweaking like > > this seems like a very odd interface. > > > > > + } > > > + > > > + st->clkin_freq = readin; > > > + break; > > > + case ADF4360_FREQ_PFD: > > > + ret = kstrtoul(buf, 10, &readin); > > > + if (ret) > > > + break; > > > + > > > + if ((readin > ADF4360_MAX_PFD_RATE) || (readin == 0)) { Ditto. > > > + ret = -EINVAL; > > > + break; > > > + } > > > + > > > + st->pfd_freq = readin; > > > + break; > > > + default: > > > + ret = -EINVAL; Nevertheless 'break;' is good to have even here. > > > + } > > > + > > > + if (ret == 0) Maybe this, or maybe if (ret) goto out_unlock; > > > + ret = adf4360_set_freq(st, st->freq_req); > > > + mutex_unlock(&st->lock); > > > + > > > + return ret ? ret : len; > > > +} ... > > > + int ret = 0; Redundant assignment. > > > + mutex_lock(&st->lock); > > > + writeval = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; > > > + writeval |= ADF4360_MUXOUT(mode & 0x7); > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), writeval); > > > + if (ret == 0) > > > + st->muxout_mode = mode & 0x7; > > > + mutex_unlock(&st->lock); Similar comment to the return check style as above. > > > + return ret; > > > +} ... > > > +static const struct iio_chan_spec_ext_info adf4360_ext_info[] = { > > > + IIO_ENUM("power_level", false, &adf4360_pwr_lvl_modes_available), > > > + { }, No need to have comma for terminator line. > > > +}; ... > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + bool lk_det; > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_FREQUENCY: > > > + if (adf4360_is_powerdown(st)) > > > + return -EBUSY; > > > + > > > + lk_det = (ADF4360_MUXOUT_LOCK_DETECT | ADF4360_MUXOUT_OD_LD) & > > > + st->muxout_mode; > > > + if (lk_det && st->muxout_gpio) { > > > + if (!gpiod_get_value(st->muxout_gpio)) { > > > + dev_dbg(&st->spi->dev, "PLL un-locked\n"); > > > + return -EBUSY; > > > + } > > > + } > > > + > > > + *val = st->freq_req; > > > + return IIO_VAL_INT; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return 0; How this is possible? ... > > > + default: > > > + ret = -EINVAL; break; ... > > > + struct adf4360_state *st = iio_priv(indio_dev); > > > + int ret = 0; Would be better to have this assignment... > > > + > > > + if (reg >= ADF4360_REG_NUM) > > > + return -EFAULT; > > > + > > > + mutex_lock(&st->lock); > > > + if (readval) { > > > + *readval = st->regs[reg]; ...here. > > > + } else { > > > + writeval &= 0xFFFFFC; What this magic does? Make a define using GENMASK(). > > > + ret = adf4360_write_reg(st, reg, writeval); > > > + } > > > + mutex_unlock(&st->lock); > > > + > > > + return ret; > > > +} ... > > > + /* ADF4360 PLLs are write only devices, try to probe using GPIO. */ > > > + for (i = 0; i < 4; i++) { > > > + if (i & 1) > > > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_DVDD); > > > + else > > > + val = ADF4360_MUXOUT(ADF4360_MUXOUT_GND); > > > + > > > + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); > > > + if (ret) > > > + return ret; > > > + > > > + ret = gpiod_get_value(st->muxout_gpio); > > > + if (ret ^ (i & 1)) { I guess == or != is better than ^ here. > > > + dev_err(dev, "Probe failed (muxout)"); > > > + return -ENODEV; > > > + } > > > + } > > > + > > > + return 0; > > > +} ... > > Hmm. This makes me wonder why this is an IIO driver rather than a clk > > driver? Definitely needs some more information in the patch description > > or a cover letter. +1. ... > > > + ret = device_property_read_string(dev, "clock-output-names", > > > + &st->clk_out_name); Your driver is OF dependent, why to bother with device property API? > > > + if ((ret < 0) && dev->of_node) Oh, this is bad. Why do you need to check for OF node at all?! > > > + st->clk_out_name = dev->of_node->name; ... > > > + /* > > > + * ADF4360-7 to ADF4360-9 have a VCO that is tuned to a specific > > > + * range using an external inductor. These properties describe > > > + * the range selected by the external inductor. > > > + */ > > > + ret = device_property_read_u32(dev, > > > + "adi,vco-minimum-frequency-hz", > > > + &tmp); > > > + if (ret == 0) > > > + st->vco_min = max(st->info->vco_min, tmp); > > > + else > > > + st->vco_min = st->info->vco_min; Why to use tmp at all? &st->vco_min); if (ret) st->vco_min = st->info->vco_min; st->vco_min = max(st->info->vco_min, st->vco_min); > > > + ret = device_property_read_u32(dev, > > > + "adi,vco-maximum-frequency-hz", > > > + &tmp); > > > + if (ret == 0) > > > + st->vco_max = min(st->info->vco_max, tmp); > > > + else > > > + st->vco_max = st->info->vco_max; Ditto. > > > + ret = device_property_read_u32(dev, > > > + "adi,loop-filter-pfd-frequency-hz", > > > + &tmp); > > > + if (ret == 0) { > > > + st->pfd_freq = tmp; Ditto! > > > + } else { > > > + dev_err(dev, "PFD frequency property missing\n"); > > > + return ret; > > > + } > > > + > > > + ret = device_property_read_u32(dev, > > > + "adi,loop-filter-charge-pump-current-microamp", > > > + &tmp); > > > + if (ret == 0) { > > > + st->cpi = find_closest(tmp, adf4360_cpi_modes, > > > + ARRAY_SIZE(adf4360_cpi_modes)); Ditto! > > > + } else { > > > + dev_err(dev, "CPI property missing\n"); > > > + return ret; > > > + } > > > + > > > + ret = device_property_read_u32(dev, "adi,power-up-frequency-hz", &tmp); > > > + if (ret == 0) > > > + st->freq_req = tmp; > > > + else > > > + st->freq_req = st->vco_min; Ditto. > > > + ret = device_property_read_u32(dev, "adi,power-out-level-microamp", > > > + &tmp); > > > + if (ret == 0) > > > + st->power_level = find_closest(tmp, adf4360_power_lvl_microamp, > > > + ARRAY_SIZE(adf4360_power_lvl_microamp)); > > > + else > > > + st->power_level = ADF4360_PL_5; Ditto. ... > > > + if (spi->dev.of_node) > > > + indio_dev->name = spi->dev.of_node->name; Use %pOFn instead > > > + else > > > + indio_dev->name = spi_get_device_id(spi)->name; ... > > > +static const struct of_device_id adf4360_of_match[] = { > > > + { .compatible = "adi,adf4360-0", }, > > > + { .compatible = "adi,adf4360-1", }, > > > + { .compatible = "adi,adf4360-2", }, > > > + { .compatible = "adi,adf4360-3", }, > > > + { .compatible = "adi,adf4360-4", }, > > > + { .compatible = "adi,adf4360-5", }, > > > + { .compatible = "adi,adf4360-6", }, > > > + { .compatible = "adi,adf4360-7", }, > > > + { .compatible = "adi,adf4360-8", }, > > > + { .compatible = "adi,adf4360-9", }, > > > + {}, No comma here. > > > +};
diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig index 240b81502512..781236496661 100644 --- a/drivers/iio/frequency/Kconfig +++ b/drivers/iio/frequency/Kconfig @@ -39,6 +39,17 @@ config ADF4350 To compile this driver as a module, choose M here: the module will be called adf4350. +config ADF4360 + tristate "Analog Devices ADF4360 Wideband Synthesizers" + depends on SPI + depends on COMMON_CLK + help + Say yes here to build support for Analog Devices ADF4360 + Wideband Synthesizers. The driver provides direct access via sysfs. + + To compile this driver as a module, choose M here: the + module will be called adf4360. + config ADF4371 tristate "Analog Devices ADF4371/ADF4372 Wideband Synthesizers" depends on SPI diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile index 518b1e50caef..85f2f81da662 100644 --- a/drivers/iio/frequency/Makefile +++ b/drivers/iio/frequency/Makefile @@ -6,4 +6,5 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_AD9523) += ad9523.o obj-$(CONFIG_ADF4350) += adf4350.o +obj-$(CONFIG_ADF4360) += adf4360.o obj-$(CONFIG_ADF4371) += adf4371.o diff --git a/drivers/iio/frequency/adf4360.c b/drivers/iio/frequency/adf4360.c new file mode 100644 index 000000000000..49ad58092171 --- /dev/null +++ b/drivers/iio/frequency/adf4360.c @@ -0,0 +1,1263 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ADF4360 PLL with Integrated Synthesizer and VCO + * + * Copyright 2014-2019 Analog Devices Inc. + * Copyright 2019-2020 Edward Kigwana. + */ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> +#include <linux/util_macros.h> + +#include <linux/iio/iio.h> + +/* Register address macro */ +#define ADF4360_REG(x) (x) + +/* ADF4360_CTRL */ +#define ADF4360_ADDR_CPL_MSK GENMASK(3, 2) +#define ADF4360_CPL(x) FIELD_PREP(ADF4360_ADDR_CPL_MSK, x) +#define ADF4360_ADDR_MUXOUT_MSK GENMASK(7, 5) +#define ADF4360_MUXOUT(x) FIELD_PREP(ADF4360_ADDR_MUXOUT_MSK, x) +#define ADF4360_ADDR_PDP_MSK BIT(8) +#define ADF4360_PDP(x) FIELD_PREP(ADF4360_ADDR_PDP_MSK, x) +#define ADF4360_ADDR_MTLD_MSK BIT(11) +#define ADF4360_MTLD(x) FIELD_PREP(ADF4360_ADDR_MTLD_MSK, x) +#define ADF4360_ADDR_OPL_MSK GENMASK(13, 12) +#define ADF4360_OPL(x) FIELD_PREP(ADF4360_ADDR_OPL_MSK, x) +#define ADF4360_ADDR_CPI1_MSK GENMASK(16, 14) +#define ADF4360_CPI1(x) FIELD_PREP(ADF4360_ADDR_CPI1_MSK, x) +#define ADF4360_ADDR_CPI2_MSK GENMASK(19, 17) +#define ADF4360_CPI2(x) FIELD_PREP(ADF4360_ADDR_CPI2_MSK, x) +#define ADF4360_ADDR_PWR_DWN_MSK GENMASK(21, 20) +#define ADF4360_POWERDOWN(x) FIELD_PREP(ADF4360_ADDR_PWR_DWN_MSK, x) +#define ADF4360_ADDR_PRESCALER_MSK GENMASK(23, 22) +#define ADF4360_PRESCALER(x) FIELD_PREP(ADF4360_ADDR_PRESCALER_MSK, x) + +/* ADF4360_NDIV */ +#define ADF4360_ADDR_A_CNTR_MSK GENMASK(6, 2) +#define ADF4360_A_COUNTER(x) FIELD_PREP(ADF4360_ADDR_A_CNTR_MSK, x) +#define ADF4360_ADDR_B_CNTR_MSK GENMASK(20, 8) +#define ADF4360_B_COUNTER(x) FIELD_PREP(ADF4360_ADDR_B_CNTR_MSK, x) +#define ADF4360_ADDR_OUT_DIV2_MSK BIT(22) +#define ADF4360_OUT_DIV2(x) FIELD_PREP(ADF4360_ADDR_OUT_DIV2_MSK, x) +#define ADF4360_ADDR_DIV2_SEL_MSK BIT(23) +#define ADF4360_PRESCALER_DIV2(x) FIELD_PREP(ADF4360_ADDR_DIV2_SEL_MSK, x) + +/* ADF4360_RDIV */ +#define ADF4360_ADDR_R_CNTR_MSK GENMASK(15, 2) +#define ADF4360_R_COUNTER(x) FIELD_PREP(ADF4360_ADDR_R_CNTR_MSK, x) +#define ADF4360_ADDR_ABP_MSK GENMASK(17, 16) +#define ADF4360_ABP(x) FIELD_PREP(ADF4360_ADDR_ABP_MSK, x) +#define ADF4360_ADDR_BSC_MSK GENMASK(21, 20) +#define ADF4360_BSC(x) FIELD_PREP(ADF4360_ADDR_BSC_MSK, x) + +/* Specifications */ +#define ADF4360_MAX_PFD_RATE 8000000 /* 8 MHz */ +#define ADF4360_MAX_COUNTER_RATE 300000000 /* 300 MHz */ +#define ADF4360_MAX_REFIN_RATE 250000000 /* 250 MHz */ + +enum { + ADF4360_CTRL, + ADF4360_RDIV, + ADF4360_NDIV, + ADF4360_REG_NUM, +}; + +enum { + ADF4360_GEN1_PC_5, + ADF4360_GEN1_PC_10, + ADF4360_GEN1_PC_15, + ADF4360_GEN1_PC_20, +}; + +enum { + ADF4360_GEN2_PC_2_5, + ADF4360_GEN2_PC_5, + ADF4360_GEN2_PC_7_5, + ADF4360_GEN2_PC_10, +}; + +enum { + ADF4360_MUXOUT_THREE_STATE, + ADF4360_MUXOUT_LOCK_DETECT, + ADF4360_MUXOUT_NDIV, + ADF4360_MUXOUT_DVDD, + ADF4360_MUXOUT_RDIV, + ADF4360_MUXOUT_OD_LD, + ADF4360_MUXOUT_SDO, + ADF4360_MUXOUT_GND, +}; + +enum { + ADF4360_PL_3_5, + ADF4360_PL_5, + ADF4360_PL_7_5, + ADF4360_PL_11, +}; + +enum { + ADF4360_CPI_0_31, + ADF4360_CPI_0_62, + ADF4360_CPI_0_93, + ADF4360_CPI_1_25, + ADF4360_CPI_1_56, + ADF4360_CPI_1_87, + ADF4360_CPI_2_18, + ADF4360_CPI_2_50, +}; + +enum { + ADF4360_POWER_DOWN_NORMAL, + ADF4360_POWER_DOWN_SOFT_ASYNC, + ADF4360_POWER_DOWN_CE, + ADF4360_POWER_DOWN_SOFT_SYNC, + ADF4360_POWER_DOWN_REGULATOR, +}; + +enum { + ADF4360_PRESCALER_8, + ADF4360_PRESCALER_16, + ADF4360_PRESCALER_32, +}; + +enum { + ADF4360_ABP_3_0NS, + ADF4360_ABP_1_3NS, + ADF4360_ABP_6_0NS, +}; + +enum { + ADF4360_BSC_1, + ADF4360_BSC_2, + ADF4360_BSC_4, + ADF4360_BSC_8, +}; + +enum { + ID_ADF4360_0, + ID_ADF4360_1, + ID_ADF4360_2, + ID_ADF4360_3, + ID_ADF4360_4, + ID_ADF4360_5, + ID_ADF4360_6, + ID_ADF4360_7, + ID_ADF4360_8, + ID_ADF4360_9, +}; + +enum { + ADF4360_FREQ_REFIN, + ADF4360_MTLD, + ADF4360_FREQ_PFD, +}; + +static const char * const adf4360_power_level_modes[] = { + [ADF4360_PL_3_5] = "3500-uA", + [ADF4360_PL_5] = "5000-uA", + [ADF4360_PL_7_5] = "7500-uA", + [ADF4360_PL_11] = "11000-uA", +}; + +static const unsigned int adf4360_power_lvl_microamp[] = { + [ADF4360_PL_3_5] = 3500, + [ADF4360_PL_5] = 5000, + [ADF4360_PL_7_5] = 7500, + [ADF4360_PL_11] = 11000, +}; + +static const unsigned int adf4360_cpi_modes[] = { + [ADF4360_CPI_0_31] = 310, + [ADF4360_CPI_0_62] = 620, + [ADF4360_CPI_0_93] = 930, + [ADF4360_CPI_1_25] = 1250, + [ADF4360_CPI_1_56] = 1560, + [ADF4360_CPI_1_87] = 1870, + [ADF4360_CPI_2_18] = 2180, + [ADF4360_CPI_2_50] = 2500, +}; + +static const char * const adf4360_muxout_modes[] = { + [ADF4360_MUXOUT_THREE_STATE] = "three-state", + [ADF4360_MUXOUT_LOCK_DETECT] = "lock-detect", + [ADF4360_MUXOUT_NDIV] = "ndiv", + [ADF4360_MUXOUT_DVDD] = "dvdd", + [ADF4360_MUXOUT_RDIV] = "rdiv", + [ADF4360_MUXOUT_OD_LD] = "od-ld", + [ADF4360_MUXOUT_SDO] = "sdo", + [ADF4360_MUXOUT_GND] = "gnd", +}; + +static const char * const adf4360_power_down_modes[] = { + [ADF4360_POWER_DOWN_NORMAL] = "normal", + [ADF4360_POWER_DOWN_SOFT_ASYNC] = "soft-async", + [ADF4360_POWER_DOWN_CE] = "ce", + [ADF4360_POWER_DOWN_SOFT_SYNC] = "soft-sync", + [ADF4360_POWER_DOWN_REGULATOR] = "regulator", +}; + +struct adf4360_output { + struct clk_hw hw; + struct iio_dev *indio_dev; +}; + +#define to_output(_hw) container_of(_hw, struct adf4360_output, hw) + +struct adf4360_chip_info { + unsigned int vco_min; + unsigned int vco_max; + unsigned int default_cpl; +}; + +struct adf4360_state { + struct spi_device *spi; + const struct adf4360_chip_info *info; + struct adf4360_output output; + struct clk *clkin; + struct gpio_desc *muxout_gpio; + struct gpio_desc *chip_en_gpio; + struct regulator *vdd_reg; + struct mutex lock; /* Protect PLL state. */ + unsigned int part_id; + unsigned long clkin_freq; + unsigned long freq_req; + unsigned long r; + unsigned long n; + unsigned int vco_min; + unsigned int vco_max; + unsigned int pfd_freq; + unsigned int cpi; + bool pdp; + bool mtld; + unsigned int power_level; + unsigned int muxout_mode; + unsigned int power_down_mode; + bool initial_reg_seq; + const char *clk_out_name; + unsigned int regs[ADF4360_REG_NUM]; + u8 spi_data[3] ____cacheline_aligned; +}; + +static const struct adf4360_chip_info adf4360_chip_info_tbl[] = { + { /* ADF4360-0 */ + .vco_min = 2400000000U, + .vco_max = 2725000000U, + .default_cpl = ADF4360_GEN1_PC_10, + }, { /* ADF4360-1 */ + .vco_min = 2050000000U, + .vco_max = 2450000000U, + .default_cpl = ADF4360_GEN1_PC_15, + }, { /* ADF4360-2 */ + .vco_min = 1850000000U, + .vco_max = 2170000000U, + .default_cpl = ADF4360_GEN1_PC_15, + }, { /* ADF4360-3 */ + .vco_min = 1600000000U, + .vco_max = 1950000000U, + .default_cpl = ADF4360_GEN1_PC_15, + }, { /* ADF4360-4 */ + .vco_min = 1450000000U, + .vco_max = 1750000000U, + .default_cpl = ADF4360_GEN1_PC_15, + }, { /* ADF4360-5 */ + .vco_min = 1200000000U, + .vco_max = 1400000000U, + .default_cpl = ADF4360_GEN1_PC_10, + }, { /* ADF4360-6 */ + .vco_min = 1050000000U, + .vco_max = 1250000000U, + .default_cpl = ADF4360_GEN1_PC_10, + }, { /* ADF4360-7 */ + .vco_min = 350000000U, + .vco_max = 1800000000U, + .default_cpl = ADF4360_GEN1_PC_5, + }, { /* ADF4360-8 */ + .vco_min = 65000000U, + .vco_max = 400000000U, + .default_cpl = ADF4360_GEN2_PC_5, + }, { /* ADF4360-9 */ + .vco_min = 65000000U, + .vco_max = 400000000U, + .default_cpl = ADF4360_GEN2_PC_5, + } +}; + +static int adf4360_write_reg(struct adf4360_state *st, unsigned int reg, + unsigned int val) +{ + int ret; + + val |= reg; + + st->spi_data[0] = (val >> 16) & 0xff; + st->spi_data[1] = (val >> 8) & 0xff; + st->spi_data[2] = val & 0xff; + + ret = spi_write(st->spi, st->spi_data, ARRAY_SIZE(st->spi_data)); + if (ret == 0) + st->regs[reg] = val; + + return ret; +} + +/* fVCO = B * fREFIN / R */ + +static unsigned long adf4360_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + + if (st->r == 0) + return 0; + + /* + * The result is guaranteed to fit in 32-bit, but the intermediate + * result might require 64-bit. + */ + return DIV_ROUND_CLOSEST_ULL((uint64_t)parent_rate * st->n, st->r); +} + +static unsigned int adf4360_calc_prescaler(unsigned int pfd_freq, + unsigned int n, + unsigned int *out_p, + unsigned int *out_a, + unsigned int *out_b) +{ + unsigned int rate = pfd_freq * n; + unsigned int p, a, b; + + /* Make sure divider counter input frequency is low enough */ + p = 8; + while (p < 32 && rate / p > ADF4360_MAX_COUNTER_RATE) + p *= 2; + + /* + * The range of dividers that can be produced using the dual-modulus + * pre-scaler is not continuous for values of n < p*(p-1). If we end up + * with a non supported divider value, pick the next closest one. + */ + a = n % p; + b = n / p; + + if (b < 3) { + b = 3; + a = 0; + } else if (a > b) { + if (a - b < p - a) { + a = b; + } else { + a = 0; + b++; + } + } + + if (out_p) + *out_p = p; + if (out_a) + *out_a = a; + if (out_b) + *out_b = b; + + return p * b + a; +} + +static long adf4360_clk_round_rate(struct clk_hw *hw, + unsigned long rate, + unsigned long *parent_rate) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + unsigned int r, n; + unsigned int pfd_freq; + + if (*parent_rate == 0) + return 0; + + if (st->part_id == ID_ADF4360_9) + return *parent_rate * st->n / st->r; + + if (rate > st->vco_max) + return st->vco_max; + + /* ADF4360-0 to AD4370-7 have an optional by two divider */ + if (st->part_id <= ID_ADF4360_7) { + if (rate < st->vco_min / 2) + return st->vco_min / 2; + if (rate < st->vco_min && rate > st->vco_max / 2) { + if (st->vco_min - rate < rate - st->vco_max / 2) + return st->vco_min; + else + return st->vco_max / 2; + } + } else { + if (rate < st->vco_min) + return st->vco_min; + } + + r = DIV_ROUND_CLOSEST(*parent_rate, st->pfd_freq); + pfd_freq = *parent_rate / r; + n = DIV_ROUND_CLOSEST(rate, pfd_freq); + + if (st->part_id <= ID_ADF4360_7) + n = adf4360_calc_prescaler(pfd_freq, n, NULL, NULL, NULL); + + return pfd_freq * n; +} + +static inline bool adf4360_is_powerdown(struct adf4360_state *st) +{ + return (st->power_down_mode != ADF4360_POWER_DOWN_NORMAL); +} + +static int adf4360_set_freq(struct adf4360_state *st, unsigned long rate) +{ + unsigned int val_r, val_n, val_ctrl; + unsigned int pfd_freq; + unsigned long r, n; + int ret; + + if (!st->clkin_freq || (st->clkin_freq > ADF4360_MAX_REFIN_RATE)) + return -EINVAL; + + if ((rate < st->vco_min) || (rate > st->vco_max)) + return -EINVAL; + + if (adf4360_is_powerdown(st)) + ret = -EBUSY; + + r = DIV_ROUND_CLOSEST(st->clkin_freq, st->pfd_freq); + pfd_freq = st->clkin_freq / r; + n = DIV_ROUND_CLOSEST(rate, pfd_freq); + + val_ctrl = ADF4360_CPL(st->info->default_cpl) | + ADF4360_MUXOUT(st->muxout_mode) | + ADF4360_PDP(!st->pdp) | + ADF4360_MTLD(st->mtld) | + ADF4360_OPL(st->power_level) | + ADF4360_CPI1(st->cpi) | + ADF4360_CPI2(st->cpi) | + ADF4360_POWERDOWN(st->power_down_mode); + + /* ADF4360-0 to ADF4360-7 have a dual-modulous prescaler */ + if (st->part_id <= ID_ADF4360_7) { + unsigned int p, a, b; + + n = adf4360_calc_prescaler(pfd_freq, n, &p, &a, &b); + + switch (p) { + case 8: + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_8); + break; + case 16: + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_16); + break; + default: + val_ctrl |= ADF4360_PRESCALER(ADF4360_PRESCALER_32); + break; + } + + val_n = ADF4360_A_COUNTER(a) | + ADF4360_B_COUNTER(b); + + if (rate < st->vco_min) + val_n |= ADF4360_OUT_DIV2(true) | + ADF4360_PRESCALER_DIV2(true); + } else { + val_n = ADF4360_B_COUNTER(n); + } + + /* + * Always use BSC divider of 8, see Analog Devices AN-1347. + * http://www.analog.com/media/en/technical-documentation/application-notes/AN-1347.pdf + */ + val_r = ADF4360_R_COUNTER(r) | + ADF4360_ABP(ADF4360_ABP_3_0NS) | + ADF4360_BSC(ADF4360_BSC_8); + + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_RDIV), val_r); + if (ret) + return ret; + + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val_ctrl); + if (ret) + return ret; + + /* + * Allow the transient behavior of the ADF4360-7 during initial + * power-up to settle. + */ + if (st->initial_reg_seq) { + usleep_range(15000, 20000); + st->initial_reg_seq = false; + } + + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_NDIV), val_n); + if (ret) + return ret; + + st->freq_req = rate; + st->n = n; + st->r = r; + + return 0; +} + +static int adf4360_clk_set_rate(struct clk_hw *hw, + unsigned long rate, + unsigned long parent_rate) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + int ret; + + if ((parent_rate == 0) || (parent_rate > ADF4360_MAX_REFIN_RATE)) + return -EINVAL; + + mutex_lock(&st->lock); + if (st->clkin_freq != parent_rate) + st->clkin_freq = parent_rate; + + ret = adf4360_set_freq(st, rate); + mutex_unlock(&st->lock); + + return ret; +} + +static int __adf4360_power_down(struct adf4360_state *st, unsigned int mode) +{ + struct device *dev = &st->spi->dev; + unsigned int val; + int ret = 0; + + switch (mode) { + case ADF4360_POWER_DOWN_NORMAL: + if (st->vdd_reg) { + ret = regulator_enable(st->vdd_reg); + if (ret) { + dev_err(dev, "Supply enable error: %d\n", ret); + return ret; + } + } + + st->initial_reg_seq = true; + st->power_down_mode = mode; + ret = adf4360_set_freq(st, st->freq_req); + break; + case ADF4360_POWER_DOWN_SOFT_ASYNC: /* fallthrough */ + case ADF4360_POWER_DOWN_SOFT_SYNC: + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; + val |= ADF4360_POWERDOWN(mode); + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); + break; + case ADF4360_POWER_DOWN_CE: + if (st->chip_en_gpio) + gpiod_set_value(st->chip_en_gpio, 0x0); + else + return -ENODEV; + break; + case ADF4360_POWER_DOWN_REGULATOR: + if (!st->vdd_reg) + return -ENODEV; + + if (st->chip_en_gpio) + ret = __adf4360_power_down(st, ADF4360_POWER_DOWN_CE); + else + ret = __adf4360_power_down(st, + ADF4360_POWER_DOWN_SOFT_ASYNC); + + ret = regulator_disable(st->vdd_reg); + if (ret) + dev_err(dev, "Supply disable error: %d\n", ret); + break; + } + if (ret == 0) + st->power_down_mode = mode; + + return 0; +} + +static int adf4360_power_down(struct adf4360_state *st, unsigned int mode) +{ + int ret; + + mutex_lock(&st->lock); + ret = __adf4360_power_down(st, mode); + mutex_unlock(&st->lock); + + return ret; +} + +static int adf4360_clk_prepare(struct clk_hw *hw) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + + return adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); +} + +static void adf4360_clk_unprepare(struct clk_hw *hw) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + + adf4360_power_down(st, ADF4360_POWER_DOWN_SOFT_ASYNC); +} + +static int adf4360_clk_enable(struct clk_hw *hw) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + + if (st->chip_en_gpio) + gpiod_set_value(st->chip_en_gpio, 0x1); + + return 0; +} + +static void adf4360_clk_disable(struct clk_hw *hw) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + + if (st->chip_en_gpio) + adf4360_power_down(st, ADF4360_POWER_DOWN_CE); +} + +static int adf4360_clk_is_enabled(struct clk_hw *hw) +{ + struct iio_dev *indio_dev = to_output(hw)->indio_dev; + struct adf4360_state *st = iio_priv(indio_dev); + + return adf4360_is_powerdown(st); +} + +static const struct clk_ops adf4360_clk_ops = { + .recalc_rate = adf4360_clk_recalc_rate, + .round_rate = adf4360_clk_round_rate, + .set_rate = adf4360_clk_set_rate, + .prepare = adf4360_clk_prepare, + .unprepare = adf4360_clk_unprepare, + .enable = adf4360_clk_enable, + .disable = adf4360_clk_disable, + .is_enabled = adf4360_clk_is_enabled, +}; + +static ssize_t adf4360_read(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + char *buf) +{ + struct adf4360_state *st = iio_priv(indio_dev); + unsigned long val; + int ret = 0; + + switch ((u32)private) { + case ADF4360_FREQ_REFIN: + val = st->clkin_freq; + break; + case ADF4360_MTLD: + val = st->mtld; + break; + case ADF4360_FREQ_PFD: + val = st->pfd_freq; + break; + default: + ret = -EINVAL; + val = 0; + } + + return ret < 0 ? ret : sprintf(buf, "%lu\n", val); +} + +static ssize_t adf4360_write(struct iio_dev *indio_dev, + uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len) +{ + struct adf4360_state *st = iio_priv(indio_dev); + unsigned long readin, tmp; + bool mtld; + int ret = 0; + + mutex_lock(&st->lock); + switch ((u32)private) { + case ADF4360_FREQ_REFIN: + ret = kstrtoul(buf, 10, &readin); + if (ret) + break; + + if ((readin > ADF4360_MAX_REFIN_RATE) || (readin == 0)) { + ret = -EINVAL; + break; + } + + if (st->clkin) { + tmp = clk_round_rate(st->clkin, readin); + if (tmp != readin) { + ret = -EINVAL; + break; + } + + ret = clk_set_rate(st->clkin, tmp); + if (ret) + break; + } + + st->clkin_freq = readin; + break; + case ADF4360_MTLD: + ret = kstrtobool(buf, &mtld); + if (ret) + break; + + st->mtld = mtld; + break; + case ADF4360_FREQ_PFD: + ret = kstrtoul(buf, 10, &readin); + if (ret) + break; + + if ((readin > ADF4360_MAX_PFD_RATE) || (readin == 0)) { + ret = -EINVAL; + break; + } + + st->pfd_freq = readin; + break; + default: + ret = -EINVAL; + } + + if (ret == 0) + ret = adf4360_set_freq(st, st->freq_req); + mutex_unlock(&st->lock); + + return ret ? ret : len; +} + +static int adf4360_get_muxout_mode(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct adf4360_state *st = iio_priv(indio_dev); + + return st->muxout_mode; +} + +static int adf4360_set_muxout_mode(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + unsigned int mode) +{ + struct adf4360_state *st = iio_priv(indio_dev); + unsigned int writeval; + int ret = 0; + + mutex_lock(&st->lock); + writeval = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_MUXOUT_MSK; + writeval |= ADF4360_MUXOUT(mode & 0x7); + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), writeval); + if (ret == 0) + st->muxout_mode = mode & 0x7; + mutex_unlock(&st->lock); + + return ret; +} + +static const struct iio_enum adf4360_muxout_modes_available = { + .items = adf4360_muxout_modes, + .num_items = ARRAY_SIZE(adf4360_muxout_modes), + .get = adf4360_get_muxout_mode, + .set = adf4360_set_muxout_mode, +}; + +static int adf4360_get_power_down(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct adf4360_state *st = iio_priv(indio_dev); + + return st->power_down_mode; +} + +static int adf4360_set_power_down(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + unsigned int mode) +{ + struct adf4360_state *st = iio_priv(indio_dev); + + return adf4360_power_down(st, mode); +} + +static const struct iio_enum adf4360_pwr_dwn_modes_available = { + .items = adf4360_power_down_modes, + .num_items = ARRAY_SIZE(adf4360_power_down_modes), + .get = adf4360_get_power_down, + .set = adf4360_set_power_down, +}; + +static int adf4360_get_power_level(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct adf4360_state *st = iio_priv(indio_dev); + + return st->power_level; +} + +static int adf4360_set_power_level(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + unsigned int level) +{ + struct adf4360_state *st = iio_priv(indio_dev); + unsigned int val; + int ret; + + if (adf4360_is_powerdown(st)) + return -EBUSY; + + mutex_lock(&st->lock); + val = st->regs[ADF4360_CTRL] & ~ADF4360_ADDR_OPL_MSK; + val |= ADF4360_OPL(level); + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); + st->power_level = level; + mutex_unlock(&st->lock); + + return ret; +} + +static const struct iio_enum adf4360_pwr_lvl_modes_available = { + .items = adf4360_power_level_modes, + .num_items = ARRAY_SIZE(adf4360_power_level_modes), + .get = adf4360_get_power_level, + .set = adf4360_set_power_level, +}; + +#define _ADF4360_EXT_INFO(_name, _ident) { \ + .name = _name, \ + .read = adf4360_read, \ + .write = adf4360_write, \ + .private = _ident, \ + .shared = IIO_SEPARATE, \ +} + +static const struct iio_chan_spec_ext_info adf4360_ext_info[] = { + _ADF4360_EXT_INFO("refin_frequency", ADF4360_FREQ_REFIN), + _ADF4360_EXT_INFO("mute_till_lock_detect", ADF4360_MTLD), + _ADF4360_EXT_INFO("pfd_frequency", ADF4360_FREQ_PFD), + IIO_ENUM_AVAILABLE("muxout_mode", &adf4360_muxout_modes_available), + IIO_ENUM("muxout_mode", false, &adf4360_muxout_modes_available), + IIO_ENUM_AVAILABLE("power_down", &adf4360_pwr_dwn_modes_available), + IIO_ENUM("power_down", false, &adf4360_pwr_dwn_modes_available), + IIO_ENUM_AVAILABLE("power_level", &adf4360_pwr_lvl_modes_available), + IIO_ENUM("power_level", false, &adf4360_pwr_lvl_modes_available), + { }, +}; + +static const struct iio_chan_spec adf4360_chan = { + .type = IIO_ALTVOLTAGE, + .indexed = 1, + .output = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY), + .ext_info = adf4360_ext_info, +}; + +static int adf4360_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, + int *val2, + long mask) +{ + struct adf4360_state *st = iio_priv(indio_dev); + bool lk_det; + + switch (mask) { + case IIO_CHAN_INFO_FREQUENCY: + if (adf4360_is_powerdown(st)) + return -EBUSY; + + lk_det = (ADF4360_MUXOUT_LOCK_DETECT | ADF4360_MUXOUT_OD_LD) & + st->muxout_mode; + if (lk_det && st->muxout_gpio) { + if (!gpiod_get_value(st->muxout_gpio)) { + dev_dbg(&st->spi->dev, "PLL un-locked\n"); + return -EBUSY; + } + } + + *val = st->freq_req; + return IIO_VAL_INT; + default: + return -EINVAL; + } + + return 0; +}; + +static int adf4360_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, + int val2, + long mask) +{ + struct adf4360_state *st = iio_priv(indio_dev); + int ret; + + mutex_lock(&st->lock); + switch (mask) { + case IIO_CHAN_INFO_FREQUENCY: + ret = adf4360_set_freq(st, val); + break; + default: + ret = -EINVAL; + } + mutex_unlock(&st->lock); + + return ret; +} + +static int adf4360_reg_access(struct iio_dev *indio_dev, + unsigned int reg, + unsigned int writeval, + unsigned int *readval) +{ + struct adf4360_state *st = iio_priv(indio_dev); + int ret = 0; + + if (reg >= ADF4360_REG_NUM) + return -EFAULT; + + mutex_lock(&st->lock); + if (readval) { + *readval = st->regs[reg]; + } else { + writeval &= 0xFFFFFC; + ret = adf4360_write_reg(st, reg, writeval); + } + mutex_unlock(&st->lock); + + return ret; +} + +static const struct iio_info adf4360_iio_info = { + .read_raw = &adf4360_read_raw, + .write_raw = &adf4360_write_raw, + .debugfs_reg_access = &adf4360_reg_access, +}; + +static int adf4360_get_gpio(struct adf4360_state *st) +{ + struct device *dev = &st->spi->dev; + unsigned int val; + int ret, i; + + st->chip_en_gpio = devm_gpiod_get_optional(dev, "enable", + GPIOD_OUT_HIGH); + if (IS_ERR(st->chip_en_gpio)) { + dev_err(dev, "Chip enable GPIO error\n"); + return PTR_ERR(st->chip_en_gpio); + } + + if (st->chip_en_gpio) + st->power_down_mode = ADF4360_POWER_DOWN_CE; + + st->muxout_gpio = devm_gpiod_get_optional(dev, "adi,muxout", GPIOD_IN); + if (IS_ERR(st->muxout_gpio)) { + dev_err(dev, "Muxout GPIO error\n"); + return PTR_ERR(st->muxout_gpio); + } + + if (!st->muxout_gpio) + return 0; + + /* ADF4360 PLLs are write only devices, try to probe using GPIO. */ + for (i = 0; i < 4; i++) { + if (i & 1) + val = ADF4360_MUXOUT(ADF4360_MUXOUT_DVDD); + else + val = ADF4360_MUXOUT(ADF4360_MUXOUT_GND); + + ret = adf4360_write_reg(st, ADF4360_REG(ADF4360_CTRL), val); + if (ret) + return ret; + + ret = gpiod_get_value(st->muxout_gpio); + if (ret ^ (i & 1)) { + dev_err(dev, "Probe failed (muxout)"); + return -ENODEV; + } + } + + return 0; +} + +static void adf4360_clkin_disable(void *data) +{ + struct adf4360_state *st = data; + + clk_disable_unprepare(st->clkin); +} + +static int adf4360_get_clkin(struct adf4360_state *st) +{ + struct device *dev = &st->spi->dev; + struct clk *clk; + int ret; + + clk = devm_clk_get(dev, "clkin"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + ret = clk_prepare_enable(clk); + if (ret) + return ret; + + ret = devm_add_action_or_reset(dev, adf4360_clkin_disable, st); + if (ret) + return ret; + + st->clkin = clk; + st->clkin_freq = clk_get_rate(clk); + + return 0; +} + +static void adf4360_clk_del_provider(void *data) +{ + struct adf4360_state *st = data; + + of_clk_del_provider(st->spi->dev.of_node); +} + +static int adf4360_clk_register(struct adf4360_state *st) +{ + struct spi_device *spi = st->spi; + struct clk_init_data init; + struct clk *clk; + const char *parent_name; + int ret; + + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0); + if (!parent_name) + return -EINVAL; + + init.name = st->clk_out_name; + init.ops = &adf4360_clk_ops; + init.flags = CLK_SET_RATE_GATE; + init.parent_names = &parent_name; + init.num_parents = 1; + + st->output.hw.init = &init; + + clk = devm_clk_register(&spi->dev, &st->output.hw); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + ret = of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get, clk); + if (ret) + return ret; + + return devm_add_action_or_reset(&spi->dev, adf4360_clk_del_provider, st); +} + +static int adf4360_parse_dt(struct adf4360_state *st) +{ + struct device *dev = &st->spi->dev; + u32 tmp; + int ret; + + ret = device_property_read_string(dev, "clock-output-names", + &st->clk_out_name); + if ((ret < 0) && dev->of_node) + st->clk_out_name = dev->of_node->name; + + if (st->part_id >= ID_ADF4360_7) { + /* + * ADF4360-7 to ADF4360-9 have a VCO that is tuned to a specific + * range using an external inductor. These properties describe + * the range selected by the external inductor. + */ + ret = device_property_read_u32(dev, + "adi,vco-minimum-frequency-hz", + &tmp); + if (ret == 0) + st->vco_min = max(st->info->vco_min, tmp); + else + st->vco_min = st->info->vco_min; + + ret = device_property_read_u32(dev, + "adi,vco-maximum-frequency-hz", + &tmp); + if (ret == 0) + st->vco_max = min(st->info->vco_max, tmp); + else + st->vco_max = st->info->vco_max; + } else { + st->vco_min = st->info->vco_min; + st->vco_max = st->info->vco_max; + } + + st->pdp = device_property_read_bool(dev, "adi,loop-filter-inverting"); + + ret = device_property_read_u32(dev, + "adi,loop-filter-pfd-frequency-hz", + &tmp); + if (ret == 0) { + st->pfd_freq = tmp; + } else { + dev_err(dev, "PFD frequency property missing\n"); + return ret; + } + + ret = device_property_read_u32(dev, + "adi,loop-filter-charge-pump-current-microamp", + &tmp); + if (ret == 0) { + st->cpi = find_closest(tmp, adf4360_cpi_modes, + ARRAY_SIZE(adf4360_cpi_modes)); + } else { + dev_err(dev, "CPI property missing\n"); + return ret; + } + + ret = device_property_read_u32(dev, "adi,power-up-frequency-hz", &tmp); + if (ret == 0) + st->freq_req = tmp; + else + st->freq_req = st->vco_min; + + ret = device_property_read_u32(dev, "adi,power-out-level-microamp", + &tmp); + if (ret == 0) + st->power_level = find_closest(tmp, adf4360_power_lvl_microamp, + ARRAY_SIZE(adf4360_power_lvl_microamp)); + else + st->power_level = ADF4360_PL_5; + + return 0; +} + +static int adf4360_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + const struct spi_device_id *id = spi_get_device_id(spi); + struct adf4360_state *st; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + + mutex_init(&st->lock); + + spi_set_drvdata(spi, indio_dev); + + st->spi = spi; + st->info = &adf4360_chip_info_tbl[id->driver_data]; + st->part_id = id->driver_data; + st->muxout_mode = ADF4360_MUXOUT_LOCK_DETECT; + st->mtld = true; + + ret = adf4360_parse_dt(st); + if (ret) { + dev_err(&spi->dev, "Parsing properties failed (%d)\n", ret); + return -ENODEV; + } + + indio_dev->dev.parent = &spi->dev; + + if (spi->dev.of_node) + indio_dev->name = spi->dev.of_node->name; + else + indio_dev->name = spi_get_device_id(spi)->name; + + indio_dev->info = &adf4360_iio_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = &adf4360_chan; + indio_dev->num_channels = 1; + st->output.indio_dev = indio_dev; + + ret = adf4360_get_gpio(st); + if (ret) + return ret; + + ret = adf4360_get_clkin(st); + if (ret) + return ret; + + st->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd"); + if (IS_ERR(st->vdd_reg)) { + if (PTR_ERR(st->vdd_reg) != -ENODEV) { + dev_err(&spi->dev, "Regulator error\n"); + return PTR_ERR(st->vdd_reg); + } + + st->vdd_reg = NULL; + } + + ret = adf4360_power_down(st, ADF4360_POWER_DOWN_NORMAL); + if (ret) + return ret; + + ret = adf4360_clk_register(st); + if (ret) + return ret; + + return devm_iio_device_register(&spi->dev, indio_dev); +} + +static const struct of_device_id adf4360_of_match[] = { + { .compatible = "adi,adf4360-0", }, + { .compatible = "adi,adf4360-1", }, + { .compatible = "adi,adf4360-2", }, + { .compatible = "adi,adf4360-3", }, + { .compatible = "adi,adf4360-4", }, + { .compatible = "adi,adf4360-5", }, + { .compatible = "adi,adf4360-6", }, + { .compatible = "adi,adf4360-7", }, + { .compatible = "adi,adf4360-8", }, + { .compatible = "adi,adf4360-9", }, + {}, +}; +MODULE_DEVICE_TABLE(of, adf4360_of_match); + +static const struct spi_device_id adf4360_id[] = { + {"adf4360-0", ID_ADF4360_0}, + {"adf4360-1", ID_ADF4360_1}, + {"adf4360-2", ID_ADF4360_2}, + {"adf4360-3", ID_ADF4360_3}, + {"adf4360-4", ID_ADF4360_4}, + {"adf4360-5", ID_ADF4360_5}, + {"adf4360-6", ID_ADF4360_6}, + {"adf4360-7", ID_ADF4360_7}, + {"adf4360-8", ID_ADF4360_8}, + {"adf4360-9", ID_ADF4360_9}, + {} +}; + +static struct spi_driver adf4360_driver = { + .driver = { + .name = "adf4360", + .of_match_table = adf4360_of_match, + .owner = THIS_MODULE, + }, + .probe = adf4360_probe, + .id_table = adf4360_id, +}; +module_spi_driver(adf4360_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); +MODULE_AUTHOR("Edward Kigwana <ekigwana@gmail.com>"); +MODULE_DESCRIPTION("Analog Devices ADF4360 PLL");