Move icss_iep structure definition and prueth_iep_gettime to icss_iep.h
and icssg_prueth.h file respectively so that they can be used / accessed
by all icssg driver files.
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
drivers/net/ethernet/ti/icssg/icss_iep.c | 72 -------------------
drivers/net/ethernet/ti/icssg/icss_iep.h | 74 +++++++++++++++++++-
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 5 +-
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 2 +
4 files changed, 78 insertions(+), 75 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index 75c294ce6fb6..5d6d1cf78e93 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.c
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
@@ -53,78 +53,6 @@
#define IEP_CAP_CFG_CAPNR_1ST_EVENT_EN(n) BIT(LATCH_INDEX(n))
#define IEP_CAP_CFG_CAP_ASYNC_EN(n) BIT(LATCH_INDEX(n) + 10)
-enum {
- ICSS_IEP_GLOBAL_CFG_REG,
- ICSS_IEP_GLOBAL_STATUS_REG,
- ICSS_IEP_COMPEN_REG,
- ICSS_IEP_SLOW_COMPEN_REG,
- ICSS_IEP_COUNT_REG0,
- ICSS_IEP_COUNT_REG1,
- ICSS_IEP_CAPTURE_CFG_REG,
- ICSS_IEP_CAPTURE_STAT_REG,
-
- ICSS_IEP_CAP6_RISE_REG0,
- ICSS_IEP_CAP6_RISE_REG1,
-
- ICSS_IEP_CAP7_RISE_REG0,
- ICSS_IEP_CAP7_RISE_REG1,
-
- ICSS_IEP_CMP_CFG_REG,
- ICSS_IEP_CMP_STAT_REG,
- ICSS_IEP_CMP0_REG0,
- ICSS_IEP_CMP0_REG1,
- ICSS_IEP_CMP1_REG0,
- ICSS_IEP_CMP1_REG1,
-
- ICSS_IEP_CMP8_REG0,
- ICSS_IEP_CMP8_REG1,
- ICSS_IEP_SYNC_CTRL_REG,
- ICSS_IEP_SYNC0_STAT_REG,
- ICSS_IEP_SYNC1_STAT_REG,
- ICSS_IEP_SYNC_PWIDTH_REG,
- ICSS_IEP_SYNC0_PERIOD_REG,
- ICSS_IEP_SYNC1_DELAY_REG,
- ICSS_IEP_SYNC_START_REG,
- ICSS_IEP_MAX_REGS,
-};
-
-/**
- * struct icss_iep_plat_data - Plat data to handle SoC variants
- * @config: Regmap configuration data
- * @reg_offs: register offsets to capture offset differences across SoCs
- * @flags: Flags to represent IEP properties
- */
-struct icss_iep_plat_data {
- const struct regmap_config *config;
- u32 reg_offs[ICSS_IEP_MAX_REGS];
- u32 flags;
-};
-
-struct icss_iep {
- struct device *dev;
- void __iomem *base;
- const struct icss_iep_plat_data *plat_data;
- struct regmap *map;
- struct device_node *client_np;
- unsigned long refclk_freq;
- int clk_tick_time; /* one refclk tick time in ns */
- struct ptp_clock_info ptp_info;
- struct ptp_clock *ptp_clock;
- struct mutex ptp_clk_mutex; /* PHC access serializer */
- u32 def_inc;
- s16 slow_cmp_inc;
- u32 slow_cmp_count;
- const struct icss_iep_clockops *ops;
- void *clockops_data;
- u32 cycle_time_ns;
- u32 perout_enabled;
- bool pps_enabled;
- int cap_cmp_irq;
- u64 period;
- u32 latch_enable;
- struct work_struct work;
-};
-
/**
* icss_iep_get_count_hi() - Get the upper 32 bit IEP counter
* @iep: Pointer to structure representing IEP.
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.h b/drivers/net/ethernet/ti/icssg/icss_iep.h
index 803a4b714893..9283123349e8 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.h
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.h
@@ -12,7 +12,79 @@
#include <linux/ptp_clock_kernel.h>
#include <linux/regmap.h>
-struct icss_iep;
+enum {
+ ICSS_IEP_GLOBAL_CFG_REG,
+ ICSS_IEP_GLOBAL_STATUS_REG,
+ ICSS_IEP_COMPEN_REG,
+ ICSS_IEP_SLOW_COMPEN_REG,
+ ICSS_IEP_COUNT_REG0,
+ ICSS_IEP_COUNT_REG1,
+ ICSS_IEP_CAPTURE_CFG_REG,
+ ICSS_IEP_CAPTURE_STAT_REG,
+
+ ICSS_IEP_CAP6_RISE_REG0,
+ ICSS_IEP_CAP6_RISE_REG1,
+
+ ICSS_IEP_CAP7_RISE_REG0,
+ ICSS_IEP_CAP7_RISE_REG1,
+
+ ICSS_IEP_CMP_CFG_REG,
+ ICSS_IEP_CMP_STAT_REG,
+ ICSS_IEP_CMP0_REG0,
+ ICSS_IEP_CMP0_REG1,
+ ICSS_IEP_CMP1_REG0,
+ ICSS_IEP_CMP1_REG1,
+
+ ICSS_IEP_CMP8_REG0,
+ ICSS_IEP_CMP8_REG1,
+ ICSS_IEP_SYNC_CTRL_REG,
+ ICSS_IEP_SYNC0_STAT_REG,
+ ICSS_IEP_SYNC1_STAT_REG,
+ ICSS_IEP_SYNC_PWIDTH_REG,
+ ICSS_IEP_SYNC0_PERIOD_REG,
+ ICSS_IEP_SYNC1_DELAY_REG,
+ ICSS_IEP_SYNC_START_REG,
+ ICSS_IEP_MAX_REGS,
+};
+
+/**
+ * struct icss_iep_plat_data - Plat data to handle SoC variants
+ * @config: Regmap configuration data
+ * @reg_offs: register offsets to capture offset differences across SoCs
+ * @flags: Flags to represent IEP properties
+ */
+struct icss_iep_plat_data {
+ const struct regmap_config *config;
+ u32 reg_offs[ICSS_IEP_MAX_REGS];
+ u32 flags;
+};
+
+struct icss_iep {
+ struct device *dev;
+ void __iomem *base;
+ const struct icss_iep_plat_data *plat_data;
+ struct regmap *map;
+ struct device_node *client_np;
+ unsigned long refclk_freq;
+ int clk_tick_time; /* one refclk tick time in ns */
+ struct ptp_clock_info ptp_info;
+ struct ptp_clock *ptp_clock;
+ struct mutex ptp_clk_mutex; /* PHC access serializer */
+ spinlock_t irq_lock; /* CMP IRQ vs icss_iep_ptp_enable access */
+ u32 def_inc;
+ s16 slow_cmp_inc;
+ u32 slow_cmp_count;
+ const struct icss_iep_clockops *ops;
+ void *clockops_data;
+ u32 cycle_time_ns;
+ u32 perout_enabled;
+ bool pps_enabled;
+ int cap_cmp_irq;
+ u64 period;
+ u32 latch_enable;
+ struct work_struct work;
+};
+
extern const struct icss_iep_clockops prueth_iep_clockops;
/* Firmware specific clock operations */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 85c9797b6238..778b41c16f2d 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -325,7 +325,7 @@ static int emac_phy_connect(struct prueth_emac *emac)
return 0;
}
-static u64 prueth_iep_gettime(void *clockops_data, struct ptp_system_timestamp *sts)
+u64 prueth_iep_gettime(void *clockops_data, struct ptp_system_timestamp *sts)
{
u32 hi_rollover_count, hi_rollover_count_r;
struct prueth_emac *emac = clockops_data;
@@ -384,7 +384,8 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
sc_desc.cyclecounter0_set = cyclecount & GENMASK(31, 0);
sc_desc.cyclecounter1_set = (cyclecount & GENMASK(63, 32)) >> 32;
sc_desc.iepcount_set = ns % cycletime;
- sc_desc.CMP0_current = cycletime - 4; //Count from 0 to (cycle time)-4
+ /* Count from 0 to (cycle time) - emac->iep->def_inc */
+ sc_desc.CMP0_current = cycletime - emac->iep->def_inc;
memcpy_toio(sc_descp, &sc_desc, sizeof(sc_desc));
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 6cb1dce8b309..79e8577caf91 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -362,6 +362,8 @@ void icssg_stats_work_handler(struct work_struct *work);
void emac_update_hardware_stats(struct prueth_emac *emac);
int emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name);
+u64 prueth_iep_gettime(void *clockops_data, struct ptp_system_timestamp *sts);
+
/* Common functions */
void prueth_cleanup_rx_chns(struct prueth_emac *emac,
struct prueth_rx_chn *rx_chn,
--
2.34.1On Thu, Aug 08, 2024 at 04:37:59PM +0530, MD Danish Anwar wrote:
> - struct ptp_clock *ptp_clock;
> - struct mutex ptp_clk_mutex; /* PHC access serializer */
> - u32 def_inc;
> - s16 slow_cmp_inc;
[ cut ]
> + struct ptp_clock *ptp_clock;
> + struct mutex ptp_clk_mutex; /* PHC access serializer */
> + spinlock_t irq_lock; /* CMP IRQ vs icss_iep_ptp_enable access */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The patch adds this new struct member. When you're moving code around, please
just move the code. Don't fix checkpatch warnings or do any other cleanups.
> + u32 def_inc;
> + s16 slow_cmp_inc;
> + u32 slow_cmp_count;
regards,
dan carpenter
Hi Dan, On 10/08/24 1:40 am, Dan Carpenter wrote: > On Thu, Aug 08, 2024 at 04:37:59PM +0530, MD Danish Anwar wrote: >> - struct ptp_clock *ptp_clock; >> - struct mutex ptp_clk_mutex; /* PHC access serializer */ >> - u32 def_inc; >> - s16 slow_cmp_inc; > > [ cut ] > >> + struct ptp_clock *ptp_clock; >> + struct mutex ptp_clk_mutex; /* PHC access serializer */ >> + spinlock_t irq_lock; /* CMP IRQ vs icss_iep_ptp_enable access */ > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > The patch adds this new struct member. When you're moving code around, please > just move the code. Don't fix checkpatch warnings or do any other cleanups. > My bad. I didn't notice this new struct member was introduced. I will take care of it. Also apart from doing the code movement, this patch also does the following change. Instead of hardcoding the value 4, the patch uses emac->iep->def_inc. Since the iep->def_inc is now accessible from drivers/net/ethernet/ti/icssg/icssg_prueth.c @@ -384,7 +384,8 @@ static void prueth_iep_settime(void *clockops_data, u64 ns) sc_desc.cyclecounter0_set = cyclecount & GENMASK(31, 0); sc_desc.cyclecounter1_set = (cyclecount & GENMASK(63, 32)) >> 32; sc_desc.iepcount_set = ns % cycletime; - sc_desc.CMP0_current = cycletime - 4; //Count from 0 to (cycle time)-4 + /* Count from 0 to (cycle time) - emac->iep->def_inc */ + sc_desc.CMP0_current = cycletime - emac->iep->def_inc; memcpy_toio(sc_descp, &sc_desc, sizeof(sc_desc)); Should I keep the above change as it is or should I split it into a separate patch and make this patch strictly for code movement. I kept the above change as part of this patch because it is related with the code movement. Moving the iep structure from icss_iep.c to icss_iep.h makes it accessible to icssg_prueth.c so I thought keeping them together will be a better idea. Please let me know if this is okay. >> + u32 def_inc; >> + s16 slow_cmp_inc; >> + u32 slow_cmp_count; > > regards, > dan carpenter -- Thanks and Regards, Danish
On Mon, Aug 12, 2024 at 11:11:21AM +0530, MD Danish Anwar wrote: > Hi Dan, > > On 10/08/24 1:40 am, Dan Carpenter wrote: > > On Thu, Aug 08, 2024 at 04:37:59PM +0530, MD Danish Anwar wrote: > >> - struct ptp_clock *ptp_clock; > >> - struct mutex ptp_clk_mutex; /* PHC access serializer */ > >> - u32 def_inc; > >> - s16 slow_cmp_inc; > > > > [ cut ] > > > >> + struct ptp_clock *ptp_clock; > >> + struct mutex ptp_clk_mutex; /* PHC access serializer */ > >> + spinlock_t irq_lock; /* CMP IRQ vs icss_iep_ptp_enable access */ > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > The patch adds this new struct member. When you're moving code around, please > > just move the code. Don't fix checkpatch warnings or do any other cleanups. > > > > My bad. I didn't notice this new struct member was introduced. I will > take care of it. > > Also apart from doing the code movement, this patch also does the > following change. Instead of hardcoding the value 4, the patch uses > emac->iep->def_inc. Since the iep->def_inc is now accessible from > drivers/net/ethernet/ti/icssg/icssg_prueth.c > > @@ -384,7 +384,8 @@ static void prueth_iep_settime(void *clockops_data, > u64 ns) > sc_desc.cyclecounter0_set = cyclecount & GENMASK(31, 0); > sc_desc.cyclecounter1_set = (cyclecount & GENMASK(63, 32)) >> 32; > sc_desc.iepcount_set = ns % cycletime; > - sc_desc.CMP0_current = cycletime - 4; //Count from 0 to (cycle time)-4 > + /* Count from 0 to (cycle time) - emac->iep->def_inc */ > + sc_desc.CMP0_current = cycletime - emac->iep->def_inc; > > memcpy_toio(sc_descp, &sc_desc, sizeof(sc_desc)); > > > Should I keep the above change as it is or should I split it into a > separate patch and make this patch strictly for code movement. I kept > the above change as part of this patch because it is related with the > code movement. Moving the iep structure from icss_iep.c to icss_iep.h > makes it accessible to icssg_prueth.c so I thought keeping them together > will be a better idea. > > Please let me know if this is okay. > Huh, I saw that the comment had changed but I assumed that was because checkpatch had complained. I didn't notice that the 4 changed to emac->iep->def_inc. That's the kind of change that we do really want to notice. Yep. Please, could you split that out into a separate patch. regards, dan carpenter
© 2016 - 2025 Red Hat, Inc.