diff mbox series

[1/2] staging: iio: frequency: ad9833: Get frequency value statically

Message ID 20190201093638.26068-1-biabeniamin@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] staging: iio: frequency: ad9833: Get frequency value statically | expand

Commit Message

Beniamin Bia Feb. 1, 2019, 9:36 a.m. UTC
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(-)

Comments

Jonathan Cameron Feb. 1, 2019, 11:24 a.m. UTC | #1
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))
Beniamin Bia Feb. 1, 2019, 12:25 p.m. UTC | #2
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
Jonathan Cameron Feb. 1, 2019, 12:42 p.m. UTC | #3
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))  
> 
>
diff mbox series

Patch

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