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 3DDC942FC4; Thu, 3 Aug 2023 11:58:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B6EEB4282D; Thu, 3 Aug 2023 11:58:11 +0200 (CEST) Received: from mgamail.intel.com (unknown [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 32BD34161A for ; Thu, 3 Aug 2023 11:58:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1691056690; x=1722592690; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=CxmVg0wyphVjkISMrugIWnkwy+QArutFiIyDAD6t6IM=; b=VZXRSmgJ87YG9ZzZSSqJQcYEXR0/2rA8FUyecj+ydM1w8zK5lAyC0Ok4 /yutBpHhyNkaPVWZAAt74OMpy/CuXNU5f1+1z+TpYf3p+S1eOi0Kr2JnC K76H3Je1TGZ/eNgblcVMC+VaG4bAbmRORlS41owYyBWNg8v/sqrB++bxp jy2cnlaTsBVTSYdI9FLIplciZ7xZ669nnxGuOhloS/nlxI53BJKthVWc2 x2U3vUlmdG2jbJgYuZh9AiLAoIJG4c5pakc0bm7yN8tsybMtzigm7n0Yo VLqS3fMzL3mGcMN8I7/xLlKBDv8QbcDZNu7g3HRXOEbtRd4xsU6apSjO2 w==; X-IronPort-AV: E=McAfee;i="6600,9927,10790"; a="433668901" X-IronPort-AV: E=Sophos;i="6.01,251,1684825200"; d="scan'208";a="433668901" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Aug 2023 02:57:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10790"; a="729505187" X-IronPort-AV: E=Sophos;i="6.01,251,1684825200"; d="scan'208";a="729505187" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga002.jf.intel.com with ESMTP; 03 Aug 2023 02:57:53 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 3 Aug 2023 02:57:52 -0700 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27 via Frontend Transport; Thu, 3 Aug 2023 02:57:52 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.43) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.27; Thu, 3 Aug 2023 02:57:52 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GPT6SUqhKnd0EvZkLmGk1/SqUTQeqo6jk2I0q2hbY9vrTJFiR2X8Uxj4fsNkIcR1cq8SfN9vaHFg3gs68OCigoFeLS+NClh/l0mVee0FCNFsoc+TI5q3YacrXQOVcy+BaDSew1xbRsGesl4McgiU2yKXk6QK4lGvZ6YnWCPCNtuJ9HwsnU86TV2gfpBXst2YLdW6GQelYVMCd8bIiWbPx/X9ZgPT3+ZvdVNXZnB3cWYUltD3TEXkv13QwTAv2+/SsiZNSQAKJSewDMj9DH+YEzZ+G4AbgmEvOdjIyZn01ofiBdh278vVwusJvdiu4JX6q8V+YvfKyddwn0sYzhpTUw== 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=xaoB0e1cPwcH9kqyPJvKGPSDX46YZCMEn4vnbLRzt/k=; b=DJ8wrlbfETcZa4lf7dq6oBs5EvvG51v7em4KacfEcvRTgdvAvk8gnPUYWYvRl0R2Vtn+1zyOho3BM7G2UDgrXWdCQl+Rmmu+wiuR3xF6DZQJUdXPGtzXCiM3esXSy426jPwMHrudFQtlR9nJPCh4ecDom6xrNXCIQ9jy6A0/+PamOAPCgtFAusmpQXMGQIgsDlIHLQuoOeyA+Z2vsLe1imXfj9iGupx4pXh5Mz10oWWeHbDvoHYs/G3otIrXfmyopuPOvx17GXcoLTSwj/X1wPCqtwdPTEx+2iEjnGYOXtlnM+QbmCOhUWFrzPCyBPFfc4waW7eUS2suYQvA3I4KBA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by CH3PR11MB8314.namprd11.prod.outlook.com (2603:10b6:610:170::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6631.47; Thu, 3 Aug 2023 09:57:51 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::9802:65f0:c441:780f]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::9802:65f0:c441:780f%7]) with mapi id 15.20.6631.046; Thu, 3 Aug 2023 09:57:50 +0000 Date: Thu, 3 Aug 2023 10:57:43 +0100 From: Bruce Richardson To: David Marchand CC: , , , , , Anatoly Burakov , Timothy McDaniel , Gaetan Rivet Subject: Re: [PATCH 06/14] pci: define some command constants Message-ID: References: <20230803075038.307012-1-david.marchand@redhat.com> <20230803075038.307012-7-david.marchand@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20230803075038.307012-7-david.marchand@redhat.com> X-ClientProxiedBy: DUZPR01CA0242.eurprd01.prod.exchangelabs.com (2603:10a6:10:4b5::14) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|CH3PR11MB8314:EE_ X-MS-Office365-Filtering-Correlation-Id: bbbe41c4-3345-4641-a00c-08db9408192a X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: R0SpfMjZKybnRVXwgeCZbTbFZFbedyUdIXI0SsWak1x9KQuD2zwGOW5iffe5AjUujaIJjrI+XmFAOW1LyX7PxVpU22A+r8ickqpm/Ln0LBKVIibs+nJGJxf62r0+Sts9HNnQXqJwSFbonN0SULc+if21KcpkpPTkkLEDXf3TbLfvfMSj4aIAdTh0pANQe36y6r3bfVeugdoleWpJ0TH/O+3UHlTZH3ujdRuN+OcMDmlyZYCXbzYs6aTjPpg0U2BAnySvqAYrkT+m1AUZirA6wpDchRWZ51u/M9fWreyDkiZewNQPtgsjgONREK0cnIp0WSiAH7gGhLqBpZjEKEPC/pJ3o3wkLDArX6c9RaCxohsuC+zvEnfCTeG3oYrN9VTWCzD1A5HHHbnZa1Dl4J1yCCfXaQ5GcIQuazzZYxjyvaLK1JAlmlP77Jm88IR+5zvi4FFOEl7370JK3klFepEvHqhKGSdEAu4bSqUKATXkprThneObe/aLElADR2zUH5p66urMaYKKwljcyTA1drEd6SoWrk2d8qmuzl4X2K5Sp7PTh51uY5SReTLXvXkqoPq8 X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(376002)(396003)(366004)(136003)(39860400002)(346002)(451199021)(83380400001)(6506007)(26005)(186003)(41300700001)(2906002)(66946007)(4326008)(66476007)(66556008)(5660300002)(6916009)(316002)(44832011)(8676002)(8936002)(6666004)(6486002)(6512007)(478600001)(54906003)(38100700002)(82960400001)(86362001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?JqvsAQbzX1NcuyGWShoqJE95j+hscfemmROD90/R4oddqzIWSODbTt/pQ7Cs?= =?us-ascii?Q?1OCEopHzh4VbSXXKiGP9CHsFnkWnl3y5Pa/6NG20pTjemLZgugcgD1uIdDM6?= =?us-ascii?Q?oye55EII27TIpHWx8P9kj0uIZGAnF0402tJp8BJEXwM8WG+FP/2/tEaM+cS9?= =?us-ascii?Q?SJC/mnK+PJG+tmmQhQGwyUfzass80XIIeXaw6ekwvMmrugVAIBhvZrK9bDmN?= =?us-ascii?Q?uwgl8kOdDrF4Lgii8U88y+1JVaeyYqqyWOjJjPIvn2459oRQmIuL3bys/awg?= =?us-ascii?Q?HJ1NugwiF+TeWBmgzT85Z8sdLjbwotrdNqrNH7OBHyTpZ2r+FN5rxYsxFCqh?= =?us-ascii?Q?8aHLUR3ragQF0vuRP4MyOPrVUvFyG100nu5HE1Q3kwEzRXu10moPtMt6de97?= =?us-ascii?Q?mEuad6pEsSxoHQ/+S5npNhb5uLY8PcnTAlTXi+8wGtzH9tGirnvR32TJ62LW?= =?us-ascii?Q?e5iaUTCJ2oTXUW5LK2fhU7lIzmuDqgdpSJA0GvmpQFPtettooUzyFwTFyEyc?= =?us-ascii?Q?drwtA1dC99qOGOeXJT+xluzH5fI8ZAS7q5DHDxDslD5bzjlDVO5xLz39kBR4?= =?us-ascii?Q?OGp8cwo4WXlWSA+s3SaVkyz7E/sEewCUeLWKOW2c8mR/rogvqAHvwYmWkkgK?= =?us-ascii?Q?aDEI3iC1QODceYL4azniNRYPWs4if3HH1hjNCKmnTqwNOOrJy4fRukCIjBDn?= =?us-ascii?Q?zIsJQSj03ZL0pgUcy/JAZpP6s6RcAY8GJ1XpUIEKfXYIeAuG8E+9JnP0TtY/?= =?us-ascii?Q?ZLf536tsu5ESDvIa5m2Oh1bmstaH5SKJQzrRTOg6TBOGjoWY+eV0dsC/6Tj3?= =?us-ascii?Q?gczaOyZKwFh3sOHGpZrvFTYj707uZVD9Y0gi2IpsII6gEWrGclR5qFeh/Qhn?= =?us-ascii?Q?O6IBDQcX+x6ewEVXFK61fp52mTms9hS72BDsGFzAB9pVQ+oMp3uzo+j9nxfu?= =?us-ascii?Q?xaYm+isACwC52Hbh+6Sf4Xw1/EMjodCjq/bDjkimBXugFdZfM88mzmFoTNOK?= =?us-ascii?Q?iSGKotm7tjEwNbCHq/ZVFbXVpSc0oXwiaKBSxL1zHPYrvCVfuSM/7s1XsO61?= =?us-ascii?Q?3AyzQXLuHYSfrUN0GkzULLf5QuZajhe4wrozkUNrYba+uq2/slCeSORcIPZJ?= =?us-ascii?Q?rwMS9UxRaS6sMUMIh5B0v3gFv3IoNIiQxhnXrUHmu0Ra1ZblsTmJqzsFiyO8?= =?us-ascii?Q?G80znWxvlqMym2Z8hXz0E3ezWvA3f2xrQbyrSYmdBgJyLChGdOa+q+0QRZ/1?= =?us-ascii?Q?5Avp4g8wl96DsiIwGff3e7Weyy3qO/O+8Yhg10TuhaiSAGg/XO3UBM53zsp8?= =?us-ascii?Q?tiNe+aggFuxO0JgA60r++15O9SdEGpkLWyoaa4AZN7tdRMpmFwLcs+Fgazuq?= =?us-ascii?Q?9TStbM3SNcRvDLHhSlOlc/Q2Koj7Rhqw+Q+NJzQJyt55A8v8TvQ5pY0Io/Bh?= =?us-ascii?Q?4OSqTABQWuU/oOdC7rxC5gF0byXiG38WPxKAca7o66ZekIhwcQ/O8LyPXkVI?= =?us-ascii?Q?CqAny58yLFXX2ZvgNukMGSXbCF2zJDHQ6f/wzmsoJYUs5bBhnYUIftXPrufR?= =?us-ascii?Q?ZUvd5Ubeft237uvjeJyaAkN3oj6zzdYdJBoTH7S/c+47lP9/LUWA4MGN8N/P?= =?us-ascii?Q?iQ=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: bbbe41c4-3345-4641-a00c-08db9408192a X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Aug 2023 09:57:50.7031 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: LaGb5IzKiiRAo2Ll4SbqhOmxHaP6DvdWJIToPFF0JNWxj0TRTOUVTm7uslwafUmUJ54UFqtMjnmtZS7C7KfUCbi6kCupj8+purYzZP6gF54= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8314 X-OriginatorOrg: intel.com 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 Thu, Aug 03, 2023 at 09:50:29AM +0200, David Marchand wrote: > Define some PCI command constants and use them in existing drivers. > > Signed-off-by: David Marchand > --- For the idea, all good, but for the implementation, one comment inline below. With the below reworked: Acked-by: Bruce Richardson > drivers/bus/pci/linux/pci_vfio.c | 8 ++++---- > drivers/event/dlb2/pf/dlb2_main.c | 8 +++----- > lib/pci/rte_pci.h | 4 +++- > 3 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c > index 6d13cafdcf..f96b3ce7fb 100644 > --- a/drivers/bus/pci/linux/pci_vfio.c > +++ b/drivers/bus/pci/linux/pci_vfio.c > @@ -156,18 +156,18 @@ pci_vfio_enable_bus_memory(struct rte_pci_device *dev, int dev_fd) > return -1; > } > > - ret = pread64(dev_fd, &cmd, sizeof(cmd), offset + PCI_COMMAND); > + ret = pread64(dev_fd, &cmd, sizeof(cmd), offset + RTE_PCI_COMMAND); > > if (ret != sizeof(cmd)) { > RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n"); > return -1; > } > > - if (cmd & PCI_COMMAND_MEMORY) > + if (cmd & RTE_PCI_COMMAND_MEMORY) > return 0; > > - cmd |= PCI_COMMAND_MEMORY; > - ret = pwrite64(dev_fd, &cmd, sizeof(cmd), offset + PCI_COMMAND); > + cmd |= RTE_PCI_COMMAND_MEMORY; > + ret = pwrite64(dev_fd, &cmd, sizeof(cmd), offset + RTE_PCI_COMMAND); > > if (ret != sizeof(cmd)) { > RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n"); > diff --git a/drivers/event/dlb2/pf/dlb2_main.c b/drivers/event/dlb2/pf/dlb2_main.c > index c6606a9bee..6dbaa2ff97 100644 > --- a/drivers/event/dlb2/pf/dlb2_main.c > +++ b/drivers/event/dlb2/pf/dlb2_main.c > @@ -33,7 +33,6 @@ > #define DLB2_PCI_EXP_DEVCTL2 40 > #define DLB2_PCI_LNKCTL2 48 > #define DLB2_PCI_SLTCTL2 56 > -#define DLB2_PCI_CMD 4 > #define DLB2_PCI_EXP_DEVSTA 10 > #define DLB2_PCI_EXP_DEVSTA_TRPND 0x20 > #define DLB2_PCI_EXP_DEVCTL_BCR_FLR 0x8000 > @@ -47,7 +46,6 @@ > #define DLB2_PCI_ERR_ROOT_STATUS 0x30 > #define DLB2_PCI_ERR_COR_STATUS 0x10 > #define DLB2_PCI_ERR_UNCOR_STATUS 0x4 > -#define DLB2_PCI_COMMAND_INTX_DISABLE 0x400 > #define DLB2_PCI_ACS_CAP 0x4 > #define DLB2_PCI_ACS_CTRL 0x6 > #define DLB2_PCI_ACS_SV 0x1 > @@ -286,7 +284,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev) > > /* clear the PCI command register before issuing the FLR */ > > - off = DLB2_PCI_CMD; > + off = RTE_PCI_COMMAND; > cmd = 0; > if (rte_pci_write_config(pdev, &cmd, 2, off) != 2) { > DLB2_LOG_ERR("[%s()] failed to write the pci command\n", > @@ -468,9 +466,9 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev) > } > } > > - off = DLB2_PCI_CMD; > + off = RTE_PCI_COMMAND; > if (rte_pci_read_config(pdev, &cmd, 2, off) == 2) { > - cmd &= ~DLB2_PCI_COMMAND_INTX_DISABLE; > + cmd &= ~RTE_PCI_COMMAND_INTX_DISABLE; > if (rte_pci_write_config(pdev, &cmd, 2, off) != 2) { > DLB2_LOG_ERR("[%s()] failed to write the pci command\n", > __func__); > diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h > index a055a28592..bf2b2639f4 100644 > --- a/lib/pci/rte_pci.h > +++ b/lib/pci/rte_pci.h > @@ -32,10 +32,12 @@ extern "C" { > > #define RTE_PCI_VENDOR_ID 0x00 /* 16 bits */ > #define RTE_PCI_DEVICE_ID 0x02 /* 16 bits */ > -#define RTE_PCI_COMMAND 0x04 /* 16 bits */ > > /* PCI Command Register */ > +#define RTE_PCI_COMMAND 0x04 /* 16 bits */ > +#define RTE_PCI_COMMAND_MEMORY 0x2 /* Enable response in Memory space */ > #define RTE_PCI_COMMAND_MASTER 0x4 /* Bus Master Enable */ > +#define RTE_PCI_COMMAND_INTX_DISABLE 0x400 /* INTx Emulation Disable */ > It's weird here to have two defines in the same block with the same value - even if one is 0x04 and the other is 0x4. That implies that the two values don't belong in the same block of defines. Question - would enums make sense for defining any of these? It allows the groups to be named? > /* PCI Status Register */ > #define RTE_PCI_STATUS 0x06 /* 16 bits */ > -- > 2.41.0 >