From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v4 20/29] net/ena: use eal I/O device memory read/write API Date: Tue, 17 Jan 2017 19:43:13 +0530 Message-ID: <20170117141312.GA7844@localhost.localdomain> References: <1484212646-10338-1-git-send-email-jerin.jacob@caviumnetworks.com> <1484637244-7548-1-git-send-email-jerin.jacob@caviumnetworks.com> <1484637244-7548-21-git-send-email-jerin.jacob@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Cc: Santosh Shukla , , "Ananyev, Konstantin" , Thomas Monjalon , Bruce Richardson , , , Jakub Palider To: Jan =?utf-8?Q?M=C4=99dala?= Return-path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0052.outbound.protection.outlook.com [104.47.40.52]) by dpdk.org (Postfix) with ESMTP id 1B396106A for ; Tue, 17 Jan 2017 15:13:35 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 #include +#include #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