From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 32602A0093; Mon, 3 Oct 2022 10:28:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CD1E240695; Mon, 3 Oct 2022 10:28:43 +0200 (CEST) Received: from wout1-smtp.messagingengine.com (wout1-smtp.messagingengine.com [64.147.123.24]) by mails.dpdk.org (Postfix) with ESMTP id 4CECA40693 for ; Mon, 3 Oct 2022 10:28:42 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 287853200034; Mon, 3 Oct 2022 04:28:39 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Mon, 03 Oct 2022 04:28:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1664785718; x= 1664872118; bh=OJ7+IOn1ttyXXNq0xzk/xMhoQMcUIQQo4KfK3y0vid4=; b=l RnxiLjX3v0a/+VpO0XjDYDYDUT4kZyhZmfdVGCanvGKwpaeWk6LzDw/PAcgY2vOy MC7wzMkM8ApTl9omr453fQeqzW36jukCVTE0OYsUlvfeR9j3rZ2aHwcOrOx3pw76 F6ooW5/tloxZftqBb0fDq2iPljtbEhnAaeT4v4QoftEJD7gk8nv9WPuxCP3Q7qi9 INPKhgoIkCYn2zHNEhu7Ynb7xiYae0FrCBk0iUoqqb9SSeYw55LzlTrxqxZVnDMG 23QyOZPXNsa+XmOfFiZkRG9gUhc6qgY2+xy/c1pul9KhgCqMykmCyCrbLJg9otq3 p68OyolFJDVJbEtaMf9xQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1664785718; x= 1664872118; bh=OJ7+IOn1ttyXXNq0xzk/xMhoQMcUIQQo4KfK3y0vid4=; b=T 3FXI0CfNjWS+MS4pUcPBmTn2xWw1ogVCpCWfRsLcfmcwo282qxFb8KiZyUcRr4RO CrUx3CPxiEb3wEDEYWVqDgrCmEDFUi507mq5X6GpofQ+cb8uv0/a/hVM5NE7Bu3v pLjkFs7mFZYh80SUuW9VICTZBtx66/Z0JUiqpyc+HUG6sZUrqAGTYXOq9yQoZB44 fzkI1Li2688yrXCP+RNO/Ls+3Lq271aEcOT0xeg7AknHXLKXgUyW6FYsHbp/ZGC5 H1HC1dMq3iULUp+oLiwLV8GR+Et2WV7BJi5IRg3kXPD2DWRJjttoWE5AkXMI3ejL TQ6kWaC+PIj98p33BLNYg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeehledgtdegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedtjeeiieefhedtfffgvdelteeufeefheeujefgueetfedttdei kefgkeduhedtgfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 3 Oct 2022 04:28:37 -0400 (EDT) From: Thomas Monjalon To: Nicolas Chautru Cc: dev@dpdk.org, gakhil@marvell.com, maxime.coquelin@redhat.com, trix@redhat.com, mdr@ashroe.eu, bruce.richardson@intel.com, david.marchand@redhat.com, stephen@networkplumber.org, mingshan.zhang@intel.com, hemant.agrawal@nxp.com Subject: Re: [PATCH v10 6/7] bbdev: add queue related warning and status information Date: Mon, 03 Oct 2022 10:28:35 +0200 Message-ID: <3072887.zE8UqtGg2D@thomas> In-Reply-To: <20220930184605.47655-7-nicolas.chautru@intel.com> References: <1655491040-183649-6-git-send-email-nicolas.chautru@intel.com> <20220930184605.47655-1-nicolas.chautru@intel.com> <20220930184605.47655-7-nicolas.chautru@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Looking at this patch because I have been alerted about the ABI compat handling. I see some details that should have been caught in earlier reviews. 30/09/2022 20:46, Nicolas Chautru: > +/* > + * Maximum size to be used to manage the enum rte_bbdev_enqueue_status including padding for future This line is long. It is always better to split lines logically, for instance here, before "including". > + * enum insertion It could be made clear that the real enum size is smaller or equal. > + */ > +#define RTE_BBDEV_ENQ_STATUS_SIZE_MAX 6 [...] > +enum rte_bbdev_enqueue_status { > + RTE_BBDEV_ENQ_STATUS_NONE, /**< Nothing to report */ > + RTE_BBDEV_ENQ_STATUS_QUEUE_FULL, /**< Not enough room in queue */ > + RTE_BBDEV_ENQ_STATUS_RING_FULL, /**< Not enough room in ring */ > + RTE_BBDEV_ENQ_STATUS_INVALID_OP, /**< Operation was rejected as invalid */ > +}; A comment is missing at the end of the enum to remind updating the MAX. But the big question is why do we need this "MAX" value? The guideline is to avoid using such MAX value for long term compatibility. [...] > +/** > + * Converts queue status from enum to string Should be imperative form: "Convert". A dot is missing at the end of the sentence. > + * > + * @param status > + * Queue status as enum > + * > + * @returns > + * Queue status as string or NULL if op_type is invalid It is not aligned with above parameter. Choose an indentation format and keep it consistent. [...] > # added in 22.11 > rte_bbdev_device_status_str; > + rte_bbdev_enqueue_status_str; > rte_bbdev_enqueue_fft_ops; > rte_bbdev_dequeue_fft_ops; It is not alphabetical order.