From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Dave Jiang <dave.jiang@intel.com>,
linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org
Cc: dan.j.williams@intel.com, ira.weiny@intel.com,
vishal.l.verma@intel.com, alison.schofield@intel.com,
Jonathan.Cameron@huawei.com, dave@stgolabs.net,
bhelgaas@google.com, lukas@wunner.de,
Bjorn Helgaas <helgaas@kernel.org>
Subject: Re: [PATCH v4 1/4] PCI/cxl: Move PCI CXL vendor Id to a common location from CXL subsystem
Date: Tue, 9 Apr 2024 14:28:00 -0700 [thread overview]
Message-ID: <162f9331-8e42-4a0d-b2f1-56bbb780a03b@linux.intel.com> (raw)
In-Reply-To: <20240409160256.94184-2-dave.jiang@intel.com>
On 4/9/24 9:01 AM, Dave Jiang wrote:
> Move PCI_DVSEC_VENDOR_ID_CXL in CXL private code to PCI_VENDOR_ID_CXL in
> pci_ids.h in order to be utilized in PCI subsystem.
>
> The response Bjorn received from the PCI SIG was "1E98h is not a VID in our
> system, but 1E98 has already been reserved by CXL." He suggested "we should
> add '#define PCI_VENDOR_ID_CXL 0x1e98' so that if we ever *do* see such an
> assignment, we'll be more likely to flag it as an issue.
Nit: Instead of including the comments as-it-is, I think it is better just state
the conclusion.
>
> Link: https://lore.kernel.org/linux-cxl/20240402172323.GA1818777@bhelgaas/
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
> drivers/cxl/core/pci.c | 6 +++---
> drivers/cxl/core/regs.c | 2 +-
> drivers/cxl/cxlpci.h | 1 -
> drivers/cxl/pci.c | 2 +-
> drivers/perf/cxl_pmu.c | 2 +-
> include/linux/pci_ids.h | 2 ++
> 6 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 0df09bd79408..c496a9710d62 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -525,7 +525,7 @@ static int cxl_cdat_get_length(struct device *dev,
> __le32 response[2];
> int rc;
>
> - rc = pci_doe(doe_mb, PCI_DVSEC_VENDOR_ID_CXL,
> + rc = pci_doe(doe_mb, PCI_VENDOR_ID_CXL,
> CXL_DOE_PROTOCOL_TABLE_ACCESS,
> &request, sizeof(request),
> &response, sizeof(response));
> @@ -555,7 +555,7 @@ static int cxl_cdat_read_table(struct device *dev,
> __le32 request = CDAT_DOE_REQ(entry_handle);
> int rc;
>
> - rc = pci_doe(doe_mb, PCI_DVSEC_VENDOR_ID_CXL,
> + rc = pci_doe(doe_mb, PCI_VENDOR_ID_CXL,
> CXL_DOE_PROTOCOL_TABLE_ACCESS,
> &request, sizeof(request),
> rsp, sizeof(*rsp) + remaining);
> @@ -640,7 +640,7 @@ void read_cdat_data(struct cxl_port *port)
> if (!pdev)
> return;
>
> - doe_mb = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
> + doe_mb = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_CXL,
> CXL_DOE_PROTOCOL_TABLE_ACCESS);
> if (!doe_mb) {
> dev_dbg(dev, "No CDAT mailbox\n");
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 372786f80955..da52fc9e234b 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -313,7 +313,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type,
> .resource = CXL_RESOURCE_NONE,
> };
>
> - regloc = pci_find_dvsec_capability(pdev, PCI_DVSEC_VENDOR_ID_CXL,
> + regloc = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> CXL_DVSEC_REG_LOCATOR);
> if (!regloc)
> return -ENXIO;
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 93992a1c8eec..4da07727ab9c 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -13,7 +13,6 @@
> * "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
>
> /* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
> #define CXL_DVSEC_PCIE_DEVICE 0
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 2ff361e756d6..110478573296 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -817,7 +817,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> cxlds->rcd = is_cxl_restricted(pdev);
> cxlds->serial = pci_get_dsn(pdev);
> cxlds->cxl_dvsec = pci_find_dvsec_capability(
> - pdev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> + pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> if (!cxlds->cxl_dvsec)
> dev_warn(&pdev->dev,
> "Device DVSEC not present, skip CXL.mem init\n");
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 308c9969642e..a1b742b1a735 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -345,7 +345,7 @@ static ssize_t cxl_pmu_event_sysfs_show(struct device *dev,
>
> /* For CXL spec defined events */
> #define CXL_PMU_EVENT_CXL_ATTR(_name, _gid, _msk) \
> - CXL_PMU_EVENT_ATTR(_name, PCI_DVSEC_VENDOR_ID_CXL, _gid, _msk)
> + CXL_PMU_EVENT_ATTR(_name, PCI_VENDOR_ID_CXL, _gid, _msk)
>
> static struct attribute *cxl_pmu_event_attrs[] = {
> CXL_PMU_EVENT_CXL_ATTR(clock_ticks, CXL_PMU_GID_CLOCK_TICKS, BIT(0)),
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index a0c75e467df3..7dfbf6d96b3d 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2607,6 +2607,8 @@
>
> #define PCI_VENDOR_ID_ALIBABA 0x1ded
>
> +#define PCI_VENDOR_ID_CXL 0x1e98
> +
> #define PCI_VENDOR_ID_TEHUTI 0x1fc9
> #define PCI_DEVICE_ID_TEHUTI_3009 0x3009
> #define PCI_DEVICE_ID_TEHUTI_3010 0x3010
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
next prev parent reply other threads:[~2024-04-09 21:28 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 16:01 [PATCH 0/4 v4] PCI: Add Secondary Bus Reset (SBR) support for CXL Dave Jiang
2024-04-09 16:01 ` [PATCH v4 1/4] PCI/cxl: Move PCI CXL vendor Id to a common location from CXL subsystem Dave Jiang
2024-04-09 21:28 ` Kuppuswamy Sathyanarayanan [this message]
2024-04-09 22:51 ` Dan Williams
2024-04-09 16:01 ` [PATCH v4 2/4] PCI: Add check for CXL Secondary Bus Reset Dave Jiang
2024-04-09 21:39 ` Kuppuswamy Sathyanarayanan
2024-04-09 21:56 ` Dave Jiang
2024-04-11 2:33 ` Kuppuswamy Sathyanarayanan
2024-04-09 22:56 ` Dan Williams
2024-04-09 16:01 ` [PATCH v4 3/4] PCI: Create new reset method to force SBR for CXL Dave Jiang
2024-04-26 19:46 ` Dan Williams
2024-04-27 6:19 ` Lukas Wunner
2024-04-27 17:07 ` Dan Williams
2024-04-09 16:01 ` [PATCH v4 4/4] cxl: Add post reset warning if reset results in loss of previously committed HDM decoders Dave Jiang
2024-04-26 20:03 ` Dan Williams
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=162f9331-8e42-4a0d-b2f1-56bbb780a03b@linux.intel.com \
--to=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=helgaas@kernel.org \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.