[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <SJ0PR03MB6778D5319F22812F1D2FBE7899099@SJ0PR03MB6778.namprd03.prod.outlook.com>
Date: Fri, 18 Nov 2022 10:33:03 +0000
From: "Sa, Nuno" <Nuno.Sa@...log.com>
To: "Miclaus, Antoniu" <Antoniu.Miclaus@...log.com>,
"jic23@...nel.org" <jic23@...nel.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "Miclaus, Antoniu" <Antoniu.Miclaus@...log.com>
Subject: RE: [PATCH v3 2/2] iio: frequency: adf4377: add support for ADF4377
> From: Antoniu Miclaus <antoniu.miclaus@...log.com>
> Sent: Tuesday, November 15, 2022 12:01 PM
> To: jic23@...nel.org; robh+dt@...nel.org;
> krzysztof.kozlowski+dt@...aro.org; linux-iio@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org
> Cc: Miclaus, Antoniu <Antoniu.Miclaus@...log.com>
> Subject: [PATCH v3 2/2] iio: frequency: adf4377: add support for ADF4377
>
> [External]
>
> The ADF4377 is a high performance, ultralow jitter, dual output integer-N
> phased locked loop (PLL) with integrated voltage controlled oscillator
> (VCO) ideally suited for data converter and mixed signal front end (MxFE)
> clock applications.
>
> Datasheet: https://www.analog.com/media/en/technical-
> documentation/data-sheets/adf4377.pdf
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> changes in v3:
> - add register name/value for the macro definitions
> - use `__aligned(IIO_DMA_MINALIGN)`
> - add extra spacing for `reg_sequence` structure
> - check frequency value before reg writes in `adf4377_set_freq` function
> - remove ternary operators inside `adf4377_write` and `adf4377_read`
> - remove comma after null terminator
> drivers/iio/frequency/Kconfig | 10 +
> drivers/iio/frequency/Makefile | 1 +
> drivers/iio/frequency/adf4377.c | 994
> ++++++++++++++++++++++++++++++++
> 3 files changed, 1005 insertions(+)
> create mode 100644 drivers/iio/frequency/adf4377.c
>
> diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
> index f3702f36436c..9e85dfa58508 100644
> --- a/drivers/iio/frequency/Kconfig
> +++ b/drivers/iio/frequency/Kconfig
> @@ -50,6 +50,16 @@ config ADF4371
> To compile this driver as a module, choose M here: the
> module will be called adf4371.
>
> +config ADF4377
> + tristate "Analog Devices ADF4377 Microwave Wideband Synthesizer"
> + depends on SPI && COMMON_CLK
> + help
> + Say yes here to build support for Analog Devices ADF4377
> Microwave
> + Wideband Synthesizer.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called adf4377.
> +
> config ADMV1013
> tristate "Analog Devices ADMV1013 Microwave Upconverter"
> depends on SPI && COMMON_CLK
> diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
> index 48add732f1d3..b616c29b4a08 100644
> --- a/drivers/iio/frequency/Makefile
> +++ b/drivers/iio/frequency/Makefile
> @@ -7,6 +7,7 @@
> obj-$(CONFIG_AD9523) += ad9523.o
> obj-$(CONFIG_ADF4350) += adf4350.o
> obj-$(CONFIG_ADF4371) += adf4371.o
> +obj-$(CONFIG_ADF4377) += adf4377.o
> obj-$(CONFIG_ADMV1013) += admv1013.o
> obj-$(CONFIG_ADMV1014) += admv1014.o
> obj-$(CONFIG_ADMV4420) += admv4420.o
> diff --git a/drivers/iio/frequency/adf4377.c
> b/drivers/iio/frequency/adf4377.c
> new file mode 100644
> index 000000000000..26abecbd51e0
> --- /dev/null
> +++ b/drivers/iio/frequency/adf4377.c
> @@ -0,0 +1,994 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADF4377 driver
> + *
> + * Copyright 2022 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/property.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regmap.h>
> +#include <linux/units.h>
> +
> +#include <asm/unaligned.h>
> +
...
> +
> +/* Specifications */
> +#define ADF4377_SPI_READ_CMD BIT(7)
> +#define ADF4377_MAX_VCO_FREQ (12800ULL *
> HZ_PER_MHZ)
> +#define ADF4377_MIN_VCO_FREQ (6400ULL *
> HZ_PER_MHZ)
> +#define ADF4377_MAX_REFIN_FREQ (1000 *
> HZ_PER_MHZ)
> +#define ADF4377_MIN_REFIN_FREQ (10 * HZ_PER_MHZ)
> +#define ADF4377_MAX_FREQ_PFD (500 * HZ_PER_MHZ)
> +#define ADF4377_MIN_FREQ_PFD (3 * HZ_PER_MHZ)
> +#define ADF4377_MAX_CLKPN_FREQ
> ADF4377_MAX_VCO_FREQ
Well, I think we are more than fine in using s64 :). Anyways, as the write
side is still a bit weird, let's stay with extended attr.
...
> +static int adf4377_get_freq(struct adf4377_state *st, u64 *freq)
> +{
> + unsigned int ref_div_factor, n_int;
> + u64 clkin_freq;
> + int ret;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_read(st->regmap, 0x12, &ref_div_factor);
> + if (ret)
> + goto exit;
> +
> + ret = regmap_bulk_read(st->regmap, 0x10, st->buf, sizeof(st->buf));
> + if (ret)
> + goto exit;
> +
> + clkin_freq = clk_get_rate(st->clkin);
> + ref_div_factor = FIELD_GET(ADF4377_0012_R_DIV_MSK,
> ref_div_factor);
> + n_int = FIELD_GET(ADF4377_0010_N_INT_LSB_MSK |
> ADF4377_0011_N_INT_MSB_MSK,
> + get_unaligned_le16(&st->buf));
> +
> + *freq = div_u64(clkin_freq, ref_div_factor) * n_int;
> +exit:
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int adf4377_set_freq(struct adf4377_state *st, u64 freq)
> +{
> + unsigned int read_val;
> + u64 f_vco;
> + int ret;
Could initialize ret to -EINVAL and save one line of code :)...
Anyways...
Reviewed-by: Nuno Sá <nuno.sa@...log.com>
Powered by blists - more mailing lists