[PATCH v1 1/4] pps: generators: tio: split pps_gen_tio.h

Raag Jadav posted 4 patches 9 months, 3 weeks ago
There is a newer version of this series
[PATCH v1 1/4] pps: generators: tio: split pps_gen_tio.h
Posted by Raag Jadav 9 months, 3 weeks ago
Split macros and structure definition to header file for better
maintainability.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 drivers/pps/generators/pps_gen_tio.c | 30 +------------------
 drivers/pps/generators/pps_gen_tio.h | 45 ++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 29 deletions(-)
 create mode 100644 drivers/pps/generators/pps_gen_tio.h

diff --git a/drivers/pps/generators/pps_gen_tio.c b/drivers/pps/generators/pps_gen_tio.c
index 6c46b46c66cd..7f2aab1219af 100644
--- a/drivers/pps/generators/pps_gen_tio.c
+++ b/drivers/pps/generators/pps_gen_tio.c
@@ -5,8 +5,6 @@
  * Copyright (C) 2024 Intel Corporation
  */
 
-#include <linux/bitfield.h>
-#include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/container_of.h>
 #include <linux/device.h>
@@ -21,33 +19,7 @@
 
 #include <asm/cpu_device_id.h>
 
-#define TIOCTL			0x00
-#define TIOCOMPV		0x10
-#define TIOEC			0x30
-
-/* Control Register */
-#define TIOCTL_EN			BIT(0)
-#define TIOCTL_DIR			BIT(1)
-#define TIOCTL_EP			GENMASK(3, 2)
-#define TIOCTL_EP_RISING_EDGE		FIELD_PREP(TIOCTL_EP, 0)
-#define TIOCTL_EP_FALLING_EDGE		FIELD_PREP(TIOCTL_EP, 1)
-#define TIOCTL_EP_TOGGLE_EDGE		FIELD_PREP(TIOCTL_EP, 2)
-
-/* Safety time to set hrtimer early */
-#define SAFE_TIME_NS			(10 * NSEC_PER_MSEC)
-
-#define MAGIC_CONST			(NSEC_PER_SEC - SAFE_TIME_NS)
-#define ART_HW_DELAY_CYCLES		2
-
-struct pps_tio {
-	struct pps_gen_source_info gen_info;
-	struct pps_gen_device *pps_gen;
-	struct hrtimer timer;
-	void __iomem *base;
-	u32 prev_count;
-	spinlock_t lock;
-	struct device *dev;
-};
+#include "pps_gen_tio.h"
 
 static inline u32 pps_tio_read(u32 offset, struct pps_tio *tio)
 {
diff --git a/drivers/pps/generators/pps_gen_tio.h b/drivers/pps/generators/pps_gen_tio.h
new file mode 100644
index 000000000000..78d4d7c25221
--- /dev/null
+++ b/drivers/pps/generators/pps_gen_tio.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Intel PPS signal Generator Driver
+ *
+ * Copyright (C) 2025 Intel Corporation
+ */
+
+#ifndef _PPS_GEN_TIO_H_
+#define _PPS_GEN_TIO_H_
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/hrtimer.h>
+#include <linux/pps_gen_kernel.h>
+#include <linux/spinlock_types.h>
+
+#define TIOCTL			0x00
+#define TIOCOMPV		0x10
+#define TIOEC			0x30
+
+/* Control Register */
+#define TIOCTL_EN			BIT(0)
+#define TIOCTL_DIR			BIT(1)
+#define TIOCTL_EP			GENMASK(3, 2)
+#define TIOCTL_EP_RISING_EDGE		FIELD_PREP(TIOCTL_EP, 0)
+#define TIOCTL_EP_FALLING_EDGE		FIELD_PREP(TIOCTL_EP, 1)
+#define TIOCTL_EP_TOGGLE_EDGE		FIELD_PREP(TIOCTL_EP, 2)
+
+/* Safety time to set hrtimer early */
+#define SAFE_TIME_NS			(10 * NSEC_PER_MSEC)
+
+#define MAGIC_CONST			(NSEC_PER_SEC - SAFE_TIME_NS)
+#define ART_HW_DELAY_CYCLES		2
+
+struct pps_tio {
+	struct pps_gen_source_info gen_info;
+	struct pps_gen_device *pps_gen;
+	struct hrtimer timer;
+	void __iomem *base;
+	u32 prev_count;
+	spinlock_t lock;
+	struct device *dev;
+};
+
+#endif /* _PPS_GEN_TIO_H_ */
-- 
2.34.1
Re: [PATCH v1 1/4] pps: generators: tio: split pps_gen_tio.h
Posted by Andy Shevchenko 9 months, 3 weeks ago
On Wed, Feb 26, 2025 at 11:45:24AM +0530, Raag Jadav wrote:
> Split macros and structure definition to header file for better
> maintainability.

> +#ifndef _PPS_GEN_TIO_H_
> +#define _PPS_GEN_TIO_H_
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/hrtimer.h>
> +#include <linux/pps_gen_kernel.h>
> +#include <linux/spinlock_types.h>

This missing time.h and types.h.

Also you need to add a forward declaration for the device.

struct device;

> +#define TIOCTL			0x00
> +#define TIOCOMPV		0x10
> +#define TIOEC			0x30
> +
> +/* Control Register */
> +#define TIOCTL_EN			BIT(0)
> +#define TIOCTL_DIR			BIT(1)
> +#define TIOCTL_EP			GENMASK(3, 2)
> +#define TIOCTL_EP_RISING_EDGE		FIELD_PREP(TIOCTL_EP, 0)
> +#define TIOCTL_EP_FALLING_EDGE		FIELD_PREP(TIOCTL_EP, 1)
> +#define TIOCTL_EP_TOGGLE_EDGE		FIELD_PREP(TIOCTL_EP, 2)
> +
> +/* Safety time to set hrtimer early */
> +#define SAFE_TIME_NS			(10 * NSEC_PER_MSEC)
> +
> +#define MAGIC_CONST			(NSEC_PER_SEC - SAFE_TIME_NS)
> +#define ART_HW_DELAY_CYCLES		2
> +
> +struct pps_tio {
> +	struct pps_gen_source_info gen_info;
> +	struct pps_gen_device *pps_gen;
> +	struct hrtimer timer;
> +	void __iomem *base;
> +	u32 prev_count;
> +	spinlock_t lock;
> +	struct device *dev;
> +};
> +
> +#endif /* _PPS_GEN_TIO_H_ */

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 1/4] pps: generators: tio: split pps_gen_tio.h
Posted by Raag Jadav 9 months, 3 weeks ago
On Wed, Feb 26, 2025 at 02:45:26PM +0200, Andy Shevchenko wrote:
> On Wed, Feb 26, 2025 at 11:45:24AM +0530, Raag Jadav wrote:
> > Split macros and structure definition to header file for better
> > maintainability.
> 
> > +#ifndef _PPS_GEN_TIO_H_
> > +#define _PPS_GEN_TIO_H_
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/hrtimer.h>
> > +#include <linux/pps_gen_kernel.h>
> > +#include <linux/spinlock_types.h>
> 
> This missing time.h and types.h.

