All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: iio: ad9832: Moved contents of header file to source file
@ 2017-02-26 21:43 Narcisa Ana Maria Vasile
  2017-02-27  9:20 ` Lars-Peter Clausen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2017-02-26 21:43 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh
  Cc: linux-iio, outreachy-kernel, Narcisa Ana Maria Vasile

The contents of the header file are used only by this single source file.

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 117 +++++++++++++++++++++++++++++-
 drivers/staging/iio/frequency/ad9832.h | 128 ---------------------------------
 2 files changed, 116 insertions(+), 129 deletions(-)
 delete mode 100644 drivers/staging/iio/frequency/ad9832.h

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index a5b2f06..1b26843 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -20,7 +20,122 @@
 #include <linux/iio/sysfs.h>
 #include "dds.h"
 
-#include "ad9832.h"
+/* Registers */
+
+#define AD9832_FREQ0LL		0x0
+#define AD9832_FREQ0HL		0x1
+#define AD9832_FREQ0LM		0x2
+#define AD9832_FREQ0HM		0x3
+#define AD9832_FREQ1LL		0x4
+#define AD9832_FREQ1HL		0x5
+#define AD9832_FREQ1LM		0x6
+#define AD9832_FREQ1HM		0x7
+#define AD9832_PHASE0L		0x8
+#define AD9832_PHASE0H		0x9
+#define AD9832_PHASE1L		0xA
+#define AD9832_PHASE1H		0xB
+#define AD9832_PHASE2L		0xC
+#define AD9832_PHASE2H		0xD
+#define AD9832_PHASE3L		0xE
+#define AD9832_PHASE3H		0xF
+
+#define AD9832_PHASE_SYM	0x10
+#define AD9832_FREQ_SYM		0x11
+#define AD9832_PINCTRL_EN	0x12
+#define AD9832_OUTPUT_EN	0x13
+
+/* Command Control Bits */
+
+#define AD9832_CMD_PHA8BITSW	0x1
+#define AD9832_CMD_PHA16BITSW	0x0
+#define AD9832_CMD_FRE8BITSW	0x3
+#define AD9832_CMD_FRE16BITSW	0x2
+#define AD9832_CMD_FPSELECT	0x6
+#define AD9832_CMD_SYNCSELSRC	0x8
+#define AD9832_CMD_SLEEPRESCLR	0xC
+
+#define AD9832_FREQ		BIT(11)
+#define AD9832_PHASE(x)		(((x) & 3) << 9)
+#define AD9832_SYNC		BIT(13)
+#define AD9832_SELSRC		BIT(12)
+#define AD9832_SLEEP		BIT(13)
+#define AD9832_RESET		BIT(12)
+#define AD9832_CLR		BIT(11)
+#define CMD_SHIFT		12
+#define ADD_SHIFT		8
+#define AD9832_FREQ_BITS	32
+#define AD9832_PHASE_BITS	12
+#define RES_MASK(bits)		((1 << (bits)) - 1)
+
+/**
+ * struct ad9832_state - driver instance specific data
+ * @spi:		spi_device
+ * @avdd:		supply regulator for the analog section
+ * @dvdd:		supply regulator for the digital section
+ * @mclk:		external master clock
+ * @ctrl_fp:		cached frequency/phase control word
+ * @ctrl_ss:		cached sync/selsrc control word
+ * @ctrl_src:		cached sleep/reset/clr word
+ * @xfer:		default spi transfer
+ * @msg:		default spi message
+ * @freq_xfer:		tuning word spi transfer
+ * @freq_msg:		tuning word spi message
+ * @phase_xfer:		tuning word spi transfer
+ * @phase_msg:		tuning word spi message
+ * @data:		spi transmit buffer
+ * @phase_data:		tuning word spi transmit buffer
+ * @freq_data:		tuning word spi transmit buffer
+ */
+
+struct ad9832_state {
+	struct spi_device		*spi;
+	struct regulator		*avdd;
+	struct regulator		*dvdd;
+	unsigned long			mclk;
+	unsigned short			ctrl_fp;
+	unsigned short			ctrl_ss;
+	unsigned short			ctrl_src;
+	struct spi_transfer		xfer;
+	struct spi_message		msg;
+	struct spi_transfer		freq_xfer[4];
+	struct spi_message		freq_msg;
+	struct spi_transfer		phase_xfer[2];
+	struct spi_message		phase_msg;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	union {
+		__be16			freq_data[4]____cacheline_aligned;
+		__be16			phase_data[2];
+		__be16			data;
+	};
+};
+
+/*
+ * TODO: struct ad9832_platform_data needs to go into include/linux/iio
+ */
+
+/**
+ * struct ad9832_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
+ * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
+ * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
+ * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
+ */
+
+struct ad9832_platform_data {
+	unsigned long		mclk;
+	unsigned long		freq0;
+	unsigned long		freq1;
+	unsigned short		phase0;
+	unsigned short		phase1;
+	unsigned short		phase2;
+	unsigned short		phase3;
+};
 
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
deleted file mode 100644
index 1b08b04..0000000
--- a/drivers/staging/iio/frequency/ad9832.h
+++ /dev/null
@@ -1,128 +0,0 @@
-/*
- * AD9832 SPI DDS driver
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later.
- */
-#ifndef IIO_DDS_AD9832_H_
-#define IIO_DDS_AD9832_H_
-
-/* Registers */
-
-#define AD9832_FREQ0LL		0x0
-#define AD9832_FREQ0HL		0x1
-#define AD9832_FREQ0LM		0x2
-#define AD9832_FREQ0HM		0x3
-#define AD9832_FREQ1LL		0x4
-#define AD9832_FREQ1HL		0x5
-#define AD9832_FREQ1LM		0x6
-#define AD9832_FREQ1HM		0x7
-#define AD9832_PHASE0L		0x8
-#define AD9832_PHASE0H		0x9
-#define AD9832_PHASE1L		0xA
-#define AD9832_PHASE1H		0xB
-#define AD9832_PHASE2L		0xC
-#define AD9832_PHASE2H		0xD
-#define AD9832_PHASE3L		0xE
-#define AD9832_PHASE3H		0xF
-
-#define AD9832_PHASE_SYM	0x10
-#define AD9832_FREQ_SYM		0x11
-#define AD9832_PINCTRL_EN	0x12
-#define AD9832_OUTPUT_EN	0x13
-
-/* Command Control Bits */
-
-#define AD9832_CMD_PHA8BITSW	0x1
-#define AD9832_CMD_PHA16BITSW	0x0
-#define AD9832_CMD_FRE8BITSW	0x3
-#define AD9832_CMD_FRE16BITSW	0x2
-#define AD9832_CMD_FPSELECT	0x6
-#define AD9832_CMD_SYNCSELSRC	0x8
-#define AD9832_CMD_SLEEPRESCLR	0xC
-
-#define AD9832_FREQ		BIT(11)
-#define AD9832_PHASE(x)		(((x) & 3) << 9)
-#define AD9832_SYNC		BIT(13)
-#define AD9832_SELSRC		BIT(12)
-#define AD9832_SLEEP		BIT(13)
-#define AD9832_RESET		BIT(12)
-#define AD9832_CLR		BIT(11)
-#define CMD_SHIFT		12
-#define ADD_SHIFT		8
-#define AD9832_FREQ_BITS	32
-#define AD9832_PHASE_BITS	12
-#define RES_MASK(bits)		((1 << (bits)) - 1)
-
-/**
- * struct ad9832_state - driver instance specific data
- * @spi:		spi_device
- * @avdd:		supply regulator for the analog section
- * @dvdd:		supply regulator for the digital section
- * @mclk:		external master clock
- * @ctrl_fp:		cached frequency/phase control word
- * @ctrl_ss:		cached sync/selsrc control word
- * @ctrl_src:		cached sleep/reset/clr word
- * @xfer:		default spi transfer
- * @msg:		default spi message
- * @freq_xfer:		tuning word spi transfer
- * @freq_msg:		tuning word spi message
- * @phase_xfer:		tuning word spi transfer
- * @phase_msg:		tuning word spi message
- * @data:		spi transmit buffer
- * @phase_data:		tuning word spi transmit buffer
- * @freq_data:		tuning word spi transmit buffer
- */
-
-struct ad9832_state {
-	struct spi_device		*spi;
-	struct regulator		*avdd;
-	struct regulator		*dvdd;
-	unsigned long			mclk;
-	unsigned short			ctrl_fp;
-	unsigned short			ctrl_ss;
-	unsigned short			ctrl_src;
-	struct spi_transfer		xfer;
-	struct spi_message		msg;
-	struct spi_transfer		freq_xfer[4];
-	struct spi_message		freq_msg;
-	struct spi_transfer		phase_xfer[2];
-	struct spi_message		phase_msg;
-	/*
-	 * DMA (thus cache coherency maintenance) requires the
-	 * transfer buffers to live in their own cache lines.
-	 */
-	union {
-		__be16			freq_data[4]____cacheline_aligned;
-		__be16			phase_data[2];
-		__be16			data;
-	};
-};
-
-/*
- * TODO: struct ad9832_platform_data needs to go into include/linux/iio
- */
-
-/**
- * struct ad9832_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
- * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
- * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
- * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
- */
-
-struct ad9832_platform_data {
-	unsigned long		mclk;
-	unsigned long		freq0;
-	unsigned long		freq1;
-	unsigned short		phase0;
-	unsigned short		phase1;
-	unsigned short		phase2;
-	unsigned short		phase3;
-};
-
-#endif /* IIO_DDS_AD9832_H_ */
-- 
1.9.1



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

* Re: [PATCH] staging: iio: ad9832: Moved contents of header file to source file
  2017-02-26 21:43 [PATCH] staging: iio: ad9832: Moved contents of header file to source file Narcisa Ana Maria Vasile
@ 2017-02-27  9:20 ` Lars-Peter Clausen
  2017-02-27 15:29 ` [PATCH v2] " Narcisa Ana Maria Vasile
  2017-02-27 17:28 ` [PATCH v3] staging: iio: ad9832: Moved contents of the header to the " Narcisa Ana Maria Vasile
  2 siblings, 0 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2017-02-27  9:20 UTC (permalink / raw)
  To: Narcisa Ana Maria Vasile, Michael.Hennerich, jic23, knaack.h,
	pmeerw, gregkh
  Cc: linux-iio, outreachy-kernel

