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 3EBB442D15; Wed, 21 Jun 2023 16:30:45 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B8A9D4068E; Wed, 21 Jun 2023 16:30:44 +0200 (CEST) Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2063.outbound.protection.outlook.com [40.107.223.63]) by mails.dpdk.org (Postfix) with ESMTP id D8C4B4003C for ; Wed, 21 Jun 2023 16:30:42 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YoPGMsciZ+Ki62guNHKF4rDqVlUSF5ei2j1F2yLK7f21MccRla+Z/gU17t6xYO2iHjccthh9m9LDEpyikDvxqZapEIWeicO+gm0shYSB1t0NYvP/ZnY5RCEBQ3Z1Xtul4HxM6rkG0K/MHUx6SYlActkYDEMC6n3ZRe3eyd61AU6SQlLrq4LvVrb1x1Fs4zgpkB7YmSuGZmnYtovbSLMmHR4aEM/t49mKfDePDNv0+gn3KDfrN8LnI/1ynpDjOJJLWGa1YaF+4z7Xt+hwI6pV58n9XLm5z9In3jqk/pm8qD55jOGMp9+JNbIY94qUOBlnLn5p6vf6txERI2uEIYOarw== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=gR7xGG5f3GI+Xqzq4+frCCyUVfjQk0GU7kiw/OBaQ6E=; b=g54ZkPQJklQCIQSAqvVexh0OR887ViXk0+DZmZxhSXbjSsocEo/KDLFaEgBc6SzSYDDZ/45J64le+Z2zxBRxS5+xODfnz4B2UvSWLVbzM5/HpKLhmK24Dwu0QVIn7o4fR4u8HTZEn7RoEAGaqv2N6pDFhTRBsGU6mjSlyaA9Wim7FHfjZeFeKMxr8bKVVukFEWw2HqSxUrgUJd8psD3CEDbwHem0SYOAryqkkwzhUUzyMyXmh/7nKwyvuPKn5F+VuBl6v0AX2Zch69lXQz8el9W8+KM2dyzB2Ut4Bvjsvz6Ag17KXOsDNdsye7qNrFO7ZEZ2jY6yvJJ6+JxYcNI4eQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=gR7xGG5f3GI+Xqzq4+frCCyUVfjQk0GU7kiw/OBaQ6E=; b=mDeHZUNHQffzxzLNj30EpPaV+9Pac15rzx6w+FfPwTuC54VfqCj4CogDBl9AQra4RdaRsm423HBtKcOQ4AEe8e5IsPW4koasMAbm2k3RB+FikeAMgIYBp03lwoUZHRqXSSW5JVdZ/aNLkdD9aLODWoOEvazy3rzam/dvKon35Qo= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by PH7PR12MB7282.namprd12.prod.outlook.com (2603:10b6:510:209::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6500.37; Wed, 21 Jun 2023 14:30:40 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::16e3:326c:5c2a:be42]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::16e3:326c:5c2a:be42%3]) with mapi id 15.20.6521.023; Wed, 21 Jun 2023 14:30:40 +0000 Message-ID: <4450bbf1-797f-6474-7c02-67957a89b0b0@amd.com> Date: Wed, 21 Jun 2023 15:30:35 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v4 03/34] common/sfc_efx/base: add API to list HW tables Content-Language: en-US To: Ivan Malov Cc: dev@dpdk.org, Andrew Rybchenko , Denis Pryazhennikov , Andy Moreton References: <20230601195538.8265-1-ivan.malov@arknetworks.am> <20230607130245.8048-1-ivan.malov@arknetworks.am> <20230607130245.8048-4-ivan.malov@arknetworks.am> <1170a8ae-f3f6-88b1-c48c-ce4bb74710bc@amd.com> <7dc198b0-1b9d-3eb5-4a69-933ded9af0d9@arknetworks.am> From: Ferruh Yigit In-Reply-To: <7dc198b0-1b9d-3eb5-4a69-933ded9af0d9@arknetworks.am> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0037.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:152::6) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|PH7PR12MB7282:EE_ X-MS-Office365-Filtering-Correlation-Id: 0b55c163-650d-438e-26c1-08db72641679 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: wyvb3sLGkh8Ji8OI/ZII63nBwGSIOktQVZ8g3tSwmeZICBpNqZyp7SL3RSoH3Bq9OF+4J4IhE2KNpPzO4iVMXErYzTmkaxq9VBnmupOe59lZJ/+SrgEupmYbD7ifg6Gtd6K8gH3jJCkjq/LT2NYNEzYP1WlXP46MrTaJuJIoo5t2YNWrpT0ajCakCdIc4Oj9OAEHcaY3M1yZ7Og1IHvBlAfiB6eNB5DCLOhfNvVzXkm32+ZMHZMeCx1CVWBsHmWgmO6NJnhgYuiv4K76Y4Ri8w8e2nIyhYIK11r67peSQ3wTKC8QNJzsHN30vIjnL0ZYgNegxgvI6y55iCng4V9ydjJLFib/57nRZNTNOTNo/MLZsnspIFdigQ2Ci4imBaK5rtJNpRQk56zhr/qLK3oIv8+wQCNB0f/45CGB5SO6iId0NiEoZKJjJkIIwxUcJx2/mrtNmbp3budkaqLUaAxNUHrFfC5L3jHv3ftn0voLPiS9ofUGfWzU67gvlCgOu2o8fcu+aFBx9NJ+rPqvJUhe8eDvcXbN69vzXJaaPPh96qnuUDSCnp9yOfwZmnLYmo9bH3MCzPK+EAPs8wTnzwXPoIw9feiAvsz4WKKQ3hhXrH2f9BKaumHnpA20ytpPa4d81t3E0ecrO/jw106fvM3sVA== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(4636009)(366004)(136003)(346002)(39860400002)(396003)(376002)(451199021)(54906003)(186003)(6486002)(6666004)(53546011)(86362001)(31696002)(6512007)(6506007)(26005)(478600001)(2616005)(31686004)(38100700002)(83380400001)(316002)(66946007)(66556008)(66476007)(4326008)(6916009)(5660300002)(44832011)(2906002)(41300700001)(36756003)(8936002)(8676002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UGRPakwwT0k0a1hJSDhIc1ZyOEVyNFlDTVFJdzBnWFJINDM0TGtvcmhqMUxw?= =?utf-8?B?aVUya3RxTTI3YVJnSG5XcmhYcGh4VVVuaHpWb2tremdVRURmZzBNOXVGREls?= =?utf-8?B?TEk4blpTM25NM0pGRHBaek0yWWxkVFB5V0ZHcTQ3dkQ4Qmp1NFNBWE8wYnNs?= =?utf-8?B?SG54TTNRWmN4TnVRZEwwY2JETjBoM1Z5TndMVkpOaHdrdXVQNWJEZ0k0M0Rz?= =?utf-8?B?RkFtRzlxQkF1ZkJUR0o1ZSt5VGxjb0FtVStDVHE0bzl4Mnl6bURnaSsrd3pY?= =?utf-8?B?V3MxY2VBcVNlQlM4ZENtNnJaWUJLMG1WN3RibCtCcjNua0Nhdm1rQ3B1azVQ?= =?utf-8?B?bE40UEp2ODZTUy9rUUZtaUtZWGwrY01LM004dWtaek81enN4S3FqR3o1b0py?= =?utf-8?B?TUxsTkJ3YjhRdlh3SUJORUZ1eVZhdC9BZmZYMG45VHk3YXZJc1VKV3I1REc1?= =?utf-8?B?V3F6VlNpaUhTRTRMOTBGajdCcEpDcXFLOWVzd3JiTVZOVENqVjhFM2gvaVVn?= =?utf-8?B?bHpOOHYxRlZKSWx3UWFTQjRIMXZyT3BmeTdxTEpTK3B6dlBnRFhqVzlEQUtx?= =?utf-8?B?VUFZV0wrRTJUL0VHbVBMMzhHbE5uc0xUVFFZVWJuMW5DQmpKU3E5R1NTdE9q?= =?utf-8?B?Y25BRTJNWTlGRDBDVDl5NTJEakhSWlRTcDliR3RIVnY3NjFPNThWcmtLd0Ir?= =?utf-8?B?Y08vOThPM3hmelZZa2pYNXFoc0tVdzJsTnhETVVMK3JvK2ZLYk5TTE9qcWtW?= =?utf-8?B?UWh3Nm9NN3FSNE5HekJDWmIxRHBOcWdKOFJMcEhMQnd1aWEwdjRRQU43Y2gr?= =?utf-8?B?d2RjM0I3U3VtemZyb2VFSGV4SXpLYU9GcDBOeDg0NGNwVWhJMkRvNWp1eDRa?= =?utf-8?B?dWNUVUtxMnlIaHJUVHlMTHEySE1CVnNPbU9ST3F4Sis1Q3dDaWQwWXJlemJK?= =?utf-8?B?V2d3V1ROd0J1b2cyV0lPc2RFZHFkRCthMDNvblhGOUxrUkYrdE1PbUpLek1s?= =?utf-8?B?eWowT0RyRExyd0hNZ1dMUVNKc0JiU0JNMXErN1JGZlJvVllCWjd3cGVqRnZx?= =?utf-8?B?Wk9zeXZUbmMyZ2s3QWFnZnJFZGxVbUg1WHpiYTNIa1lIWE9lekhHRWxGSFNn?= =?utf-8?B?UE9FSURjUEg3L2dOQmdEbFhUY1V6VzZZeXREVFRieVN4dzVnN2pzeUlrd3hU?= =?utf-8?B?Y2RUSGo3YjFIMDNEMEpzUkRRWjg0WXhnanE1bkVKQVRQK2hEdGJHVDhyeng0?= =?utf-8?B?RUNXTE5zZWQzdi9BcVVMeHhDRGt1MkRoZVFrTGFlbmllUU1TYVN3ZkVnUkNP?= =?utf-8?B?VGRxL05SYU42MktNajlmc1BsZWU3UlBidi9MUXo2VktJb1RDVmp2eVlmUVJz?= =?utf-8?B?ejVQdUJJdDlYQWdxa3RoY21vR1dWSCswU3Y0NWcxcHd4K25ydzBMNGJLUU14?= =?utf-8?B?YzdxN2FpOGdBL0VBVURnMjVBOEs3ZnE5QkZvR2t4akpIVHJqNEUyblBWZllu?= =?utf-8?B?VFNTNWxyZ3FyOUsxTG9zNGloMjBTZEJETDdKd1Y3S0F0dmMvK3lZNEZUSExa?= =?utf-8?B?NlF1L2xYelVMelBSNFFPc25yQjVRWUFjc2NsaVVmZzBEY3FFZGxZT2Vnckdu?= =?utf-8?B?Zk1jcDdTOWtKeFcvNC9ZRXk1aURwM25xdExJZ0U1bUVCUGVXSG5MeE9iNzFC?= =?utf-8?B?Y2FNQ0JNb3BzNnBPN0RVNFRydkRqSDFWTlR3Y016VE9CS3R1Z3pOZ3VmTlNP?= =?utf-8?B?UzNYWjg3Q1V3elk0MzNLdkxNQWhhcWovSUZNKzlIaDQ2VjhteTJvU29oWllz?= =?utf-8?B?Y3grMXo2d1lOYUFxOXhXbm9jbWdwL1M1NGwrRUF6UHNsUHdrajZrNm9RUWdX?= =?utf-8?B?MjlXTmlGNCt4bWNLdjJpWXprS1ZmT1VpWlNGbUtiTVVvbXhJNG82cWxPUUNQ?= =?utf-8?B?N1I1bTdBcWJjeC9qS0pYb0lwdG1Dc2ZRcTZMaWtwN2dGMTRmQUJJNDdZRlhJ?= =?utf-8?B?NnZyQU0rNWt6a0NsamNycjRmTXl1WEltdUVRa3UzRlQzbkJkOXR4N09sZjAy?= =?utf-8?B?Tzc4ekdqVXdiSVUwRWNyU0Y3cjA3eko3VHFLR3VIZUlpZEZlTVh0eStVVUJK?= =?utf-8?Q?5WFJfvilVOpbO8GP73ZlypCW7?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0b55c163-650d-438e-26c1-08db72641679 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jun 2023 14:30:40.4409 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: nZKNtEROAdIO8jv7Z2W3osxpD2Sz6SZ+o3ix31jM3HbQy7X901BAM+r/DMkErO3k X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7282 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 On 6/21/2023 12:09 PM, Ivan Malov wrote: > Hi Ferruh, > > Thank you so much for your review notes. You suggested to squash > some pairs of "common/sfc_base/efx" and "net/sfc" patches, so > that logical changes would be unified. I see your point. > > On the other hand, however, doing so would be rather unusual > from our internal process standpoint. Typically, we stick > with the idea that splitting patches into smaller ones is > better, as it is much easier to reason about and debug > smaller changesets rather than bigger ones. > > The thing is, the DPDK driver is not the only one based on > the common code ("libefx"), that is, we got used to > keeping things separate, even despite some of them > beging logically connected. I apologise in case > my explanation is still vague. > > If the reader comes across a libefx patch in one of the > other libefx-based projects and wants to search for it > in the other projects (DPDK), it is much easier for > them to find what they need provided that the patch > exists in DPDK in the same format, as a separate > change set with (almost) the same commit summary. > > In other words, there are pros and cons to squashing > things, well, at least in this particular series, > which is rather big and complicated. > > How about we retain the series as it is, in its current state > this time? We hope to adopt the suggested ("bigger logical > patches") next time, in our future work. What do you think? > > We would discuss this internally, with our team, and come > up with the new approach for us all to structure future > patch sets, for some other features yet to be supported. > Ack, thanks. Let me continue with this set as it is taking into account that -rc2 is so close, we can sync more for future patches. > Thank you. > > On Mon, 19 Jun 2023, Ferruh Yigit wrote: > >> On 6/7/2023 2:02 PM, Ivan Malov wrote: >>> From: Denis Pryazhennikov >>> >>> New MCDI Table Access API allows management of >>> the HW tables' content. >>> This part of API helps to list all supported tables. >>> In the near future, only the CT table is planned >>> to be used, so only one identifier for this table >>> was added to the efx. >>> New table IDs will be added as needed. >>> >>> Signed-off-by: Denis Pryazhennikov >>> Reviewed-by: Andy Moreton >>> >> >> This patch adds a function to the base code, but it is disconnected from >> the context. >> In the future if someone looks this function in git log, there is no >> easy way to see why this function added and where/how it is used at time >> it is added etc.. >> >> So, instead of making commit per function, can you please split commits >> based on functionality/logic? >> >> Please combine the commit that new function and commit where new >> function is used to single commit, making a commit per feature? >> >> >> If you are concerned about checkpatch warnings related to the component >> (like common/sfc_efx/base), please ignore it for the case when a feature >> is distributed into multiple components, and feel free to use most >> appropriate component name, I assume it will be driver component >> (net/sfc) most of the times. >> >> >> There is apply errors on CI, which prevents CI checks, can you please >> rebase set on top of latest head? >> >> >> Btw, we call 'API' to end-user facing functions, that user directly >> call, for this context better to call it 'function', but after patches >> merged probably you won't need it at all. >> >> Thanks, >> Ferruh >>