diff mbox

[dpdk-dev,v3,17/29] net/bnx2x: use eal I/O device memory read/write API

Message ID 1484212646-10338-18-git-send-email-jerin.jacob@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Jerin Jacob Jan. 12, 2017, 9:17 a.m. UTC 
  From: Santosh Shukla <santosh.shukla@caviumnetworks.com>

Replace the raw I/O device memory read/write access with eal abstraction
for I/O device memory read/write access to fix portability issues across
different architectures.

CC: Harish Patil <harish.patil@cavium.com>
CC: Rasesh Mody <rasesh.mody@cavium.com>
Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 drivers/net/bnx2x/bnx2x.h | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

Comments

Ferruh Yigit Jan. 12, 2017, 7:11 p.m. UTC | #1 
On 1/12/2017 9:17 AM, Jerin Jacob wrote:
<...>
>  
> @@ -1560,11 +1556,9 @@ bnx2x_reg_read32(struct bnx2x_softc *sc, size_t offset)
>  #define DPM_TRIGGER_TYPE 0x40
>  
>  /* Doorbell macro */
> -#define BNX2X_DB_WRITE(db_bar, val) \
> -	*((volatile uint32_t *)(db_bar)) = (val)
> +#define BNX2X_DB_WRITE(db_bar, val) rte_write32_relaxed((val), (db_bar))

What is the rule to use relaxed version or not?

I don't know about memory barrier requirements, if it is easy, would you
mind explaining? Because I have same question for many different parts
of this patchset.

Thanks,
ferruh
Jerin Jacob Jan. 13, 2017, 8:32 a.m. UTC | #2 
On Thu, Jan 12, 2017 at 07:11:18PM +0000, Ferruh Yigit wrote:
> On 1/12/2017 9:17 AM, Jerin Jacob wrote:
> <...>
> >  
> > @@ -1560,11 +1556,9 @@ bnx2x_reg_read32(struct bnx2x_softc *sc, size_t offset)
> >  #define DPM_TRIGGER_TYPE 0x40
> >  
> >  /* Doorbell macro */
> > -#define BNX2X_DB_WRITE(db_bar, val) \
> > -	*((volatile uint32_t *)(db_bar)) = (val)
> > +#define BNX2X_DB_WRITE(db_bar, val) rte_write32_relaxed((val), (db_bar))
> 
> What is the rule to use relaxed version or not?

Here is the logic followed across the driver change:

1) If the top level code is in the pattern like "explicit barrier" and
then io write. Then second the io write operation can be relaxed.
2) If the code runs in slow path, To make patch clean and scope limited,
use only nonrelaxed versions.
 
> I don't know about memory barrier requirements, if it is easy, would you
> mind explaining? Because I have same question for many different parts
> of this patchset.
> 
> Thanks,
> ferruh
diff mbox

Patch

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 5cefea4..59064d8 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -18,6 +18,7 @@ 
 
 #include <rte_byteorder.h>
 #include <rte_spinlock.h>
+#include <rte_io.h>
 
 #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 #ifndef __LITTLE_ENDIAN
@@ -1419,8 +1420,7 @@  bnx2x_reg_write8(struct bnx2x_softc *sc, size_t offset, uint8_t val)
 {
 	PMD_DEBUG_PERIODIC_LOG(DEBUG, "offset=0x%08lx val=0x%02x",
 			       (unsigned long)offset, val);
-	*((volatile uint8_t*)
-	  ((uintptr_t)sc->bar[BAR0].base_addr + offset)) = val;
+	rte_write8(val, ((uint8_t *)sc->bar[BAR0].base_addr + offset));
 }
 
 static inline void
@@ -1433,8 +1433,8 @@  bnx2x_reg_write16(struct bnx2x_softc *sc, size_t offset, uint16_t val)
 #endif
 	PMD_DEBUG_PERIODIC_LOG(DEBUG, "offset=0x%08lx val=0x%04x",
 			       (unsigned long)offset, val);
-	*((volatile uint16_t*)
-	  ((uintptr_t)sc->bar[BAR0].base_addr + offset)) = val;
+	rte_write16(val, ((uint8_t *)sc->bar[BAR0].base_addr + offset));
+
 }
 
 static inline void
@@ -1448,8 +1448,7 @@  bnx2x_reg_write32(struct bnx2x_softc *sc, size_t offset, uint32_t val)
 
 	PMD_DEBUG_PERIODIC_LOG(DEBUG, "offset=0x%08lx val=0x%08x",
 			       (unsigned long)offset, val);
-	*((volatile uint32_t*)
-	  ((uintptr_t)sc->bar[BAR0].base_addr + offset)) = val;
+	rte_write32(val, ((uint8_t *)sc->bar[BAR0].base_addr + offset));
 }
 
 static inline uint8_t
@@ -1457,8 +1456,7 @@  bnx2x_reg_read8(struct bnx2x_softc *sc, size_t offset)
 {
 	uint8_t val;
 
-	val = (uint8_t)(*((volatile uint8_t*)
-			  ((uintptr_t)sc->bar[BAR0].base_addr + offset)));
+	val = rte_read8((uint8_t *)sc->bar[BAR0].base_addr + offset);
 	PMD_DEBUG_PERIODIC_LOG(DEBUG, "offset=0x%08lx val=0x%02x",
 			       (unsigned long)offset, val);
 
@@ -1476,8 +1474,7 @@  bnx2x_reg_read16(struct bnx2x_softc *sc, size_t offset)
 			    (unsigned long)offset);
 #endif
 
-	val = (uint16_t)(*((volatile uint16_t*)
-			   ((uintptr_t)sc->bar[BAR0].base_addr + offset)));
+	val = rte_read16(((uint8_t *)sc->bar[BAR0].base_addr + offset));
 	PMD_DEBUG_PERIODIC_LOG(DEBUG, "offset=0x%08lx val=0x%08x",
 			       (unsigned long)offset, val);
 
@@ -1495,8 +1492,7 @@  bnx2x_reg_read32(struct bnx2x_softc *sc, size_t offset)
 			    (unsigned long)offset);
 #endif
 
-	val = (uint32_t)(*((volatile uint32_t*)
-			   ((uintptr_t)sc->bar[BAR0].base_addr + offset)));
+	val = rte_read32(((uint8_t *)sc->bar[BAR0].base_addr + offset));
 	PMD_DEBUG_PERIODIC_LOG(DEBUG, "offset=0x%08lx val=0x%08x",
 			       (unsigned long)offset, val);
 
@@ -1560,11 +1556,9 @@  bnx2x_reg_read32(struct bnx2x_softc *sc, size_t offset)
 #define DPM_TRIGGER_TYPE 0x40
 
 /* Doorbell macro */
-#define BNX2X_DB_WRITE(db_bar, val) \
-	*((volatile uint32_t *)(db_bar)) = (val)
+#define BNX2X_DB_WRITE(db_bar, val) rte_write32_relaxed((val), (db_bar))
 
-#define BNX2X_DB_READ(db_bar) \
-	*((volatile uint32_t *)(db_bar))
+#define BNX2X_DB_READ(db_bar) rte_read32_relaxed(db_bar)
 
 #define DOORBELL_ADDR(sc, offset) \
 	(volatile uint32_t *)(((char *)(sc)->bar[BAR1].base_addr + (offset)))