On 05/04/2012 02:31 PM, michael.hennerich@xxxxxxxxxx wrote:
> From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>
Various bits inline, one thing I don't follow is why the number of
channels is controlled by platform data? Normally we just export
them whether or not they are connected...
> Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
> ---
> .../Documentation/frequency/sysfs-bus-iio-ad9523 | 31 +
> drivers/staging/iio/frequency/Kconfig | 22 +-
> drivers/staging/iio/frequency/Makefile | 3 +-
> drivers/staging/iio/frequency/ad9523.c | 1040 ++++++++++++++++++++
> drivers/staging/iio/frequency/ad9523.h | 112 +++
> 5 files changed, 1206 insertions(+), 2 deletions(-)
> create mode 100644 drivers/staging/iio/Documentation/frequency/sysfs-bus-iio-ad9523
> create mode 100644 drivers/staging/iio/frequency/ad9523.c
> create mode 100644 drivers/staging/iio/frequency/ad9523.h
>
> diff --git a/drivers/staging/iio/Documentation/frequency/sysfs-bus-iio-ad9523 b/drivers/staging/iio/Documentation/frequency/sysfs-bus-iio-ad9523
> new file mode 100644
> index 0000000..79328d1
> --- /dev/null
> +++ b/drivers/staging/iio/Documentation/frequency/sysfs-bus-iio-ad9523
> @@ -0,0 +1,31 @@
> +What: /sys/bus/iio/devices/iio:deviceX/status_pllY_lock_detect
> +What: /sys/bus/iio/devices/iio:deviceX/status_pllY_feedback_clock
> +What: /sys/bus/iio/devices/iio:deviceX/status_pllY_reference_clock
> +What: /sys/bus/iio/devices/iio:deviceX/status_reference_a
> +What: /sys/bus/iio/devices/iio:deviceX/status_reference_b
> +What: /sys/bus/iio/devices/iio:deviceX/status_reference_test
> +What: /sys/bus/iio/devices/iio:deviceX/status_vcxo
> +KernelVersion: 3.4.0
> +Contact: linux-iio@xxxxxxxxxxxxxxx
> +Description:
> + Reading returns either 'OK' or 'FAIL'. 'OK' means that the
> + clock in question is present or the pllY is locked.
I'd break this into two descriptions. One for pllY locked and one for
the clocks.
Also I have no idea what the reference tests are form this description!
>'FAIL'
> + means that the clock is missing or the pllY is unlocked.
These look like the sort of thing that has previously led to discussion of
an out of band means of reporting 'device failures' within the kernel.
Still I'm not aware of anyone having actually worked on that, so I guess
we need something like that. I wonder if OK / FAIL is the best option.
Would have suggested a simple 1/0 pair but obviously for somethings
we'd need very specific naming to indicate whether 1 was a good thing
or a bad thing. Maybe an alternative naming might be (putting this
out there mainly to open up debate - not because I necessarily think
it improves on what you have!)
pllY_locked
pllY_feedback_clk_present
pllY_reference_clk_present
looking at the datasheet, aren't refa and refb two separate reference
clk inputs? Thats not clear in current naming if true.
When you get down to it I'm not entirely sure on what you attributes
all mean, so on that basis the naming / description need some work!
I had to look up what a vcxo actually was.. x = crystal?
> +
> +What: /sys/bus/iio/devices/iio:deviceX/store_eeprom
> +KernelVersion: 3.4.0
> +Contact: linux-iio@xxxxxxxxxxxxxxx
> +Description:
> + Writing '1' stores the current device configuration into
> + on-chip EEPROM. After power-up or chip reset the device will
> + automatically load the saved configuration.
This is a fairly common thing to see on devices. The complexity lies in
that
sometimes only a subset of stuff is saved... I guess there is no harm
in what you have here, but we may well want to think about it a bit more
at a later date!
> +
> +What: /sys/bus/iio/devices/iio:deviceX/sync_dividers
> +KernelVersion: 3.4.0
> +Contact: linux-iio@xxxxxxxxxxxxxxx
> +Description:
> + Writing '1' triggers the clock distribution synchronization
> + functionality. All dividers are reset and the channels start
> + with their predefined phase offsets (out_altvoltageY_phase).
> + Writing this file has the effect as driving the external
> + /SYNC pin low.
> diff --git a/drivers/staging/iio/frequency/Kconfig b/drivers/staging/iio/frequency/Kconfig
> index 93b7141..7b2c0c3 100644
> --- a/drivers/staging/iio/frequency/Kconfig
> +++ b/drivers/staging/iio/frequency/Kconfig
> @@ -1,6 +1,25 @@
> #
> -# Direct Digital Synthesis drivers
> +# Frequency
> +# Direct Digital Synthesis drivers (DDS)
> +# Clock Distribution device drivers
> +# Phase-Locked Loop (PLL) frequency synthesizers
> #
> +
> +menu "Frequency Synthesizers DDS/PLL"
a newline here?
> +menu "Clock Generator/Distribution"
> +
> +config AD9523
> + tristate "Analog Devices AD9523 Low Jitter Clock Generator"
> + depends on SPI
> + help
> + Say yes here to build support for Analog Devices AD9523 Low Jitter
> + Clock Generator. The driver provides direct access via sysfs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called ad9523.
> +
> +endmenu
> +
> menu "Direct Digital Synthesis"
>
> config AD5930
> @@ -59,3 +78,4 @@ config AD9951
> ad9951, provides direct access via sysfs.
>
> endmenu
> +endmenu
> diff --git a/drivers/staging/iio/frequency/Makefile b/drivers/staging/iio/frequency/Makefile
> index 1477461..e0157b6 100644
> --- a/drivers/staging/iio/frequency/Makefile
> +++ b/drivers/staging/iio/frequency/Makefile
> @@ -1,5 +1,5 @@
> #
> -# Makefile for Direct Digital Synthesis drivers
> +# Makefile iio/frequency
> #
>
> obj-$(CONFIG_AD5930) += ad5930.o
> @@ -9,3 +9,4 @@ obj-$(CONFIG_AD9850) += ad9850.o
> obj-$(CONFIG_AD9852) += ad9852.o
> obj-$(CONFIG_AD9910) += ad9910.o
> obj-$(CONFIG_AD9951) += ad9951.o
> +obj-$(CONFIG_AD9523) += ad9523.o
> diff --git a/drivers/staging/iio/frequency/ad9523.c b/drivers/staging/iio/frequency/ad9523.c
> new file mode 100644
> index 0000000..bb43386
> --- /dev/null
> +++ b/drivers/staging/iio/frequency/ad9523.c
> @@ -0,0 +1,1040 @@
> +/*
> + * AD9523 SPI Low Jitter Clock Generator
> + *
> + * Copyright 2012 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/spi/spi.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include "ad9523.h"
> +
> +#define AD9523_READ (1 << 15)
> +#define AD9523_WRITE (0 << 15)
> +#define AD9523_CNT(x) (((x) - 1) << 13)
> +#define AD9523_ADDR(x) ((x) & 0xFFF)
> +
> +#define AD9523_R1B (1 << 16)
> +#define AD9523_R2B (2 << 16)
> +#define AD9523_R3B (3 << 16)
> +#define AD9523_TRANSF_LEN(x) ((x) >> 16)
> +
> +#define AD9523_SERIAL_PORT_CONFIG (AD9523_R1B | 0x0)
> +#define AD9523_VERSION_REGISTER (AD9523_R1B | 0x2)
> +#define AD9523_PART_REGISTER (AD9523_R1B | 0x3)
> +#define AD9523_READBACK_CTRL (AD9523_R1B | 0x4)
> +
> +#define AD9523_EEPROM_CUSTOMER_VERSION_ID (AD9523_R2B | 0x6)
> +
> +#define AD9523_PLL1_REF_A_DIVIDER (AD9523_R2B | 0x11)
> +#define AD9523_PLL1_REF_B_DIVIDER (AD9523_R2B | 0x13)
> +#define AD9523_PLL1_REF_TEST_DIVIDER (AD9523_R1B | 0x14)
> +#define AD9523_PLL1_FEEDBACK_DIVIDER (AD9523_R2B | 0x17)
> +#define AD9523_PLL1_CHARGE_PUMP_CTRL (AD9523_R2B | 0x19)
> +#define AD9523_PLL1_INPUT_RECEIVERS_CTRL (AD9523_R1B | 0x1A)
> +#define AD9523_PLL1_REF_CTRL (AD9523_R1B | 0x1B)
> +#define AD9523_PLL1_MISC_CTRL (AD9523_R1B | 0x1C)
> +#define AD9523_PLL1_LOOP_FILTER_CTRL (AD9523_R1B | 0x1D)
> +
> +#define AD9523_PLL2_CHARGE_PUMP (AD9523_R1B | 0xF0)
> +#define AD9523_PLL2_FEEDBACK_DIVIDER_AB (AD9523_R1B | 0xF1)
> +#define AD9523_PLL2_CTRL (AD9523_R1B | 0xF2)
> +#define AD9523_PLL2_VCO_CTRL (AD9523_R1B | 0xF3)
> +#define AD9523_PLL2_VCO_DIVIDER (AD9523_R1B | 0xF4)
> +#define AD9523_PLL2_LOOP_FILTER_CTRL (AD9523_R2B | 0xF6)
> +#define AD9523_PLL2_R2_DIVIDER (AD9523_R1B | 0xF7)
> +
> +#define AD9523_CHANNEL_CLOCK_DIST(ch) (AD9523_R3B | (0x192 + 3 * ch))
> +
> +#define AD9523_PLL1_OUTPUT_CTRL (AD9523_R1B | 0x1BA)
> +#define AD9523_PLL1_OUTPUT_CHANNEL_CTRL (AD9523_R1B | 0x1BB)
> +
> +#define AD9523_READBACK_0 (AD9523_R1B | 0x22C)
> +#define AD9523_READBACK_1 (AD9523_R1B | 0x22D)
> +
> +#define AD9523_STATUS_SIGNALS (AD9523_R3B | 0x232)
> +#define AD9523_POWER_DOWN_CTRL (AD9523_R1B | 0x233)
> +#define AD9523_IO_UPDATE (AD9523_R1B | 0x234)
> +
> +#define AD9523_EEPROM_DATA_XFER_STATUS (AD9523_R1B | 0xB00)
> +#define AD9523_EEPROM_ERROR_READBACK (AD9523_R1B | 0xB01)
> +#define AD9523_EEPROM_CTRL1 (AD9523_R1B | 0xB02)
> +#define AD9523_EEPROM_CTRL2 (AD9523_R1B | 0xB03)
> +
> +/* AD9523_SERIAL_PORT_CONFIG */
> +
> +#define AD9523_SER_CONF_SDO_ACTIVE (1 << 7)
> +#define AD9523_SER_CONF_SOFT_RESET (1 << 5)
> +
> +/* AD9523_READBACK_CTRL */
> +#define AD9523_READBACK_CTRL_READ_BUFFERED (1 << 0)
> +
> +/* AD9523_PLL1_CHARGE_PUMP_CTRL */
> +#define AD9523_PLL1_CHARGE_PUMP_CURRENT_nA(x) (((x) / 500) & 0x7F)
> +#define AD9523_PLL1_CHARGE_PUMP_TRISTATE (1 << 7)
> +#define AD9523_PLL1_CHARGE_PUMP_MODE_NORMAL (3 << 8)
> +#define AD9523_PLL1_CHARGE_PUMP_MODE_PUMP_DOWN (2 << 8)
> +#define AD9523_PLL1_CHARGE_PUMP_MODE_PUMP_UP (1 << 8)
> +#define AD9523_PLL1_CHARGE_PUMP_MODE_TRISTATE (0 << 8)
> +#define AD9523_PLL1_BACKLASH_PW_MIN (0 << 10)
> +#define AD9523_PLL1_BACKLASH_PW_LOW (1 << 10)
> +#define AD9523_PLL1_BACKLASH_PW_HIGH (2 << 10)
> +#define AD9523_PLL1_BACKLASH_PW_MAX (3 << 10)
> +
> +/* AD9523_PLL1_INPUT_RECEIVERS_CTRL */
> +#define AD9523_PLL1_REF_TEST_RCV_EN (1 << 7)
> +#define AD9523_PLL1_REFB_DIFF_RCV_EN (1 << 6)
> +#define AD9523_PLL1_REFA_DIFF_RCV_EN (1 << 5)
> +#define AD9523_PLL1_REFB_RCV_EN (1 << 4)
> +#define AD9523_PLL1_REFA_RCV_EN (1 << 3)
> +#define AD9523_PLL1_REFA_REFB_PWR_CTRL_EN (1 << 2)
> +#define AD9523_PLL1_OSC_IN_CMOS_NEG_INP_EN (1 << 1)
> +#define AD9523_PLL1_OSC_IN_DIFF_EN (1 << 0)
> +
> +/* AD9523_PLL1_REF_CTRL */
> +#define AD9523_PLL1_BYPASS_REF_TEST_DIV_EN (1 << 7)
> +#define AD9523_PLL1_BYPASS_FEEDBACK_DIV_EN (1 << 6)
> +#define AD9523_PLL1_ZERO_DELAY_MODE_INT (1 << 5)
> +#define AD9523_PLL1_ZERO_DELAY_MODE_EXT (0 << 5)
> +#define AD9523_PLL1_OSC_IN_PLL_FEEDBACK_EN (1 << 4)
> +#define AD9523_PLL1_ZD_IN_CMOS_NEG_INP_EN (1 << 3)
> +#define AD9523_PLL1_ZD_IN_DIFF_EN (1 << 2)
> +#define AD9523_PLL1_REFB_CMOS_NEG_INP_EN (1 << 1)
> +#define AD9523_PLL1_REFA_CMOS_NEG_INP_EN (1 << 0)
> +
> +/* AD9523_PLL1_MISC_CTRL */
> +#define AD9523_PLL1_REFB_INDEP_DIV_CTRL_EN (1 << 7)
> +#define AD9523_PLL1_OSC_CTRL_FAIL_VCC_BY2_EN (1 << 6)
> +#define AD9523_PLL1_REF_MODE(x) ((x) << 2)
> +#define AD9523_PLL1_BYPASS_REFB_DIV (1 << 1)
> +#define AD9523_PLL1_BYPASS_REFA_DIV (1 << 0)
> +
> +/* AD9523_PLL1_LOOP_FILTER_CTRL */
> +#define AD9523_PLL1_LOOP_FILTER_RZERO(x) ((x) & 0xF)
> +
> +/* AD9523_PLL2_CHARGE_PUMP */
> +#define AD9523_PLL2_CHARGE_PUMP_CURRENT_nA(x) ((x) / 3500)
> +
> +/* AD9523_PLL2_FEEDBACK_DIVIDER_AB */
> +#define AD9523_PLL2_FB_NDIV_A_CNT(x) (((x) & 0x3) << 6)
> +#define AD9523_PLL2_FB_NDIV_B_CNT(x) (((x) & 0x3F) << 0)
> +#define AD9523_PLL2_FB_NDIV(a, b) (4 * (b) + (a))
> +
> +/* AD9523_PLL2_CTRL */
> +#define AD9523_PLL2_CHARGE_PUMP_MODE_NORMAL (3 << 0)
> +#define AD9523_PLL2_CHARGE_PUMP_MODE_PUMP_DOWN (2 << 0)
> +#define AD9523_PLL2_CHARGE_PUMP_MODE_PUMP_UP (1 << 0)
> +#define AD9523_PLL2_CHARGE_PUMP_MODE_TRISTATE (0 << 0)
> +#define AD9523_PLL2_BACKLASH_PW_MIN (0 << 2)
> +#define AD9523_PLL2_BACKLASH_PW_LOW (1 << 2)
> +#define AD9523_PLL2_BACKLASH_PW_HIGH (2 << 2)
> +#define AD9523_PLL2_BACKLASH_PW_MAX (3 << 1)
> +#define AD9523_PLL2_BACKLASH_CTRL_EN (1 << 4)
> +#define AD9523_PLL2_FREQ_DOUBLER_EN (1 << 5)
> +#define AD9523_PLL2_LOCK_DETECT_PWR_DOWN_EN (1 << 7)
> +
> +/* AD9523_PLL2_VCO_CTRL */
> +#define AD9523_PLL2_VCO_CALIBRATE (1 << 1)
> +#define AD9523_PLL2_FORCE_VCO_MIDSCALE (1 << 2)
> +#define AD9523_PLL2_FORCE_REFERENCE_VALID (1 << 3)
> +#define AD9523_PLL2_FORCE_RELEASE_SYNC (1 << 4)
> +
> +/* AD9523_PLL2_VCO_DIVIDER */
> +#define AD9523_PLL2_VCO_DIV_M1(x) ((((x) - 3) & 0x3) << 0)
> +#define AD9523_PLL2_VCO_DIV_M2(x) ((((x) - 3) & 0x3) << 4)
> +#define AD9523_PLL2_VCO_DIV_M1_PWR_DOWN_EN (1 << 2)
> +#define AD9523_PLL2_VCO_DIV_M2_PWR_DOWN_EN (1 << 6)
> +
> +/* AD9523_PLL2_LOOP_FILTER_CTRL */
> +#define AD9523_PLL2_LOOP_FILTER_CPOLE1(x) (((x) & 0x7) << 0)
> +#define AD9523_PLL2_LOOP_FILTER_RZERO(x) (((x) & 0x7) << 3)
> +#define AD9523_PLL2_LOOP_FILTER_RPOLE2(x) (((x) & 0x7) << 6)
> +#define AD9523_PLL2_LOOP_FILTER_RZERO_BYPASS_EN (1 << 8)
> +
> +/* AD9523_PLL2_R2_DIVIDER */
> +#define AD9523_PLL2_R2_DIVIDER_VAL(x) (((x) & 0x1F) << 0)
> +
> +/* AD9523_CHANNEL_CLOCK_DIST */
> +#define AD9523_CLK_DIST_DIV_PHASE(x) (((x) & 0x3F) << 18)
> +#define AD9523_CLK_DIST_DIV_PHASE_REV(x) ((ret >> 18) & 0x3F)
> +#define AD9523_CLK_DIST_DIV(x) ((((x) - 1) & 0x3FF) << 8)
> +#define AD9523_CLK_DIST_DIV_REV(x) (((ret >> 8) & 0x3FF) + 1)
> +#define AD9523_CLK_DIST_INV_DIV_OUTPUT_EN (1 << 7)
> +#define AD9523_CLK_DIST_IGNORE_SYNC_EN (1 << 6)
> +#define AD9523_CLK_DIST_PWR_DOWN_EN (1 << 5)
> +#define AD9523_CLK_DIST_LOW_PWR_MODE_EN (1 << 4)
> +#define AD9523_CLK_DIST_DRIVER_MODE(x) (((x) & 0xF) << 0)
> +
> +/* AD9523_PLL1_OUTPUT_CTRL */
> +#define AD9523_PLL1_OUTP_CTRL_VCO_DIV_SEL_CH6_M2 (1 << 7)
> +#define AD9523_PLL1_OUTP_CTRL_VCO_DIV_SEL_CH5_M2 (1 << 6)
> +#define AD9523_PLL1_OUTP_CTRL_VCO_DIV_SEL_CH4_M2 (1 << 5)
> +#define AD9523_PLL1_OUTP_CTRL_CMOS_DRV_WEAK (1 << 4)
> +#define AD9523_PLL1_OUTP_CTRL_OUTPUT_DIV_1 (0 << 0)
> +#define AD9523_PLL1_OUTP_CTRL_OUTPUT_DIV_2 (1 << 0)
> +#define AD9523_PLL1_OUTP_CTRL_OUTPUT_DIV_4 (2 << 0)
> +#define AD9523_PLL1_OUTP_CTRL_OUTPUT_DIV_8 (4 << 0)
> +#define AD9523_PLL1_OUTP_CTRL_OUTPUT_DIV_16 (8 << 0)
> +
> +/* AD9523_PLL1_OUTPUT_CHANNEL_CTRL */
> +#define AD9523_PLL1_OUTP_CH_CTRL_OUTPUT_PWR_DOWN_EN (1 << 7)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCO_DIV_SEL_CH9_M2 (1 << 6)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCO_DIV_SEL_CH8_M2 (1 << 5)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCO_DIV_SEL_CH7_M2 (1 << 4)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCXO_SRC_SEL_CH3 (1 << 3)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCXO_SRC_SEL_CH2 (1 << 2)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCXO_SRC_SEL_CH1 (1 << 1)
> +#define AD9523_PLL1_OUTP_CH_CTRL_VCXO_SRC_SEL_CH0 (1 << 0)
> +
> +/* AD9523_READBACK_0 */
> +#define AD9523_READBACK_0_STAT_PLL2_REF_CLK (1 << 7)
> +#define AD9523_READBACK_0_STAT_PLL2_FB_CLK (1 << 6)
> +#define AD9523_READBACK_0_STAT_VCXO (1 << 5)
> +#define AD9523_READBACK_0_STAT_REF_TEST (1 << 4)
> +#define AD9523_READBACK_0_STAT_REFB (1 << 3)
> +#define AD9523_READBACK_0_STAT_REFA (1 << 2)
> +#define AD9523_READBACK_0_STAT_PLL2_LD (1 << 1)
> +#define AD9523_READBACK_0_STAT_PLL1_LD (1 << 0)
> +
> +/* AD9523_READBACK_1 */
> +#define AD9523_READBACK_1_HOLDOVER_ACTIVE (1 << 3)
> +#define AD9523_READBACK_1_AUTOMODE_SEL_REFB (1 << 2)
> +#define AD9523_READBACK_1_VCO_CALIB_IN_PROGRESS (1 << 0)
> +
> +/* AD9523_STATUS_SIGNALS */
> +#define AD9523_STATUS_SIGNALS_SYNC_MAN_CTRL (1 << 16)
> +#define AD9523_STATUS_MONITOR_01_PLL12_LOCKED (0x302)
> +/* AD9523_POWER_DOWN_CTRL */
> +#define AD9523_POWER_DOWN_CTRL_PLL1_PWR_DOWN (1 << 2)
> +#define AD9523_POWER_DOWN_CTRL_PLL2_PWR_DOWN (1 << 1)
> +#define AD9523_POWER_DOWN_CTRL_DIST_PWR_DOWN (1 << 0)
> +
> +/* AD9523_IO_UPDATE */
> +#define AD9523_IO_UPDATE_EN (1 << 0)
> +
> +/* AD9523_EEPROM_DATA_XFER_STATUS */
> +#define AD9523_EEPROM_DATA_XFER_IN_PROGRESS (1 << 0)
> +
> +/* AD9523_EEPROM_ERROR_READBACK */
> +#define AD9523_EEPROM_ERROR_READBACK_FAIL (1 << 0)
> +
> +/* AD9523_EEPROM_CTRL1 */
> +#define AD9523_EEPROM_CTRL1_SOFT_EEPROM (1 << 1)
> +#define AD9523_EEPROM_CTRL1_EEPROM_WRITE_PROT_DIS (1 << 0)
> +
> +/* AD9523_EEPROM_CTRL2 */
> +#define AD9523_EEPROM_CTRL2_REG2EEPROM (1 << 0)
> +
> +#define AD9523_NUM_CHAN 14
> +#define AD9523_NUM_CHAN_ALT_CLK_SRC 10
> +
Hmm. these are convenient but a little nasty. Maybe a comment
and defining the first in terms of the second would help?
> +#define AD_IF(_pde, _a) ((pdata->_pde) ? _a : 0)
> +#define AD_IFE(_pde, _a, _b) ((pdata->_pde) ? _a : _b)
> +
> +enum {
> + AD9523_STAT_PLL1_LD,
> + AD9523_STAT_PLL2_LD,
> + AD9523_STAT_REFA,
> + AD9523_STAT_REFB,
> + AD9523_STAT_REF_TEST,
> + AD9523_STAT_VCXO,
> + AD9523_STAT_PLL2_FB_CLK,
> + AD9523_STAT_PLL2_REF_CLK,
> + AD9523_SYNC,
> + AD9523_EEPROM,
> +};
> +
> +enum {
> + AD9523_VCO1,
> + AD9523_VCO2,
> + AD9523_VCXO,
> + AD9523_NUM_CLK_SRC,
> +};
> +
> +struct ad9523_state {
> + struct spi_device *spi;
> + struct regulator *reg;
> + struct ad9523_platform_data *pdata;
> + struct iio_chan_spec ad9523_channels[AD9523_NUM_CHAN];
> +
> + unsigned long vcxo_freq;
> + unsigned long vco_freq;
> + unsigned long vco_out_freq[AD9523_NUM_CLK_SRC];
> + unsigned char vco_out_map[AD9523_NUM_CHAN_ALT_CLK_SRC];
> +
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + union {
> + __be32 d32;
> + u8 d8[4];
> + } data[2] ____cacheline_aligned;
> +};
> +
> +static int ad9523_read(struct iio_dev *indio_dev, unsigned addr)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + struct spi_message m;
> + int ret;
> + struct spi_transfer t[] = {
> + {
> + .tx_buf = &st->data[0].d8[2],
> + .len = 2,
any reason not to drop this setting of cs_change to it's default?
> + .cs_change = 0,
> + }, {
This address setting bit is a bit cunning. Perhaps a comment will make
it more obvious what you are up to for a casual reader?
> + .rx_buf = &st->data[1].d8[4 - AD9523_TRANSF_LEN(addr)],
> + .len = AD9523_TRANSF_LEN(addr),
> + },
> + };
> +
> + spi_message_init(&m);
> + spi_message_add_tail(&t[0], &m);
> + spi_message_add_tail(&t[1], &m);
> +
> + st->data[0].d32 = cpu_to_be32(AD9523_READ |
> + AD9523_CNT(AD9523_TRANSF_LEN(addr)) |
> + AD9523_ADDR(addr));
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret >= 0)
spi_sync is defined to return zero on success. Is the > necessary
to avoid a false warning?
> + ret = be32_to_cpu(st->data[1].d32) & (0xFFFFFF >>
> + (8 * (3 - AD9523_TRANSF_LEN(addr))));
> + else
> + dev_err(&indio_dev->dev, "read failed (%d)", ret);
> +
> + return ret;
> +};
> +
> +static int ad9523_write(struct iio_dev *indio_dev, unsigned addr, unsigned val)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + struct spi_message m;
> + int ret;
> + struct spi_transfer t[] = {
> + {
> + .tx_buf = &st->data[0].d8[2],
> + .len = 2,
again, why set to default.
> + .cs_change = 0,
> + }, {
> + .tx_buf = &st->data[1].d8[4 - AD9523_TRANSF_LEN(addr)],
> + .len = AD9523_TRANSF_LEN(addr),
> + },
> + };
> +
> + spi_message_init(&m);
> + spi_message_add_tail(&t[0], &m);
> + spi_message_add_tail(&t[1], &m);
> +
> + st->data[0].d32 = cpu_to_be32(AD9523_WRITE |
> + AD9523_CNT(AD9523_TRANSF_LEN(addr)) |
> + AD9523_ADDR(addr));
> + st->data[1].d32 = cpu_to_be32(val);
> +
> + ret = spi_sync(st->spi, &m);
> +
> + if (ret < 0)
> + dev_err(&indio_dev->dev, "write failed (%d)", ret);
> +
> + return ret;
> +}
> +
> +static int ad9523_io_update(struct iio_dev *indio_dev)
> +{
> + return ad9523_write(indio_dev, AD9523_IO_UPDATE, AD9523_IO_UPDATE_EN);
> +}
> +
> +static int ad9523_vco_out_map(struct iio_dev *indio_dev,
> + unsigned ch, bool out)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + int ret;
> + unsigned mask;
> +
> + switch (ch) {
> + case 0 ... 3:
> + ret = ad9523_read(indio_dev, AD9523_PLL1_OUTPUT_CHANNEL_CTRL);
> + if (ret < 0)
> + break;
> + mask = AD9523_PLL1_OUTP_CH_CTRL_VCXO_SRC_SEL_CH0 << ch;
> + if (out) {
> + ret |= mask;
> + out = 2;
> + } else {
> + ret &= ~mask;
> + }
> + ret = ad9523_write(indio_dev,
> + AD9523_PLL1_OUTPUT_CHANNEL_CTRL, ret);
> + break;
> + case 4 ... 6:
> + ret = ad9523_read(indio_dev, AD9523_PLL1_OUTPUT_CTRL);
> + if (ret < 0)
> + break;
> + mask = AD9523_PLL1_OUTP_CTRL_VCO_DIV_SEL_CH4_M2 << (ch - 4);
> + if (out)
> + ret |= mask;
> + else
> + ret &= ~mask;
> + ret = ad9523_write(indio_dev, AD9523_PLL1_OUTPUT_CTRL, ret);
> + break;
> + case 7 ... 9:
> + ret = ad9523_read(indio_dev, AD9523_PLL1_OUTPUT_CHANNEL_CTRL);
> + if (ret < 0)
> + break;
> + mask = AD9523_PLL1_OUTP_CH_CTRL_VCO_DIV_SEL_CH7_M2 << (ch - 7);
> + if (out)
> + ret |= mask;
> + else
> + ret &= ~mask;
> + ret = ad9523_write(indio_dev,
> + AD9523_PLL1_OUTPUT_CHANNEL_CTRL, ret);
> + break;
> + default:
> + return 0;
> + }
> +
> + st->vco_out_map[ch] = out;
blank line please
> + return ret;
> +}
> +
> +static int ad9523_set_clock_provider(struct iio_dev *indio_dev,
> + unsigned ch, unsigned long freq)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + long tmp1, tmp2;
> + bool use_alt_clk_src;
> +
> + switch (ch) {
> + case 0 ... 3:
> + if (freq == st->vco_out_freq[AD9523_VCXO])
> + use_alt_clk_src = true;
> + else
> + use_alt_clk_src = false;
> +
> + break;
> + case 4 ... 9:
> + tmp1 = st->vco_out_freq[AD9523_VCO1] / freq;
> + tmp2 = st->vco_out_freq[AD9523_VCO2] / freq;
> + tmp1 *= freq;
> + tmp2 *= freq;
> + if (abs(tmp1 - freq) > abs(tmp2 - freq))
do this directly?
use_alt_clk_src = (abs(tmp1 - freq) > abs(tmp2 - freq));
> + use_alt_clk_src = true;
> + else
> + use_alt_clk_src = false;
> + break;
> + default:
Is this a valid route? If not I'd just use einval to make it
clear to readers that this shouldn't happen.
> + return 0;
> + }
> +
> + return ad9523_vco_out_map(indio_dev, ch, use_alt_clk_src);
> +}
> +
> +static ssize_t ad9523_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + unsigned long readin, tmp;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &readin);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&indio_dev->mlock);
> + switch ((u32)this_attr->address) {
> + case AD9523_SYNC:
I'd break these two cases out into separate functions. This one
ends up looking ugglier than it really is!
> + ret = ad9523_read(indio_dev, AD9523_STATUS_SIGNALS);
> + if (ret < 0)
> + goto out;
> + tmp = ret;
> + tmp |= AD9523_STATUS_SIGNALS_SYNC_MAN_CTRL;
> + ret = ad9523_write(indio_dev, AD9523_STATUS_SIGNALS, tmp);
> + if (ret < 0)
> + goto out;
> + ad9523_io_update(indio_dev);
> + tmp &= ~AD9523_STATUS_SIGNALS_SYNC_MAN_CTRL;
> + ret = ad9523_write(indio_dev, AD9523_STATUS_SIGNALS, tmp);
> + if (ret < 0)
> + goto out;
> + break;
> + case AD9523_EEPROM:
> + if (readin != 1) {
> + ret = -EINVAL;
> + goto out;
> + }
> + ret = ad9523_write(indio_dev, AD9523_EEPROM_CTRL1,
> + AD9523_EEPROM_CTRL1_EEPROM_WRITE_PROT_DIS);
> + if (ret < 0)
> + goto out;
> + ret = ad9523_write(indio_dev, AD9523_EEPROM_CTRL2,
> + AD9523_EEPROM_CTRL2_REG2EEPROM);
> + if (ret < 0)
> + goto out;
> +
> + tmp = 4;
> + do {
> + msleep(16);
> + ret = ad9523_read(indio_dev,
> + AD9523_EEPROM_DATA_XFER_STATUS);
> + if (ret < 0)
> + goto out;
> + } while ((ret & AD9523_EEPROM_DATA_XFER_IN_PROGRESS) && tmp--);
> +
> + ret = ad9523_write(indio_dev, AD9523_EEPROM_CTRL1, 0);
> + if (ret < 0)
> + goto out;
> +
> + ret = ad9523_read(indio_dev, AD9523_EEPROM_ERROR_READBACK);
> + if (ret < 0)
> + goto out;
> +
> + if (ret & AD9523_EEPROM_ERROR_READBACK_FAIL) {
> + dev_err(&indio_dev->dev, "Verify EEPROM failed");
> + ret = -EIO;
> + }
> + break;
> + default:
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + ret = ad9523_io_update(indio_dev);
> +out:
> + mutex_unlock(&indio_dev->mlock);
> +
> + return ret ? ret : len;
> +}
> +
> +static ssize_t ad9523_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + int ret = 0;
> +
> + mutex_lock(&indio_dev->mlock);
> + ret = ad9523_read(indio_dev, AD9523_READBACK_0);
> + if (ret >= 0) {
> + ret = sprintf(buf, "%s\n", (ret & (1 <<
> + (u32)this_attr->address)) ? "OK" : "FAIL");
> + }
> + mutex_unlock(&indio_dev->mlock);
> +
> + return ret;
> +}
> +
> +static IIO_DEVICE_ATTR(status_pll1_lock_detect, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_PLL1_LD);
> +
> +static IIO_DEVICE_ATTR(status_pll2_lock_detect, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_PLL2_LD);
> +
> +static IIO_DEVICE_ATTR(status_reference_a, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_REFA);
> +
> +static IIO_DEVICE_ATTR(status_reference_b, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_REFB);
> +
> +static IIO_DEVICE_ATTR(status_reference_test, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_REF_TEST);
> +
> +static IIO_DEVICE_ATTR(status_vcxo, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_VCXO);
> +
> +static IIO_DEVICE_ATTR(status_pll2_feedback_clock, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_PLL2_FB_CLK);
> +
> +static IIO_DEVICE_ATTR(status_pll2_reference_clock, S_IRUGO,
> + ad9523_show,
> + NULL,
> + AD9523_STAT_PLL2_REF_CLK);
> +
> +static IIO_DEVICE_ATTR(sync_dividers, S_IWUSR,
> + NULL,
> + ad9523_store,
> + AD9523_SYNC);
> +
> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR,
> + NULL,
> + ad9523_store,
> + AD9523_EEPROM);
> +
> +static struct attribute *ad9523_attributes[] = {
> + &iio_dev_attr_sync_dividers.dev_attr.attr,
> + &iio_dev_attr_store_eeprom.dev_attr.attr,
> + &iio_dev_attr_status_pll1_lock_detect.dev_attr.attr,
> + &iio_dev_attr_status_pll2_lock_detect.dev_attr.attr,
> + &iio_dev_attr_status_reference_a.dev_attr.attr,
> + &iio_dev_attr_status_reference_b.dev_attr.attr,
> + &iio_dev_attr_status_reference_test.dev_attr.attr,
> + &iio_dev_attr_status_vcxo.dev_attr.attr,
> + &iio_dev_attr_status_pll2_feedback_clock.dev_attr.attr,
> + &iio_dev_attr_status_pll2_reference_clock.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group ad9523_attribute_group = {
> + .attrs = ad9523_attributes,
> +};
> +
> +static int ad9523_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long m)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + unsigned code;
> + int ret;
> +
> + mutex_lock(&indio_dev->mlock);
> + ret = ad9523_read(indio_dev, AD9523_CHANNEL_CLOCK_DIST(chan->channel));
> + mutex_unlock(&indio_dev->mlock);
> +
> + if (ret < 0)
> + return ret;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_RAW:
> + *val = !(ret & AD9523_CLK_DIST_PWR_DOWN_EN);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_FREQUENCY:
> + *val = st->vco_out_freq[st->vco_out_map[chan->channel]] /
> + AD9523_CLK_DIST_DIV_REV(ret);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_PHASE:
> + code = (AD9523_CLK_DIST_DIV_PHASE_REV(ret) * 3141592) /
> + AD9523_CLK_DIST_DIV_REV(ret);
> + *val = code / 1000000;
> + *val2 = (code % 1000000) * 10;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> +};
> +
> +static int ad9523_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long mask)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + unsigned reg;
> + int ret, tmp, code;
> +
> + mutex_lock(&indio_dev->mlock);
> + ret = ad9523_read(indio_dev, AD9523_CHANNEL_CLOCK_DIST(chan->channel));
> + if (ret < 0)
> + goto out;
> +
> + reg = ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (val)
> + reg &= ~AD9523_CLK_DIST_PWR_DOWN_EN;
> + else
> + reg |= AD9523_CLK_DIST_PWR_DOWN_EN;
> + break;
> + case IIO_CHAN_INFO_FREQUENCY:
> + ret = ad9523_set_clock_provider(indio_dev, chan->channel, val);
> + if (ret < 0)
> + goto out;
> + tmp = st->vco_out_freq[st->vco_out_map[chan->channel]] / val;
> + tmp = clamp(tmp, 1, 1024);
> + reg &= ~(0x3FF << 8);
> + reg |= AD9523_CLK_DIST_DIV(tmp);
> + break;
> + case IIO_CHAN_INFO_PHASE:
> + code = val * 1000000 + val2 % 1000000;
> + tmp = (code * AD9523_CLK_DIST_DIV_REV(ret)) / 3141592;
> + tmp = clamp(tmp, 0, 63);
> + reg &= ~AD9523_CLK_DIST_DIV_PHASE(~0);
> + reg |= AD9523_CLK_DIST_DIV_PHASE(tmp);
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = ad9523_write(indio_dev, AD9523_CHANNEL_CLOCK_DIST(chan->channel),
> + reg);
> + if (ret < 0)
> + goto out;
> +
> + ad9523_io_update(indio_dev);
> +out:
> + mutex_unlock(&indio_dev->mlock);
> + return ret;
> +}
> +
> +static int ad9523_reg_access(struct iio_dev *indio_dev,
> + unsigned reg, unsigned writeval,
> + unsigned *readval)
> +{
> + int ret;
> +
> + mutex_lock(&indio_dev->mlock);
> + if (readval == NULL) {
> + ret = ad9523_write(indio_dev, reg | AD9523_R1B, writeval);
> + ad9523_io_update(indio_dev);
> + } else {
> + ret = ad9523_read(indio_dev, reg | AD9523_R1B);
> + if (ret < 0)
> + return ret;
> + *readval = ret;
> + ret = 0;
> + }
> + mutex_unlock(&indio_dev->mlock);
> +
> + return ret;
> +}
> +
> +static const struct iio_info ad9523_info = {
> + .read_raw = &ad9523_read_raw,
> + .write_raw = &ad9523_write_raw,
> + .debugfs_reg_access = &ad9523_reg_access,
> + .attrs = &ad9523_attribute_group,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static int ad9523_setup(struct iio_dev *indio_dev)
> +{
> + struct ad9523_state *st = iio_priv(indio_dev);
> + struct ad9523_platform_data *pdata = st->pdata;
> + struct ad9523_channel_spec *chan;
odd spacing above.
> + unsigned active_mask = 0;
> + int ret, i;
> +
> + ret = ad9523_write(indio_dev, AD9523_SERIAL_PORT_CONFIG,
> + AD9523_SER_CONF_SOFT_RESET |
> + (st->spi->mode & SPI_3WIRE ? 0 :
> + AD9523_SER_CONF_SDO_ACTIVE));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_READBACK_CTRL,
> + AD9523_READBACK_CTRL_READ_BUFFERED);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_io_update(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * PLL1 Setup
> + */
> + ret = ad9523_write(indio_dev, AD9523_PLL1_REF_A_DIVIDER,
> + pdata->refa_r_div);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL1_REF_B_DIVIDER,
> + pdata->refb_r_div);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL1_FEEDBACK_DIVIDER,
> + pdata->pll1_feedback_div);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL1_CHARGE_PUMP_CTRL,
> + AD9523_PLL1_CHARGE_PUMP_CURRENT_nA(pdata->
> + pll1_charge_pump_current_nA) |
> + AD9523_PLL1_CHARGE_PUMP_MODE_NORMAL |
> + AD9523_PLL1_BACKLASH_PW_MIN);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL1_INPUT_RECEIVERS_CTRL,
> + AD_IF(refa_diff_rcv_en, AD9523_PLL1_REFA_RCV_EN) |
> + AD_IF(refb_diff_rcv_en, AD9523_PLL1_REFB_RCV_EN) |
> + AD_IF(osc_in_diff_en, AD9523_PLL1_OSC_IN_DIFF_EN) |
> + AD_IF(osc_in_cmos_neg_inp_en,
> + AD9523_PLL1_OSC_IN_CMOS_NEG_INP_EN) |
> + AD_IF(refa_diff_rcv_en, AD9523_PLL1_REFA_DIFF_RCV_EN) |
> + AD_IF(refb_diff_rcv_en, AD9523_PLL1_REFB_DIFF_RCV_EN));
> + if (ret < 0)
> + return ret;
> +
> +
excess blank lines.
> + ret = ad9523_write(indio_dev, AD9523_PLL1_REF_CTRL,
> + AD_IF(zd_in_diff_en, AD9523_PLL1_ZD_IN_DIFF_EN) |
> + AD_IF(zd_in_cmos_neg_inp_en,
> + AD9523_PLL1_ZD_IN_CMOS_NEG_INP_EN) |
> + AD_IF(zero_delay_mode_internal_en,
> + AD9523_PLL1_ZERO_DELAY_MODE_INT) |
> + AD_IF(osc_in_feedback_en, AD9523_PLL1_OSC_IN_PLL_FEEDBACK_EN) |
> + AD_IF(refa_cmos_neg_inp_en, AD9523_PLL1_REFA_CMOS_NEG_INP_EN) |
> + AD_IF(refb_cmos_neg_inp_en, AD9523_PLL1_REFB_CMOS_NEG_INP_EN));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL1_MISC_CTRL,
> + AD9523_PLL1_REFB_INDEP_DIV_CTRL_EN |
> + AD9523_PLL1_REF_MODE(pdata->ref_mode));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL1_LOOP_FILTER_CTRL,
> + AD9523_PLL1_LOOP_FILTER_RZERO(pdata->pll1_loop_filter_rzero));
> + if (ret < 0)
> + return ret;
> + /*
> + * PLL2 Setup
> + */
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_CHARGE_PUMP,
> + AD9523_PLL2_CHARGE_PUMP_CURRENT_nA(pdata->
> + pll2_charge_pump_current_nA));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_FEEDBACK_DIVIDER_AB,
> + AD9523_PLL2_FB_NDIV_A_CNT(pdata->pll2_ndiv_a_cnt) |
> + AD9523_PLL2_FB_NDIV_B_CNT(pdata->pll2_ndiv_b_cnt));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_CTRL,
> + AD9523_PLL2_CHARGE_PUMP_MODE_NORMAL |
> + AD9523_PLL2_BACKLASH_CTRL_EN |
> + AD_IF(pll2_freq_doubler_en, AD9523_PLL2_FREQ_DOUBLER_EN));
> + if (ret < 0)
> + return ret;
> +
> + st->vco_freq = (pdata->vcxo_freq * (pdata->pll2_freq_doubler_en ? 2 : 1)
> + / pdata->pll2_r2_div) * AD9523_PLL2_FB_NDIV(pdata->
> + pll2_ndiv_a_cnt, pdata->pll2_ndiv_b_cnt);
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_VCO_CTRL,
> + AD9523_PLL2_VCO_CALIBRATE);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_VCO_DIVIDER,
> + AD9523_PLL2_VCO_DIV_M1(pdata->pll2_vco_diff_m1) |
> + AD9523_PLL2_VCO_DIV_M2(pdata->pll2_vco_diff_m2) |
> + AD_IFE(pll2_vco_diff_m1, 0,
> + AD9523_PLL2_VCO_DIV_M1_PWR_DOWN_EN) |
> + AD_IFE(pll2_vco_diff_m2, 0,
> + AD9523_PLL2_VCO_DIV_M2_PWR_DOWN_EN));
> + if (ret < 0)
> + return ret;
> +
> + if (pdata->pll2_vco_diff_m1)
> + st->vco_out_freq[AD9523_VCO1] =
> + st->vco_freq / pdata->pll2_vco_diff_m1;
> +
> + if (pdata->pll2_vco_diff_m2)
> + st->vco_out_freq[AD9523_VCO2] =
> + st->vco_freq / pdata->pll2_vco_diff_m2;
> +
> + st->vco_out_freq[AD9523_VCXO] = pdata->vcxo_freq;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_R2_DIVIDER,
> + AD9523_PLL2_R2_DIVIDER_VAL(pdata->pll2_r2_div));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_PLL2_LOOP_FILTER_CTRL,
> + AD9523_PLL2_LOOP_FILTER_CPOLE1(pdata->cpole1) |
> + AD9523_PLL2_LOOP_FILTER_RZERO(pdata->rzero) |
> + AD9523_PLL2_LOOP_FILTER_RPOLE2(pdata->rpole2) |
> + AD_IF(rzero_bypass_en,
> + AD9523_PLL2_LOOP_FILTER_RZERO_BYPASS_EN));
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < pdata->num_channels; i++) {
> + chan = &pdata->channels[i];
> + if (chan->channel_num < AD9523_NUM_CHAN) {
> + active_mask |= (1 << chan->channel_num);
> + ret = ad9523_write(indio_dev,
> + AD9523_CHANNEL_CLOCK_DIST(chan->channel_num),
> + AD9523_CLK_DIST_DRIVER_MODE(chan->driver_mode) |
> + AD9523_CLK_DIST_DIV(chan->channel_divider) |
> + AD9523_CLK_DIST_DIV_PHASE(chan->divider_phase) |
> + (chan->sync_ignore_en ?
> + AD9523_CLK_DIST_IGNORE_SYNC_EN : 0) |
> + (chan->divider_output_invert_en ?
> + AD9523_CLK_DIST_INV_DIV_OUTPUT_EN : 0) |
> + (chan->low_power_mode_en ?
> + AD9523_CLK_DIST_LOW_PWR_MODE_EN : 0) |
> + (chan->output_dis ?
> + AD9523_CLK_DIST_PWR_DOWN_EN : 0));
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_vco_out_map(indio_dev, chan->channel_num,
> + chan->use_alt_clock_src);
> + if (ret < 0)
> + return ret;
> +
Hmm. by making this all dynamic we can't have a nice clean static array
to define things. Please explain why we need to be able to mask
channels in the board config for this device?
I'm not saying it isn't valid, merely that you haven't explained why!
> + st->ad9523_channels[i].type = IIO_ALTVOLTAGE;
> + st->ad9523_channels[i].output = 1;
> + st->ad9523_channels[i].indexed = 1;
> + st->ad9523_channels[i].channel = chan->channel_num;
> + st->ad9523_channels[i].extend_name =
> + chan->extended_name;
> + st->ad9523_channels[i].info_mask =
> + IIO_CHAN_INFO_RAW |
> + IIO_CHAN_INFO_PHASE_SEPARATE_BIT |
> + IIO_CHAN_INFO_FREQUENCY_SEPARATE_BIT;
> + }
> + }
> +
> + for (i = 0; i < AD9523_NUM_CHAN; i++)
> + if (!(active_mask & (1 << i)))
> + ad9523_write(indio_dev,
> + AD9523_CHANNEL_CLOCK_DIST(i),
> + AD9523_CLK_DIST_DRIVER_MODE(TRISTATE) |
> + AD9523_CLK_DIST_PWR_DOWN_EN);
> +
> + ret = ad9523_write(indio_dev, AD9523_POWER_DOWN_CTRL, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_write(indio_dev, AD9523_STATUS_SIGNALS,
> + AD9523_STATUS_MONITOR_01_PLL12_LOCKED);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad9523_io_update(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int __devinit ad9523_probe(struct spi_device *spi)
> +{
> + struct ad9523_platform_data *pdata = spi->dev.platform_data;
> + struct iio_dev *indio_dev;
> + struct ad9523_state *st;
> + int ret;
> +
> + if (!pdata) {
> + dev_err(&spi->dev, "no platform data?\n");
> + return -EINVAL;
> + }
> +
> + indio_dev = iio_device_alloc(sizeof(*st));
> + if (indio_dev == NULL)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> +
> + st->reg = regulator_get(&spi->dev, "vcc");
> + if (!IS_ERR(st->reg)) {
> + ret = regulator_enable(st->reg);
> + if (ret)
> + goto error_put_reg;
> + }
> +
> + spi_set_drvdata(spi, indio_dev);
> + st->spi = spi;
> + st->pdata = pdata;
> +
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->name = (pdata->name[0] != 0) ? pdata->name :
> + spi_get_device_id(spi)->name;
> + indio_dev->info = &ad9523_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = st->ad9523_channels;
> + indio_dev->num_channels = pdata->num_channels;
> +
> + ret = ad9523_setup(indio_dev);
> + if (ret < 0)
> + goto error_disable_reg;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto error_disable_reg;
> +
> + dev_info(&spi->dev, "probed %s\n", indio_dev->name);
> +
> + return 0;
> +
> +error_disable_reg:
> + if (!IS_ERR(st->reg))
> + regulator_disable(st->reg);
> +error_put_reg:
> + if (!IS_ERR(st->reg))
> + regulator_put(st->reg);
> +
> + iio_device_free(indio_dev);
> +
> + return ret;
> +}
> +
> +static int __devexit ad9523_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct ad9523_state *st = iio_priv(indio_dev);
> + struct regulator *reg = st->reg;
> +
> + iio_device_unregister(indio_dev);
> +
> + if (!IS_ERR(reg)) {
> + regulator_disable(reg);
> + regulator_put(reg);
> + }
> +
> + iio_device_free(indio_dev);
> +
> + return 0;
> +}
> +
> +static const struct spi_device_id ad9523_id[] = {
> + {"ad9523", 9523},
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, ad9523_id);
> +
> +static struct spi_driver ad9523_driver = {
> + .driver = {
> + .name = "ad9523",
> + .owner = THIS_MODULE,
> + },
> + .probe = ad9523_probe,
> + .remove = __devexit_p(ad9523_remove),
> + .id_table = ad9523_id,
> +};
> +module_spi_driver(ad9523_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <hennerich@xxxxxxxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Analog Devices AD9523 CLOCKDIST/PLL");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/staging/iio/frequency/ad9523.h b/drivers/staging/iio/frequency/ad9523.h
> new file mode 100644
> index 0000000..17bba03
> --- /dev/null
> +++ b/drivers/staging/iio/frequency/ad9523.h
> @@ -0,0 +1,112 @@
> +/*
> + * AD9523 SPI Low Jitter Clock Generator
> + *
> + * Copyright 2012 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#ifndef IIO_PLL_AD9523_H_
> +#define IIO_PLL_AD9523_H_
> +
> +/*
> + * TODO: move to include/linux/iio
> + */
> +
> +enum outp_drv_mode {
> + TRISTATE,
> + LVPECL_8mA,
> + LVDS_4mA,
> + LVDS_7mA,
> + HSTL0_16mA,
> + HSTL1_8mA,
> + CMOS_CONF1,
> + CMOS_CONF2,
> + CMOS_CONF3,
> + CMOS_CONF4,
> + CMOS_CONF5,
> + CMOS_CONF6,
> + CMOS_CONF7,
> + CMOS_CONF8,
> + CMOS_CONF9
> +};
> +
> +enum ref_sel_mode {
> + NONEREVERTIVE_STAY_ON_REFB,
> + REVERT_TO_REFA,
> + SELECT_REFA,
> + SELECT_REFB,
> + EXT_REF_SEL
> +};
> +
I'd like to see this documented.
> +struct ad9523_channel_spec {
> + unsigned channel_num;
> + bool divider_output_invert_en;
> + bool sync_ignore_en;
> + bool low_power_mode_en;
> + /* CH0..CH3 VCXO, CH4..CH9 VCO2 */
> + bool use_alt_clock_src;
> + bool output_dis;
> + enum outp_drv_mode driver_mode;
> + unsigned char divider_phase;
> + unsigned short channel_divider;
> + char extended_name[16];
> +};
> +
> +/*
> + * struct ad9523_platform_data - platform specific information
> + */
> +
> +struct ad9523_platform_data {
> + unsigned long vcxo_freq;
> +
> + /* Differential/ Single-Ended Input Configuration */
> + bool refa_diff_rcv_en;
> + bool refb_diff_rcv_en;
> + bool zd_in_diff_en;
> + bool osc_in_diff_en;
> +
> + /*
> + * Valid if differential input disabled
> + * if not true defaults to pos input
> + */
> + bool refa_cmos_neg_inp_en;
> + bool refb_cmos_neg_inp_en;
> + bool zd_in_cmos_neg_inp_en;
> + bool osc_in_cmos_neg_inp_en;
> +
> + /* PLL1 Setting */
> + unsigned short refa_r_div;
> + unsigned short refb_r_div;
> + unsigned short pll1_feedback_div;
> + unsigned short pll1_charge_pump_current_nA;
> + bool zero_delay_mode_internal_en;
> + bool osc_in_feedback_en;
> + unsigned char pll1_loop_filter_rzero;
> +
> + /* Reference */
> + enum ref_sel_mode ref_mode;
> +
> + /* PLL2 Setting */
> + unsigned int pll2_charge_pump_current_nA;
> + unsigned char pll2_ndiv_a_cnt;
> + unsigned char pll2_ndiv_b_cnt;
> + bool pll2_freq_doubler_en;
> + unsigned char pll2_r2_div;
> + unsigned char pll2_vco_diff_m1; /* 3..5 */
> + unsigned char pll2_vco_diff_m2; /* 3..5 */
> +
> + /* Loop Filter PLL2 */
> + unsigned char rpole2;
> + unsigned char rzero;
> + unsigned char cpole1;
> + bool rzero_bypass_en;
> +
> + /* Output Channel Configuration */
> + int num_channels;
> + struct ad9523_channel_spec *channels;
> +
> + char name[SPI_NAME_SIZE];
> +};
> +
> +#endif /* IIO_PLL_AD9523_H_ */
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
