[PATCH] Add struct ad9832_platform_data to the include/linux/iio

[PATCH] Add struct ad9832_platform_data to the include/linux/iio

[Madhumitha Prabakaran]

Add struct ad9832_platform_data to the include/linux/iio
for maintaining code organization and clarity.

Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c |  3 +--
 drivers/staging/iio/frequency/ad9832.h | 34 --------------------------
 include/linux/iio/frequency/ad9832.h   | 30 +++++++++++++++++++++++
 3 files changed, 31 insertions(+), 36 deletions(-)
 delete mode 100644 drivers/staging/iio/frequency/ad9832.h
 create mode 100644 include/linux/iio/frequency/ad9832.h

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 6f9eebd6c7ee..675adfb07b5d 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -19,8 +19,7 @@
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-
-#include "ad9832.h"
+#include <linux/iio/frequency/ad9832.h>
 
 #include "dds.h"
 
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
deleted file mode 100644
index 98dfbd9289ab..000000000000
--- a/drivers/staging/iio/frequency/ad9832.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * AD9832 SPI DDS driver
- *
- * Copyright 2011 Analog Devices Inc.
- */
-#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		freq0;
-	unsigned long		freq1;
-	unsigned short		phase0;
-	unsigned short		phase1;
-	unsigned short		phase2;
-	unsigned short		phase3;
-};
-
-#endif /* IIO_DDS_AD9832_H_ */
diff --git a/include/linux/iio/frequency/ad9832.h b/include/linux/iio/frequency/ad9832.h
new file mode 100644
index 000000000000..517d954636db
--- /dev/null
+++ b/include/linux/iio/frequency/ad9832.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * AD9832 SPI DDS driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ */
+#ifndef IIO_DDS_AD9832_H_
+#define IIO_DDS_AD9832_H_
+
+/**
+ * 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 freq0;
+	unsigned long freq1;
+	unsigned short phase0;
+	unsigned short phase1;
+	unsigned short phase2;
+	unsigned short phase3;
+};
+
+#endif /* IIO_DDS_AD9832_H_ */
+
-- 
2.25.1

Re: [PATCH] Add struct ad9832_platform_data to the include/linux/iio

[Greg Kroah-Hartman]

On Fri, Jul 07, 2023 at 04:15:53PM -0500, Madhumitha Prabakaran wrote:
> Add struct ad9832_platform_data to the include/linux/iio
> for maintaining code organization and clarity.
> 
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> ---
>  drivers/staging/iio/frequency/ad9832.c |  3 +--
>  drivers/staging/iio/frequency/ad9832.h | 34 --------------------------
>  include/linux/iio/frequency/ad9832.h   | 30 +++++++++++++++++++++++

No, not yet, sorry.  Staging drivers should be self-contained, why does
this .c file need a .h file at all anyway?  It should all just be in the
.c file, can you do that instead?

thanks,

greg k-h

Re: [PATCH] Add struct ad9832_platform_data to the include/linux/iio

[Jonathan Cameron]

On Sat, 8 Jul 2023 13:10:29 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Fri, Jul 07, 2023 at 04:15:53PM -0500, Madhumitha Prabakaran wrote:
> > Add struct ad9832_platform_data to the include/linux/iio
> > for maintaining code organization and clarity.
> > 
> > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > ---
> >  drivers/staging/iio/frequency/ad9832.c |  3 +--
> >  drivers/staging/iio/frequency/ad9832.h | 34 --------------------------
> >  include/linux/iio/frequency/ad9832.h   | 30 +++++++++++++++++++++++  
> 
> No, not yet, sorry.  Staging drivers should be self-contained, why does
> this .c file need a .h file at all anyway?  It should all just be in the
> .c file, can you do that instead?

This is an aged driver so still has definitions that would be included
from board files, hence the header.

So Madhumitha, if you are looking at getting this driver out of staging
(which would be great!) then first job is convert it from platform data
to device tree (or better yet generic firmware bindings using linux/property.h)
A side effect of that is the header would go away as equivalent job would be
done by the dt-bindings yaml file.

Jonathan

> 
> thanks,
> 
> greg k-h

Re: [PATCH] Add struct ad9832_platform_data to the include/linux/iio

[Madhumitha Prabakaran]

On 7/8/23 09:45, Jonathan Cameron wrote:
> On Sat, 8 Jul 2023 13:10:29 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>
>> On Fri, Jul 07, 2023 at 04:15:53PM -0500, Madhumitha Prabakaran wrote:
>>> Add struct ad9832_platform_data to the include/linux/iio
>>> for maintaining code organization and clarity.
>>>
>>> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
>>> ---
>>>   drivers/staging/iio/frequency/ad9832.c |  3 +--
>>>   drivers/staging/iio/frequency/ad9832.h | 34 --------------------------
>>>   include/linux/iio/frequency/ad9832.h   | 30 +++++++++++++++++++++++
>> No, not yet, sorry.  Staging drivers should be self-contained, why does
>> this .c file need a .h file at all anyway?  It should all just be in the
>> .c file, can you do that instead?
> This is an aged driver so still has definitions that would be included
> from board files, hence the header.
>
> So Madhumitha, if you are looking at getting this driver out of staging
> (which would be great!) then first job is convert it from platform data
> to device tree (or better yet generic firmware bindings using linux/property.h)

Sure, I will take a look and work on convert it from platform data to 
generic

firmware bindings.

> A side effect of that is the header would go away as equivalent job would be
> done by the dt-bindings yaml file.
>
> Jonathan
>
>> thanks,
>>
>> greg k-h