1. Patchew
  2. linux
  3. [PATCH v7 0/6] platform/x86/intel/vsec: Prep for ACPI PMT discovery
  4. View patch

[PATCH v7 1/6] platform/x86/intel/vsec: Refactor base_addr handling

David E. Box posted 6 patches 1 month ago
[PATCH v7 1/6] platform/x86/intel/vsec: Refactor base_addr handling
Posted by David E. Box 1 month ago 
The base_addr field in intel_vsec_platform_info was originally added to
support devices that emulate PCI VSEC capabilities in MMIO. Previously,
the code would check at registration time whether base_addr was set,
falling back to the PCI BAR if not.

Refactor this by making base_addr an explicit function parameter. This
clarifies ownership of the value and removes conditional logic from
intel_vsec_add_dev(). It also enables making intel_vsec_platform_info
const in a later patch, since the function no longer needs to write to
info->base_addr.

No functional change intended.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
Changes in v7:
  - No change

Changes in v6:
  - No change

Changes in v5:
  - No change

Changes in v4:
  - No change

Changes in v3:
  - No change

Changes in v2:
  - Use pci_resource_start() macro instead of direct pdev->resource array
    access (suggested by Ilpo)

 drivers/platform/x86/intel/vsec.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 5059d320edf8..46966edca03b 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -271,14 +271,13 @@ EXPORT_SYMBOL_NS_GPL(intel_vsec_add_aux, "INTEL_VSEC");
 
 static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *header,
 			      struct intel_vsec_platform_info *info,
