On Thu, 19 Dec 2019 15:37:54 +0200
Beniamin Bia <beniamin.bia@xxxxxxxxxx> wrote:
> From: Stefan Popa <stefan.popa@xxxxxxxxxx>
>
> The ADF4371/ADF4372 devices support individual configurations for the
> output channels. Each child node represents a channel for which
> "power-up-frequency" and "output-enable" optional properties are currently
> supported.
>
> If the "power-up-frequency" is specified for a channel, the driver checks
> if the value provided is in sync with the frequencies set on the other
> channels. This limitation is due to the fact that all the channel
> frequencies are derived from the VCO fundamental frequency.
>
> If the "output-enable" property is specified, then the channel will be
> enabled, otherwise, the driver will initialize the defaults (RF8x will
> be the only enabled channel).
>
> Signed-off-by: Stefan Popa <stefan.popa@xxxxxxxxxx>
> Signed-off-by: Beniamin Bia <beniamin.bia@xxxxxxxxxx>
The code looks good. Question outstanding is the one I raised in review
of the binding doc. What's this for?
Thanks,
Jonathan
> ---
> drivers/iio/frequency/adf4371.c | 79 ++++++++++++++++++++++++++++++++-
> 1 file changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
> index 7d77ebdbea82..e2a599b912e5 100644
> --- a/drivers/iio/frequency/adf4371.c
> +++ b/drivers/iio/frequency/adf4371.c
> @@ -154,10 +154,16 @@ struct adf4371_chip_info {
> const struct iio_chan_spec *channels;
> };
>
> +struct adf4371_channel_config {
> + bool enable;
> + unsigned long long freq;
> +};
> +
> struct adf4371_state {
> struct spi_device *spi;
> struct regmap *regmap;
> struct clk *clkin;
> + struct adf4371_channel_config channel_cfg[4];
> /*
> * Lock for accessing device registers. Some operations require
> * multiple consecutive R/W operations, during which the device
> @@ -175,6 +181,7 @@ struct adf4371_state {
> unsigned int mod2;
> unsigned int rf_div_sel;
> unsigned int ref_div_factor;
> + bool mute_till_lock_en;
> u8 buf[10] ____cacheline_aligned;
> };
>
> @@ -480,6 +487,36 @@ static const struct iio_info adf4371_info = {
> .debugfs_reg_access = &adf4371_reg_access,
> };
>
> +static int adf4371_channel_config(struct adf4371_state *st)
> +{
> + unsigned long long rate;
> + int i, ret;
> +
> + for (i = 0; i < st->chip_info->num_channels; i++) {
> + if (st->channel_cfg[i].freq == 0)
> + continue;
> +
> + rate = adf4371_pll_fract_n_get_rate(st, i);
> + if (rate == 0) {
> + ret = adf4371_set_freq(st, st->channel_cfg[i].freq, i);
> + if (ret < 0)
> + return ret;
> + } else if (rate != st->channel_cfg[i].freq) {
> + dev_err(&st->spi->dev,
> + "Clock rate for chanel %d is not in sync\n", i);
> + return -EINVAL;
> + }
> + /* Powerup channel if the property was specified in the dt */
> + if (st->channel_cfg[i].enable) {
> + ret = adf4371_channel_power_down(st, i, false);
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int adf4371_setup(struct adf4371_state *st)
> {
> unsigned int synth_timeout = 2, timeout = 1, vco_alc_timeout = 1;
> @@ -497,7 +534,7 @@ static int adf4371_setup(struct adf4371_state *st)
> return ret;
>
> /* Mute to Lock Detect */
> - if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en")) {
> + if (st->mute_till_lock_en) {
> ret = regmap_update_bits(st->regmap, ADF4371_REG(0x25),
> ADF4371_MUTE_LD_MSK,
> ADF4371_MUTE_LD(1));
> @@ -545,7 +582,11 @@ static int adf4371_setup(struct adf4371_state *st)
> st->buf[3] = synth_timeout;
> st->buf[4] = ADF4371_VCO_ALC_TOUT(vco_alc_timeout);
>
> - return regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5);
> + ret = regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5);
> + if (ret < 0)
> + return 0;
> +
> + return adf4371_channel_config(st);
> }
>
> static void adf4371_clk_disable(void *data)
> @@ -555,6 +596,36 @@ static void adf4371_clk_disable(void *data)
> clk_disable_unprepare(st->clkin);
> }
>
> +static int adf4371_parse_dt(struct adf4371_state *st)
> +{
> + unsigned char num_channels;
> + unsigned int channel;
> + struct fwnode_handle *child;
> + int ret;
> +
> + if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en"))
> + st->mute_till_lock_en = true;
> +
> + num_channels = device_get_child_node_count(&st->spi->dev);
> + if (num_channels > st->chip_info->num_channels)
> + return -EINVAL;
> +
> + device_for_each_child_node(&st->spi->dev, child) {
> + ret = fwnode_property_read_u32(child, "reg", &channel);
> + if (ret)
> + return ret;
> +
> + ret = fwnode_property_present(child, "adi,output-enable");
> + st->channel_cfg[channel].enable = ret;
> +
> + fwnode_property_read_u64(child,
> + "adi,power-up-frequency",
> + &st->channel_cfg[channel].freq);
> + }
> +
> + return 0;
> +}
> +
> static int adf4371_probe(struct spi_device *spi)
> {
> const struct spi_device_id *id = spi_get_device_id(spi);
> @@ -602,6 +673,10 @@ static int adf4371_probe(struct spi_device *spi)
>
> st->clkin_freq = clk_get_rate(st->clkin);
>
> + ret = adf4371_parse_dt(st);
> + if (ret < 0)
> + return ret;
> +
> ret = adf4371_setup(st);
> if (ret < 0) {
> dev_err(&spi->dev, "ADF4371 setup failed\n");
