From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3F742A0523; Tue, 30 Jun 2020 19:01:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DCA5A1BF46; Tue, 30 Jun 2020 19:01:02 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 319781BF44 for ; Tue, 30 Jun 2020 19:01:01 +0200 (CEST) IronPort-SDR: EQ3ujl+pDnNCjp8v5uhMu+AhmTYCvp96ItDl2DWW7T7dX1R1sne/Zf4b1VrqvrPX0WDYhS8i+5 b77VDZrovorw== X-IronPort-AV: E=McAfee;i="6000,8403,9667"; a="134603042" X-IronPort-AV: E=Sophos;i="5.75,298,1589266800"; d="scan'208";a="134603042" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2020 10:00:53 -0700 IronPort-SDR: 4GT1/LXIHzUL7ldYE6SFzlsoj2z+ubQTfyLKdzWHEp3Surh3AxVMffJokO/c78/Fzr7Tt3E4TF kgkJCkM9c/7g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,298,1589266800"; d="scan'208";a="454676328" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.213.246.78]) ([10.213.246.78]) by orsmga005.jf.intel.com with ESMTP; 30 Jun 2020 10:00:52 -0700 To: Hemant Agrawal , dev@dpdk.org Cc: Sachin Saxena References: <20200527132326.1382-1-hemant.agrawal@nxp.com> <20200527132326.1382-16-hemant.agrawal@nxp.com> From: Ferruh Yigit Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJsBBMBCgBWAhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEABQkKqZZ8FiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl6ha3sXGHZrczovL2tl eXMub3BlbnBncC5vcmcACgkQ+TPrQ98TYR8uLA//QwltuFliUWe60xwmu9sY38c1DXvX67wk UryQ1WijVdIoj4H8cf/s2KtyIBjc89R254KMEfJDao/LrXqJ69KyGKXFhFPlF3VmFLsN4XiT PSfxkx8s6kHVaB3O183p4xAqnnl/ql8nJ5ph9HuwdL8CyO5/7dC/MjZ/mc4NGq5O9zk3YRGO lvdZAp5HW9VKW4iynvy7rl3tKyEqaAE62MbGyfJDH3C/nV/4+mPc8Av5rRH2hV+DBQourwuC ci6noiDP6GCNQqTh1FHYvXaN4GPMHD9DX6LtT8Fc5mL/V9i9kEVikPohlI0WJqhE+vQHFzR2 1q5nznE+pweYsBi3LXIMYpmha9oJh03dJOdKAEhkfBr6n8BWkWQMMiwfdzg20JX0o7a/iF8H 4dshBs+dXdIKzPfJhMjHxLDFNPNH8zRQkB02JceY9ESEah3wAbzTwz+e/9qQ5OyDTQjKkVOo cxC2U7CqeNt0JZi0tmuzIWrfxjAUulVhBmnceqyMOzGpSCQIkvalb6+eXsC9V1DZ4zsHZ2Mx Hi+7pCksdraXUhKdg5bOVCt8XFmx1MX4AoV3GWy6mZ4eMMvJN2hjXcrreQgG25BdCdcxKgqp e9cMbCtF+RZax8U6LkAWueJJ1QXrav1Jk5SnG8/5xANQoBQKGz+yFiWcgEs9Tpxth15o2v59 gXK5Ag0EV9ZMvgEQAKc0Db17xNqtSwEvmfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ES YpV8QWj0xK4YM0dLxnDU2IYxjEshSB1TqAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4Ai bPtrHuIXWQOBECcVZTTOdZYGAzaYzxiAONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxD UQljeNvKYt1lZE/gAUUxNLWsYyTT+22/vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/ 3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35piVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVj sM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQI3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdc q9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYHfVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH7 1PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZqw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFB VOQOxCvwRG2QCgcJ/UTn5vlivul+cThi6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI 8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJlRr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYC GwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNhHwUCXqFrngUJCKxSYAAKCRD5M+tD3xNhH3YWD/9b cUiWaHJasX+OpiuZ1Li5GG3m9aw4lR/k2lET0UPRer2Jy1JsL+uqzdkxGvPqzFTBXgx/6Byz EMa2mt6R9BCyR286s3lxVS5Bgr5JGB3EkpPcoJT3A7QOYMV95jBiiJTy78Qdzi5LrIu4tW6H o0MWUjpjdbR01cnj6EagKrDx9kAsqQTfvz4ff5JIFyKSKEHQMaz1YGHyCWhsTwqONhs0G7V2 0taQS1bGiaWND0dIBJ/u0pU998XZhmMzn765H+/MqXsyDXwoHv1rcaX/kcZIcN3sLUVcbdxA WHXOktGTQemQfEpCNuf2jeeJlp8sHmAQmV3dLS1R49h0q7hH4qOPEIvXjQebJGs5W7s2vxbA 5u5nLujmMkkfg1XHsds0u7Zdp2n200VC4GQf8vsUp6CSMgjedHeF9zKv1W4lYXpHp576ZV7T GgsEsvveAE1xvHnpV9d7ZehPuZfYlP4qgo2iutA1c0AXZLn5LPcDBgZ+KQZTzm05RU1gkx7n gL9CdTzVrYFy7Y5R+TrE9HFUnsaXaGsJwOB/emByGPQEKrupz8CZFi9pkqPuAPwjN6Wonokv ChAewHXPUadcJmCTj78Oeg9uXR6yjpxyFjx3vdijQIYgi5TEGpeTQBymLANOYxYWYOjXk+ae dYuOYKR9nbPv+2zK9pwwQ2NXbUBystaGyQ== Message-ID: <06f953bc-48fb-6c97-bdb4-ba075ba0654b@intel.com> Date: Tue, 30 Jun 2020 18:00:50 +0100 MIME-Version: 1.0 In-Reply-To: <20200527132326.1382-16-hemant.agrawal@nxp.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 15/37] net/dpaa: add support for fmlib in dpdk X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" On 5/27/2020 2:23 PM, Hemant Agrawal wrote: > This library is required for configuring FMAN for > various flow configurations. This is a big patch with new files, looks like a new base code drop. Can you please give more explanation on the patch and what 'fmlib' is? > > Signed-off-by: Sachin Saxena > Signed-off-by: Hemant Agrawal <...> > +#if defined(FM_LIB_DBG) > + #define _fml_dbg(format, arg...) \ > + printf("fmlib [%s:%u] - " format, \ > + __func__, __LINE__, ##arg) > +#else > + #define _fml_dbg(arg...) > +#endif Shouldn't use 'printf' directly, this prevents using dynamic logging and our log APIs. Please use a registered logtype instead. > + > +/*#define FM_IOCTL_DBG*/ > + > +#if defined(FM_IOCTL_DBG) > + #define _fm_ioctl_dbg(format, arg...) \ > + printk("fm ioctl [%s:%u](cpu:%u) - " format, \ > + __func__, __LINE__, smp_processor_id(), ##arg) printk? :) > +#else > +# define _fm_ioctl_dbg(arg...) > +#endif > + > +/** > + @Group lnx_ioctl_ncsw_grp NetCommSw Linux User-Space (IOCTL) API > + @{ > +*/ > + > +#define NCSW_IOC_TYPE_BASE 0xe0 > + /**< defines the IOCTL type for all the NCSW Linux module commands */ > + > +/** > + @Group lnx_usr_FM_grp Frame Manager API > + > + @Description FM API functions, definitions and enums. > + > + @{ > +*/ There are lots of checkpatch warning in the block comment syntax, about missing " * " on each line. Other base dpaa/dpaa2 base code seems have it in the block comments, if this won't create a maintance problem, what do you think to fix the syntax on comments? <...> > + e_IOC_FM_PCD_PRS_COUNTERS_SHIM_PARSE_RESULT_RETURNED_WITH_ERR, > + /**< Parser counter - counts the number of times SHIM parse result is returned with errors. */ > + e_IOC_FM_PCD_PRS_COUNTERS_SOFT_PRS_CYCLES, > + /**< Parser counter - counts the number of cycles spent executing soft parser instruction (including stall cycles). */ > + e_IOC_FM_PCD_PRS_COUNTERS_SOFT_PRS_STALL_CYCLES, > + /**< Parser counter - counts the number of cycles stalled waiting for parser internal memory reads while executing soft parser instruction. */ Can you please break long lines? <...> > +#if 0 > +TODO: unused IOCTL > +/** > + @Function FM_PCD_ModifyCounter > + > + @Description Writes a value to an enabled counter. Use "0" to reset the counter. > + > + @Param[in] ioc_fm_pcd_counters_params_t - The requested counter parameters. > + > + @Return 0 on success; Error code otherwise. > +*/ > +#define FM_PCD_IOC_MODIFY_COUNTER _IOW(FM_IOC_TYPE_BASE, FM_PCD_IOC_NUM(10), ioc_fm_pcd_counters_params_t) > +#define FM_PCD_IOC_SET_COUNTER FM_PCD_IOC_MODIFY_COUNTER > +#endif Can you please remove dead code? <...> > +/** > + @Description Enumeration type for selecting the policer profile packet frame length selector > +*/ > +typedef enum ioc_fm_pcd_plcr_frame_length_select { > + e_IOC_FM_PCD_PLCR_L2_FRM_LEN, /**< L2 frame length */ > + e_IOC_FM_PCD_PLCR_L3_FRM_LEN, /**< L3 frame length */ > + e_IOC_FM_PCD_PLCR_L4_FRM_LEN, /**< L4 frame length */ > + e_IOC_FM_PCD_PLCR_FULL_FRM_LEN /**< Full frame length */ > +} ioc_fm_pcd_plcr_frame_length_select; > + > +/** > + @Description Enumeration type for selecting roll-back frame > +*/ > +typedef enum ioc_fm_pcd_plcr_roll_back_frame_select { > + e_IOC_FM_PCD_PLCR_ROLLBACK_L2_FRM_LEN, /**< Rollback L2 frame length */ > + e_IOC_FM_PCD_PLCR_ROLLBACK_FULL_FRM_LEN /**< Rollback Full frame length */ > +} ioc_fm_pcd_plcr_roll_back_frame_select; Please fix the leading whitespace for above two enums.