Re: [PATCH RESEND V2 3/4] iio: frequency: adf4350: Add support for clock consumer framework

Michael Hennerich <michael.hennerich@xxxxxxxxxx> · Wed, 5 Jun 2013 09:32:04 +0200

On 06/04/2013 07:44 PM, Jonathan Cameron wrote:
On 06/03/2013 02:30 PM, michael.hennerich@xxxxxxxxxx wrote:
From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>

Preferably get clkin (PLL reference clock) from clock framework

Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
ERROR: "clk_round_rate" [drivers/iio/frequency/adf4350.ko] undefined!
make[1]: *** [__modpost] Error 1

on my arm test build. Sorry, I was being lazy before and hadn't done
any test builds till I tried merging it.  Backed out the merge of this
patch for now.

clk.h does say that api is optional for machine classes.  No idea what you want to
do about this.

The CLK API is optional in the driver - so I prefer to keep it like that 
and don't make
the driver depend on COMMON_CLK.

The simplest and probably the best workaround is to guard the CLK 
Round/Set code with
#if defined(CONFIG_COMMON_CLK).

Thoughts?


Incidentally, if you want to play squash the false warnings I also get...

   CC [M]  drivers/iio/frequency/adf4350.o
drivers/iio/frequency/adf4350.c: In function 'adf4350_read':
drivers/iio/frequency/adf4350.c:294:21: warning: 'val' may be used uninitialized in this function

which I don't think is addressed in this series.  IIRC it is an obvious false positive
so I've never bothered mentioning it before :)
Yes it is a false positive - and my compilers doesn't warn on it.
Anyways I can initialize val...

-Michael


---
  drivers/iio/frequency/adf4350.c |   58 +++++++++++++++++++++++++++++++++------
  1 file changed, 49 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index e76d4ac..f6849c8 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -1,7 +1,7 @@
  /*
   * ADF4350/ADF4351 SPI Wideband Synthesizer driver
   *
- * Copyright 2012 Analog Devices Inc.
+ * Copyright 2012-2013 Analog Devices Inc.
   *
   * Licensed under the GPL-2.
   */
@@ -17,6 +17,7 @@
  #include <linux/gcd.h>
  #include <linux/gpio.h>
  #include <asm/div64.h>
+#include <linux/clk.h>
  
  #include <linux/iio/iio.h>
  #include <linux/iio/sysfs.h>