On 02/26/2017 10:43 PM, Narcisa Ana Maria Vasile wrote:
> The contents of the header file are used only by this single source file.
> 
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>

Hi,

Moving the registers define and state struct is OK. But the platform data
struct should stay in a separate header as it is supposed to be used from
somewhere else other than the driver. See the TODO comment right above the
struct.

[...]
> +/*
> + * TODO: struct ad9832_platform_data needs to go into include/linux/iio
> + */
> +
> +/**
> + * struct ad9832_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
> + * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
> + * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
> + * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
> + */
> +
> +struct ad9832_platform_data {
> +	unsigned long		mclk;
> +	unsigned long		freq0;
> +	unsigned long		freq1;
> +	unsigned short		phase0;
> +	unsigned short		phase1;
> +	unsigned short		phase2;
> +	unsigned short		phase3;
> +};
>  
>  static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {



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

* [PATCH v2] staging: iio: ad9832: Moved contents of header file to source file
  2017-02-26 21:43 [PATCH] staging: iio: ad9832: Moved contents of header file to source file Narcisa Ana Maria Vasile
  2017-02-27  9:20 ` Lars-Peter Clausen
@ 2017-02-27 15:29 ` Narcisa Ana Maria Vasile
  2017-02-27 15:36   ` [Outreachy kernel] " Daniel Baluta
  2017-02-27 15:37   ` Julia Lawall
  2017-02-27 17:28 ` [PATCH v3] staging: iio: ad9832: Moved contents of the header to the " Narcisa Ana Maria Vasile
  2 siblings, 2 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2017-02-27 15:29 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh
  Cc: outreachy-kernel, linux-iio, Narcisa Ana Maria Vasile

The contents of the header file are used only by this single source
file.

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
Changes in v2:
    Removed the platform data struct from the ad9832.c file and added it
    back to the ad9832.h header file, since this struct is supposed to be
    used from somewhere else, other than the driver.
---
 drivers/staging/iio/frequency/ad9832.c | 27 ++-----------------------
 drivers/staging/iio/frequency/ad9832.h | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 25 deletions(-)
 create mode 100644 drivers/staging/iio/frequency/ad9832.h

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 1b26843..8d40c8e 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -20,6 +20,8 @@
 #include <linux/iio/sysfs.h>
 #include "dds.h"
 
+#include "ad9832.h"
+
 /* Registers */
 
 #define AD9832_FREQ0LL		0x0
@@ -112,31 +114,6 @@ struct ad9832_state {
 	};
 };
 
-/*
- * TODO: struct ad9832_platform_data needs to go into include/linux/iio
- */
-
-/**
- * struct ad9832_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
- * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
- * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
- * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
- */
-
-struct ad9832_platform_data {
-	unsigned long		mclk;
-	unsigned long		freq0;
-	unsigned long		freq1;
-	unsigned short		phase0;
-	unsigned short		phase1;
-	unsigned short		phase2;
-	unsigned short		phase3;
-};
-
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout *
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
new file mode 100644
index 0000000..39d326c
--- /dev/null
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -0,0 +1,36 @@
+/*
+ * AD9832 SPI DDS driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+#ifndef IIO_DDS_AD9832_H_
+#define IIO_DDS_AD9832_H_
+
+/*
+ * TODO: struct ad9832_platform_data needs to go into include/linux/iio
+ */
+
+/**
+ * struct ad9832_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
+ * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
+ * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
+ * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
+ */
+
+struct ad9832_platform_data {
+	unsigned long		mclk;
+	unsigned long		freq0;
+	unsigned long		freq1;
+	unsigned short		phase0;
+	unsigned short		phase1;
+	unsigned short		phase2;
+	unsigned short		phase3;
+};
+
+#endif /* IIO_DDS_AD9832_H_ */
-- 
1.9.1



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

* Re: [Outreachy kernel] [PATCH v2] staging: iio: ad9832: Moved contents of header file to source file
  2017-02-27 15:29 ` [PATCH v2] " Narcisa Ana Maria Vasile
@ 2017-02-27 15:36   ` Daniel Baluta
  2017-02-27 15:37   ` Julia Lawall
  1 sibling, 0 replies; 9+ messages in thread
From: Daniel Baluta @ 2017-02-27 15:36 UTC (permalink / raw)
  To: Narcisa Ana Maria Vasile
  Cc: Lars-Peter Clausen, Hennerich, Michael, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald, Greg Kroah-Hartman,
	outreachy-kernel, linux-iio

On Mon, Feb 27, 2017 at 5:29 PM, Narcisa Ana Maria Vasile
<narcisaanamaria12@gmail.com> wrote:
> The contents of the header file are used only by this single source
> file.
>
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> ---
> Changes in v2:
>     Removed the platform data struct from the ad9832.c file and added it
>     back to the ad9832.h header file, since this struct is supposed to be
>     used from somewhere else, other than the driver.
> ---
>  drivers/staging/iio/frequency/ad9832.c | 27 ++-----------------------
>  drivers/staging/iio/frequency/ad9832.h | 36 ++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 25 deletions(-)
>  create mode 100644 drivers/staging/iio/frequency/ad9832.h
>
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 1b26843..8d40c8e 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -20,6 +20,8 @@
>  #include <linux/iio/sysfs.h>
>  #include "dds.h"
>
> +#include "ad9832.h"
> +
>  /* Registers */
>
>  #define AD9832_FREQ0LL         0x0
> @@ -112,31 +114,6 @@ struct ad9832_state {
>         };
>  };
>
> -/*
> - * TODO: struct ad9832_platform_data needs to go into include/linux/iio
> - */
> -
> -/**
> - * struct ad9832_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
> - * @phase1:            power up phase1 value [0..4095] correlates with 0..2PI
> - * @phase2:            power up phase2 value [0..4095] correlates with 0..2PI
> - * @phase3:            power up phase3 value [0..4095] correlates with 0..2PI
> - */
> -
> -struct ad9832_platform_data {
> -       unsigned long           mclk;
> -       unsigned long           freq0;
> -       unsigned long           freq1;
> -       unsigned short          phase0;
> -       unsigned short          phase1;
> -       unsigned short          phase2;
> -       unsigned short          phase3;
> -};
> -
>  static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>         unsigned long long freqreg = (u64)fout *
> diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
> new file mode 100644
> index 0000000..39d326c
> --- /dev/null
> +++ b/drivers/staging/iio/frequency/ad9832.h
> @@ -0,0 +1,36 @@
> +/*
> + * AD9832 SPI DDS driver
> + *
> + * Copyright 2011 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +#ifndef IIO_DDS_AD9832_H_
> +#define IIO_DDS_AD9832_H_
> +
> +/*
> + * TODO: struct ad9832_platform_data needs to go into include/linux/iio
> + */
> +
> +/**
> + * struct ad9832_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
> + * @phase1:            power up phase1 value [0..4095] correlates with 0..2PI
> + * @phase2:            power up phase2 value [0..4095] correlates with 0..2PI
> + * @phase3:            power up phase3 value [0..4095] correlates with 0..2PI
> + */
> +
> +struct ad9832_platform_data {
> +       unsigned long           mclk;
> +       unsigned long           freq0;
> +       unsigned long           freq1;
> +       unsigned short          phase0;
> +       unsigned short          phase1;
> +       unsigned short          phase2;
> +       unsigned short          phase3;
> +};
> +
> +#endif /* IIO_DDS_AD9832_H_ */

