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
