* [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs
@ 2020-01-28 11:13 Alexandru Ardelean
2020-01-28 11:13 ` [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL Alexandru Ardelean
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Alexandru Ardelean @ 2020-01-28 11:13 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel
Cc: ekigwana, jic23, lars, robh+dt, Alexandru Ardelean
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>
---
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[] = {
+ [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");
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL
2020-01-28 11:13 [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Alexandru Ardelean
@ 2020-01-28 11:13 ` Alexandru Ardelean
2020-02-02 14:47 ` Jonathan Cameron
2020-02-05 18:24 ` Rob Herring
2020-01-28 11:13 ` [PATCH v3 3/3] MAINTAINERS: add entry for ADF4360 PLL driver Alexandru Ardelean
2020-02-02 14:45 ` [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Jonathan Cameron
2 siblings, 2 replies; 10+ messages in thread
From: Alexandru Ardelean @ 2020-01-28 11:13 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel
Cc: ekigwana, jic23, lars, robh+dt, Alexandru Ardelean
From: Edward Kigwana <ekigwana@gmail.com>
This change adds the device-tree bindings documentation for the ADF4360
family of PLLs.
Signed-off-by: Edward Kigwana <ekigwana@gmail.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../bindings/iio/frequency/adi,adf4360.yaml | 137 ++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
new file mode 100644
index 000000000000..895e2cb2b300
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2019-2020 Edward Kigwana
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,adf4360.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADF4360 PLL device driver
+
+maintainers:
+ - Lars-Peter Clausen <lars@metafoo.de>
+ - Edward Kigwana <ekigwana@gmail.com>
+
+description: |
+ Bindings for the Analog Devices ADF4360 family of clock generator phase-locked
+ loop (PLL) devices with an integrated voltage-controlled oscillator (VCO).
+ Each of the parts in the family supports a specific frequency range.
+ Datasheets can be found here:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-0.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-1.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-2.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-3.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-4.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-5.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-6.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-7.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-8.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-9.pdf
+
+properties:
+ compatible:
+ pattern: '^adi,adf4360-[0-9]$'
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: clkin
+
+ '#clock-cells':
+ const: 0
+
+ adi,loop-filter-pfd-frequency-hz:
+ description: |
+ The phase-frequency-detector frequency that the external loop filter was
+ designed for.
+ allOf:
+ - minimum: 25000
+ - maximum: 8000000
+ maxItems: 1
+
+ adi,loop-filter-charger-pump-current-microamp:
+ description: |
+ The charge pump current that the external loop filter was designed for.
+ The provided value is clamped to the closest enumerated value.
+ enum: [ 310, 620, 930, 1250, 1560, 1870, 2180, 2500 ]
+
+ adi,vco-minimum-frequency-hz:
+ description: |
+ Required for ADF4360-7, ADF4360-8 and ADF4360-9. Minimum VCO frequency
+ that can be supported by the tuning range set by the external inductor.
+ maxItems: 1
+
+ adi,vco-maximum-frequency-hz:
+ description: |
+ Required for ADF4360-7, ADF4360-8 and ADF4360-9. Maximum VCO frequency
+ that can be supported by the tuning range set by the external inductor.
+ maxItems: 1
+
+ adi,loop-filter-inverting:
+ description: Indicates that the external loop filter is an inverting filter.
+ type: boolean
+
+ adi,power-up-frequency-hz:
+ description: |
+ PLL tunes to the set frequency on probe or defaults to either the minimum
+ for the part or value set using adi,vco-minimum-frequency-hz.
+ maxItems: 1
+
+ vdd-supply:
+ description: |
+ vdd supply is used to enable or disable chip when regulator power down
+ mode is set. Other power down modes are used to mitigate the case of a
+ shared regulator.
+
+ enable-gpios:
+ description: |
+ Chip enable gpio is used to enable or disable chip when chip enable power
+ down mode is set.
+ maxItems: 1
+
+ adi,muxout-gpios:
+ description: |
+ MUX out gpio is used to detect chip and test pll lock state on read when
+ muxout control is set to lock detect.
+ maxItems: 1
+
+ adi,power-out-level-microamp:
+ description: |
+ Chip support setting of output power level. This property is optional.
+ If it is not provided by default 11000 uA will be set.
+ enum: [ 3500, 5000, 7500, 11000 ]
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - adi,loop-filter-charge-pump-current
+ - adi,loop-filter-pfd-frequency-hz
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pll@0 {
+ compatible = "adi,adf4360-7";
+ reg = <0>;
+ spi-max-frequency = <2000000>;
+ clocks = <&ref_clock>;
+ #clock-cells = <0>;
+ clock-names = "clkin";
+ clock-output-names = "adf4360-7";
+
+ adi,loop-filter-charge-pump-current = <5>;
+ adi,loop-filter-pfd-frequency-hz = <2500000>;
+ adi,vco-minimum-frequency-hz = <700000000>;
+ adi,vco-maximum-frequency-hz = <840000000>;
+ };
+ };
+...
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] MAINTAINERS: add entry for ADF4360 PLL driver
2020-01-28 11:13 [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Alexandru Ardelean
2020-01-28 11:13 ` [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL Alexandru Ardelean
@ 2020-01-28 11:13 ` Alexandru Ardelean
2020-02-03 10:26 ` Andy Shevchenko
2020-02-02 14:45 ` [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Jonathan Cameron
2 siblings, 1 reply; 10+ messages in thread
From: Alexandru Ardelean @ 2020-01-28 11:13 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel
Cc: ekigwana, jic23, lars, robh+dt, Alexandru Ardelean
From: Edward Kigwana <ekigwana@gmail.com>
Add entry in the MAINTAINERS file for the ADF4360 PLL driver.
Signed-off-by: Edward Kigwana <ekigwana@gmail.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index e699fe378e71..d7a404084ad9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -462,6 +462,14 @@ ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
M: Jiri Kosina <jikos@kernel.org>
S: Maintained
+ADF4360 PLL DRIVER
+M: Edward Kigwana <ekigwana@gmail.com>
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
+F: drivers/iio/frequency/adf4360.c
+
ADF7242 IEEE 802.15.4 RADIO DRIVER
M: Michael Hennerich <michael.hennerich@analog.com>
W: https://wiki.analog.com/ADF7242
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs
2020-01-28 11:13 [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Alexandru Ardelean
2020-01-28 11:13 ` [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL Alexandru Ardelean
2020-01-28 11:13 ` [PATCH v3 3/3] MAINTAINERS: add entry for ADF4360 PLL driver Alexandru Ardelean
@ 2020-02-02 14:45 ` Jonathan Cameron
2020-02-03 11:18 ` Ardelean, Alexandru
2 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2020-02-02 14:45 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, devicetree, linux-kernel, ekigwana, lars, robh+dt
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");
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL
2020-01-28 11:13 ` [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL Alexandru Ardelean
@ 2020-02-02 14:47 ` Jonathan Cameron
2020-02-05 18:24 ` Rob Herring
1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2020-02-02 14:47 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, devicetree, linux-kernel, ekigwana, lars, robh+dt
On Tue, 28 Jan 2020 13:13:01 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Edward Kigwana <ekigwana@gmail.com>
>
> This change adds the device-tree bindings documentation for the ADF4360
> family of PLLs.
>
> Signed-off-by: Edward Kigwana <ekigwana@gmail.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../bindings/iio/frequency/adi,adf4360.yaml | 137 ++++++++++++++++++
> 1 file changed, 137 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
> new file mode 100644
> index 000000000000..895e2cb2b300
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
> @@ -0,0 +1,137 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright 2019-2020 Edward Kigwana
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,adf4360.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADF4360 PLL device driver
This isn't a driver, so don't mention "device driver" It's a binding
for the hardware.
> +
> +maintainers:
> + - Lars-Peter Clausen <lars@metafoo.de>
> + - Edward Kigwana <ekigwana@gmail.com>
> +
> +description: |
> + Bindings for the Analog Devices ADF4360 family of clock generator phase-locked
> + loop (PLL) devices with an integrated voltage-controlled oscillator (VCO).
> + Each of the parts in the family supports a specific frequency range.
> + Datasheets can be found here:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-0.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-1.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-3.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-4.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-5.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-6.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-7.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-8.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-9.pdf
> +
> +properties:
> + compatible:
> + pattern: '^adi,adf4360-[0-9]$'
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: clkin
> +
> + '#clock-cells':
> + const: 0
> +
> + adi,loop-filter-pfd-frequency-hz:
> + description: |
> + The phase-frequency-detector frequency that the external loop filter was
> + designed for.
> + allOf:
> + - minimum: 25000
> + - maximum: 8000000
> + maxItems: 1
> +
> + adi,loop-filter-charger-pump-current-microamp:
> + description: |
> + The charge pump current that the external loop filter was designed for.
> + The provided value is clamped to the closest enumerated value.
> + enum: [ 310, 620, 930, 1250, 1560, 1870, 2180, 2500 ]
> +
> + adi,vco-minimum-frequency-hz:
> + description: |
> + Required for ADF4360-7, ADF4360-8 and ADF4360-9. Minimum VCO frequency
> + that can be supported by the tuning range set by the external inductor.
> + maxItems: 1
> +
> + adi,vco-maximum-frequency-hz:
> + description: |
> + Required for ADF4360-7, ADF4360-8 and ADF4360-9. Maximum VCO frequency
> + that can be supported by the tuning range set by the external inductor.
> + maxItems: 1
> +
> + adi,loop-filter-inverting:
> + description: Indicates that the external loop filter is an inverting filter.
> + type: boolean
> +
> + adi,power-up-frequency-hz:
> + description: |
> + PLL tunes to the set frequency on probe or defaults to either the minimum
> + for the part or value set using adi,vco-minimum-frequency-hz.
> + maxItems: 1
> +
> + vdd-supply:
> + description: |
> + vdd supply is used to enable or disable chip when regulator power down
> + mode is set. Other power down modes are used to mitigate the case of a
> + shared regulator.
> +
> + enable-gpios:
> + description: |
> + Chip enable gpio is used to enable or disable chip when chip enable power
> + down mode is set.
> + maxItems: 1
> +
> + adi,muxout-gpios:
> + description: |
> + MUX out gpio is used to detect chip and test pll lock state on read when
> + muxout control is set to lock detect.
> + maxItems: 1
This is a debug feature I think? If so probably shouldn't be in dt, but
perhaps I am missing something?
> +
> + adi,power-out-level-microamp:
> + description: |
> + Chip support setting of output power level. This property is optional.
> + If it is not provided by default 11000 uA will be set.
> + enum: [ 3500, 5000, 7500, 11000 ]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - adi,loop-filter-charge-pump-current
> + - adi,loop-filter-pfd-frequency-hz
> +
> +examples:
> + - |
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pll@0 {
> + compatible = "adi,adf4360-7";
> + reg = <0>;
> + spi-max-frequency = <2000000>;
> + clocks = <&ref_clock>;
> + #clock-cells = <0>;
> + clock-names = "clkin";
> + clock-output-names = "adf4360-7";
> +
> + adi,loop-filter-charge-pump-current = <5>;
> + adi,loop-filter-pfd-frequency-hz = <2500000>;
> + adi,vco-minimum-frequency-hz = <700000000>;
> + adi,vco-maximum-frequency-hz = <840000000>;
> + };
> + };
> +...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] MAINTAINERS: add entry for ADF4360 PLL driver
2020-01-28 11:13 ` [PATCH v3 3/3] MAINTAINERS: add entry for ADF4360 PLL driver Alexandru Ardelean
@ 2020-02-03 10:26 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-03 10:26 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, devicetree, Linux Kernel Mailing List, ekigwana,
Jonathan Cameron, Lars-Peter Clausen, Rob Herring
On Tue, Jan 28, 2020 at 1:13 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> From: Edward Kigwana <ekigwana@gmail.com>
>
> Add entry in the MAINTAINERS file for the ADF4360 PLL driver.
> +ADF4360 PLL DRIVER
> +M: Edward Kigwana <ekigwana@gmail.com>
> +W: http://ez.analog.com/community/linux-device-drivers
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
> +F: drivers/iio/frequency/adf4360.c
Had you run parse-maintainers.pl afterwards to see if everything is okay?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs
2020-02-02 14:45 ` [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Jonathan Cameron
@ 2020-02-03 11:18 ` Ardelean, Alexandru
2020-02-03 11:27 ` Jonathan Cameron
2020-02-03 14:10 ` Andy Shevchenko
0 siblings, 2 replies; 10+ messages in thread
From: Ardelean, Alexandru @ 2020-02-03 11:18 UTC (permalink / raw)
To: jic23; +Cc: ekigwana, devicetree, linux-kernel, linux-iio, robh+dt, lars
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");
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs
2020-02-03 11:18 ` Ardelean, Alexandru
@ 2020-02-03 11:27 ` Jonathan Cameron
2020-02-03 14:10 ` Andy Shevchenko
1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2020-02-03 11:27 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: jic23, ekigwana, devicetree, linux-kernel, linux-iio, robh+dt, lars
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");
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs
2020-02-03 11:18 ` Ardelean, Alexandru
2020-02-03 11:27 ` Jonathan Cameron
@ 2020-02-03 14:10 ` Andy Shevchenko
1 sibling, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-03 14:10 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: jic23, ekigwana, devicetree, linux-kernel, linux-iio, robh+dt, lars
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.
> > > +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL
2020-01-28 11:13 ` [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL Alexandru Ardelean
2020-02-02 14:47 ` Jonathan Cameron
@ 2020-02-05 18:24 ` Rob Herring
1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2020-02-05 18:24 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, devicetree, linux-kernel, ekigwana, jic23, lars
On Tue, Jan 28, 2020 at 01:13:01PM +0200, Alexandru Ardelean wrote:
> From: Edward Kigwana <ekigwana@gmail.com>
>
> This change adds the device-tree bindings documentation for the ADF4360
> family of PLLs.
>
> Signed-off-by: Edward Kigwana <ekigwana@gmail.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../bindings/iio/frequency/adi,adf4360.yaml | 137 ++++++++++++++++++
> 1 file changed, 137 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
> new file mode 100644
> index 000000000000..895e2cb2b300
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf4360.yaml
> @@ -0,0 +1,137 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright 2019-2020 Edward Kigwana
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,adf4360.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADF4360 PLL device driver
> +
> +maintainers:
> + - Lars-Peter Clausen <lars@metafoo.de>
> + - Edward Kigwana <ekigwana@gmail.com>
> +
> +description: |
> + Bindings for the Analog Devices ADF4360 family of clock generator phase-locked
> + loop (PLL) devices with an integrated voltage-controlled oscillator (VCO).
> + Each of the parts in the family supports a specific frequency range.
> + Datasheets can be found here:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-0.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-1.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-3.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-4.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-5.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-6.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-7.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-8.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ADF4360-9.pdf
> +
> +properties:
> + compatible:
> + pattern: '^adi,adf4360-[0-9]$'
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: clkin
> +
> + '#clock-cells':
> + const: 0
> +
> + adi,loop-filter-pfd-frequency-hz:
> + description: |
> + The phase-frequency-detector frequency that the external loop filter was
> + designed for.
> + allOf:
> + - minimum: 25000
> + - maximum: 8000000
> + maxItems: 1
You're mixing array and scalar properties. Drop 'maxItems' and min/max
don't need to be under 'allOf'.
> +
> + adi,loop-filter-charger-pump-current-microamp:
> + description: |
> + The charge pump current that the external loop filter was designed for.
> + The provided value is clamped to the closest enumerated value.
> + enum: [ 310, 620, 930, 1250, 1560, 1870, 2180, 2500 ]
> +
> + adi,vco-minimum-frequency-hz:
> + description: |
> + Required for ADF4360-7, ADF4360-8 and ADF4360-9. Minimum VCO frequency
> + that can be supported by the tuning range set by the external inductor.
> + maxItems: 1
> +
> + adi,vco-maximum-frequency-hz:
> + description: |
> + Required for ADF4360-7, ADF4360-8 and ADF4360-9. Maximum VCO frequency
> + that can be supported by the tuning range set by the external inductor.
> + maxItems: 1
> +
> + adi,loop-filter-inverting:
> + description: Indicates that the external loop filter is an inverting filter.
> + type: boolean
> +
> + adi,power-up-frequency-hz:
> + description: |
> + PLL tunes to the set frequency on probe or defaults to either the minimum
> + for the part or value set using adi,vco-minimum-frequency-hz.
> + maxItems: 1
> +
> + vdd-supply:
> + description: |
> + vdd supply is used to enable or disable chip when regulator power down
> + mode is set. Other power down modes are used to mitigate the case of a
> + shared regulator.
> +
> + enable-gpios:
> + description: |
> + Chip enable gpio is used to enable or disable chip when chip enable power
> + down mode is set.
> + maxItems: 1
> +
> + adi,muxout-gpios:
> + description: |
> + MUX out gpio is used to detect chip and test pll lock state on read when
> + muxout control is set to lock detect.
> + maxItems: 1
> +
> + adi,power-out-level-microamp:
> + description: |
> + Chip support setting of output power level. This property is optional.
> + If it is not provided by default 11000 uA will be set.
> + enum: [ 3500, 5000, 7500, 11000 ]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - adi,loop-filter-charge-pump-current
> + - adi,loop-filter-pfd-frequency-hz
> +
> +examples:
> + - |
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pll@0 {
> + compatible = "adi,adf4360-7";
> + reg = <0>;
> + spi-max-frequency = <2000000>;
> + clocks = <&ref_clock>;
> + #clock-cells = <0>;
> + clock-names = "clkin";
> + clock-output-names = "adf4360-7";
> +
> + adi,loop-filter-charge-pump-current = <5>;
> + adi,loop-filter-pfd-frequency-hz = <2500000>;
> + adi,vco-minimum-frequency-hz = <700000000>;
> + adi,vco-maximum-frequency-hz = <840000000>;
> + };
> + };
> +...
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-05 18:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-28 11:13 [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Alexandru Ardelean
2020-01-28 11:13 ` [PATCH v3 2/3] dt-bindings: iio: frequency: Add docs for ADF4360 PLL Alexandru Ardelean
2020-02-02 14:47 ` Jonathan Cameron
2020-02-05 18:24 ` Rob Herring
2020-01-28 11:13 ` [PATCH v3 3/3] MAINTAINERS: add entry for ADF4360 PLL driver Alexandru Ardelean
2020-02-03 10:26 ` Andy Shevchenko
2020-02-02 14:45 ` [PATCH v3 1/3] iio: frequency: adf4360: Add support for ADF4360 PLLs Jonathan Cameron
2020-02-03 11:18 ` Ardelean, Alexandru
2020-02-03 11:27 ` Jonathan Cameron
2020-02-03 14:10 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).