Hi,

It seems that you have sent a diff relative to v1. This isn't OK,
because v1 isn't already applied
to maintainers tree.

Please send a patch against the original code.

thanks,
Daniel.


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

* Re: [Outreachy kernel] [PATCH v2] staging: iio: ad9832: Moved contents of header file to source file
  2017-02-27 15:29 ` [PATCH v2] " Narcisa Ana Maria Vasile
  2017-02-27 15:36   ` [Outreachy kernel] " Daniel Baluta
@ 2017-02-27 15:37   ` Julia Lawall
  1 sibling, 0 replies; 9+ messages in thread
From: Julia Lawall @ 2017-02-27 15:37 UTC (permalink / raw)
  To: Narcisa Ana Maria Vasile
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	outreachy-kernel, linux-iio



On Mon, 27 Feb 2017, Narcisa Ana Maria Vasile wrote:

> The contents of the header file are used only by this single source
> file.
>
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> ---
> Changes in v2:
>     Removed the platform data struct from the ad9832.c file and added it
>     back to the ad9832.h header file, since this struct is supposed to be
>     used from somewhere else, other than the driver.

Did someone accept your previous patch?  Normally, you should always make
your patches against what is in Greg's tree, and not against your own
previous patch.  So I would expect to see only - lines in the .h file; now
you are removing less, but not adding anything.

julia



> ---
>  drivers/staging/iio/frequency/ad9832.c | 27 ++-----------------------
>  drivers/staging/iio/frequency/ad9832.h | 36 ++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 25 deletions(-)
>  create mode 100644 drivers/staging/iio/frequency/ad9832.h
>
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 1b26843..8d40c8e 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -20,6 +20,8 @@
>  #include <linux/iio/sysfs.h>
>  #include "dds.h"
>
> +#include "ad9832.h"
> +
>  /* Registers */
>
>  #define AD9832_FREQ0LL		0x0
> @@ -112,31 +114,6 @@ struct ad9832_state {
>  	};
>  };
>
> -/*
> - * TODO: struct ad9832_platform_data needs to go into include/linux/iio
> - */
> -
> -/**
> - * struct ad9832_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
> - * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
> - * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
> - * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
> - */
> -
> -struct ad9832_platform_data {
> -	unsigned long		mclk;
> -	unsigned long		freq0;
> -	unsigned long		freq1;
> -	unsigned short		phase0;
> -	unsigned short		phase1;
> -	unsigned short		phase2;
> -	unsigned short		phase3;
> -};
> -
>  static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>  	unsigned long long freqreg = (u64)fout *
> diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
> new file mode 100644
> index 0000000..39d326c
> --- /dev/null
> +++ b/drivers/staging/iio/frequency/ad9832.h
> @@ -0,0 +1,36 @@
> +/*
> + * AD9832 SPI DDS driver
> + *
> + * Copyright 2011 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +#ifndef IIO_DDS_AD9832_H_
> +#define IIO_DDS_AD9832_H_
> +
> +/*
> + * TODO: struct ad9832_platform_data needs to go into include/linux/iio
> + */
> +
> +/**
> + * struct ad9832_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
> + * @phase1:		power up phase1 value [0..4095] correlates with 0..2PI
> + * @phase2:		power up phase2 value [0..4095] correlates with 0..2PI
> + * @phase3:		power up phase3 value [0..4095] correlates with 0..2PI
> + */
> +
> +struct ad9832_platform_data {
> +	unsigned long		mclk;
> +	unsigned long		freq0;
> +	unsigned long		freq1;
> +	unsigned short		phase0;
> +	unsigned short		phase1;
> +	unsigned short		phase2;
> +	unsigned short		phase3;
> +};
> +
> +#endif /* IIO_DDS_AD9832_H_ */
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1488209351-351-1-git-send-email-narcisaanamaria12%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


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

* [PATCH v3] staging: iio: ad9832: Moved contents of the header to the source file
  2017-02-26 21:43 [PATCH] staging: iio: ad9832: Moved contents of header file to source file Narcisa Ana Maria Vasile
  2017-02-27  9:20 ` Lars-Peter Clausen
  2017-02-27 15:29 ` [PATCH v2] " Narcisa Ana Maria Vasile
@ 2017-02-27 17:28 ` Narcisa Ana Maria Vasile
  2017-03-01 17:12   ` Narcisa Vasile
  2017-03-02 19:10   ` Jonathan Cameron
  2 siblings, 2 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2017-02-27 17:28 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh
  Cc: outreachy-kernel, linux-iio, Narcisa Ana Maria Vasile