@@ -33,6 +34,7 @@ struct adf4350_state {
  	struct spi_device		*spi;
  	struct regulator		*reg;
  	struct adf4350_platform_data	*pdata;
+	struct clk			*clk;
  	unsigned long			clkin;
  	unsigned long			chspc; /* Channel Spacing */
  	unsigned long			fpfd; /* Phase Frequency Detector */
@@ -43,7 +45,7 @@ struct adf4350_state {
  	unsigned			r4_rf_div_sel;
  	unsigned long			regs[6];
  	unsigned long			regs_hw[6];
-
+	unsigned long long		freq_req;
  	/*
  	 * DMA (thus cache coherency maintenance) requires the
  	 * transfer buffers to live in their own cache lines.
@@ -52,7 +54,6 @@ struct adf4350_state {
  };
  
  static struct adf4350_platform_data default_pdata = {
-	.clkin = 122880000,
  	.channel_spacing = 10000,
  	.r2_user_settings = ADF4350_REG2_PD_POLARITY_POS |
  			    ADF4350_REG2_CHARGE_PUMP_CURR_uA(2500),
@@ -235,6 +236,7 @@ static int adf4350_set_freq(struct adf4350_state *st, unsigned long long freq)
  		ADF4350_REG4_MUTE_TILL_LOCK_EN));
  
  	st->regs[ADF4350_REG5] = ADF4350_REG5_LD_PIN_MODE_DIGITAL;
+	st->freq_req = freq;
  
  	return adf4350_sync_config(st);
  }
@@ -246,6 +248,7 @@ static ssize_t adf4350_write(struct iio_dev *indio_dev,
  {
  	struct adf4350_state *st = iio_priv(indio_dev);
  	unsigned long long readin;
+	unsigned long tmp;
  	int ret;
  
  	ret = kstrtoull(buf, 10, &readin);
@@ -258,10 +261,23 @@ static ssize_t adf4350_write(struct iio_dev *indio_dev,
  		ret = adf4350_set_freq(st, readin);
  		break;
  	case ADF4350_FREQ_REFIN:
-		if (readin > ADF4350_MAX_FREQ_REFIN)
+		if (readin > ADF4350_MAX_FREQ_REFIN) {
  			ret = -EINVAL;
-		else
-			st->clkin = readin;
+			break;
+		}
+
+		if (st->clk) {
+			tmp = clk_round_rate(st->clk, readin);
+			if (tmp != readin) {
+				ret = -EINVAL;
+				break;
+			}
+			ret = clk_set_rate(st->clk, tmp);
+			if (ret < 0)
+				break;
+		}
+		st->clkin = readin;
+		ret = adf4350_set_freq(st, st->freq_req);
  		break;
  	case ADF4350_FREQ_RESOLUTION:
  		if (readin == 0)
@@ -308,6 +324,9 @@ static ssize_t adf4350_read(struct iio_dev *indio_dev,
  			}
  		break;
  	case ADF4350_FREQ_REFIN:
+		if (st->clk)
+			st->clkin = clk_get_rate(st->clk);
+
  		val = st->clkin;
  		break;
  	case ADF4350_FREQ_RESOLUTION:
@@ -318,6 +337,7 @@ static ssize_t adf4350_read(struct iio_dev *indio_dev,
  		break;
  	default:
  		ret = -EINVAL;
+		val = 0;
  	}
  	mutex_unlock(&indio_dev->mlock);
  
@@ -360,14 +380,24 @@ static int adf4350_probe(struct spi_device *spi)
  	struct adf4350_platform_data *pdata = spi->dev.platform_data;
  	struct iio_dev *indio_dev;
  	struct adf4350_state *st;
+	struct clk *clk = NULL;
  	int ret;
  
  	if (!pdata) {
  		dev_warn(&spi->dev, "no platform data? using default\n");
-
  		pdata = &default_pdata;
  	}
  
+	if (!pdata->clkin) {
+		clk = clk_get(&spi->dev, "clkin");
+		if (IS_ERR(clk))
+			return -EPROBE_DEFER;
+
+		ret = clk_prepare_enable(clk);
+		if (ret < 0)
+			return ret;
+	}
+
  	indio_dev = iio_device_alloc(sizeof(*st));
  	if (indio_dev == NULL)
  		return -ENOMEM;
@@ -395,7 +425,12 @@ static int adf4350_probe(struct spi_device *spi)
  	indio_dev->num_channels = 1;
  
  	st->chspc = pdata->channel_spacing;
-	st->clkin = pdata->clkin;
+	if (clk) {
+		st->clk = clk;
+		st->clkin = clk_get_rate(clk);
+	} else {
+		st->clkin = pdata->clkin;
+	}
  
  	st->min_out_freq = spi_get_device_id(spi)->driver_data == 4351 ?
  		ADF4351_MIN_OUT_FREQ : ADF4350_MIN_OUT_FREQ;
@@ -435,6 +470,8 @@ error_put_reg:
  	if (!IS_ERR(st->reg))
  		regulator_put(st->reg);
  
+	if (clk)
+		clk_disable_unprepare(clk);
  	iio_device_free(indio_dev);
  
  	return ret;
@@ -451,6 +488,9 @@ static int adf4350_remove(struct spi_device *spi)
  
  	iio_device_unregister(indio_dev);
  
+	if (st->clk)
+		clk_disable_unprepare(st->clk);
+
  	if (!IS_ERR(reg)) {
  		regulator_disable(reg);
  		regulator_put(reg);
@@ -481,6 +521,6 @@ static struct spi_driver adf4350_driver = {
  };
  module_spi_driver(adf4350_driver);
  
-MODULE_AUTHOR("Michael Hennerich <hennerich@xxxxxxxxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Michael Hennerich <michael.hennerich@xxxxxxxxxx>");
  MODULE_DESCRIPTION("Analog Devices ADF4350/ADF4351 PLL");
  MODULE_LICENSE("GPL v2");



--
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


--
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







