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 402ABA0524; Wed, 14 Apr 2021 10:17:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B10F31617DC; Wed, 14 Apr 2021 10:17:44 +0200 (CEST) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id 20A111617D9 for ; Wed, 14 Apr 2021 10:17:43 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 684B25C01CF; Wed, 14 Apr 2021 04:17:42 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Wed, 14 Apr 2021 04:17:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm3; bh= SnJjKTV1B2Qi7aKRt7+SL5hYvlNUaaq0U+KFoKGlG5U=; b=AZlwAMFZOTiLWQA4 qW9lDXCXSOBSmenEMsTI87X5GiWxoO8WzGIiE1Kfnj4woKBGlnBB7ohplhEUvhpC HznPtpHUrvlVMqa2iD6QVTOI9YGfY2sM3BiKauEgElNEnIYxyJcCjMhwqbJW6PCT hjHBPO3paSwQhQQ6qmR5XDK2YKlJeQfegNJ+c3M05o+2XwA2CMwJf54MhuSV+QmZ fEyZR70EzjohQ6hColGPkjpbBSSdg9BUfnwMrHlfUdAtrw7kr5U3OLnE1Pyt4LKg 9A4Nxdpt5LxqynCwRR/DWcbBT3TXIKJQpAak3Ko4jpbmQW9V1IUQ2emE0W5K6vhn Qiev1w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=SnJjKTV1B2Qi7aKRt7+SL5hYvlNUaaq0U+KFoKGlG 5U=; b=OvB2bKtO9Sb70uhxrFccrP4nsOVSJBncfMuBOOJZUeRn+AvJs/VPgjvxm axbrugh8jJb9hdSi6clz9b/I9Rkea97iXRqj+9BleIRkcOyf3pOXymwaC46NvgcB 21lUzqY7+e6ubVBXEXDS8J9Ga7wVsfNypvJ2ijNBOsMu08KsXuk+f8cweFtWvog/ D/WQZd/GfkIKdduc+U642jnVL5D3v1Ut9khOsYmNsL0nMSmGZERGWgey0EFwOGBf JIW1p8FD5Cq4gDsu5VimXO3ke5nU3mwZ/cUvVd4CSZaIk5t0yX2uXTs85qkdGUMB 56Yt7U74OQ4Pm2IamGaeC1gUQy+jw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrudeluddgtddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecukfhppeejjedrudefgedrvddtfedrudekgeenucevlhhushhtvghruf hiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghl ohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 5A5A424005B; Wed, 14 Apr 2021 04:17:41 -0400 (EDT) From: Thomas Monjalon To: Xueming Li , "Wang, Haiyue" Cc: "dev@dpdk.org" , Asaf Penso , Parav Pandit , Ray Kinsella Date: Wed, 14 Apr 2021 10:17:39 +0200 Message-ID: <1710155.qeZJibJ7ot@thomas> In-Reply-To: References: <20210311130113.31382-1-xuemingl@nvidia.com> <20210413032329.25551-1-xuemingl@nvidia.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v1] bus/auxiliary: introduce auxiliary bus 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 Sender: "dev" 14/04/2021 04:59, Wang, Haiyue: > From: Xueming Li [...] > > +void > > +auxiliary_on_scan(struct rte_auxiliary_device *dev) > > +{ > > + struct rte_devargs *devargs; > > + > > + devargs = auxiliary_devargs_lookup(dev->name); > > + dev->device.devargs = devargs; > > Can be simple as: > > dev->device.devargs = auxiliary_devargs_lookup(dev->name); > > > +} > > + > > +/* > > + * Match the auxiliary Driver and Device using driver function. > > + */ > > +bool > > +auxiliary_match(const struct rte_auxiliary_driver *auxiliary_drv, > > + const struct rte_auxiliary_device *auxiliary_dev) > > How about these auxiliary variable name style ? > > const struct rte_auxiliary_driver *aux_drv, > const struct rte_auxiliary_device *aux_dev +1 [...] > > +static int > > +rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver *dr, > > + struct rte_auxiliary_device *dev) > > +{ > > + int ret; > > + enum rte_iova_mode iova_mode; > > + > > RCT style ? > enum rte_iova_mode iova_mode; > int ret; I don't see the benefit of reverse christmas tree. > > + if ((dr->drv_flags & RTE_AUXILIARY_DRV_NEED_IOVA_AS_VA) > 0 && > > '(dr->drv_flags & RTE_AUXILIARY_DRV_NEED_IOVA_AS_VA)' should work, no need '> 0' Yes it's acceptable to consider bit testing as a boolean. [...] > > +static int > > +auxiliary_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t len) > > +{ > > + struct rte_auxiliary_device *adev = RTE_DEV_TO_AUXILIARY(dev); > > How about to use 'aux_dev', instead of 'adev' ? > > > + > > + if (!adev || !adev->driver) { > > ' RTE_DEV_TO_AUXILIARY' is container of 'dev', so it should check 'dev != NULL', > not '!adev'. ; -) Yes and should be explicit NULL comparison. [...] > > --- /dev/null > > +++ b/drivers/bus/auxiliary/linux/auxiliary.c > ^ > | > Seems no need to add one more directory 'linux' layer, as the meson said "linux only". I disagree. Linux sub-directory is more explicit. And who knows? There could be an implementation on other OSes in future.