Moved the contents of the header(ad9832.h) into the corresponding source file
with the exception of the platform data struct which is supposed to be
used from somewhere else other than the driver.

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
Changes in v3:
   -The changes are now made against the original code
---
 drivers/staging/iio/frequency/ad9832.c | 92 ++++++++++++++++++++++++++++++++++
 drivers/staging/iio/frequency/ad9832.h | 92 ----------------------------------
 2 files changed, 92 insertions(+), 92 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index a5b2f06..8d40c8e 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -22,6 +22,98 @@
 
 #include "ad9832.h"
 
+/* Registers */
+
+#define AD9832_FREQ0LL		0x0
+#define AD9832_FREQ0HL		0x1
+#define AD9832_FREQ0LM		0x2
+#define AD9832_FREQ0HM		0x3
+#define AD9832_FREQ1LL		0x4
+#define AD9832_FREQ1HL		0x5
+#define AD9832_FREQ1LM		0x6
+#define AD9832_FREQ1HM		0x7
+#define AD9832_PHASE0L		0x8
+#define AD9832_PHASE0H		0x9
+#define AD9832_PHASE1L		0xA
+#define AD9832_PHASE1H		0xB
+#define AD9832_PHASE2L		0xC
+#define AD9832_PHASE2H		0xD
+#define AD9832_PHASE3L		0xE
+#define AD9832_PHASE3H		0xF
+
+#define AD9832_PHASE_SYM	0x10
+#define AD9832_FREQ_SYM		0x11
+#define AD9832_PINCTRL_EN	0x12
+#define AD9832_OUTPUT_EN	0x13
+
+/* Command Control Bits */
+
+#define AD9832_CMD_PHA8BITSW	0x1
+#define AD9832_CMD_PHA16BITSW	0x0
+#define AD9832_CMD_FRE8BITSW	0x3
+#define AD9832_CMD_FRE16BITSW	0x2
+#define AD9832_CMD_FPSELECT	0x6
+#define AD9832_CMD_SYNCSELSRC	0x8
+#define AD9832_CMD_SLEEPRESCLR	0xC
+
+#define AD9832_FREQ		BIT(11)
+#define AD9832_PHASE(x)		(((x) & 3) << 9)
+#define AD9832_SYNC		BIT(13)
+#define AD9832_SELSRC		BIT(12)
+#define AD9832_SLEEP		BIT(13)
+#define AD9832_RESET		BIT(12)
+#define AD9832_CLR		BIT(11)
+#define CMD_SHIFT		12
+#define ADD_SHIFT		8
+#define AD9832_FREQ_BITS	32
+#define AD9832_PHASE_BITS	12
+#define RES_MASK(bits)		((1 << (bits)) - 1)
+
+/**
+ * struct ad9832_state - driver instance specific data
+ * @spi:		spi_device
+ * @avdd:		supply regulator for the analog section
+ * @dvdd:		supply regulator for the digital section
+ * @mclk:		external master clock
+ * @ctrl_fp:		cached frequency/phase control word
+ * @ctrl_ss:		cached sync/selsrc control word
+ * @ctrl_src:		cached sleep/reset/clr word
+ * @xfer:		default spi transfer
+ * @msg:		default spi message
+ * @freq_xfer:		tuning word spi transfer
+ * @freq_msg:		tuning word spi message
+ * @phase_xfer:		tuning word spi transfer
+ * @phase_msg:		tuning word spi message
+ * @data:		spi transmit buffer
+ * @phase_data:		tuning word spi transmit buffer
+ * @freq_data:		tuning word spi transmit buffer
+ */
+
+struct ad9832_state {
+	struct spi_device		*spi;
+	struct regulator		*avdd;
+	struct regulator		*dvdd;
+	unsigned long			mclk;
+	unsigned short			ctrl_fp;
+	unsigned short			ctrl_ss;
+	unsigned short			ctrl_src;
+	struct spi_transfer		xfer;
+	struct spi_message		msg;
+	struct spi_transfer		freq_xfer[4];
+	struct spi_message		freq_msg;
+	struct spi_transfer		phase_xfer[2];
+	struct spi_message		phase_msg;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	union {
+		__be16			freq_data[4]____cacheline_aligned;
+		__be16			phase_data[2];
+		__be16			data;
+	};
+};
+
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout *
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
index 1b08b04..39d326c 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -8,98 +8,6 @@
 #ifndef IIO_DDS_AD9832_H_
 #define IIO_DDS_AD9832_H_
 
