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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.