On 05/14/2012 03:09 PM, Michael Hennerich wrote:
> On 05/12/2012 08:19 PM, Jonathan Cameron wrote:
>> 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...
> HI Jonathan,
>
> Thanks for the review.
>
> There are a few things in the output channel config, which are
> clearly platform specific, such as the output driver mode.
> A channel without proper output driver set, won't work.
> Things like output driver shouldn't be runtime controllable -
> Therefore we hide unused channels.
Fair enough. Guess it makes sense for output devices...
>
>>> 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!
> REF_TEST is an reference clock bypass test input for PLL1. REF_TEST is
> similar to REFA or REFB, but bypasses the REF selection and switchover
> logic.
> REF_TEST is typically not used.
> I'll add some comment.
Cool.
>
>>> '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.
> Reference_a and reference_b are two separate inputs.
> Do you mean we should better name it.
yes or at least I was putting it up as an alternative.
Actually to be generic maybe a second numeric index rather
than a,b? Or do we think this is obscure enough that
there is no need to generalize that far?
>
> pll2_locked
> pll2_feedback_clk_present
> pll2_reference_clk_present
>
> pll1_locked
> pll1_reference_clk_a_present
> pll1_reference_clk_b_present
> pll1_reference_clk_test_present
>
> With this naming a 0/1 pair would make sense.
Good point.
>
>
>>
>> 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?
> Voltage Controlled Crystal Oscillator.
> X for Crystal is a pretty common.
>>
>>> +
>>> +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?
> Will do - I mainly introduced these to avoid excess line breaks.
>>> +#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?
> leftover by some debug code.
> I'll remove.
>>> + .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?
> I'll add some comments.
cool.
>>> + .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?
> No. I fix it up.
>>> + 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));
> Right...
>>> + 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.
> 10..14 is nothing to do, thus it's a valid route and we return 0,
> instead of taking actions.
Fair enough then. Could you add a comment to that effect please.
>
>>> + 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!
> Ok
>>> + 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!
> See my comment above. Parts of the channel configuration
> belong to platform data. I don't have problems exposing unused ADC inputs,
> however exposing controls for something that could cause unwanted
> radiations
> is a different story. On purpose we explicitly disable all channels that
> are not
> in the pdata channel array.
> If a channel is on purpose configured, it is assumed that it has proper
> termination, and that traces are routed in a fashion that unwanted
> radiation
> is not going to be a problem. (FCC, CE, etc. qualification)
Fair enough.
>
>>
>>> + 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.
> ok
>>> +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
>>
>
>
--
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
