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

[Jonathan Cameron <jic23@kernel.org>]

On 06/03/2013 02:30 PM, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Preferably get clkin (PLL reference clock) from clock framework
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>

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.

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

> ---
>  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@blackfin.uclinux.org>");
> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
>  MODULE_DESCRIPTION("Analog Devices ADF4350/ADF4351 PLL");
>  MODULE_LICENSE("GPL v2");
>