diff mbox series

[2/3] iio: frequency: adf4371: Introduce channel child nodes

Message ID 20191219133755.26109-2-beniamin.bia@analog.com (mailing list archive)
State New, archived
Headers show
Series [1/3] iio: frequency: adf4371: Create a power down/up function | expand

Commit Message

Beniamin Bia Dec. 19, 2019, 1:37 p.m. UTC
From: Stefan Popa <stefan.popa@analog.com>

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@analog.com>
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/iio/frequency/adf4371.c | 79 ++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron Dec. 23, 2019, 11:02 a.m. UTC | #1
On Thu, 19 Dec 2019 15:37:54 +0200
Beniamin Bia <beniamin.bia@analog.com> wrote:

> From: Stefan Popa <stefan.popa@analog.com>
> 
> 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@analog.com>
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
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");
diff mbox series

Patch

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");