linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically
@ 2019-02-01  9:36 Beniamin Bia
  2019-02-01  9:36 ` [PATCH 2/2] staging: iio: frequency: ad9833: Load clock using clock framework Beniamin Bia
  2019-02-01 11:24 ` [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Jonathan Cameron
  0 siblings, 2 replies; 5+ messages in thread
From: Beniamin Bia @ 2019-02-01  9:36 UTC (permalink / raw)
  To: linux-iio; +Cc: Beniamin Bia, Beniamin Bia

The value of frequency is taken from ad9834.c instead of platform data

Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/staging/iio/frequency/ad9834.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 995acdd7c942..d92d4bf71261 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -99,6 +99,16 @@ enum ad9834_supported_device_ids {
 	ID_AD9838,
 };
 
+static struct ad9834_platform_data default_config = {
+	.mclk = 25000000,
+	.freq0 = 1000000,
+	.freq1 = 5000000,
+	.phase0 = 512,
+	.phase1 = 1024,
+	.en_div2 = false,
+	.en_signbit_msb_out = false,
+};
+
 static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
@@ -391,16 +401,13 @@ static const struct iio_info ad9833_info = {
 
 static int ad9834_probe(struct spi_device *spi)
 {
-	struct ad9834_platform_data *pdata = dev_get_platdata(&spi->dev);
+	struct ad9834_platform_data *pdata;
 	struct ad9834_state *st;
 	struct iio_dev *indio_dev;
 	struct regulator *reg;
 	int ret;
 
-	if (!pdata) {
-		dev_dbg(&spi->dev, "no platform data?\n");
-		return -ENODEV;
-	}
+	pdata = &default_config;
 
 	reg = devm_regulator_get(&spi->dev, "avdd");
 	if (IS_ERR(reg))
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] staging: iio: frequency: ad9833: Load clock using clock framework
  2019-02-01  9:36 [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Beniamin Bia
@ 2019-02-01  9:36 ` Beniamin Bia
  2019-02-01 11:24 ` [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Jonathan Cameron
  1 sibling, 0 replies; 5+ messages in thread
From: Beniamin Bia @ 2019-02-01  9:36 UTC (permalink / raw)
  To: linux-iio; +Cc: Beniamin Bia, Beniamin Bia

The clock frequency is loaded from device-tree using clock framework
instead of old platform data structure. The change allow configuration of
the device via device-trees and better initialization sequence.
This is part of broader effort to add device-tree support to this driver
and take it out from staging.

Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/staging/iio/frequency/ad9834.c | 24 +++++++++++++++++++-----
 drivers/staging/iio/frequency/ad9834.h |  2 --
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index d92d4bf71261..3b25065c1a72 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -6,6 +6,7 @@
  * Licensed under the GPL-2.
  */
 
+#include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/workqueue.h>
 #include <linux/device.h>
@@ -71,7 +72,7 @@
 struct ad9834_state {
 	struct spi_device		*spi;
 	struct regulator		*reg;
-	unsigned int			mclk;
+	struct clk			*mclk;
 	unsigned short			control;
 	unsigned short			devid;
 	struct spi_transfer		xfer;
@@ -100,7 +101,6 @@ enum ad9834_supported_device_ids {
 };
 
 static struct ad9834_platform_data default_config = {
-	.mclk = 25000000,
 	.freq0 = 1000000,
 	.freq1 = 5000000,
 	.phase0 = 512,
@@ -120,12 +120,15 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 static int ad9834_write_frequency(struct ad9834_state *st,
 				  unsigned long addr, unsigned long fout)
 {
+	unsigned long clk_freq;
 	unsigned long regval;
 
-	if (fout > (st->mclk / 2))
+	clk_freq = clk_get_rate(st->mclk);
+
+	if (fout > (clk_freq / 2))
 		return -EINVAL;
 
-	regval = ad9834_calc_freqreg(st->mclk, fout);
+	regval = ad9834_calc_freqreg(clk_freq, fout);
 
 	st->freq_data[0] = cpu_to_be16(addr | (regval &
 				       RES_MASK(AD9834_FREQ_BITS / 2)));
@@ -405,6 +408,7 @@ static int ad9834_probe(struct spi_device *spi)
 	struct ad9834_state *st;
 	struct iio_dev *indio_dev;
 	struct regulator *reg;
+	struct clk *clk;
 	int ret;
 
 	pdata = &default_config;
@@ -419,6 +423,14 @@ static int ad9834_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	clk = devm_clk_get(&spi->dev, NULL);
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable master clock\n");
+		return ret;
+	}
+
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev) {
 		ret = -ENOMEM;
@@ -427,7 +439,7 @@ static int ad9834_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, indio_dev);
 	st = iio_priv(indio_dev);
 	mutex_init(&st->lock);
-	st->mclk = pdata->mclk;
+	st->mclk = clk;
 	st->spi = spi;
 	st->devid = spi_get_device_id(spi)->driver_data;
 	st->reg = reg;
@@ -501,6 +513,7 @@ static int ad9834_probe(struct spi_device *spi)
 
 error_disable_reg:
 	regulator_disable(reg);
+	clk_disable_unprepare(st->mclk);
 
 	return ret;
 }
@@ -512,6 +525,7 @@ static int ad9834_remove(struct spi_device *spi)
 
 	iio_device_unregister(indio_dev);
 	regulator_disable(st->reg);
+	clk_disable_unprepare(st->mclk);
 
 	return 0;
 }
diff --git a/drivers/staging/iio/frequency/ad9834.h b/drivers/staging/iio/frequency/ad9834.h
index ae620f38eb49..16562dc10812 100644
--- a/drivers/staging/iio/frequency/ad9834.h
+++ b/drivers/staging/iio/frequency/ad9834.h
@@ -14,7 +14,6 @@
 
 /**
  * struct ad9834_platform_data - platform specific information
- * @mclk:		master clock in Hz
  * @freq0:		power up freq0 tuning word in Hz
  * @freq1:		power up freq1 tuning word in Hz
  * @phase0:		power up phase0 value [0..4095] correlates with 0..2PI
@@ -27,7 +26,6 @@
  */
 
 struct ad9834_platform_data {
-	unsigned int		mclk;
 	unsigned int		freq0;
 	unsigned int		freq1;
 	unsigned short		phase0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically
  2019-02-01  9:36 [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Beniamin Bia
  2019-02-01  9:36 ` [PATCH 2/2] staging: iio: frequency: ad9833: Load clock using clock framework Beniamin Bia
@ 2019-02-01 11:24 ` Jonathan Cameron
  2019-02-01 12:25   ` Bia, Beniamin
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2019-02-01 11:24 UTC (permalink / raw)
  To: Beniamin Bia; +Cc: linux-iio, Beniamin Bia

On Fri, 1 Feb 2019 11:36:37 +0200
Beniamin Bia <biabeniamin@gmail.com> wrote:

> The value of frequency is taken from ad9834.c instead of platform data

Why?  I would rather see this move over to DT than take aways the flexibility
that was previously there.

Jonathan

> 
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
> ---
>  drivers/staging/iio/frequency/ad9834.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index 995acdd7c942..d92d4bf71261 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -99,6 +99,16 @@ enum ad9834_supported_device_ids {
>  	ID_AD9838,
>  };
>  
> +static struct ad9834_platform_data default_config = {
> +	.mclk = 25000000,
> +	.freq0 = 1000000,
> +	.freq1 = 5000000,
> +	.phase0 = 512,
> +	.phase1 = 1024,
> +	.en_div2 = false,
> +	.en_signbit_msb_out = false,
> +};
> +
>  static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>  	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
> @@ -391,16 +401,13 @@ static const struct iio_info ad9833_info = {
>  
>  static int ad9834_probe(struct spi_device *spi)
>  {
> -	struct ad9834_platform_data *pdata = dev_get_platdata(&spi->dev);
> +	struct ad9834_platform_data *pdata;
>  	struct ad9834_state *st;
>  	struct iio_dev *indio_dev;
>  	struct regulator *reg;
>  	int ret;
>  
> -	if (!pdata) {
> -		dev_dbg(&spi->dev, "no platform data?\n");
> -		return -ENODEV;
> -	}
> +	pdata = &default_config;
>  
>  	reg = devm_regulator_get(&spi->dev, "avdd");
>  	if (IS_ERR(reg))



^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically
  2019-02-01 11:24 ` [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Jonathan Cameron
@ 2019-02-01 12:25   ` Bia, Beniamin
  2019-02-01 12:42     ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: Bia, Beniamin @ 2019-02-01 12:25 UTC (permalink / raw)
  To: Jonathan Cameron, Beniamin Bia; +Cc: linux-iio

The mclk is loaded from device tree in the next patch. This was just a intermediate step.
But what should i do with the rest of the fields?
Frequency and phase are the default configuration of the device and they don't belong in device tree.
They could also be modified from sysfs.
Should i remove the struct and write directly the value in probe function?

Ben
________________________________________
From: Jonathan Cameron [jonathan.cameron@huawei.com]
Sent: Friday, February 01, 2019 1:24 PM
To: Beniamin Bia
Cc: linux-iio@vger.kernel.org; Bia, Beniamin
Subject: Re: [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically

[External]


On Fri, 1 Feb 2019 11:36:37 +0200
Beniamin Bia <biabeniamin@gmail.com> wrote:

> The value of frequency is taken from ad9834.c instead of platform data

Why?  I would rather see this move over to DT than take aways the flexibility
that was previously there.

Jonathan

>
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
> ---
>  drivers/staging/iio/frequency/ad9834.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index 995acdd7c942..d92d4bf71261 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -99,6 +99,16 @@ enum ad9834_supported_device_ids {
>       ID_AD9838,
>  };
>
> +static struct ad9834_platform_data default_config = {
> +     .mclk = 25000000,
> +     .freq0 = 1000000,
> +     .freq1 = 5000000,
> +     .phase0 = 512,
> +     .phase1 = 1024,
> +     .en_div2 = false,
> +     .en_signbit_msb_out = false,
> +};
> +
>  static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>       unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
> @@ -391,16 +401,13 @@ static const struct iio_info ad9833_info = {
>
>  static int ad9834_probe(struct spi_device *spi)
>  {
> -     struct ad9834_platform_data *pdata = dev_get_platdata(&spi->dev);
> +     struct ad9834_platform_data *pdata;
>       struct ad9834_state *st;
>       struct iio_dev *indio_dev;
>       struct regulator *reg;
>       int ret;
>
> -     if (!pdata) {
> -             dev_dbg(&spi->dev, "no platform data?\n");
> -             return -ENODEV;
> -     }
> +     pdata = &default_config;
>
>       reg = devm_regulator_get(&spi->dev, "avdd");
>       if (IS_ERR(reg))



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically
  2019-02-01 12:25   ` Bia, Beniamin
@ 2019-02-01 12:42     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2019-02-01 12:42 UTC (permalink / raw)
  To: Bia, Beniamin; +Cc: Beniamin Bia, linux-iio

On Fri, 1 Feb 2019 12:25:29 +0000
"Bia, Beniamin" <Beniamin.Bia@analog.com> wrote:

> The mclk is loaded from device tree in the next patch. This was just a intermediate step.
> But what should i do with the rest of the fields?
> Frequency and phase are the default configuration of the device and they don't belong in device tree.
> They could also be modified from sysfs.
> Should i remove the struct and write directly the value in probe function?

Yes, that would make sense.

Jonathan

> 
> Ben
> ________________________________________
> From: Jonathan Cameron [jonathan.cameron@huawei.com]
> Sent: Friday, February 01, 2019 1:24 PM
> To: Beniamin Bia
> Cc: linux-iio@vger.kernel.org; Bia, Beniamin
> Subject: Re: [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically
> 
> [External]
> 
> 
> On Fri, 1 Feb 2019 11:36:37 +0200
> Beniamin Bia <biabeniamin@gmail.com> wrote:
> 
> > The value of frequency is taken from ad9834.c instead of platform data  
> 
> Why?  I would rather see this move over to DT than take aways the flexibility
> that was previously there.
> 
> Jonathan
> 
> >
> > Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
> > ---
> >  drivers/staging/iio/frequency/ad9834.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> > index 995acdd7c942..d92d4bf71261 100644
> > --- a/drivers/staging/iio/frequency/ad9834.c
> > +++ b/drivers/staging/iio/frequency/ad9834.c
> > @@ -99,6 +99,16 @@ enum ad9834_supported_device_ids {
> >       ID_AD9838,
> >  };
> >
> > +static struct ad9834_platform_data default_config = {
> > +     .mclk = 25000000,
> > +     .freq0 = 1000000,
> > +     .freq1 = 5000000,
> > +     .phase0 = 512,
> > +     .phase1 = 1024,
> > +     .en_div2 = false,
> > +     .en_signbit_msb_out = false,
> > +};
> > +
> >  static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
> >  {
> >       unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
> > @@ -391,16 +401,13 @@ static const struct iio_info ad9833_info = {
> >
> >  static int ad9834_probe(struct spi_device *spi)
> >  {
> > -     struct ad9834_platform_data *pdata = dev_get_platdata(&spi->dev);
> > +     struct ad9834_platform_data *pdata;
> >       struct ad9834_state *st;
> >       struct iio_dev *indio_dev;
> >       struct regulator *reg;
> >       int ret;
> >
> > -     if (!pdata) {
> > -             dev_dbg(&spi->dev, "no platform data?\n");
> > -             return -ENODEV;
> > -     }
> > +     pdata = &default_config;
> >
> >       reg = devm_regulator_get(&spi->dev, "avdd");
> >       if (IS_ERR(reg))  
> 
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-02-01 12:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-01  9:36 [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Beniamin Bia
2019-02-01  9:36 ` [PATCH 2/2] staging: iio: frequency: ad9833: Load clock using clock framework Beniamin Bia
2019-02-01 11:24 ` [PATCH 1/2] staging: iio: frequency: ad9833: Get frequency value statically Jonathan Cameron
2019-02-01 12:25   ` Bia, Beniamin
2019-02-01 12:42     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).