Aren't they guaranteed by hrtimer.h and spinlock_types.h?

Raag
Re: [PATCH v1 1/4] pps: generators: tio: split pps_gen_tio.h
Posted by Andy Shevchenko 9 months, 3 weeks ago
Thu, Feb 27, 2025 at 07:08:43AM +0200, Raag Jadav kirjoitti:
> On Wed, Feb 26, 2025 at 02:45:26PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 26, 2025 at 11:45:24AM +0530, Raag Jadav wrote:

...

> > > +#include <linux/bitfield.h>
> > > +#include <linux/bits.h>
> > > +#include <linux/hrtimer.h>
> > > +#include <linux/pps_gen_kernel.h>
> > > +#include <linux/spinlock_types.h>
> > 
> > This missing time.h and types.h.
> 
> Aren't they guaranteed by hrtimer.h and spinlock_types.h?

Nope, why? HR timer is only for HR timer specific definitions and APIs, how
does they are related?

time.h due to NSEC_PER_USEC (or what was there?) and types.h due to u32.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 1/4] pps: generators: tio: split pps_gen_tio.h
Posted by Rodolfo Giometti 9 months, 3 weeks ago
On 26/02/25 07:15, Raag Jadav wrote:
> Split macros and structure definition to header file for better
> maintainability.
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

