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 7984342FC4; Thu, 3 Aug 2023 13:52:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6402440C35; Thu, 3 Aug 2023 13:52:11 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 5442D406BA for ; Thu, 3 Aug 2023 13:52:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1691063529; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TEFbV1FXMffyb/1EjvGhV01wGjhds3L2YAA+0wZBiMs=; b=ccefEVlp/kM8HA4CjCsBQH54U1lOYoLj7PwvfhL1cOQfaF8rw+kamvf+5dl7LaPhKEH158 Z2HEZASpZc5tFOV0VJZ9K3558Ld6DZtsE/GlQQ31NTYv00yARXz9cF69HUC9LQxiHRu6/O 7jUzlImFsdGgwOeM85uH9MMIuSBnma4= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-367-ZlWOIYz0Nh-no-Ex0RaGhQ-1; Thu, 03 Aug 2023 07:52:08 -0400 X-MC-Unique: ZlWOIYz0Nh-no-Ex0RaGhQ-1 Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2b9bf49342dso9539951fa.1 for ; Thu, 03 Aug 2023 04:52:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691063527; x=1691668327; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TEFbV1FXMffyb/1EjvGhV01wGjhds3L2YAA+0wZBiMs=; b=iaf7+8Go0pqBswXtLCC4QNgClliSNd93jZHkBnV1HQB3qwtMp1EI/RFuLt+jf15MMr lb9GPWWn3c9TaAQHJHH+GGApetmXAvy2WGqSeY3vaFWzc/y7yqVFjFeMTS/ZUk2peitr 3ZQMMpsngtpWFD9GyX5QkKdN1VVbKy6Y0KGBtTAN1tS+CKuYRUDz0cmSD2WWSzwrCeYE fKkfvMA1/bDyxfi9Ld8xlMr/gZSkOQ/N+zZkd3A2ligUgB0RE42lWJUiz0OXbJEkUPc1 8+2mT5+9Ow7whtvqze6gBI7zE9N6uKe9gYmj5c+O03PK7Nv1n6npPmpSgkh7niqtdq7x oTjA== X-Gm-Message-State: ABy/qLb9vStU3hXQaFO90EIonHGMu6P7rsLDa9CrKV/mebjUAZRkxSVP Lita8rXXqyRf81UEoWNeWs+brHk4KFczND0s3jSukayEOxywQzrwrOkgT0yCptqDK3wdTh3szoe HtVXNzmBEyvc1vwXDNCo= X-Received: by 2002:a2e:9782:0:b0:2b9:ea5f:36b with SMTP id y2-20020a2e9782000000b002b9ea5f036bmr6920952lji.52.1691063527230; Thu, 03 Aug 2023 04:52:07 -0700 (PDT) X-Google-Smtp-Source: APBJJlHcKaoUW82n4IAKgtzyqCFLoCIBYfOeq+XxoQuP8XyOWaXNNRLNS7OK517ougHqT4moDrykaMrEU00WBLm2jKg= X-Received: by 2002:a2e:9782:0:b0:2b9:ea5f:36b with SMTP id y2-20020a2e9782000000b002b9ea5f036bmr6920944lji.52.1691063526927; Thu, 03 Aug 2023 04:52:06 -0700 (PDT) MIME-Version: 1.0 References: <20230803075038.307012-1-david.marchand@redhat.com> <20230803075038.307012-7-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Thu, 3 Aug 2023 13:51:54 +0200 Message-ID: Subject: Re: [PATCH 06/14] pci: define some command constants To: Bruce Richardson Cc: dev@dpdk.org, thomas@monjalon.net, ferruh.yigit@amd.com, chenbo.xia@intel.com, nipun.gupta@amd.com, Anatoly Burakov , Timothy McDaniel , Gaetan Rivet X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 3, 2023 at 11:58=E2=80=AFAM Bruce Richardson wrote: > > 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/p= ci_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 =3D pread64(dev_fd, &cmd, sizeof(cmd), offset + PCI_COMMAND); > > + ret =3D pread64(dev_fd, &cmd, sizeof(cmd), offset + RTE_PCI_COMMA= ND); > > > > if (ret !=3D sizeof(cmd)) { > > RTE_LOG(ERR, EAL, "Cannot read command from PCI config sp= ace!\n"); > > return -1; > > } > > > > - if (cmd & PCI_COMMAND_MEMORY) > > + if (cmd & RTE_PCI_COMMAND_MEMORY) > > return 0; > > > > - cmd |=3D PCI_COMMAND_MEMORY; > > - ret =3D pwrite64(dev_fd, &cmd, sizeof(cmd), offset + PCI_COMMAND)= ; > > + cmd |=3D RTE_PCI_COMMAND_MEMORY; > > + ret =3D pwrite64(dev_fd, &cmd, sizeof(cmd), offset + RTE_PCI_COMM= AND); > > > > if (ret !=3D sizeof(cmd)) { > > RTE_LOG(ERR, EAL, "Cannot write command to PCI config spa= ce!\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 =3D DLB2_PCI_CMD; > > + off =3D RTE_PCI_COMMAND; > > cmd =3D 0; > > if (rte_pci_write_config(pdev, &cmd, 2, off) !=3D 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 =3D DLB2_PCI_CMD; > > + off =3D RTE_PCI_COMMAND; > > if (rte_pci_read_config(pdev, &cmd, 2, off) =3D=3D 2) { > > - cmd &=3D ~DLB2_PCI_COMMAND_INTX_DISABLE; > > + cmd &=3D ~RTE_PCI_COMMAND_INTX_DISABLE; > > if (rte_pci_write_config(pdev, &cmd, 2, off) !=3D 2) { > > DLB2_LOG_ERR("[%s()] failed to write the pci comm= and\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 Mem= ory 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 value= s > 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? That's something I will look into, or at least better organise the defines.= . --=20 David Marchand