From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v3 16/29] net/bnxt: use eal I/O device memory
 read/write API
Date: Thu, 12 Jan 2017 19:10:24 +0000
Message-ID: <f90a509c-b2ab-347a-a18b-3c9df19f7314@intel.com>
References: <1482832175-27199-1-git-send-email-jerin.jacob@caviumnetworks.com>
 <1484212646-10338-1-git-send-email-jerin.jacob@caviumnetworks.com>
 <1484212646-10338-17-git-send-email-jerin.jacob@caviumnetworks.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Cc: konstantin.ananyev@intel.com, thomas.monjalon@6wind.com,
 bruce.richardson@intel.com, jianbo.liu@linaro.org, viktorin@rehivetech.com,
 santosh.shukla@caviumnetworks.com, Stephen Hurd <stephen.hurd@broadcom.com>,
 Ajit Khaparde <ajit.khaparde@broadcom.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id CEB6BF6D2
 for <dev@dpdk.org>; Thu, 12 Jan 2017 20:10:28 +0100 (CET)
In-Reply-To: <1484212646-10338-17-git-send-email-jerin.jacob@caviumnetworks.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 1/12/2017 9:17 AM, Jerin Jacob wrote:
<...>
>  #define B_CP_DB_REARM(cpr, raw_cons)					\
> -		(*(uint32_t *)((cpr)->cp_doorbell) = (DB_CP_REARM_FLAGS | \
> -				RING_CMP(cpr->cp_ring_struct, raw_cons)))
> +	rte_write32((DB_CP_REARM_FLAGS |				\

Just asking, can this be rte_write32_relaxed() since there is explicit
memory barrier defined for B_CP_DIS_DB but not for here?

> +		    RING_CMP(((cpr)->cp_ring_struct), raw_cons)),	\
> +		    ((cpr)->cp_doorbell))
>  
>  #define B_CP_DIS_DB(cpr, raw_cons)					\
> -		rte_smp_wmb();						\
> -		(*(uint32_t *)((cpr)->cp_doorbell) = (DB_CP_FLAGS |	\
> -				RING_CMP(cpr->cp_ring_struct, raw_cons)))
> +	rte_write32((DB_CP_FLAGS |					\
> +		    RING_CMP(((cpr)->cp_ring_struct), raw_cons)),	\
> +		    ((cpr)->cp_doorbell))
>  

<...>

> @@ -80,11 +82,12 @@ static int bnxt_hwrm_send_message_locked(struct bnxt *bp, void *msg,
>  	for (; i < bp->max_req_len; i += 4) {
>  		bar = (uint8_t *)bp->bar0 + i;
>  		*(volatile uint32_t *)bar = 0;

Should this line be removed?

> +		rte_write32(0, bar);
>  	}
>  
>  	/* Ring channel doorbell */
>  	bar = (uint8_t *)bp->bar0 + 0x100;
> -	*(volatile uint32_t *)bar = 1;
> +	rte_write32(1, bar);
>  
>  	/* Poll for the valid bit */
>  	for (i = 0; i < HWRM_CMD_TIMEOUT; i++) {
<...>