From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <gage.eads@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 3B0C65599
for <dev@dpdk.org>; Thu, 23 Mar 2017 17:46:09 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple;
d=intel.com; i=@intel.com; q=dns/txt; s=intel;
t=1490287570; x=1521823570;
h=from:to:cc:subject:date:message-id:references:
in-reply-to:content-transfer-encoding:mime-version;
bh=+hvPF82l3/g729UMQtN+gVG22wGgFVoWD59RNr/61eo=;
b=JR37uTkn/tQxuGmaZ9Xnc3XX+s3mxJIntG7hZWXbkfrtvIAz0VChmUIY
xIHuW9JqPGB8Ae7TOb7gXE+3hFMkmQ==;
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
23 Mar 2017 09:46:09 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.36,210,1486454400"; d="scan'208";a="1126279574"
Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206])
by fmsmga001.fm.intel.com with ESMTP; 23 Mar 2017 09:46:08 -0700
Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by
FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS)
id 14.3.319.2; Thu, 23 Mar 2017 09:46:08 -0700
Received: from fmsmsx108.amr.corp.intel.com ([169.254.9.101]) by
fmsmsx117.amr.corp.intel.com ([169.254.3.194]) with mapi id 14.03.0319.002;
Thu, 23 Mar 2017 09:46:07 -0700
From: "Eads, Gage" <gage.eads@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>, "Richardson,
Bruce" <bruce.richardson@intel.com>, "Van Haaren, Harry"
<harry.van.haaren@intel.com>, "hemant.agrawal@nxp.com"
<hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
"santosh.shukla@caviumnetworks.com" <santosh.shukla@caviumnetworks.com>
Thread-Topic: [dpdk-dev] [PATCH 08/39] event/octeontx: add mailbox support
Thread-Index: AQHSlEO6b7KsckKiv0WXcGZHNvrttaGiuj4w
Date: Thu, 23 Mar 2017 16:46:07 +0000
Message-ID: <9184057F7FC11744A2107296B6B8EB1E01E9073C@FMSMSX108.amr.corp.intel.com>
References: <1488562101-6658-1-git-send-email-jerin.jacob@caviumnetworks.com>
<1488562101-6658-9-git-send-email-jerin.jacob@caviumnetworks.com>
In-Reply-To: <1488562101-6658-9-git-send-email-jerin.jacob@caviumnetworks.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.1.200.108]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 08/39] event/octeontx: add mailbox support
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
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>
X-List-Received-Date: Thu, 23 Mar 2017 16:46:10 -0000
Hi Jerin,
I identified a few issues below.
Thanks,
Gage
<snip>
> +static inline void
> +mbox_send_requeust(struct mbox *m, struct octeontx_mbox_hdr *hdr,
> + const void *txmsg, uint16_t txsize)
"requeust" -> "request"
<snip>
> +
> +static inline int
> +mbox_wait_response(struct mbox *m, struct octeontx_mbox_hdr *hdr,
> + void *rxmsg, uint16_t rxsize)
> +{
> + int res =3D 0, wait;
> + uint16_t len;
> + struct mbox_ram_hdr rx_hdr;
> + uint64_t *ram_mbox_hdr =3D (uint64_t *)m->ram_mbox_base;
> + uint8_t *ram_mbox_msg =3D m->ram_mbox_base + sizeof(struct
> +mbox_ram_hdr);
> +
> + /* Wait for response */
> + wait =3D MBOX_WAIT_TIME;
> + while (wait > 0) {
> + rte_delay_us(100);
> + rx_hdr.u64 =3D rte_read64(ram_mbox_hdr);
> + if (rx_hdr.chan_state =3D=3D MBOX_CHAN_STATE_RES)
> + break;
> + wait -=3D 10;
> + }
'wait' is in units of milliseconds ("Mbox operation timeout in milliseconds=
"), so the function subtracts 10 ms after spinning for 100 us. Is that inte=
ntional?
> +
> + hdr->res_code =3D rx_hdr.res_code;
> + m->tag_own++;
> +
> + /* Tag mismatch */
> + if (m->tag_own !=3D rx_hdr.tag) {
> + res =3D -EBADR;
> + goto error;
> + }
> +
> + /* PF nacked the msg */
> + if (rx_hdr.res_code !=3D MBOX_RET_SUCCESS) {
> + res =3D -EBADMSG;
> + goto error;
> + }
> +
> + /* Timeout */
> + if (wait <=3D 0) {
> + res =3D -ETIMEDOUT;
> + goto error;
> + }
Will a timeout mean rx_hdr is invalid? If so, the timeout error should be c=
hecked before checking for a PF nack or tag mismatch.
<snip>
> +static inline int
> +mbox_send(struct mbox *m, struct octeontx_mbox_hdr *hdr, const void
> *txmsg,
> + uint16_t txsize, void *rxmsg, uint16_t rxsize) {
> + int res =3D -EINVAL;
> +
> + if (m->init_once =3D=3D 0 || hdr =3D=3D NULL ||
> + txsize > MAX_RAM_MBOX_LEN || rxsize >
> MAX_RAM_MBOX_LEN) {
> + ssovf_log_err("Invalid init_once=3D%d hdr=3D%p txsz=3D%d rxsz=3D%d",
> + m->init_once, hdr, txsize, rxsize);
> + return res;
> + }
> +
> + rte_spinlock_lock(&m->lock);
> +
> + mbox_send_requeust(m, hdr, txmsg, txsize);
"requeust" -> "request"