-			      unsigned long cap_id)
+			      unsigned long cap_id, u64 base_addr)
 {
 	struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
 	struct resource __free(kfree) *res = NULL;
 	struct resource *tmp;
 	struct device *parent;
 	unsigned long quirks = info->quirks;
-	u64 base_addr;
 	int i;
 
 	if (info->parent)
@@ -310,11 +309,6 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
 	if (quirks & VSEC_QUIRK_TABLE_SHIFT)
 		header->offset >>= TABLE_OFFSET_SHIFT;
 
-	if (info->base_addr)
-		base_addr = info->base_addr;
-	else
-		base_addr = pdev->resource[header->tbir].start;
-
 	/*
 	 * The DVSEC/VSEC contains the starting offset and count for a block of
 	 * discovery tables. Create a resource array of these tables to the
@@ -412,7 +406,8 @@ static int get_cap_id(u32 header_id, unsigned long *cap_id)
 
 static int intel_vsec_register_device(struct pci_dev *pdev,
 				      struct intel_vsec_header *header,
-				      struct intel_vsec_platform_info *info)
+				      struct intel_vsec_platform_info *info,
+				      u64 base_addr)
 {
 	const struct vsec_feature_dependency *consumer_deps;
 	struct vsec_priv *priv;
@@ -428,7 +423,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev,
 	 * For others using the exported APIs, add the device directly.
 	 */
 	if (!pci_match_id(intel_vsec_pci_ids, pdev))
-		return intel_vsec_add_dev(pdev, header, info, cap_id);
+		return intel_vsec_add_dev(pdev, header, info, cap_id, base_addr);
 
 	priv = pci_get_drvdata(pdev);
 	if (priv->state[cap_id] == STATE_REGISTERED ||
@@ -444,7 +439,7 @@ static int intel_vsec_register_device(struct pci_dev *pdev,
 
 	consumer_deps = get_consumer_dependencies(priv, cap_id);
 	if (!consumer_deps || suppliers_ready(priv, consumer_deps, cap_id)) {
-		ret = intel_vsec_add_dev(pdev, header, info, cap_id);
+		ret = intel_vsec_add_dev(pdev, header, info, cap_id, base_addr);
 		if (ret)
 			priv->state[cap_id] = STATE_SKIP;
 		else
@@ -464,7 +459,7 @@ static bool intel_vsec_walk_header(struct pci_dev *pdev,
 	int ret;
 
 	for ( ; *header; header++) {
-		ret = intel_vsec_register_device(pdev, *header, info);
+		ret = intel_vsec_register_device(pdev, *header, info, info->base_addr);
 		if (!ret)
 			have_devices = true;
 	}
@@ -512,7 +507,8 @@ static bool intel_vsec_walk_dvsec(struct pci_dev *pdev,
 		pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2, &hdr);
 		header.id = PCI_DVSEC_HEADER2_ID(hdr);
 
-		ret = intel_vsec_register_device(pdev, &header, info);
+		ret = intel_vsec_register_device(pdev, &header, info,
+						 pci_resource_start(pdev, header.tbir));
 		if (ret)
 			continue;
 
@@ -557,7 +553,8 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
 		header.tbir = INTEL_DVSEC_TABLE_BAR(table);
 		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
 
-		ret = intel_vsec_register_device(pdev, &header, info);
+		ret = intel_vsec_register_device(pdev, &header, info,
+						 pci_resource_start(pdev, header.tbir));
 		if (ret)
 			continue;
 
-- 
2.43.0
RE: [PATCH v7 1/6] platform/x86/intel/vsec: Refactor base_addr handling
Posted by Ruhl, Michael J 1 month ago 

>-----Original Message-----
>From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of David E.
>Box
>Sent: Tuesday, March 3, 2026 12:20 AM
>To: thomas.hellstrom@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>irenic.rajneesh@gmail.com; ilpo.jarvinen@linux.intel.com;
>srinivas.pandruvada@linux.intel.com; intel-xe@lists.freedesktop.org; dri-
>devel@lists.freedesktop.org; xi.pardee@linux.intel.com
>Cc: david.e.box@linux.intel.com; hansg@kernel.org; linux-
>kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
>Subject: [PATCH v7 1/6] platform/x86/intel/vsec: Refactor base_addr handling
>
>The base_addr field in intel_vsec_platform_info was originally added to
>support devices that emulate PCI VSEC capabilities in MMIO. Previously,
>the code would check at registration time whether base_addr was set,
>falling back to the PCI BAR if not.

It looks like this value could be set in the various static info struct (tgl_info, etc).

Would this be a reasonably thing to set/do?

Should the documentation be updated to specify specific use cases?

>Refactor this by making base_addr an explicit function parameter. This
>clarifies ownership of the value and removes conditional logic from
>intel_vsec_add_dev(). It also enables making intel_vsec_platform_info
>const in a later patch, since the function no longer needs to write to
>info->base_addr.

This makes the base_addr usage a lot more clear.

Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

M

>No functional change intended.
>
>Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>---
>Changes in v7:
>  - No change
>
>Changes in v6:
>  - No change
>
>Changes in v5:
>  - No change
>
>Changes in v4:
>  - No change
>
>Changes in v3:
>  - No change
>
>Changes in v2:
>  - Use pci_resource_start() macro instead of direct pdev->resource array
>    access (suggested by Ilpo)
>
> drivers/platform/x86/intel/vsec.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>index 5059d320edf8..46966edca03b 100644
>--- a/drivers/platform/x86/intel/vsec.c
>+++ b/drivers/platform/x86/intel/vsec.c
>@@ -271,14 +271,13 @@ EXPORT_SYMBOL_NS_GPL(intel_vsec_add_aux,
>"INTEL_VSEC");
>
> static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header
>*header,
> 			      struct intel_vsec_platform_info *info,
>-			      unsigned long cap_id)
>+			      unsigned long cap_id, u64 base_addr)
> {
> 	struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
> 	struct resource __free(kfree) *res = NULL;
> 	struct resource *tmp;
> 	struct device *parent;
> 	unsigned long quirks = info->quirks;
>-	u64 base_addr;
> 	int i;
>
> 	if (info->parent)
>@@ -310,11 +309,6 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
>struct intel_vsec_header *he
> 	if (quirks & VSEC_QUIRK_TABLE_SHIFT)
> 		header->offset >>= TABLE_OFFSET_SHIFT;
>
>-	if (info->base_addr)
>-		base_addr = info->base_addr;
>-	else
>-		base_addr = pdev->resource[header->tbir].start;
>-
> 	/*
> 	 * The DVSEC/VSEC contains the starting offset and count for a block of
> 	 * discovery tables. Create a resource array of these tables to the
>@@ -412,7 +406,8 @@ static int get_cap_id(u32 header_id, unsigned long
>*cap_id)
>
> static int intel_vsec_register_device(struct pci_dev *pdev,
> 				      struct intel_vsec_header *header,
>-				      struct intel_vsec_platform_info *info)
>+				      struct intel_vsec_platform_info *info,
>+				      u64 base_addr)
> {
> 	const struct vsec_feature_dependency *consumer_deps;
> 	struct vsec_priv *priv;
>@@ -428,7 +423,7 @@ static int intel_vsec_register_device(struct pci_dev
>*pdev,
> 	 * For others using the exported APIs, add the device directly.
> 	 */
> 	if (!pci_match_id(intel_vsec_pci_ids, pdev))
>-		return intel_vsec_add_dev(pdev, header, info, cap_id);
>+		return intel_vsec_add_dev(pdev, header, info, cap_id,
>base_addr);
>
> 	priv = pci_get_drvdata(pdev);
> 	if (priv->state[cap_id] == STATE_REGISTERED ||
>@@ -444,7 +439,7 @@ static int intel_vsec_register_device(struct pci_dev
>*pdev,
>
> 	consumer_deps = get_consumer_dependencies(priv, cap_id);
> 	if (!consumer_deps || suppliers_ready(priv, consumer_deps, cap_id)) {
>-		ret = intel_vsec_add_dev(pdev, header, info, cap_id);
>+		ret = intel_vsec_add_dev(pdev, header, info, cap_id,
>base_addr);
> 		if (ret)
> 			priv->state[cap_id] = STATE_SKIP;
> 		else
>@@ -464,7 +459,7 @@ static bool intel_vsec_walk_header(struct pci_dev
>*pdev,
> 	int ret;
>
> 	for ( ; *header; header++) {
>-		ret = intel_vsec_register_device(pdev, *header, info);
>+		ret = intel_vsec_register_device(pdev, *header, info, info-
>>base_addr);
> 		if (!ret)
> 			have_devices = true;
> 	}
>@@ -512,7 +507,8 @@ static bool intel_vsec_walk_dvsec(struct pci_dev
>*pdev,
> 		pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2,
>&hdr);
> 		header.id = PCI_DVSEC_HEADER2_ID(hdr);
>
>-		ret = intel_vsec_register_device(pdev, &header, info);
>+		ret = intel_vsec_register_device(pdev, &header, info,
>+						 pci_resource_start(pdev,
>header.tbir));
> 		if (ret)
> 			continue;
>
>@@ -557,7 +553,8 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
> 		header.tbir = INTEL_DVSEC_TABLE_BAR(table);
> 		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
>
>-		ret = intel_vsec_register_device(pdev, &header, info);
>+		ret = intel_vsec_register_device(pdev, &header, info,
>+						 pci_resource_start(pdev,
>header.tbir));
> 		if (ret)
> 			continue;
>
>--
>2.43.0

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.