Re: [PATCH v4 20/29] net/ena: use eal I/O device memory read/write API

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

On Tue, Jan 17, 2017 at 01:51:38PM +0100, Jan Mędala wrote:
> Jerin, Santosh,

Hello Jan,

> 
> As you are introducing improved I/O access API, I would like to ask to
> change ENA platform code to:
> 
> * #define ENA_REG_WRITE32(value, reg) rte_write32_relaxed((value), (reg))*
> * #define ENA_REG_READ32(reg) rte_read32_relaxed((reg))*
> 
> There is no more need to have read/write functions within platform code (
> *readl()*, *writel()*, *writel_relaxed()*), as we can directly map our API
> macro to DPDK platform implementation.
> Also I would prefer to keep API within *drivers/net/ena/base/ena_eth_com.h*

I have tried to make change as per your requirement. looks like, read32/write
has been called from following files.

drivers/net/ena/base/ena_eth_com.c
drivers/net/ena/base/ena_com.c

So, Is make sense to keep it in drivers/net/ena/base/ena_eth_com.h or
in drivers/net/ena/base/ena_com.h ?

Following diff is an working build-able version. If you are not happy with the
change then can you send a patch so that in can include in v5 or if you
want me to change the let me know(What is the change required wrt the
following change)

➜ [dpdk-master] $ git diff
diff --git a/drivers/net/ena/base/ena_com.h
b/drivers/net/ena/base/ena_com.h
index e534592..f34e416 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -42,9 +42,13 @@
 #if defined(__linux__) && !defined(__KERNEL__)
 #include <rte_lcore.h>
 #include <rte_spinlock.h>
+#include <rte_io.h>
 #define __iomem
 #endif
 
+#define ENA_REG_WRITE32(value, reg) rte_write32_relaxed((value), (reg))
+#define ENA_REG_READ32(reg) rte_read32_relaxed((reg))
+
 #define ENA_MAX_NUM_IO_QUEUES          128U
 /* We need to queues for each IO (on for Tx and one for Rx) */
 #define ENA_TOTAL_NUM_QUEUES           (2 * (ENA_MAX_NUM_IO_QUEUES))
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h
b/drivers/net/ena/base/ena_plat_dpdk.h
index 87c3bf1..ee81648 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -224,19 +224,6 @@ typedef uint64_t dma_addr_t;
 #define ENA_MEM_ALLOC(dmadev, size) rte_zmalloc(NULL, size, 1)
 #define ENA_MEM_FREE(dmadev, ptr) ({ENA_TOUCH(dmadev); rte_free(ptr);
})
 
-static inline void writel(u32 value, volatile void  *addr)
-{
-       *(volatile u32 *)addr = value;
-}
-
-static inline u32 readl(const volatile void *addr)
-{
-       return *(const volatile u32 *)addr;
-}
-
-#define ENA_REG_WRITE32(value, reg) writel((value), (reg))
-#define ENA_REG_READ32(reg) readl((reg))
-
 #define ATOMIC32_INC(i32_ptr) rte_atomic32_inc(i32_ptr)
 #define ATOMIC32_DEC(i32_ptr) rte_atomic32_dec(i32_ptr)
 #define ATOMIC32_SET(i32_ptr, val) rte_atomic32_set(i32_ptr, v

> and map define ENA_REG_WRITE32 to relaxed version (when barrier is needed
> we call it explicitly in driver code, there is no need to have
> ENA_REG_WRITE32_RELAXED). That will help us to manage code together between
> the different platforms.
> 
> Should I do the change for ENA or do you want to keep the current flow and
> take care of it?
> 
> ​Best regards
> Jan