-/* Registers */
-
-#define AD9832_FREQ0LL		0x0
-#define AD9832_FREQ0HL		0x1
-#define AD9832_FREQ0LM		0x2
-#define AD9832_FREQ0HM		0x3
-#define AD9832_FREQ1LL		0x4
-#define AD9832_FREQ1HL		0x5
-#define AD9832_FREQ1LM		0x6
-#define AD9832_FREQ1HM		0x7
-#define AD9832_PHASE0L		0x8
-#define AD9832_PHASE0H		0x9
-#define AD9832_PHASE1L		0xA
-#define AD9832_PHASE1H		0xB
-#define AD9832_PHASE2L		0xC
-#define AD9832_PHASE2H		0xD
-#define AD9832_PHASE3L		0xE
-#define AD9832_PHASE3H		0xF
-
-#define AD9832_PHASE_SYM	0x10
-#define AD9832_FREQ_SYM		0x11
-#define AD9832_PINCTRL_EN	0x12
-#define AD9832_OUTPUT_EN	0x13
-
-/* Command Control Bits */
-
-#define AD9832_CMD_PHA8BITSW	0x1
-#define AD9832_CMD_PHA16BITSW	0x0
-#define AD9832_CMD_FRE8BITSW	0x3
-#define AD9832_CMD_FRE16BITSW	0x2
-#define AD9832_CMD_FPSELECT	0x6
-#define AD9832_CMD_SYNCSELSRC	0x8
-#define AD9832_CMD_SLEEPRESCLR	0xC
-
-#define AD9832_FREQ		BIT(11)
-#define AD9832_PHASE(x)		(((x) & 3) << 9)
-#define AD9832_SYNC		BIT(13)
-#define AD9832_SELSRC		BIT(12)
-#define AD9832_SLEEP		BIT(13)
-#define AD9832_RESET		BIT(12)
-#define AD9832_CLR		BIT(11)
-#define CMD_SHIFT		12
-#define ADD_SHIFT		8
-#define AD9832_FREQ_BITS	32
-#define AD9832_PHASE_BITS	12
-#define RES_MASK(bits)		((1 << (bits)) - 1)
-
-/**
- * struct ad9832_state - driver instance specific data
- * @spi:		spi_device
- * @avdd:		supply regulator for the analog section
- * @dvdd:		supply regulator for the digital section
- * @mclk:		external master clock
- * @ctrl_fp:		cached frequency/phase control word
- * @ctrl_ss:		cached sync/selsrc control word
- * @ctrl_src:		cached sleep/reset/clr word
- * @xfer:		default spi transfer
- * @msg:		default spi message
- * @freq_xfer:		tuning word spi transfer
- * @freq_msg:		tuning word spi message
- * @phase_xfer:		tuning word spi transfer
- * @phase_msg:		tuning word spi message
- * @data:		spi transmit buffer
- * @phase_data:		tuning word spi transmit buffer
- * @freq_data:		tuning word spi transmit buffer
- */
-
-struct ad9832_state {
-	struct spi_device		*spi;
-	struct regulator		*avdd;
-	struct regulator		*dvdd;
-	unsigned long			mclk;
-	unsigned short			ctrl_fp;
-	unsigned short			ctrl_ss;
-	unsigned short			ctrl_src;
-	struct spi_transfer		xfer;
-	struct spi_message		msg;
-	struct spi_transfer		freq_xfer[4];
-	struct spi_message		freq_msg;
-	struct spi_transfer		phase_xfer[2];
-	struct spi_message		phase_msg;
-	/*
-	 * DMA (thus cache coherency maintenance) requires the
-	 * transfer buffers to live in their own cache lines.
-	 */
-	union {
-		__be16			freq_data[4]____cacheline_aligned;
-		__be16			phase_data[2];
-		__be16			data;
-	};
-};
-
 /*
  * TODO: struct ad9832_platform_data needs to go into include/linux/iio
  */
-- 
1.9.1



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

* Re: [PATCH v3] staging: iio: ad9832: Moved contents of the header to the source file
  2017-02-27 17:28 ` [PATCH v3] staging: iio: ad9832: Moved contents of the header to the " Narcisa Ana Maria Vasile
@ 2017-03-01 17:12   ` Narcisa Vasile
  2017-03-01 18:14     ` Lars-Peter Clausen
  2017-03-02 19:10   ` Jonathan Cameron
  1 sibling, 1 reply; 9+ messages in thread
From: Narcisa Vasile @ 2017-03-01 17:12 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	linux-iio, narcisaanamaria12


