e1000e: Include clocksource.h to get CLOCKSOURCE_MASK.

PATCH In review David Miller 4 participants 22 messages Dec 31, 2014 – Jan 5, 2015

Subsystems (7)

Networking Network drivers Intel Ethernet Microblaze architecture +3 more subsystems

lore

Share & tools

lore

David Miller

author maintainer

2014-12-31 23:33 UTC #0 lore raw

PATCH 1/2 e1000e: Include clocksource.h to get CLOCKSOURCE_MASK.

Signed-off-by David S. Miller <[email protected]>

---
 drivers/net/ethernet/intel/e1000e/netdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/intel/e1000e/netdev.c +1 -0

index e14fd85..2537d36a 100644
@@ -43,6 +43,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/aer.h>
 #include <linux/prefetch.h>
+#include <linux/clocksource.h>
 
 #include "e1000.h"

Signature · 1 line

2.1.0

Jeff Kirsher

Acked-by on v1 1/2

2014-12-31 23:42 UTC #1 lore raw

Quoted 2 lines · David Miller

Signed-off-by David S. Miller <[email protected]>

Acked-by Jeff Kirsher <[email protected]>

Quoted 3 lines

---
 drivers/net/ethernet/intel/e1000e/netdev.c | 1 +
 1 file changed, 1 insertion(+)

Richard Cochran

2015-01-01 10:45 UTC #2 lore raw

PATCH 1/7 timecounter: provide a macro to initialize the cyclecounter mask field.

In reply to David Miller

There is no need for users of the timecounter/cyclecounter code to include clocksource.h just for a single macro.

Signed-off-by Richard Cochran <[email protected]>

---
 include/linux/timecounter.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

include/linux/timecounter.h +4 -1

index 74f4549..4382035 100644
@@ -19,6 +19,9 @@
 
 #include <linux/types.h>
 
+/* simplify initialization of mask field */
+#define CYCLECOUNTER_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
+
 /**
  * struct cyclecounter - hardware abstraction for a free running counter
  *	Provides completely state-free accessors to the underlying hardware.

@@ -29,7 +32,7 @@
  * @read:		returns the current cycle value
  * @mask:		bitmask for two's complement
  *			subtraction of non 64 bit counters,
- *			see CLOCKSOURCE_MASK() helper macro
+ *			see CYCLECOUNTER_MASK() helper macro
  * @mult:		cycle to nanosecond multiplier
  * @shift:		cycle to nanosecond divisor (power of two)
  */

Signature · 1 line

1.7.10.4

David Laight

2015-01-05 13:21 UTC #3 lore raw

RE: [PATCH net-next 1/7] timecounter: provide a macro to initialize the cyclecounter mask field.

Quoted 13 lines

There is no need for users of the timecounter/cyclecounter code to include clocksource.h just for a single macro.
Signed-off-by Richard Cochran <[email protected]>
---
 include/linux/timecounter.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
include/linux/timecounter.h +0 -0
index 74f4549..4382035 100644

include/linux/timecounter.h line 23

Quoted 5 lines

include/linux/timecounter.h +2 -0

@@ -19,6 +19,9 @@
 
 #include <linux/types.h>
 
+/* simplify initialization of mask field */
+#define CYCLECOUNTER_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)

That has me chasing through the C integer promotion rules. Better might be:

	((bits) < 64 ? (1ULL << (bits)) - 1 : (((1ULL << 63) - 1) << 1) + 1)

I actually suspect there is a standard definition somewhere?

David

Richard Cochran

2015-01-05 13:43 UTC #4 lore raw

Re: [PATCH net-next 1/7] timecounter: provide a macro to initialize the cyclecounter mask field.

Quoted 7 lines · David Laight

include/linux/timecounter.h line 23
Quoted 2 lines
include/linux/timecounter.h +2 -0
+/* simplify initialization of mask field */
+#define CYCLECOUNTER_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
That has me chasing through the C integer promotion rules. Better might be:
	((bits) < 64 ? (1ULL << (bits)) - 1 : (((1ULL << 63) - 1) << 1) + 1)
I actually suspect there is a standard definition somewhere?

This is an exact copy of CLOCKSOURCE_MASK, and if wrong, then both are wrong. In any case, I can't see any issue here. Is not

	(some_int_type) -1

always equal to

	0xf...(width of type)

for all integer types, when using 2s compliment?

Thanks
Richard

David Laight

