RE: [PATCH v7 3/6] platform/x86/intel/vsec: Decouple add/link helpers from PCI

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of David
>E. Box
>Sent: Tuesday, March 3, 2026 12:20 AM
>To: thomas.hellstrom@xxxxxxxxxxxxxxx; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>;
>irenic.rajneesh@xxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx;
>srinivas.pandruvada@xxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx; dri-
>devel@xxxxxxxxxxxxxxxxxxxxx; xi.pardee@xxxxxxxxxxxxxxx
>Cc: david.e.box@xxxxxxxxxxxxxxx; hansg@xxxxxxxxxx; linux-
>kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx
>Subject: [PATCH v7 3/6] platform/x86/intel/vsec: Decouple add/link helpers
>from PCI
>
>This refactor is prepares for adding ACPI-enumerated PMT endpoints. While
>intel_vsec is bound to PCI today, some helpers are used by code that will
>also register PMT endpoints from non-PCI (ACPI) paths. Clean up
>PCI-specific plumbing where it isn’t strictly required and rely on
>generic struct device where possible.
>
>Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
>Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
>---
>Changes in v7:
>  - Correct the remaining struct device * argument to intel_vsec_add_aux().
>    When dropping the unused first argument in v6, the device parameter was
>    inadvertently changed from &vsec_dev->auxdev.dev to the PCI device.
>    Restore the aux device.
>
>Changes in v6:
>  - No change
>
>Changes in v5:
>  - No change
>
>Changes in v4:
>  - No change
>
>Changes in v3:
>  - No change
>
>Changes in v2:
>  - No change (previous patch 1)
>
> drivers/platform/x86/intel/vsec.c      | 13 +++++++++----
> drivers/platform/x86/intel/vsec_tpmi.c |  2 +-
> include/linux/intel_vsec.h             |  2 +-
> 3 files changed, 11 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>index e0096be605d9..938648b9ef09 100644
>--- a/drivers/platform/x86/intel/vsec.c
>+++ b/drivers/platform/x86/intel/vsec.c
>@@ -158,18 +158,23 @@ static bool vsec_driver_present(int cap_id)
>  */
> static const struct pci_device_id intel_vsec_pci_ids[];
>
>-static int intel_vsec_link_devices(struct pci_dev *pdev, struct device *dev,
>+static int intel_vsec_link_devices(struct device *parent, struct device *dev,
> 				   int consumer_id)

This functionality is explicitly for pci devices?

would intel_vsec_pci_link_devices()  (or something similar) be a better name?

Regardless,

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

M

> {
> 	const struct vsec_feature_dependency *deps;
> 	enum vsec_device_state *state;
> 	struct device **suppliers;
> 	struct vsec_priv *priv;
>+	struct pci_dev *pdev;
> 	int supplier_id;
>
> 	if (!consumer_id)
> 		return 0;
>
>+	if (!dev_is_pci(parent))
>+		return 0;
>+
>+	pdev = to_pci_dev(parent);
> 	if (!pci_match_id(intel_vsec_pci_ids, pdev))
> 		return 0;
>
>@@ -204,7 +209,7 @@ static int intel_vsec_link_devices(struct pci_dev *pdev,
>struct device *dev,
> 	return 0;
> }
>
>-int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
>+int intel_vsec_add_aux(struct device *parent,
> 		       struct intel_vsec_device *intel_vsec_dev,
> 		       const char *name)
> {
>@@ -252,7 +257,7 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct
>device *parent,
> 	if (ret)
> 		goto cleanup_aux;
>
>-	ret = intel_vsec_link_devices(pdev, &auxdev->dev, intel_vsec_dev-
>>cap_id);
>+	ret = intel_vsec_link_devices(parent, &auxdev->dev, intel_vsec_dev-
>>cap_id);
> 	if (ret)
> 		goto cleanup_aux;
>
>@@ -343,7 +348,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
>struct intel_vsec_header *he
> 	 * Pass the ownership of intel_vsec_dev and resource within it to
> 	 * intel_vsec_add_aux()
> 	 */
>-	return intel_vsec_add_aux(pdev, parent, no_free_ptr(intel_vsec_dev),
>+	return intel_vsec_add_aux(parent, no_free_ptr(intel_vsec_dev),
> 				  intel_vsec_name(header->id));
> }
>
>diff --git a/drivers/platform/x86/intel/vsec_tpmi.c
>b/drivers/platform/x86/intel/vsec_tpmi.c
>index 98846e88d3d0..2298b6361094 100644
>--- a/drivers/platform/x86/intel/vsec_tpmi.c
>+++ b/drivers/platform/x86/intel/vsec_tpmi.c
>@@ -655,7 +655,7 @@ static int tpmi_create_device(struct intel_tpmi_info
>*tpmi_info,
> 	 * feature_vsec_dev and res memory are also freed as part of
> 	 * device deletion.
> 	 */
>-	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
>+	return intel_vsec_add_aux(&vsec_dev->auxdev.dev,
> 				  feature_vsec_dev, feature_id_name);
> }
>
>diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
>index d551174b0049..49a746ec0128 100644
>--- a/include/linux/intel_vsec.h
>+++ b/include/linux/intel_vsec.h
>@@ -184,7 +184,7 @@ struct pmt_feature_group {
> 	struct telemetry_region	regions[];
> };
>
>-int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
>+int intel_vsec_add_aux(struct device *parent,
> 		       struct intel_vsec_device *intel_vsec_dev,
> 		       const char *name);
>
>--
>2.43.0