>-----Original Message-----
>From: Intel-xe <[email protected]> On Behalf Of David E.
>Box
>Sent: Tuesday, March 3, 2026 12:20 AM
>To: [email protected]; Vivi, Rodrigo <[email protected]>;
>[email protected]; [email protected];
>[email protected]; [email protected]; dri-
>[email protected]; [email protected]
>Cc: [email protected]; [email protected]; linux-
>[email protected]; [email protected]
>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 <[email protected]>
M
>No functional change intended.
>
>Signed-off-by: David E. Box <[email protected]>
>---
>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