2015-01-05 13:53 UTC #5 lore raw

RE: [PATCH net-next 1/7] timecounter: provide a macro to initialize the cyclecounter mask field.

Quoted 12 lines · David Laight

Quoted 7 lines
include/linux/timecounter.h line 23
Quoted 2 lines
include/linux/timecounter.h +2 -0
+/* simplify initialization of mask field */
+#define CYCLECOUNTER_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
That has me chasing through the C integer promotion rules. Better might be:
	((bits) < 64 ? (1ULL << (bits)) - 1 : (((1ULL << 63) - 1) << 1) + 1)
I actually suspect there is a standard definition somewhere?
This is an exact copy of CLOCKSOURCE_MASK, and if wrong, then both are wrong. In any case, I can't see any issue here. Is not

Quoted 7 lines

	(some_int_type) -1
always equal to
	0xf...(width of type)
for all integer types, when using 2s compliment?

As I said, it leaves me chasing through the promotion rules (which I probably know if I actually think hard enough).

Thinking... ~0ULL would be nice and simple and correct.

David

Richard Cochran

2015-01-01 10:39 UTC #6 lore raw

PATCH 2/7 bnx2x: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -1

index 2c95132..0758c8b 100644
@@ -14610,7 +14610,7 @@ static void bnx2x_init_cyclecounter(struct bnx2x *bp)
 {
 	memset(&bp->cyclecounter, 0, sizeof(bp->cyclecounter));
 	bp->cyclecounter.read = bnx2x_cyclecounter_read;
-	bp->cyclecounter.mask = CLOCKSOURCE_MASK(64);
+	bp->cyclecounter.mask = CYCLECOUNTER_MASK(64);
 	bp->cyclecounter.shift = 1;
 	bp->cyclecounter.mult = 1;
 }

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-01 10:39 UTC #7 lore raw

PATCH 3/7 e1000e: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/intel/e1000e/netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/intel/e1000e/netdev.c +1 -1

