From: Dan Williams <dan.j.williams@intel.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: linux-cxl@vger.kernel.org, Linux PCI <linux-pci@vger.kernel.org>,
Alison Schofield <alison.schofield@intel.com>,
Ira Weiny <ira.weiny@intel.com>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [PATCH 02/23] cxl: Flesh out register names
Date: Tue, 23 Nov 2021 20:24:53 -0800 [thread overview]
Message-ID: <CAPcyv4gcK_UCSjKq4LUz=AgrNsWPp+peOyzRo2NHa4Be=yCtUQ@mail.gmail.com> (raw)
In-Reply-To: <20211120000250.1663391-3-ben.widawsky@intel.com>
On Fri, Nov 19, 2021 at 4:03 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> Get a better naming scheme in place for upcoming additions.
I prefer that subject lines and changelog have at least one concrete
detail. Writing that out would identify that "rename REGLOC to
REG_LOCATOR", is separate from "drop the unused PCI_DVSEC_ID_CXL
definition", is different from "drop redundant prefixes of 'PCI' and
'DVSEC' from defines".
With some concrete details added to the changelog you can add:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> ---
> Changes since RFCv2:
> Use some abbreviations (Jonathan)
> Prefix everything with CXL (Jonathan)
> Remove new additions (Dan)
>
> Original discussion motivating this occurred here:
> https://lore.kernel.org/linux-pci/20210913190131.xiiszmno46qie7v5@intel.com/
> ---
> drivers/cxl/pci.c | 14 +++++++-------
> drivers/cxl/pci.h | 19 ++++++++++---------
> 2 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 8dc91fd3396a..a6ea9811a05b 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -403,10 +403,10 @@ static int cxl_map_regs(struct cxl_dev_state *cxlds, struct cxl_register_map *ma
> static void cxl_decode_regblock(u32 reg_lo, u32 reg_hi,
> struct cxl_register_map *map)
> {
> - map->block_offset =
> - ((u64)reg_hi << 32) | (reg_lo & CXL_REGLOC_ADDR_MASK);
> - map->barno = FIELD_GET(CXL_REGLOC_BIR_MASK, reg_lo);
> - map->reg_type = FIELD_GET(CXL_REGLOC_RBI_MASK, reg_lo);
> + map->block_offset = ((u64)reg_hi << 32) |
> + (reg_lo & CXL_DVSEC_REG_LOCATOR_BLOCK_OFF_LOW_MASK);
> + map->barno = FIELD_GET(CXL_DVSEC_REG_LOCATOR_BIR_MASK, reg_lo);
> + map->reg_type = FIELD_GET(CXL_DVSEC_REG_LOCATOR_BLOCK_ID_MASK, reg_lo);
> }
>
> /**
> @@ -427,15 +427,15 @@ static int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
> int regloc, i;
>
> regloc = pci_find_dvsec_capability(pdev, PCI_DVSEC_VENDOR_ID_CXL,
> - PCI_DVSEC_ID_CXL_REGLOC_DVSEC_ID);
> + CXL_DVSEC_REG_LOCATOR);
> if (!regloc)
> return -ENXIO;
>
> pci_read_config_dword(pdev, regloc + PCI_DVSEC_HEADER1, ®loc_size);
> regloc_size = FIELD_GET(PCI_DVSEC_HEADER1_LENGTH_MASK, regloc_size);
>
> - regloc += PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET;
> - regblocks = (regloc_size - PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET) / 8;
> + regloc += CXL_DVSEC_REG_LOCATOR_BLOCK1_OFFSET;
> + regblocks = (regloc_size - CXL_DVSEC_REG_LOCATOR_BLOCK1_OFFSET) / 8;
>
> for (i = 0; i < regblocks; i++, regloc += 8) {
> u32 reg_lo, reg_hi;
> diff --git a/drivers/cxl/pci.h b/drivers/cxl/pci.h
> index 7d3e4bf06b45..29b8eaef3a0a 100644
> --- a/drivers/cxl/pci.h
> +++ b/drivers/cxl/pci.h
> @@ -7,17 +7,21 @@
>
> /*
> * See section 8.1 Configuration Space Registers in the CXL 2.0
> - * Specification
> + * Specification. Names are taken straight from the specification with "CXL" and
> + * "DVSEC" redundancies removed. When obvious, abbreviations may be used.
> */
> #define PCI_DVSEC_HEADER1_LENGTH_MASK GENMASK(31, 20)
> #define PCI_DVSEC_VENDOR_ID_CXL 0x1E98
> -#define PCI_DVSEC_ID_CXL 0x0
>
> -#define PCI_DVSEC_ID_CXL_REGLOC_DVSEC_ID 0x8
> -#define PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET 0xC
> +/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
> +#define CXL_DVSEC_PCIE_DEVICE 0
>
> -/* BAR Indicator Register (BIR) */
> -#define CXL_REGLOC_BIR_MASK GENMASK(2, 0)
> +/* CXL 2.0 8.1.9: Register Locator DVSEC */
> +#define CXL_DVSEC_REG_LOCATOR 8
> +#define CXL_DVSEC_REG_LOCATOR_BLOCK1_OFFSET 0xC
> +#define CXL_DVSEC_REG_LOCATOR_BIR_MASK GENMASK(2, 0)
> +#define CXL_DVSEC_REG_LOCATOR_BLOCK_ID_MASK GENMASK(15, 8)
> +#define CXL_DVSEC_REG_LOCATOR_BLOCK_OFF_LOW_MASK GENMASK(31, 16)
>
> /* Register Block Identifier (RBI) */
> enum cxl_regloc_type {
> @@ -28,7 +32,4 @@ enum cxl_regloc_type {
> CXL_REGLOC_RBI_TYPES
> };
>
> -#define CXL_REGLOC_RBI_MASK GENMASK(15, 8)
> -#define CXL_REGLOC_ADDR_MASK GENMASK(31, 16)
> -
> #endif /* __CXL_PCI_H__ */
> --
> 2.34.0
>
next prev parent reply other threads:[~2021-11-24 4:25 UTC|newest]
Thread overview: 127+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-20 0:02 [PATCH 00/23] Add drivers for CXL ports and mem devices Ben Widawsky
2021-11-20 0:02 ` [PATCH 01/23] cxl: Rename CXL_MEM to CXL_PCI Ben Widawsky
2021-11-22 14:47 ` Jonathan Cameron
2021-11-24 4:15 ` Dan Williams
2021-11-20 0:02 ` [PATCH 02/23] cxl: Flesh out register names Ben Widawsky
2021-11-22 14:49 ` Jonathan Cameron
2021-11-24 4:24 ` Dan Williams [this message]
2021-11-20 0:02 ` [PATCH 03/23] cxl/pci: Extract device status check Ben Widawsky
2021-11-22 15:03 ` Jonathan Cameron
2021-11-24 19:30 ` Dan Williams
2021-11-20 0:02 ` [PATCH 04/23] cxl/pci: Implement Interface Ready Timeout Ben Widawsky
2021-11-22 15:02 ` Jonathan Cameron
2021-11-22 17:17 ` Ben Widawsky
2021-11-22 17:53 ` Jonathan Cameron
2021-11-24 19:56 ` Dan Williams
2021-11-25 6:17 ` Ben Widawsky
2021-11-25 7:14 ` Dan Williams
2021-11-20 0:02 ` [PATCH 05/23] cxl/pci: Don't poll doorbell for mailbox access Ben Widawsky
2021-11-22 15:11 ` Jonathan Cameron
2021-11-22 17:24 ` Ben Widawsky
2021-11-24 21:55 ` Dan Williams
2021-11-29 18:33 ` Ben Widawsky
2021-11-29 19:02 ` Dan Williams
2021-11-29 19:11 ` Ben Widawsky
2021-11-29 19:18 ` Dan Williams
2021-11-29 19:31 ` Ben Widawsky
2021-11-29 19:37 ` Dan Williams
2021-11-29 19:50 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 06/23] cxl/pci: Don't check media status for mbox access Ben Widawsky
2021-11-22 15:19 ` Jonathan Cameron
2021-11-24 21:58 ` Dan Williams
2021-11-20 0:02 ` [PATCH 07/23] cxl/pci: Add new DVSEC definitions Ben Widawsky
2021-11-22 15:22 ` Jonathan Cameron
2021-11-22 17:32 ` Ben Widawsky
2021-11-24 22:03 ` Dan Williams
2021-11-20 0:02 ` [PATCH 08/23] cxl/acpi: Map component registers for Root Ports Ben Widawsky
2021-11-22 15:51 ` Jonathan Cameron
2021-11-22 19:28 ` Ben Widawsky
2021-11-24 22:18 ` Dan Williams
2021-11-20 0:02 ` [PATCH 09/23] cxl: Introduce module_cxl_driver Ben Widawsky
2021-11-22 15:54 ` Jonathan Cameron
2021-11-24 22:22 ` Dan Williams
2021-11-20 0:02 ` [PATCH 10/23] cxl/core: Convert decoder range to resource Ben Widawsky
2021-11-22 16:08 ` Jonathan Cameron
2021-11-24 22:41 ` Dan Williams
2021-11-20 0:02 ` [PATCH 11/23] cxl/core: Document and tighten up decoder APIs Ben Widawsky
2021-11-22 16:13 ` Jonathan Cameron
2021-11-24 22:55 ` Dan Williams
2021-11-20 0:02 ` [PATCH 12/23] cxl: Introduce endpoint decoders Ben Widawsky
2021-11-22 16:20 ` Jonathan Cameron
2021-11-22 19:37 ` Ben Widawsky
2021-11-25 0:07 ` Dan Williams
2021-11-29 20:05 ` Ben Widawsky
2021-11-29 20:07 ` Dan Williams
2021-11-29 20:12 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 13/23] cxl/core: Move target population locking to caller Ben Widawsky
2021-11-22 16:33 ` Jonathan Cameron
2021-11-22 21:58 ` Ben Widawsky
2021-11-23 11:05 ` Jonathan Cameron
2021-11-25 0:34 ` Dan Williams
2021-11-20 0:02 ` [PATCH 14/23] cxl: Introduce topology host registration Ben Widawsky
2021-11-22 18:20 ` Jonathan Cameron
2021-11-22 22:30 ` Ben Widawsky
2021-11-25 1:09 ` Dan Williams
2021-11-29 21:23 ` Ben Widawsky
2021-11-29 11:42 ` Dan Carpenter
2021-11-20 0:02 ` [PATCH 15/23] cxl/core: Store global list of root ports Ben Widawsky
2021-11-22 18:22 ` Jonathan Cameron
2021-11-22 22:32 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 16/23] cxl/pci: Cache device DVSEC offset Ben Widawsky
2021-11-22 16:46 ` Jonathan Cameron
2021-11-22 22:34 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 17/23] cxl: Cache and pass DVSEC ranges Ben Widawsky
2021-11-20 4:29 ` kernel test robot
2021-11-22 17:00 ` Jonathan Cameron
2021-11-22 22:50 ` Ben Widawsky
2021-11-26 11:37 ` Jonathan Cameron
2021-11-20 0:02 ` [PATCH 18/23] cxl/pci: Implement wait for media active Ben Widawsky
2021-11-22 17:03 ` Jonathan Cameron
2021-11-22 22:57 ` Ben Widawsky
2021-11-23 11:09 ` Jonathan Cameron
2021-11-23 16:04 ` Ben Widawsky
2021-11-23 17:48 ` Bjorn Helgaas
2021-11-23 19:37 ` Ben Widawsky
2021-11-26 11:36 ` Jonathan Cameron
2021-11-20 0:02 ` [PATCH 19/23] cxl/pci: Store component register base in cxlds Ben Widawsky
2021-11-20 7:28 ` kernel test robot
2021-11-22 17:11 ` Jonathan Cameron
2021-11-22 23:01 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 20/23] cxl/port: Introduce a port driver Ben Widawsky
2021-11-20 3:14 ` kernel test robot
2021-11-20 5:38 ` kernel test robot
2021-11-22 17:41 ` Jonathan Cameron
2021-11-22 23:38 ` Ben Widawsky
2021-11-23 11:38 ` Jonathan Cameron
2021-11-23 16:14 ` Ben Widawsky
2021-11-23 18:21 ` Bjorn Helgaas
2021-11-23 22:03 ` Ben Widawsky
2021-11-23 22:36 ` Dan Williams
2021-11-23 23:38 ` Ben Widawsky
2021-11-23 23:55 ` Bjorn Helgaas
2021-11-24 0:40 ` Dan Williams
2021-11-24 6:33 ` Christoph Hellwig
2021-11-24 7:17 ` Dan Williams
2021-11-24 7:28 ` Christoph Hellwig
2021-11-24 7:33 ` Greg Kroah-Hartman
2021-11-24 7:54 ` Dan Williams
2021-11-24 8:21 ` Greg Kroah-Hartman
2021-11-24 18:24 ` Dan Williams
2021-12-02 21:24 ` Bjorn Helgaas
2021-12-03 1:38 ` Dan Williams
2021-12-03 22:03 ` Bjorn Helgaas
2021-12-04 1:24 ` Dan Williams
2021-12-07 2:56 ` Bjorn Helgaas
2021-12-07 4:48 ` Dan Williams
2021-11-24 21:31 ` Bjorn Helgaas
2021-11-20 0:02 ` [PATCH 21/23] cxl: Unify port enumeration for decoders Ben Widawsky
2021-11-22 17:48 ` Jonathan Cameron
2021-11-22 23:44 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 22/23] cxl/mem: Introduce cxl_mem driver Ben Widawsky
2021-11-20 0:40 ` Randy Dunlap
2021-11-21 3:55 ` Ben Widawsky
2021-11-22 18:17 ` Jonathan Cameron
2021-11-23 0:05 ` Ben Widawsky
2021-11-20 0:02 ` [PATCH 23/23] cxl/mem: Disable switch hierarchies for now Ben Widawsky
2021-11-22 18:19 ` Jonathan Cameron
2021-11-22 19:17 ` Ben Widawsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPcyv4gcK_UCSjKq4LUz=AgrNsWPp+peOyzRo2NHa4Be=yCtUQ@mail.gmail.com' \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=ben.widawsky@intel.com \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).