[-- Attachment #1.1: Type: text/plain, Size: 9701 bytes --]

Hi,
Is this patch correct now or should I improve it ?

Narcisa

On Monday, February 27, 2017 at 7:28:46 PM UTC+2, Narcisa Vasile wrote:
>
> Moved the contents of the header(ad9832.h) into the corresponding source 
> file 
> with the exception of the platform data struct which is supposed to be 
> used from somewhere else other than the driver. 
>
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com> 
> --- 
> Changes in v3: 
>    -The changes are now made against the original code 
> --- 
>  drivers/staging/iio/frequency/ad9832.c | 92 
> ++++++++++++++++++++++++++++++++++ 
>  drivers/staging/iio/frequency/ad9832.h | 92 
> ---------------------------------- 
>  2 files changed, 92 insertions(+), 92 deletions(-) 
>
> diff --git a/drivers/staging/iio/frequency/ad9832.c 
> b/drivers/staging/iio/frequency/ad9832.c 
> index a5b2f06..8d40c8e 100644 
> --- a/drivers/staging/iio/frequency/ad9832.c 
> +++ b/drivers/staging/iio/frequency/ad9832.c 
> @@ -22,6 +22,98 @@ 
>   
>  #include "ad9832.h" 
>   
> +/* Registers */ 
> + 
> +#define AD9832_FREQ0LL                0x0 
> +#define AD9832_FREQ0HL                0x1 
> +#define AD9832_FREQ0LM                0x2 
> +#define AD9832_FREQ0HM                0x3 
> +#define AD9832_FREQ1LL                0x4 
> +#define AD9832_FREQ1HL                0x5 
> +#define AD9832_FREQ1LM                0x6 
> +#define AD9832_FREQ1HM                0x7 
> +#define AD9832_PHASE0L                0x8 
> +#define AD9832_PHASE0H                0x9 
> +#define AD9832_PHASE1L                0xA 
> +#define AD9832_PHASE1H                0xB 
> +#define AD9832_PHASE2L                0xC 
> +#define AD9832_PHASE2H                0xD 
> +#define AD9832_PHASE3L                0xE 
> +#define AD9832_PHASE3H                0xF 
> + 
> +#define AD9832_PHASE_SYM        0x10 
> +#define AD9832_FREQ_SYM                0x11 
> +#define AD9832_PINCTRL_EN        0x12 
> +#define AD9832_OUTPUT_EN        0x13 
> + 
> +/* Command Control Bits */ 
> + 
> +#define AD9832_CMD_PHA8BITSW        0x1 
> +#define AD9832_CMD_PHA16BITSW        0x0 
> +#define AD9832_CMD_FRE8BITSW        0x3 
> +#define AD9832_CMD_FRE16BITSW        0x2 
> +#define AD9832_CMD_FPSELECT        0x6 
> +#define AD9832_CMD_SYNCSELSRC        0x8 
> +#define AD9832_CMD_SLEEPRESCLR        0xC 
> + 
> +#define AD9832_FREQ                BIT(11) 
> +#define AD9832_PHASE(x)                (((x) & 3) << 9) 
> +#define AD9832_SYNC                BIT(13) 
> +#define AD9832_SELSRC                BIT(12) 
> +#define AD9832_SLEEP                BIT(13) 
> +#define AD9832_RESET                BIT(12) 
> +#define AD9832_CLR                BIT(11) 
> +#define CMD_SHIFT                12 
> +#define ADD_SHIFT                8 
> +#define AD9832_FREQ_BITS        32 
> +#define AD9832_PHASE_BITS        12 
> +#define RES_MASK(bits)                ((1 << (bits)) - 1) 
> + 
> +/** 
> + * struct ad9832_state - driver instance specific data 
> + * @spi:                spi_device 
> + * @avdd:                supply regulator for the analog section 
> + * @dvdd:                supply regulator for the digital section 
> + * @mclk:                external master clock 
> + * @ctrl_fp:                cached frequency/phase control word 
> + * @ctrl_ss:                cached sync/selsrc control word 
> + * @ctrl_src:                cached sleep/reset/clr word 
> + * @xfer:                default spi transfer 
> + * @msg:                default spi message 
> + * @freq_xfer:                tuning word spi transfer 
> + * @freq_msg:                tuning word spi message 
> + * @phase_xfer:                tuning word spi transfer 
> + * @phase_msg:                tuning word spi message 
> + * @data:                spi transmit buffer 
> + * @phase_data:                tuning word spi transmit buffer 
> + * @freq_data:                tuning word spi transmit buffer 
> + */ 
> + 
> +struct ad9832_state { 
> +        struct spi_device                *spi; 
> +        struct regulator                *avdd; 
> +        struct regulator                *dvdd; 
> +        unsigned long                        mclk; 
> +        unsigned short                        ctrl_fp; 
> +        unsigned short                        ctrl_ss; 
> +        unsigned short                        ctrl_src; 
> +        struct spi_transfer                xfer; 
> +        struct spi_message                msg; 
> +        struct spi_transfer                freq_xfer[4]; 
> +        struct spi_message                freq_msg; 
> +        struct spi_transfer                phase_xfer[2]; 
> +        struct spi_message                phase_msg; 
> +        /* 
> +         * DMA (thus cache coherency maintenance) requires the 
> +         * transfer buffers to live in their own cache lines. 
> +         */ 
> +        union { 
> +                __be16                        freq_data[4]____cacheline_aligned; 
>
> +                __be16                        phase_data[2]; 
> +                __be16                        data; 
> +        }; 
> +}; 
> + 
>  static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned 
> long fout) 
>  { 
>          unsigned long long freqreg = (u64)fout * 
> diff --git a/drivers/staging/iio/frequency/ad9832.h 
> b/drivers/staging/iio/frequency/ad9832.h 
> index 1b08b04..39d326c 100644 
> --- a/drivers/staging/iio/frequency/ad9832.h 
> +++ b/drivers/staging/iio/frequency/ad9832.h 
> @@ -8,98 +8,6 @@ 
>  #ifndef IIO_DDS_AD9832_H_ 
>  #define IIO_DDS_AD9832_H_ 
>   
> -/* Registers */ 
> - 
> -#define AD9832_FREQ0LL                0x0 
> -#define AD9832_FREQ0HL                0x1 
> -#define AD9832_FREQ0LM                0x2 
> -#define AD9832_FREQ0HM                0x3 
> -#define AD9832_FREQ1LL                0x4 
> -#define AD9832_FREQ1HL                0x5 
> -#define AD9832_FREQ1LM                0x6 
> -#define AD9832_FREQ1HM                0x7 
> -#define AD9832_PHASE0L                0x8 
> -#define AD9832_PHASE0H                0x9 
> -#define AD9832_PHASE1L                0xA 
> -#define AD9832_PHASE1H                0xB 
> -#define AD9832_PHASE2L                0xC 
> -#define AD9832_PHASE2H                0xD 
> -#define AD9832_PHASE3L                0xE 
> -#define AD9832_PHASE3H                0xF 
> - 
> -#define AD9832_PHASE_SYM        0x10 
> -#define AD9832_FREQ_SYM                0x11 
> -#define AD9832_PINCTRL_EN        0x12 
> -#define AD9832_OUTPUT_EN        0x13 
> - 
> -/* Command Control Bits */ 
> - 
> -#define AD9832_CMD_PHA8BITSW        0x1 
> -#define AD9832_CMD_PHA16BITSW        0x0 
> -#define AD9832_CMD_FRE8BITSW        0x3 
> -#define AD9832_CMD_FRE16BITSW        0x2 
> -#define AD9832_CMD_FPSELECT        0x6 
> -#define AD9832_CMD_SYNCSELSRC        0x8 
> -#define AD9832_CMD_SLEEPRESCLR        0xC 
> - 
> -#define AD9832_FREQ                BIT(11) 
> -#define AD9832_PHASE(x)                (((x) & 3) << 9) 
> -#define AD9832_SYNC                BIT(13) 
> -#define AD9832_SELSRC                BIT(12) 
> -#define AD9832_SLEEP                BIT(13) 
> -#define AD9832_RESET                BIT(12) 
> -#define AD9832_CLR                BIT(11) 
> -#define CMD_SHIFT                12 
> -#define ADD_SHIFT                8 
> -#define AD9832_FREQ_BITS        32 
> -#define AD9832_PHASE_BITS        12 
> -#define RES_MASK(bits)                ((1 << (bits)) - 1) 
> - 
> -/** 
> - * struct ad9832_state - driver instance specific data 
> - * @spi:                spi_device 
> - * @avdd:                supply regulator for the analog section 
> - * @dvdd:                supply regulator for the digital section 
> - * @mclk:                external master clock 
> - * @ctrl_fp:                cached frequency/phase control word 
> - * @ctrl_ss:                cached sync/selsrc control word 
> - * @ctrl_src:                cached sleep/reset/clr word 
> - * @xfer:                default spi transfer 
> - * @msg:                default spi message 
> - * @freq_xfer:                tuning word spi transfer 
> - * @freq_msg:                tuning word spi message 
> - * @phase_xfer:                tuning word spi transfer 
> - * @phase_msg:                tuning word spi message 
> - * @data:                spi transmit buffer 
> - * @phase_data:                tuning word spi transmit buffer 
> - * @freq_data:                tuning word spi transmit buffer 
> - */ 
> - 
> -struct ad9832_state { 
> -        struct spi_device                *spi; 
> -        struct regulator                *avdd; 
> -        struct regulator                *dvdd; 
> -        unsigned long                        mclk; 
> -        unsigned short                        ctrl_fp; 
> -        unsigned short                        ctrl_ss; 
> -        unsigned short                        ctrl_src; 
> -        struct spi_transfer                xfer; 
> -        struct spi_message                msg; 
> -        struct spi_transfer                freq_xfer[4]; 
> -        struct spi_message                freq_msg; 
> -        struct spi_transfer                phase_xfer[2]; 
> -        struct spi_message                phase_msg; 
> -        /* 
> -         * DMA (thus cache coherency maintenance) requires the 
> -         * transfer buffers to live in their own cache lines. 
> -         */ 
> -        union { 
> -                __be16                        freq_data[4]____cacheline_aligned; 
>
> -                __be16                        phase_data[2]; 
> -                __be16                        data; 
> -        }; 
> -}; 
> - 
>  /* 
>   * TODO: struct ad9832_platform_data needs to go into include/linux/iio 
>   */ 
> -- 
> 1.9.1 
>
>

[-- Attachment #1.2: Type: text/html, Size: 13846 bytes --]

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

* Re: [PATCH v3] staging: iio: ad9832: Moved contents of the header to the source file
  2017-03-01 17:12   ` Narcisa Vasile
@ 2017-03-01 18:14     ` Lars-Peter Clausen
  0 siblings, 0 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2017-03-01 18:14 UTC (permalink / raw)
  To: Narcisa Vasile, outreachy-kernel
  Cc: Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, linux-iio

On 03/01/2017 06:12 PM, Narcisa Vasile wrote:
> Hi,
> Is this patch correct now or should I improve it ?

Patch looks OK. But in general you need to allow some time for review.
People are often busy with work, tight deadlines, etc. Jonathan for example
who will apply your patch works on this in his spare time. So it might take
a few days before somebody gets the time to look at your patch. Asking about
the state of the patch after a few days is just one more e-mail to read for
the maintainer. I understand that for you your patch is the most important
patch, but maintainers get bombarded with tens if not even hundreds of
patches a day and they have to care about them all equally.



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

* Re: [PATCH v3] staging: iio: ad9832: Moved contents of the header to the source file
  2017-02-27 17:28 ` [PATCH v3] staging: iio: ad9832: Moved contents of the header to the " Narcisa Ana Maria Vasile
  2017-03-01 17:12   ` Narcisa Vasile
@ 2017-03-02 19:10   ` Jonathan Cameron
  1 sibling, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2017-03-02 19:10 UTC (permalink / raw)
  To: Narcisa Ana Maria Vasile, lars, Michael.Hennerich, knaack.h,
	pmeerw, gregkh
  Cc: outreachy-kernel, linux-iio

On 27/02/17 17:28, Narcisa Ana Maria Vasile wrote:
> Moved the contents of the header(ad9832.h) into the corresponding source file
> with the exception of the platform data struct which is supposed to be
> used from somewhere else other than the driver.
> 
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to check we haven't missed anything!

Thanks,

Jonathan
> ---
> Changes in v3:
>    -The changes are now made against the original code
> ---
>  drivers/staging/iio/frequency/ad9832.c | 92 ++++++++++++++++++++++++++++++++++
>  drivers/staging/iio/frequency/ad9832.h | 92 ----------------------------------
>  2 files changed, 92 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index a5b2f06..8d40c8e 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -22,6 +22,98 @@
>  
>  #include "ad9832.h"
>  
> +/* Registers */
> +
> +#define AD9832_FREQ0LL		0x0
> +#define AD9832_FREQ0HL		0x1
> +#define AD9832_FREQ0LM		0x2
> +#define AD9832_FREQ0HM		0x3
> +#define AD9832_FREQ1LL		0x4
> +#define AD9832_FREQ1HL		0x5
> +#define AD9832_FREQ1LM		0x6
> +#define AD9832_FREQ1HM		0x7
> +#define AD9832_PHASE0L		0x8
> +#define AD9832_PHASE0H		0x9
> +#define AD9832_PHASE1L		0xA
> +#define AD9832_PHASE1H		0xB
> +#define AD9832_PHASE2L		0xC
> +#define AD9832_PHASE2H		0xD
> +#define AD9832_PHASE3L		0xE
> +#define AD9832_PHASE3H		0xF
> +
> +#define AD9832_PHASE_SYM	0x10
> +#define AD9832_FREQ_SYM		0x11
> +#define AD9832_PINCTRL_EN	0x12
> +#define AD9832_OUTPUT_EN	0x13
> +
> +/* Command Control Bits */
> +
> +#define AD9832_CMD_PHA8BITSW	0x1
> +#define AD9832_CMD_PHA16BITSW	0x0
> +#define AD9832_CMD_FRE8BITSW	0x3
> +#define AD9832_CMD_FRE16BITSW	0x2
> +#define AD9832_CMD_FPSELECT	0x6
> +#define AD9832_CMD_SYNCSELSRC	0x8
> +#define AD9832_CMD_SLEEPRESCLR	0xC
> +
> +#define AD9832_FREQ		BIT(11)
> +#define AD9832_PHASE(x)		(((x) & 3) << 9)
> +#define AD9832_SYNC		BIT(13)
> +#define AD9832_SELSRC		BIT(12)
> +#define AD9832_SLEEP		BIT(13)
> +#define AD9832_RESET		BIT(12)
> +#define AD9832_CLR		BIT(11)
> +#define CMD_SHIFT		12
> +#define ADD_SHIFT		8
> +#define AD9832_FREQ_BITS	32
> +#define AD9832_PHASE_BITS	12
> +#define RES_MASK(bits)		((1 << (bits)) - 1)
> +
> +/**
> + * struct ad9832_state - driver instance specific data
> + * @spi:		spi_device
> + * @avdd:		supply regulator for the analog section
> + * @dvdd:		supply regulator for the digital section
> + * @mclk:		external master clock
> + * @ctrl_fp:		cached frequency/phase control word
> + * @ctrl_ss:		cached sync/selsrc control word
> + * @ctrl_src:		cached sleep/reset/clr word
> + * @xfer:		default spi transfer
> + * @msg:		default spi message
> + * @freq_xfer:		tuning word spi transfer
> + * @freq_msg:		tuning word spi message
> + * @phase_xfer:		tuning word spi transfer
> + * @phase_msg:		tuning word spi message
> + * @data:		spi transmit buffer
> + * @phase_data:		tuning word spi transmit buffer
> + * @freq_data:		tuning word spi transmit buffer
> + */
> +
> +struct ad9832_state {
> +	struct spi_device		*spi;
> +	struct regulator		*avdd;
> +	struct regulator		*dvdd;
> +	unsigned long			mclk;
> +	unsigned short			ctrl_fp;
> +	unsigned short			ctrl_ss;
> +	unsigned short			ctrl_src;
> +	struct spi_transfer		xfer;
> +	struct spi_message		msg;
> +	struct spi_transfer		freq_xfer[4];
> +	struct spi_message		freq_msg;
> +	struct spi_transfer		phase_xfer[2];
> +	struct spi_message		phase_msg;
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
> +	 */
> +	union {
> +		__be16			freq_data[4]____cacheline_aligned;
> +		__be16			phase_data[2];
> +		__be16			data;
> +	};
> +};
> +
>  static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>  	unsigned long long freqreg = (u64)fout *
> diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
> index 1b08b04..39d326c 100644
> --- a/drivers/staging/iio/frequency/ad9832.h
> +++ b/drivers/staging/iio/frequency/ad9832.h
> @@ -8,98 +8,6 @@
>  #ifndef IIO_DDS_AD9832_H_
>  #define IIO_DDS_AD9832_H_
>  
> -/* Registers */
> -
> -#define AD9832_FREQ0LL		0x0
> -#define AD9832_FREQ0HL		0x1
> -#define AD9832_FREQ0LM		0x2
> -#define AD9832_FREQ0HM		0x3
> -#define AD9832_FREQ1LL		0x4
> -#define AD9832_FREQ1HL		0x5
> -#define AD9832_FREQ1LM		0x6
> -#define AD9832_FREQ1HM		0x7
> -#define AD9832_PHASE0L		0x8
> -#define AD9832_PHASE0H		0x9
> -#define AD9832_PHASE1L		0xA
> -#define AD9832_PHASE1H		0xB
> -#define AD9832_PHASE2L		0xC
> -#define AD9832_PHASE2H		0xD
> -#define AD9832_PHASE3L		0xE
> -#define AD9832_PHASE3H		0xF
> -
> -#define AD9832_PHASE_SYM	0x10
> -#define AD9832_FREQ_SYM		0x11
> -#define AD9832_PINCTRL_EN	0x12
> -#define AD9832_OUTPUT_EN	0x13
> -
> -/* Command Control Bits */
> -
> -#define AD9832_CMD_PHA8BITSW	0x1
> -#define AD9832_CMD_PHA16BITSW	0x0
> -#define AD9832_CMD_FRE8BITSW	0x3
> -#define AD9832_CMD_FRE16BITSW	0x2
> -#define AD9832_CMD_FPSELECT	0x6
> -#define AD9832_CMD_SYNCSELSRC	0x8
> -#define AD9832_CMD_SLEEPRESCLR	0xC
> -
> -#define AD9832_FREQ		BIT(11)
> -#define AD9832_PHASE(x)		(((x) & 3) << 9)
> -#define AD9832_SYNC		BIT(13)
> -#define AD9832_SELSRC		BIT(12)
> -#define AD9832_SLEEP		BIT(13)
> -#define AD9832_RESET		BIT(12)
> -#define AD9832_CLR		BIT(11)
> -#define CMD_SHIFT		12
> -#define ADD_SHIFT		8
> -#define AD9832_FREQ_BITS	32
> -#define AD9832_PHASE_BITS	12
> -#define RES_MASK(bits)		((1 << (bits)) - 1)
> -
> -/**
> - * struct ad9832_state - driver instance specific data
> - * @spi:		spi_device
> - * @avdd:		supply regulator for the analog section
> - * @dvdd:		supply regulator for the digital section
> - * @mclk:		external master clock
> - * @ctrl_fp:		cached frequency/phase control word
> - * @ctrl_ss:		cached sync/selsrc control word
> - * @ctrl_src:		cached sleep/reset/clr word
> - * @xfer:		default spi transfer
> - * @msg:		default spi message
> - * @freq_xfer:		tuning word spi transfer
> - * @freq_msg:		tuning word spi message
> - * @phase_xfer:		tuning word spi transfer
> - * @phase_msg:		tuning word spi message
> - * @data:		spi transmit buffer
> - * @phase_data:		tuning word spi transmit buffer
> - * @freq_data:		tuning word spi transmit buffer
> - */
> -
> -struct ad9832_state {
> -	struct spi_device		*spi;
> -	struct regulator		*avdd;
> -	struct regulator		*dvdd;
> -	unsigned long			mclk;
> -	unsigned short			ctrl_fp;
> -	unsigned short			ctrl_ss;
> -	unsigned short			ctrl_src;
> -	struct spi_transfer		xfer;
> -	struct spi_message		msg;
> -	struct spi_transfer		freq_xfer[4];
> -	struct spi_message		freq_msg;
> -	struct spi_transfer		phase_xfer[2];
> -	struct spi_message		phase_msg;
> -	/*
> -	 * DMA (thus cache coherency maintenance) requires the
> -	 * transfer buffers to live in their own cache lines.
> -	 */
> -	union {
> -		__be16			freq_data[4]____cacheline_aligned;
> -		__be16			phase_data[2];
> -		__be16			data;
> -	};
> -};
> -
>  /*
>   * TODO: struct ad9832_platform_data needs to go into include/linux/iio
This comment is wrong now. Obviously not relevant to this patch, but updating
it would be useful!

Jonathan
>   */
> 



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

end of thread, other threads:[~2017-03-02 19:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-26 21:43 [PATCH] staging: iio: ad9832: Moved contents of header file to source file Narcisa Ana Maria Vasile
2017-02-27  9:20 ` Lars-Peter Clausen
2017-02-27 15:29 ` [PATCH v2] " Narcisa Ana Maria Vasile
2017-02-27 15:36   ` [Outreachy kernel] " Daniel Baluta
2017-02-27 15:37   ` Julia Lawall
2017-02-27 17:28 ` [PATCH v3] staging: iio: ad9832: Moved contents of the header to the " Narcisa Ana Maria Vasile
2017-03-01 17:12   ` Narcisa Vasile
2017-03-01 18:14     ` Lars-Peter Clausen
2017-03-02 19:10   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.