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 E8CD4A0C51; Thu, 10 Jun 2021 08:30:42 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D0AAE4067C; Thu, 10 Jun 2021 08:30:42 +0200 (CEST) Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on2048.outbound.protection.outlook.com [40.107.94.48]) by mails.dpdk.org (Postfix) with ESMTP id E3E4F4003C for ; Thu, 10 Jun 2021 08:30:40 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nwnsOOIsJrS4JpfvMGeXzP7xd51KtddV2e3vlH2HA5cSjxeaLjjniLJjolpXGVfQYwAOfFCGHi9qGH/vQTHifLd3muthu0Lp36U8aYgOsd4H36j41htYHA/X+Cq3W2oWPqeNuCZsSx1q8cpWwdbnto2b4IoUklE2NYHKZnfJ9jQjY9ZzDdTFwWLG8W5XrAqyEq7ub+BWoC3yV8rq5WTd5WI0h6utU0V29FAG9JAjbSRO3Q4ZLYKixgBiXtylxhPetp4kp+ChdSOHPZdIcL33+SYZyVzUO+aq+FmsjD+2q/2IDLHUW3XaFGQRBACT7xR6zpVVkBCZk05WYN+jR9ms2g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=JprAQfdjtBFPbXfwTdXhOELq0zfinWILk4XOCuTt+XQ=; b=Bu/QwDGQEMDuQ7nfGIlio3rwUx85CKuApZxdcsjQ6XETZR0mYknraNXvbPg5c3ainTtTpyutDj1yF5Xcp6yIzXqu7Lzv+JZm73ZZ0u4DWXVI3wkebzs7GldWtCCiNnoQaRF/ff2apYnebqos0ZYUFfXXD6X5yx3nE6mG9SM/0Pb6MDstAe4Nvp9bGts94Sbe7NxvJBgxvmCU77kKWsO3t8yM8C6ash5efT4dqJrNO0550o9PGRkJKjaeYRy/h6chPlkwHuMwzt+EAs1tp8qB3ku1Sr/ygOobX56eDIJ5iv3hBv4Nj2hG2aU65zDJiOa9h4SpRqDDYnoRSPWzIR6bRw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=JprAQfdjtBFPbXfwTdXhOELq0zfinWILk4XOCuTt+XQ=; b=ggQlqpNYnuGxWJQIbpxBC/Ahi71ORu1cZF+NQlrJf7irkR5S2oJucadMhTfsaxYI+P+kbWslEFBi27ZVGW+RYcoK4S2A0npFAg7Ci4HEFLCPuvKWn9jy2H6UWIboU+s3PnVZ4kVenUmWNlaQf4OOVe26PgAEudiCyOaGDofDtgx/aQhGopdokHgH7ek+AeuxdVSjtCu1xwEH/X1uhzfOJTswfb8FrJBYIsuB3HoIEtcQlvWt/kTIDmX7szBbdAWs/FDkeX5FVUXpSpMVrAsN6yZG1MEnOA41aMaFKdwennkxzfufY+ZieOdsXXGAhmsdX8YONuOAadnP+Vaai7shxQ== Received: from DM4PR12MB5373.namprd12.prod.outlook.com (2603:10b6:5:39a::17) by DM4PR12MB5312.namprd12.prod.outlook.com (2603:10b6:5:39d::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4219.20; Thu, 10 Jun 2021 06:30:39 +0000 Received: from DM4PR12MB5373.namprd12.prod.outlook.com ([fe80::ac75:9b9a:a03f:1676]) by DM4PR12MB5373.namprd12.prod.outlook.com ([fe80::ac75:9b9a:a03f:1676%3]) with mapi id 15.20.4219.022; Thu, 10 Jun 2021 06:30:39 +0000 From: "Xueming(Steven) Li" To: NBU-Contact-Thomas Monjalon CC: "dev@dpdk.org" , Parav Pandit , Ray Kinsella , Wang Haiyue , "andrew.rybchenko@oktetlabs.ru" Thread-Topic: [dpdk-dev] [RFC v2] bus/auxiliary: introduce auxiliary bus Thread-Index: AQHXRaMW2n3Bl9LNrkiHlYEQ1JP48qsJ62AAgAMNW3A= Date: Thu, 10 Jun 2021 06:30:39 +0000 Message-ID: References: <20210413032329.25551-1-xuemingl@nvidia.com> <20210510134732.2174-1-xuemingl@nvidia.com> <2237980.2ZMkXp7bNk@thomas> In-Reply-To: <2237980.2ZMkXp7bNk@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: monjalon.net; dkim=none (message not signed) header.d=none;monjalon.net; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [223.104.211.130] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 045078c0-e626-4bc4-d62e-08d92bd943eb x-ms-traffictypediagnostic: DM4PR12MB5312: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: WUVoAWvaiifUduUOgU+UBAsdjRv1/sB7517KpkM7DAgLWT9UKrn6IF1QXIhchIkR/6y+8xPIJRoqPM6GbL0TdPuLWm9NwraPfMQsaH2cmf83KT+YTsznDnGLZzO5hnw0Xw/4cXY5JBpjOQDoatl+gGcOA/ul7ah2ci8JQKR0c9io1ncWuajNJLeKe7iBPZSFaL48D1yYXpHAwywlvj5yd2bxke3vqqFJn84v3kf2QCm2D8vSQMF76bGkbXmQzt2cUyZ1SOxhWmrkOYq7IpQ9mfUNN5s0KncV0jQMI7iLmC22uX0woei1aSohIPqdpcLQ93t2TK8HRc5suJ7VC1n+Cg0lFDFJRNyncNtSYGpPRpHGfqDsMfhxbH/vNZALRWbeetcFkoNKZGDUjXRvB3Ouey6eURu550UN0g5cjUsp+0eO2QnQ536xo1Rrg7/Lc4ZiIforkIoinh+NSyf7HNh3D7DfS2N7JFxyyMK5LIYY7J7t+qkqWwGmCj2duJM7RP0o4VpNzUy9yNk4uP395tyaTGBiO0IVLrPKV9HCiwb6/yvSlWBmjLUn8flWHbPFTgp1lqqxZ9QPA/f0rRByQfu3nhvR91PEyGYUGjVErl9oLz93xhD6nrUAq0c0UNPtYWeRpwJjeDopmJvKyPtHu1EGoOqIz+OKf9mnXLTLZvUnxxz0wzByx6eDKTpVqZ7DjpsrKAlU8KbHLPzlrxHaFEzrVNaPmK8Fg36lfvoCG5lBO9oDmRmTfkzf5Iv/40TAxqKa8+lSKU2CIZm5F7xmMSUuVQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM4PR12MB5373.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(136003)(39860400002)(376002)(346002)(366004)(396003)(76116006)(8936002)(66946007)(64756008)(66476007)(66446008)(186003)(66556008)(33656002)(2906002)(8676002)(86362001)(38100700002)(122000001)(54906003)(71200400001)(316002)(26005)(55016002)(52536014)(6506007)(9686003)(53546011)(83380400001)(7696005)(4326008)(6916009)(478600001)(966005)(5660300002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?lneglibi1yVl4ZvfAKevhNmQP2xyFwb0jjQ17uey1nqRaIYrCUMDN/254PTF?= =?us-ascii?Q?MVaSOFnM7QZcMbaGYjAKJ2dt/buhyHFnRLcgy8E5YAPGaLuMJtEh5tfU8yzT?= =?us-ascii?Q?GaJuS8Ka5eMn6Zi+SOYGNKezHT6xJtSMNuDTUUJCvTKfAtM8IO3/EfwMzMCw?= =?us-ascii?Q?78wvziIv2XlLOvp1A22oWvVp1CYWpM676YQ/7bhkGZoUqoxPLYoUg5obigAT?= =?us-ascii?Q?BZvMG8xT6W7hdByeXQfof4FfHwsui5p6+o7UrGUFpV13XZG77qWjtGYaKBK1?= =?us-ascii?Q?Sm/g02S5zJIIfaqjdgv1xPjCql+G7qBwnL3j+dVhR/ZOtc9hd6mdrfL1q2iI?= =?us-ascii?Q?XCyOxHoGrTEc2W/00BVZeoZ3SgC8AxP64yWAKrdB0YxzsIfjx8KVqsNaabiP?= =?us-ascii?Q?4DtPxEfQtWYElv3A0cz3phdXk7yyNynx+JbjP/BVrG58MIX8ZudZb3S4JTOf?= =?us-ascii?Q?rYaBsiUMox0zGpjAZpauKrfEiGW8hftD3aefSIMb3NpomnFwu1iIWN56Jsdq?= =?us-ascii?Q?/BriX5T/Ro/C65PRjdlq8zHfadRwwnHshobSrTf5a1L69S3QlABRkt8nvEmf?= =?us-ascii?Q?VQH/KpcbqvHq8XfJEkqda9vZQFipGz5GMrumdoVJfEM0G/Amd58DbFd+75ud?= =?us-ascii?Q?NC2UnaP4cG2O1lJE/KAfklGS/Rnrf1Yv3tCaTzj82EGyCAe8s34zhS+ZZexg?= =?us-ascii?Q?/OXTkM1PZxDpSp3oP86h3mWQvkABg9TuKfRKLpdmW1uQSP0qdAOcnBBgTMVb?= =?us-ascii?Q?6KeEG2tszbgmsFv30KFxyrwwX+Dry0C4A3kdEH6hqZPNcOlZKZtXeTHQ2OwE?= =?us-ascii?Q?CZewfDO74yt51cZxp3VIwV5Y5n9wKd3Hek25wj18vSGIGa8MkQIOol2Dq4x7?= =?us-ascii?Q?F24n6er6FATa9PE/3gF9fBV/poUpWhvnkL3hv31MlbKrjtcxruN/vifoJFOO?= =?us-ascii?Q?/RxIcdZDuh4LcCim228Q/xCDIDA7q9m8W+7Csl0InYVFqULzrx1TilpEk837?= =?us-ascii?Q?OWqE4Qhf8PxjcQI/az15F0UNgIFwQLJz6FJoQ8PvxdE1m9cREDO29qI6bwzr?= =?us-ascii?Q?LUVvLWT91I0NQhqgX508480vB3a/Gf5TkyD67XuBlwZ71AECBzR7Uc64cRLR?= =?us-ascii?Q?1sO1K/OEYBmJVSqFPG4M1YsGFJKE5ZVfP2BMQA8DK5gMBV8Exk1XyA1S0qCY?= =?us-ascii?Q?vGEdlhWfLjrfPZXTYeHAM269UUDxnRDGA677Cdc2dcTbZnOvEXFcO9hQhDW5?= =?us-ascii?Q?JRDTVTaKp+x/fZ/bzC+Pnjnv1hOiAwmdgQqjukiTwnAJ0gZ2geYeP/JCZauj?= =?us-ascii?Q?QMtyUPseb7Zu/COu+Lt5jUa9?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5373.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 045078c0-e626-4bc4-d62e-08d92bd943eb X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jun 2021 06:30:39.5429 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Qorx8nCjfyP6wFgky0ziiyl0ev7rTF9KEGqo90t0yWu7smB4hCk7Wlgwd2TOlsrQsKNakwT73VFMvQkOwKqYtQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5312 Subject: Re: [dpdk-dev] [RFC v2] 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" > -----Original Message----- > From: Thomas Monjalon > Sent: Tuesday, June 8, 2021 3:53 PM > To: Xueming(Steven) Li > Cc: dev@dpdk.org; Parav Pandit ; Ray Kinsella ; Wang Haiyue ; > andrew.rybchenko@oktetlabs.ru > Subject: Re: [dpdk-dev] [RFC v2] bus/auxiliary: introduce auxiliary bus >=20 > 10/05/2021 15:47, Xueming Li: > > Auxiliary [1] provides a way to split function into child-devices >=20 > Auxiliary -> Auxiliary bus >=20 > > representing sub-domains of functionality. Each auxiliary_device >=20 > auxiliary_device -> auxiliary device >=20 > > represents a part of its parent functionality. > > > > Auxiliary device is identified by unique device name, sysfs path: > > /sys/bus/auxiliary/devices/ > > > > [1] kernel auxiliary bus document: > > https://www.kernel.org/doc/html/latest/driver-api/auxiliary_bus.html > > > > Signed-off-by: Xueming Li > [...] > > +++ b/drivers/bus/auxiliary/auxiliary_common.c > [...] > > +int auxiliary_bus_logtype; >=20 > You don't need to declare this variable, it is done in a macro. >=20 > > + > > +static struct rte_devargs * > > +auxiliary_devargs_lookup(const char *name) { > > + struct rte_devargs *devargs; > > + > > + RTE_EAL_DEVARGS_FOREACH("auxiliary", devargs) { >=20 > "auxiliary" should be defined as a macro. >=20 > [...] > > +/* > > + * Test whether the auxiliary device exist */ __rte_weak bool >=20 > The comment should explain it is a stub for OS not supporting auxiliary b= us. >=20 > > +auxiliary_dev_exists(const char *name) { > > + RTE_SET_USED(name); > > + return false; > > +} > > + > > +/* > > + * Scan the content of the auxiliary bus, and the devices in the > > +devices > > + * list > > + */ >=20 > Same here about the stub comment. >=20 > > +__rte_weak int > > +auxiliary_scan(void) > > +{ > > + return 0; > > +} > > + >=20 > Please insert a comment here. >=20 > > +void > > +auxiliary_on_scan(struct rte_auxiliary_device *aux_dev) { > > + aux_dev->device.devargs =3D auxiliary_devargs_lookup(aux_dev->name); > > +} > > + > > +/* > > + * Match the auxiliary Driver and Device using driver function. >=20 > driver and device do not need capital letters >=20 > > + */ > > +bool > > +auxiliary_match(const struct rte_auxiliary_driver *aux_drv, > > + const struct rte_auxiliary_device *aux_dev) { > > + if (aux_drv->match =3D=3D NULL) > > + return false; > > + return aux_drv->match(aux_dev->name); } > > + > > +/* > > + * Call the probe() function of the driver. > > + */ > > +static int > > +rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver *dr, > > + struct rte_auxiliary_device *dev) > > +{ > > + enum rte_iova_mode iova_mode; > > + int ret; > > + > > + if ((dr =3D=3D NULL) || (dev =3D=3D NULL)) > > + return -EINVAL; > > + > > + /* The device is not blocked; Check if driver supports it */ >=20 > Please keep all the comments more uniform by starting with a capital > and ends with a point. >=20 > [...] > > +RTE_REGISTER_BUS(auxiliary, auxiliary_bus.bus); > > +RTE_LOG_REGISTER(auxiliary_bus_logtype, bus.auxiliary, NOTICE); >=20 > Please replace with RTE_LOG_REGISTER_DEFAULT. >=20 > [...] > > +++ b/drivers/bus/auxiliary/linux/auxiliary.c > [...] > > +/** > > + * @file > > + * Linux auxiliary probing. > > + */ >=20 > No need of doxygen comment in a .c file. >=20 > [...] > > +++ b/drivers/bus/auxiliary/meson.build > > @@ -0,0 +1,11 @@ > > +# SPDX-License-Identifier: BSD-3-Clause > > +# Copyright 2021 Mellanox Technologies, Ltd > > + > > +headers =3D files('rte_bus_auxiliary.h') > > +sources =3D files('auxiliary_common.c', > > + 'auxiliary_params.c') > > +if is_linux > > + sources +=3D files('linux/auxiliary.c') > > +endif > > +deps +=3D ['kvargs'] >=20 > Please change the indent with spaces > and check with devtools/check-meson.py >=20 > [...] > > +++ b/drivers/bus/auxiliary/private.h > [...] > > +/* Auxiliary bus iterators */ > > +#define FOREACH_DEVICE_ON_AUXILIARYBUS(p) \ >=20 > Please avoid tabs at the end of the line. > A space is enough before the backslash. >=20 > > + TAILQ_FOREACH(p, &(auxiliary_bus.device_list), next) > > + > > +#define FOREACH_DRIVER_ON_AUXILIARYBUS(p) \ > > + TAILQ_FOREACH(p, &(auxiliary_bus.driver_list), next) > > + > > +/** > > + * Test whether the auxiliary device exist > > + * > > + * @param name > > + * Auxiliary device name > > + * @return > > + * true on exists, false otherwise > > + */ > > +bool auxiliary_dev_exists(const char *name); >=20 > Given it is not an API, no need of doxygen. > And the function is so simple that no need of comment at all. >=20 > [...] > > +++ b/drivers/bus/auxiliary/rte_bus_auxiliary.h > > +#ifndef _RTE_BUS_AUXILIARY_H_ > > +#define _RTE_BUS_AUXILIARY_H_ >=20 > No need of underscores before and after. > (yes I know a lot of DPDK files use such scheme) >=20 > > + > > +/** > > + * @file > > + * > > + * RTE Auxiliary Bus Interface. >=20 > No need of RTE, it has no meaning here. >=20 > > + */ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* Forward declarations */ > > +struct rte_auxiliary_driver; > > +struct rte_auxiliary_bus; > > +struct rte_auxiliary_device; > > + > > +/** > > + * Match function for the driver to decide if device can be handled. > > + * > > + * @param name > > + * Pointer to the auxiliary device name. > > + * @return > > + * Whether the driver can handle the auxiliary device. > > + */ > > +typedef bool(*rte_auxiliary_match_t) (const char *name); >=20 > I disagree with the earlier comment asking for typedef pointer > (based on one of my patches). > Actually Andrew's suggestion makes sense: > http://mails.dpdk.org/archives/dev/2021-June/210665.html >=20 > [...] > > +++ b/drivers/bus/auxiliary/version.map > > +DPDK_21 { > > + local: *; > > +}; >=20 > We don't need this empty section. >=20 > > + > > +EXPERIMENTAL { > > + global: > > + >=20 > As asked by Ray, we should add this line: > # added in 21.08 >=20 > > + rte_auxiliary_register; > > + rte_auxiliary_unregister; > > +}; >=20 > Release notes are missing. >=20 Thanks for all the good catches, will update and reflect in next version.