index e14fd85..332a298 100644
@@ -4189,7 +4189,7 @@ static int e1000_sw_init(struct e1000_adapter *adapter)
 	/* Setup hardware time stamping cyclecounter */
 	if (adapter->flags & FLAG_HAS_HW_TIMESTAMP) {
 		adapter->cc.read = e1000e_cyclecounter_read;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mask = CYCLECOUNTER_MASK(64);
 		adapter->cc.mult = 1;
 		/* cc.shift set in e1000e_get_base_tininca() */

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-01 10:39 UTC #8 lore raw

PATCH 4/7 igb: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/intel/igb/igb_ptp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/igb/igb_ptp.c +2 -2

index 1d27f2d..5e7a4e3 100644
@@ -765,7 +765,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82576;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mask = CYCLECOUNTER_MASK(64);
 		adapter->cc.mult = 1;
 		adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
 		/* Dial the nominal frequency. */

@@ -785,7 +785,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82580;
-		adapter->cc.mask = CLOCKSOURCE_MASK(IGB_NBITS_82580);
+		adapter->cc.mask = CYCLECOUNTER_MASK(IGB_NBITS_82580);
 		adapter->cc.mult = 1;
 		adapter->cc.shift = 0;
 		/* Enable the timer functions by clearing bit 31. */

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-01 10:39 UTC #9 lore raw

PATCH 5/7 ixgbe: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c +1 -1

index 47c29ea..79c00f5 100644
@@ -793,7 +793,7 @@ void ixgbe_ptp_start_cyclecounter(struct ixgbe_adapter *adapter)
 
 	memset(&adapter->cc, 0, sizeof(adapter->cc));
 	adapter->cc.read = ixgbe_ptp_read;
-	adapter->cc.mask = CLOCKSOURCE_MASK(64);
+	adapter->cc.mask = CYCLECOUNTER_MASK(64);
 	adapter->cc.shift = shift;
 	adapter->cc.mult = 1;

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-01 10:40 UTC #10 lore raw

PATCH 6/7 mlx4: include clocksource.h again

In reply to David Miller

This driver uses the function, clocksource_khz2mult, and so it really must include clocksource.h.

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/mellanox/mlx4/en_clock.c +1 -0

index e9cce4f..90b5309 100644
@@ -32,6 +32,7 @@
  */
 
 #include <linux/mlx4/device.h>
+#include <linux/clocksource.h>
 
 #include "mlx4_en.h"

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-01 10:40 UTC #11 lore raw

PATCH 7/7 microblaze: include the new timecounter header.

In reply to David Miller

The timecounter/cyclecounter code has moved, so users need the new include.

Signed-off-by Richard Cochran <[email protected]>

---
 arch/microblaze/kernel/timer.c |    1 +
 1 file changed, 1 insertion(+)

arch/microblaze/kernel/timer.c +1 -0

index dd96f0e..c897745 100644
@@ -17,6 +17,7 @@
 #include <linux/clockchips.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/timecounter.h>
 #include <asm/cpuinfo.h>
 
 static void __iomem *timer_baseaddr;

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-01 10:46 UTC #12 lore raw

cover Fixing the "Time Counter fixes and improvements"

In reply to David Miller

Dave,

I did not catch the missing includes in my x86 and arm testing, because those archs somehow do include clocksource.h for the drivers in question. Sorry.

This is how I would like to fix the header fallout. We really should decouple the timecounter/cyclecounter code from the clocksource code where possible.

Thanks,
Richard

Richard Cochran (7):
timecounter: provide a macro to initialize the cyclecounter mask
field.
bnx2x: convert to CYCLECOUNTER_MASK macro.
e1000e: convert to CYCLECOUNTER_MASK macro.
igb: convert to CYCLECOUNTER_MASK macro.
ixgbe: convert to CYCLECOUNTER_MASK macro.
mlx4: include clocksource.h again
microblaze: include the new timecounter header.

 arch/microblaze/kernel/timer.c                   |    1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c       |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c         |    4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c     |    2 +-
 drivers/net/ethernet/mellanox/mlx4/en_clock.c    |    1 +
 include/linux/timecounter.h                      |    5 ++++-
 7 files changed, 11 insertions(+), 6 deletions(-)

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #13 lore raw

v2 cover Fixing the "Time Counter fixes and improvements"

In reply to David Miller

Dave,

For this series I had only tested the build with ARCH=x86 and arm, but others like sparc64, microblaze, powerpc, and s390 will fail because they somehow don't indirectly include clocksource.h for the drivers in question.

This series fixes the build issues reported by:
kbuild test robot <[email protected]>

Thanks,
Richard

 arch/microblaze/kernel/timer.c                   |    1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c       |    3 +--
 drivers/net/ethernet/intel/igb/igb_ptp.c         |    5 ++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c     |    2 +-
 drivers/net/ethernet/mellanox/mlx4/en_clock.c    |    1 +
 include/linux/timecounter.h                      |    5 ++++-
 7 files changed, 11 insertions(+), 8 deletions(-)

Signature · 1 line

1.7.10.4

David Miller

author maintainer

2015-01-02 21:48 UTC #14 lore raw

Re: [PATCH net-next V2 0/7] Fixing the "Time Counter fixes and improvements"

From: Richard Cochran <[email protected]>
Date: Fri, 2 Jan 2015 20:22:02 +0100

Quoted 7 lines

For this series I had only tested the build with ARCH=x86 and arm, but others like sparc64, microblaze, powerpc, and s390 will fail because they somehow don't indirectly include clocksource.h for the drivers in question.
This series fixes the build issues reported by:
kbuild test robot <[email protected]>

Series applied, thanks Richard.

Richard Cochran

2015-01-02 19:29 UTC #15 lore raw

PATCH v2 1/7 timecounter: provide a macro to initialize the cyclecounter mask field.

In reply to David Miller

There is no need for users of the timecounter/cyclecounter code to include clocksource.h just for a single macro.

Signed-off-by Richard Cochran <[email protected]>

---
 include/linux/timecounter.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

include/linux/timecounter.h +4 -1

index 74f4549..4382035 100644
@@ -19,6 +19,9 @@
 
 #include <linux/types.h>
 
+/* simplify initialization of mask field */
+#define CYCLECOUNTER_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
+
 /**
  * struct cyclecounter - hardware abstraction for a free running counter
  *	Provides completely state-free accessors to the underlying hardware.

@@ -29,7 +32,7 @@
  * @read:		returns the current cycle value
  * @mask:		bitmask for two's complement
  *			subtraction of non 64 bit counters,
- *			see CLOCKSOURCE_MASK() helper macro
+ *			see CYCLECOUNTER_MASK() helper macro
  * @mult:		cycle to nanosecond multiplier
  * @shift:		cycle to nanosecond divisor (power of two)
  */

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #16 lore raw

PATCH v2 2/7 bnx2x: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -1

index 2c95132..0758c8b 100644
@@ -14610,7 +14610,7 @@ static void bnx2x_init_cyclecounter(struct bnx2x *bp)
 {
 	memset(&bp->cyclecounter, 0, sizeof(bp->cyclecounter));
 	bp->cyclecounter.read = bnx2x_cyclecounter_read;
-	bp->cyclecounter.mask = CLOCKSOURCE_MASK(64);
+	bp->cyclecounter.mask = CYCLECOUNTER_MASK(64);
 	bp->cyclecounter.shift = 1;
 	bp->cyclecounter.mult = 1;
 }

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #17 lore raw

PATCH v2 3/7 e1000e: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/intel/e1000e/netdev.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/net/ethernet/intel/e1000e/netdev.c +1 -2

index 2537d36a..332a298 100644
@@ -43,7 +43,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/aer.h>
 #include <linux/prefetch.h>
-#include <linux/clocksource.h>
 
 #include "e1000.h"

@@ -4190,7 +4189,7 @@ static int e1000_sw_init(struct e1000_adapter *adapter)
 	/* Setup hardware time stamping cyclecounter */
 	if (adapter->flags & FLAG_HAS_HW_TIMESTAMP) {
 		adapter->cc.read = e1000e_cyclecounter_read;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mask = CYCLECOUNTER_MASK(64);
 		adapter->cc.mult = 1;
 		/* cc.shift set in e1000e_get_base_tininca() */

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #18 lore raw

PATCH v2 4/7 igb: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/intel/igb/igb_ptp.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/net/ethernet/intel/igb/igb_ptp.c +2 -3

index 8baf3fd..5e7a4e3 100644
@@ -19,7 +19,6 @@
 #include <linux/device.h>
 #include <linux/pci.h>
 #include <linux/ptp_classify.h>
-#include <linux/clocksource.h>
 
 #include "igb.h"

@@ -766,7 +765,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82576;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mask = CYCLECOUNTER_MASK(64);
 		adapter->cc.mult = 1;
 		adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
 		/* Dial the nominal frequency. */

@@ -786,7 +785,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82580;
-		adapter->cc.mask = CLOCKSOURCE_MASK(IGB_NBITS_82580);
+		adapter->cc.mask = CYCLECOUNTER_MASK(IGB_NBITS_82580);
 		adapter->cc.mult = 1;
 		adapter->cc.shift = 0;
 		/* Enable the timer functions by clearing bit 31. */

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #19 lore raw

PATCH v2 5/7 ixgbe: convert to CYCLECOUNTER_MASK macro.

In reply to David Miller

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c +1 -1

index 47c29ea..79c00f5 100644
@@ -793,7 +793,7 @@ void ixgbe_ptp_start_cyclecounter(struct ixgbe_adapter *adapter)
 
 	memset(&adapter->cc, 0, sizeof(adapter->cc));
 	adapter->cc.read = ixgbe_ptp_read;
-	adapter->cc.mask = CLOCKSOURCE_MASK(64);
+	adapter->cc.mask = CYCLECOUNTER_MASK(64);
 	adapter->cc.shift = shift;
 	adapter->cc.mult = 1;

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #20 lore raw

PATCH v2 6/7 mlx4: include clocksource.h again

In reply to David Miller

This driver uses the function, clocksource_khz2mult, and so it really must include clocksource.h.

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/mellanox/mlx4/en_clock.c +1 -0

index e9cce4f..90b5309 100644
@@ -32,6 +32,7 @@
  */
 
 #include <linux/mlx4/device.h>
+#include <linux/clocksource.h>
 
 #include "mlx4_en.h"

Signature · 1 line

1.7.10.4

Richard Cochran

2015-01-02 19:22 UTC #21 lore raw

PATCH v2 7/7 microblaze: include the new timecounter header.

In reply to David Miller

The timecounter/cyclecounter code has moved, so users need the new include.

Signed-off-by Richard Cochran <[email protected]>

---
 arch/microblaze/kernel/timer.c |    1 +
 1 file changed, 1 insertion(+)

arch/microblaze/kernel/timer.c +1 -0

index dd96f0e..c897745 100644
@@ -17,6 +17,7 @@
 #include <linux/clockchips.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/timecounter.h>
 #include <asm/cpuinfo.h>
 
 static void __iomem *timer_baseaddr;

Signature · 1 line

1.7.10.4