Re: [PATCH v4 14/16] platform/x86/intel/pmc/ssram: Add ACPI discovery scaffolding

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 14 May 2026, David E. Box wrote:

> Prepare the SSRAM telemetry driver for ACPI-based discovery by adding the
> common initialization path and selection framework needed for both PCI and
> ACPI resource discovery.
> 
> At this stage, existing supported devices continue to use the PCI path.
> This change lays the groundwork for follow-on patches that wire platform
> IDs to the ACPI policy path.
> 
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - Replaced local raw ACPI discovery pointer type u32 (*)[4] with
>      acpi_disc_t in SSRAM ACPI initialization path.
> 
> V3 - No changes
> 
> V2 changes:
>   - Fixed cleanup patterns using __free() attributes
>   - Addressed Ilpo's recommendations for safer cleanup.h patterns
> 
>  .../platform/x86/intel/pmc/ssram_telemetry.c  | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> index 597bfb7ad822..1a9f1e67cbe1 100644
> --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2023, Intel Corporation.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
>  #include <linux/device.h>
> @@ -29,14 +30,17 @@ DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
>  
>  enum resource_method {
>  	RES_METHOD_PCI,
> +	RES_METHOD_ACPI,
>  };
>  
>  struct ssram_type {
>  	enum resource_method method;
> +	enum pmc_index p_index;
>  };
>  
>  static const struct ssram_type pci_main = {
>  	.method = RES_METHOD_PCI,
> +	.p_index = PMC_IDX_MAIN,
>  };
>  
>  static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
> @@ -149,6 +153,67 @@ static int pmc_ssram_telemetry_pci_init(struct pci_dev *pcidev)
>  	return ret;
>  }
>  
> +static int pmc_ssram_telemetry_get_pmc_acpi(struct pci_dev *pcidev,  unsigned int pmc_idx)
> +{
> +	u64 ssram_base;
> +
> +	ssram_base = pci_resource_start(pcidev, 0);
> +	if (!ssram_base)
> +		return -ENODEV;
> +
> +	void __iomem __free(pmc_ssram_telemetry_iounmap) *ssram =
> +		ioremap(ssram_base, SSRAM_HDR_SIZE);
> +	if (!ssram)
> +		return -ENOMEM;
> +
> +	pmc_ssram_get_devid_pwrmbase(ssram, pmc_idx);
> +
> +	return 0;
> +}
> +
> +static int pmc_ssram_telemetry_acpi_init(struct pci_dev *pcidev,
> +					 enum pmc_index index)
> +{
> +	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> +	acpi_handle handle = ACPI_HANDLE(&pcidev->dev);

Move the assignment next to !handle check.

> +	struct intel_vsec_header header;
> +	struct intel_vsec_header *headers[2] = { &header, NULL };
> +	struct intel_vsec_platform_info info = { };
> +	void *dsd_buf __free(pmc_acpi_free) = buf.pointer;

Why is this here and not below the acpi function call?

> +	union acpi_object *dsd;
> +	acpi_status status;
> +	int ret;
> +
> +	if (!handle)
> +		return -ENODEV;
> +
> +	status = acpi_evaluate_object(handle, "_DSD", NULL, &buf);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	dsd = pmc_find_telem_guid(buf.pointer);

pmc_find_telem_guid(dsd_buf) ?

> +	if (!dsd)
> +		return -ENODEV;
> +
> +	acpi_disc_t disc __free(kfree) = pmc_parse_telem_dsd(dsd, &header);
> +	if (IS_ERR(disc))
> +		return PTR_ERR(disc);
> +
> +	info.headers = headers;
> +	info.caps = VSEC_CAP_TELEMETRY;
> +	info.acpi_disc = disc;
> +	info.src = INTEL_VSEC_DISC_ACPI;
> +
> +	/* This is an ACPI companion device. PCI BAR will be used for base addr. */
> +	info.base_addr = 0;
> +
> +	ret = intel_vsec_register(&pcidev->dev, &info);
> +	if (ret)
> +		return ret;
> +
> +	return pmc_ssram_telemetry_get_pmc_acpi(pcidev, index);
> +}
> +
>  /**
>   * pmc_ssram_telemetry_get_pmc_info() - Get a PMC devid and base_addr information
>   * @pmc_idx:               Index of the PMC
> @@ -189,6 +254,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  {
>  	const struct ssram_type *ssram_type;
>  	enum resource_method method;
> +	enum pmc_index index;
>  	int ret;
>  
>  	ssram_type = (const struct ssram_type *)id->driver_data;
> @@ -198,6 +264,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  	}
>  
>  	method = ssram_type->method;
> +	index = ssram_type->p_index;
>  
>  	ret = pcim_enable_device(pcidev);
>  	if (ret) {
> @@ -207,6 +274,8 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  
>  	if (method == RES_METHOD_PCI)
>  		ret = pmc_ssram_telemetry_pci_init(pcidev);
> +	else if (method == RES_METHOD_ACPI)
> +		ret = pmc_ssram_telemetry_acpi_init(pcidev, index);
>  	else
>  		ret = -EINVAL;
>  
> @@ -239,6 +308,7 @@ static struct pci_driver pmc_ssram_telemetry_driver = {
>  };
>  module_pci_driver(pmc_ssram_telemetry_driver);
>  
> +MODULE_IMPORT_NS("INTEL_PMC_CORE");
>  MODULE_IMPORT_NS("INTEL_VSEC");
>  MODULE_AUTHOR("Xi Pardee <xi.pardee@intel.com>");
>  MODULE_DESCRIPTION("Intel PMC SSRAM Telemetry driver");
> 

-- 
 i.