From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Subject: Re: [PATCH v3 03/10] crypto/dpaa2_sec: add dpaa2_sec poll mode driver Date: Tue, 24 Jan 2017 10:06:03 -0500 Message-ID: <20170124150603.GA20359@neilslaptop.think-freely.org> References: <20161222201700.20020-1-akhil.goyal@nxp.com> <20170120140509.4495-1-akhil.goyal@nxp.com> <20170120140509.4495-4-akhil.goyal@nxp.com> <20170120123237.GA15111@hmswarspite.think-freely.org> <80e5fc73-9a47-a3c5-db0a-1329810b1a39@nxp.com> <20170120193154.GB15111@hmswarspite.think-freely.org> <16826234-24c1-7511-4948-6849050bb463@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, thomas.monjalon@6wind.com, declan.doherty@intel.com, pablo.de.lara.guarch@intel.com, john.mcnamara@intel.com, Hemant Agrawal To: Akhil Goyal Return-path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id B52CB2BD1 for ; Tue, 24 Jan 2017 16:06:26 +0100 (CET) Content-Disposition: inline In-Reply-To: <16826234-24c1-7511-4948-6849050bb463@nxp.com> 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 24, 2017 at 12:04:09PM +0530, Akhil Goyal wrote: > On 1/21/2017 1:01 AM, Neil Horman wrote: > > On Fri, Jan 20, 2017 at 06:47:49PM +0530, Akhil Goyal wrote: > > > On 1/20/2017 6:02 PM, Neil Horman wrote: > > > > On Fri, Jan 20, 2017 at 07:35:02PM +0530, akhil.goyal@nxp.com wrote: > > > > > From: Akhil Goyal > > > > > > > > > > Signed-off-by: Hemant Agrawal > > > > > Signed-off-by: Akhil Goyal > > > > > --- > > > > > config/common_base | 8 + > > > > > config/defconfig_arm64-dpaa2-linuxapp-gcc | 12 + > > > > > drivers/bus/Makefile | 3 + > > > > > drivers/common/Makefile | 3 + > > > > > drivers/crypto/Makefile | 1 + > > > > > drivers/crypto/dpaa2_sec/Makefile | 77 +++++ > > > > > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 374 +++++++++++++++++++++ > > > > > drivers/crypto/dpaa2_sec/dpaa2_sec_logs.h | 70 ++++ > > > > > drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 225 +++++++++++++ > > > > > .../crypto/dpaa2_sec/rte_pmd_dpaa2_sec_version.map | 4 + > > > > > drivers/net/dpaa2/Makefile | 1 + > > > > > drivers/pool/Makefile | 4 + > > > > > mk/rte.app.mk | 6 + > > > > > 13 files changed, 788 insertions(+) > > > > > create mode 100644 drivers/crypto/dpaa2_sec/Makefile > > > > > create mode 100644 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > > > > > create mode 100644 drivers/crypto/dpaa2_sec/dpaa2_sec_logs.h > > > > > create mode 100644 drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h > > > > > create mode 100644 drivers/crypto/dpaa2_sec/rte_pmd_dpaa2_sec_version.map > > > > > > > > > NAK, you're trying to patch driver/bus/Makefile, which doesn't exist in the > > > > upstream tree, please fix your patch. > > > > > > > > I'm also opposed to the inclusion of pmds that require non-open external > > > > libraries as your documentation suggests that you require. If you need an out > > > > of tree library to support your hardware, you will recieve no benefit from the > > > > upstream community in terms of testing and maintenence, nor will the community > > > > be able to work with your hardware on arches that your library doesn't support. > > > > > > > > Neil > > > > > > > Thanks for your comments Neil. > > > dpaa2_sec driver is dependent on dpaa2 driver which is in review in other > > > thread. I have mentioned that in the cover letter. > > > Its latest version is http://dpdk.org/dev/patchwork/patch/19782/ > > > > > Sorry, I missed that comment, I'll go find the other thread and take another > > look > > > > > Also there is no external library used. The libraries that are mentioned in > > > the documentation are all part of the above dpaa2 driver patchset. > > > > > Your documentation patch doesn't seem to suggest that. From the patch: > > > > +This driver relies on external libraries and kernel drivers for resources > > +allocations and initialization. The following dependencies are not part of > > +DPDK and must be installed separately: > > + > > +- **NXP Linux SDK** > > + > > + NXP Linux software development kit (SDK) includes support for family > > + of QorIQ® ARM-Architecture-based system on chip (SoC) processors > > + and corresponding boards. > > > > .... > > > > If thats not the case, you should update the documentation. If it is the case, > > I think my initial comment is still valid... > > > > Regards > > Neil > > > > > -Akhil > > > > > > > > > > > > Thanks for pointing this out. I will update the documentation. > I found the v6 patch series that you referenced, and this set still doesn't apply cleanly to it. Theres a conflict in one of the makefiles. > Regards, > Akhil > >