diff mbox series

[v5,1/4] PCI/cxl: Move PCI CXL vendor Id to a common location from CXL subsystem

Message ID 20240429223610.1341811-2-dave.jiang@intel.com
State Superseded
Headers show
Series PCI: Add Secondary Bus Reset (SBR) support for CXL | expand

Commit Message

Dave Jiang April 29, 2024, 10:35 p.m. UTC 
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.

When uplevelling PCI_DVSEC_VENDOR_ID_CXL to a common locatoin Bjorn
suggested making it a proper PCI_VENDOR_ID_* define in
include/linux/pci_ids.h. While it is not in the PCI IDs database it is a
reserved value and Linux treats it as a 'vendor id' for all intents and
purposes [1].

Link: https://lore.kernel.org/linux-cxl/20240402172323.GA1818777@bhelgaas/
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Improve commit log. (Dan)
---
 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(-)

Comments

PJ Waskiewicz May 1, 2024, 3:37 p.m. UTC | #1 
On Mon, 2024-04-29 at 15:35 -0700, 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.
> 
> When uplevelling PCI_DVSEC_VENDOR_ID_CXL to a common locatoin Bjorn
> suggested making it a proper PCI_VENDOR_ID_* define in
> include/linux/pci_ids.h. While it is not in the PCI IDs database it
> is a
> reserved value and Linux treats it as a 'vendor id' for all intents
> and
> purposes [1].

Would you consider a patch, after this series merges, to upstream
pciutils to sync up lspci's name of this value as well?  It would be
less confusing to anyone looking at both codebases and trying to line
up #define's.

-PJ
Dave Jiang May 1, 2024, 4:04 p.m. UTC | #2 
On 5/1/24 8:37 AM, PJ Waskiewicz wrote:
> On Mon, 2024-04-29 at 15:35 -0700, 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.
>>
>> When uplevelling PCI_DVSEC_VENDOR_ID_CXL to a common locatoin Bjorn
>> suggested making it a proper PCI_VENDOR_ID_* define in
>> include/linux/pci_ids.h. While it is not in the PCI IDs database it
>> is a
>> reserved value and Linux treats it as a 'vendor id' for all intents
>> and
>> purposes [1].
> 
> Would you consider a patch, after this series merges, to upstream
> pciutils to sync up lspci's name of this value as well?  It would be
> less confusing to anyone looking at both codebases and trying to line
> up #define's.
> 

Yes I can look into doing that. 

> -PJ
Dan Williams May 1, 2024, 4:09 p.m. UTC | #3 
Dave Jiang wrote:
> 
> 
> On 5/1/24 8:37 AM, PJ Waskiewicz wrote:
> > On Mon, 2024-04-29 at 15:35 -0700, 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.
> >>
> >> When uplevelling PCI_DVSEC_VENDOR_ID_CXL to a common locatoin Bjorn
> >> suggested making it a proper PCI_VENDOR_ID_* define in
> >> include/linux/pci_ids.h. While it is not in the PCI IDs database it
> >> is a
> >> reserved value and Linux treats it as a 'vendor id' for all intents
> >> and
> >> purposes [1].
> > 
> > Would you consider a patch, after this series merges, to upstream
> > pciutils to sync up lspci's name of this value as well?  It would be
> > less confusing to anyone looking at both codebases and trying to line
> > up #define's.
> > 
> 
> Yes I can look into doing that. 

Wait, isn't this Martin's call? If pcituils follows the kernel, great,
if it does not, that's pciutil's prerogative.
diff mbox series

Patch

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 3c42f984eeaf..e1082e749c69 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -314,7 +314,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