> ---
>   drivers/pps/generators/pps_gen_tio.c | 30 +------------------
>   drivers/pps/generators/pps_gen_tio.h | 45 ++++++++++++++++++++++++++++
>   2 files changed, 46 insertions(+), 29 deletions(-)
>   create mode 100644 drivers/pps/generators/pps_gen_tio.h
> 
> diff --git a/drivers/pps/generators/pps_gen_tio.c b/drivers/pps/generators/pps_gen_tio.c
> index 6c46b46c66cd..7f2aab1219af 100644
> --- a/drivers/pps/generators/pps_gen_tio.c
> +++ b/drivers/pps/generators/pps_gen_tio.c
> @@ -5,8 +5,6 @@
>    * Copyright (C) 2024 Intel Corporation
>    */
>   
> -#include <linux/bitfield.h>
> -#include <linux/bits.h>
>   #include <linux/cleanup.h>
>   #include <linux/container_of.h>
>   #include <linux/device.h>
> @@ -21,33 +19,7 @@
>   
>   #include <asm/cpu_device_id.h>
>   
> -#define TIOCTL			0x00
> -#define TIOCOMPV		0x10
> -#define TIOEC			0x30
> -
> -/* Control Register */
> -#define TIOCTL_EN			BIT(0)
> -#define TIOCTL_DIR			BIT(1)
> -#define TIOCTL_EP			GENMASK(3, 2)
> -#define TIOCTL_EP_RISING_EDGE		FIELD_PREP(TIOCTL_EP, 0)
> -#define TIOCTL_EP_FALLING_EDGE		FIELD_PREP(TIOCTL_EP, 1)
> -#define TIOCTL_EP_TOGGLE_EDGE		FIELD_PREP(TIOCTL_EP, 2)
> -
> -/* Safety time to set hrtimer early */
> -#define SAFE_TIME_NS			(10 * NSEC_PER_MSEC)
> -
> -#define MAGIC_CONST			(NSEC_PER_SEC - SAFE_TIME_NS)
> -#define ART_HW_DELAY_CYCLES		2
> -
> -struct pps_tio {
> -	struct pps_gen_source_info gen_info;
> -	struct pps_gen_device *pps_gen;
> -	struct hrtimer timer;
> -	void __iomem *base;
> -	u32 prev_count;
> -	spinlock_t lock;
> -	struct device *dev;
> -};
> +#include "pps_gen_tio.h"
>   
>   static inline u32 pps_tio_read(u32 offset, struct pps_tio *tio)
>   {
> diff --git a/drivers/pps/generators/pps_gen_tio.h b/drivers/pps/generators/pps_gen_tio.h
> new file mode 100644
> index 000000000000..78d4d7c25221
> --- /dev/null
> +++ b/drivers/pps/generators/pps_gen_tio.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Intel PPS signal Generator Driver
> + *
> + * Copyright (C) 2025 Intel Corporation
> + */
> +
> +#ifndef _PPS_GEN_TIO_H_
> +#define _PPS_GEN_TIO_H_
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/hrtimer.h>
> +#include <linux/pps_gen_kernel.h>
> +#include <linux/spinlock_types.h>
> +
> +#define TIOCTL			0x00
> +#define TIOCOMPV		0x10
> +#define TIOEC			0x30
> +
> +/* Control Register */
> +#define TIOCTL_EN			BIT(0)
> +#define TIOCTL_DIR			BIT(1)
> +#define TIOCTL_EP			GENMASK(3, 2)
> +#define TIOCTL_EP_RISING_EDGE		FIELD_PREP(TIOCTL_EP, 0)
> +#define TIOCTL_EP_FALLING_EDGE		FIELD_PREP(TIOCTL_EP, 1)
> +#define TIOCTL_EP_TOGGLE_EDGE		FIELD_PREP(TIOCTL_EP, 2)
> +
> +/* Safety time to set hrtimer early */
> +#define SAFE_TIME_NS			(10 * NSEC_PER_MSEC)
> +
> +#define MAGIC_CONST			(NSEC_PER_SEC - SAFE_TIME_NS)
> +#define ART_HW_DELAY_CYCLES		2
> +
> +struct pps_tio {
> +	struct pps_gen_source_info gen_info;
> +	struct pps_gen_device *pps_gen;
> +	struct hrtimer timer;
> +	void __iomem *base;
> +	u32 prev_count;
> +	spinlock_t lock;
> +	struct device *dev;
> +};
> +
> +#endif /* _PPS_GEN_TIO_H_ */

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming