From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Xi Pardee <xi.pardee@linux.intel.com>
Cc: irenic.rajneesh@gmail.com, david.e.box@linux.intel.com,
platform-driver-x86@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
linux-pm@vger.kernel.org
Subject: Re: [PATCH 3/6] platform/x86/intel/pmc: Use PCI DID for PMC SSRAM device discovery
Date: Fri, 20 Mar 2026 12:45:15 +0200 (EET) [thread overview]
Message-ID: <60626f08-0214-6748-5a64-31d88d545fc4@linux.intel.com> (raw)
In-Reply-To: <20260302223214.484585-4-xi.pardee@linux.intel.com>
On Mon, 2 Mar 2026, Xi Pardee wrote:
> Update the PMC SSRAM discovery process to identify the device using its
> PCI Device ID rather than relying on a fixed PCI bus location. The
> enumeration of integrated devices on the PCI bus is no longer guaranteed
> to be consistent across CPUs.
>
> On earlier platforms, the IOE and PCH SSRAM devices were hidden from the
> BIOS, and the SOC SSRAM device is associated to telemetry regions from all
> available SSRAM devices. Starting with Nova Lake, the IOE and PCH SSRAM
> devices register their telemetry regions independently, meaning each
> telemetry region is now linked to its corresponding SSRAM device. A new
> ssram_hidden attribute has been added to the pmc_dev_info structure to
> reflect this distinction.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
> drivers/platform/x86/intel/pmc/arl.c | 4 ++--
> drivers/platform/x86/intel/pmc/core.c | 16 ++++++++++++----
> drivers/platform/x86/intel/pmc/core.h | 6 ++++--
> drivers/platform/x86/intel/pmc/lnl.c | 2 +-
> drivers/platform/x86/intel/pmc/mtl.c | 2 +-
> drivers/platform/x86/intel/pmc/ptl.c | 2 +-
> drivers/platform/x86/intel/pmc/wcl.c | 2 +-
> 7 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
> index eb23bc68340ab..95372a0807acf 100644
> --- a/drivers/platform/x86/intel/pmc/arl.c
> +++ b/drivers/platform/x86/intel/pmc/arl.c
> @@ -720,7 +720,6 @@ static int arl_h_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_
>
> static u32 ARL_PMT_DMU_GUIDS[] = {ARL_PMT_DMU_GUID, 0x0};
> struct pmc_dev_info arl_pmc_dev = {
> - .pci_func = 0,
> .dmu_guids = ARL_PMT_DMU_GUIDS,
> .regmap_list = arl_pmc_info_list,
> .map = &arl_socs_reg_map,
> @@ -729,11 +728,11 @@ struct pmc_dev_info arl_pmc_dev = {
> .resume = arl_resume,
> .init = arl_core_init,
> .sub_req = pmc_core_pmt_get_lpm_req,
> + .ssram_hidden = true,
> };
>
> static u32 ARL_H_PMT_DMU_GUIDS[] = {ARL_PMT_DMU_GUID, ARL_H_PMT_DMU_GUID, 0x0};
> struct pmc_dev_info arl_h_pmc_dev = {
> - .pci_func = 2,
> .dmu_guids = ARL_H_PMT_DMU_GUIDS,
> .regmap_list = arl_pmc_info_list,
> .map = &mtl_socm_reg_map,
> @@ -742,4 +741,5 @@ struct pmc_dev_info arl_h_pmc_dev = {
> .resume = arl_h_resume,
> .init = arl_h_core_init,
> .sub_req = pmc_core_pmt_get_lpm_req,
> + .ssram_hidden = true,
> };
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index e5b48a68cf495..7670970c995b9 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1660,12 +1660,10 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info *
> unsigned int pmc_idx;
> int ret;
>
> - pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, pmc_dev_info->pci_func));
> - if (!pcidev)
> - return -ENODEV;
> -
> for (pmc_idx = 0; pmc_idx < ARRAY_SIZE(pmcdev->pmcs); ++pmc_idx) {
> + struct pci_dev *pcidev __free(pci_dev_put) = NULL;
Please don't use = NULL; pattern with __free() because it's dangerous in
some cases, please see the long comment in cleanup.h for details why.
Place the declaration where you do the assignment.
> struct pmc *pmc;
> + u16 devid;
>
> pmc = pmcdev->pmcs[pmc_idx];
> if (!pmc)
> @@ -1674,6 +1672,15 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info *
> if (!pmc->map->lpm_req_guid)
> return -ENXIO;
>
> + if (pmc_dev_info->ssram_hidden)
> + devid = pmcdev->pmcs[PMC_IDX_MAIN]->devid;
> + else
> + devid = pmc->devid;
> +
> + pcidev = pci_get_device(PCI_VENDOR_ID_INTEL, devid, NULL);
> + if (!pcidev)
> + return -ENODEV;
> +
> ep = pmt_telem_find_and_register_endpoint(pcidev, pmc->map->lpm_req_guid, 0);
> if (IS_ERR(ep)) {
> dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %pe", ep);
> @@ -1724,6 +1731,7 @@ static int pmc_core_pmc_add(struct pmc_dev *pmcdev, unsigned int pmc_idx)
>
> pmc->map = map;
> pmc->base_addr = pmc_ssram_telemetry.base_addr;
> + pmc->devid = pmc_ssram_telemetry.devid;
> pmc->regbase = ioremap(pmc->base_addr, pmc->map->regmap_length);
>
> if (!pmc->regbase) {
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index 829b1dee3f636..31fe71b01120b 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -425,6 +425,7 @@ struct pmc_info {
> * @ltr_ign: Holds LTR ignore data while suspended
> * @num_lpm_modes: Count of enabled modes
> * @lpm_en_modes: Array of enabled modes from lowest to highest priority
> + * @devid: Device ID of the SSRAM device
> *
> * pmc contains info about one power management controller device.
> */
> @@ -436,6 +437,7 @@ struct pmc {
> u32 ltr_ign;
> u8 num_lpm_modes;
> u8 lpm_en_modes[LPM_MAX_NUM_MODES];
> + u16 devid;
> };
>
> /**
> @@ -495,7 +497,6 @@ enum pmc_index {
>
> /**
> * struct pmc_dev_info - Structure to keep PMC device info
> - * @pci_func: Function number of the primary PMC
> * @dmu_guids: List of Die Management Unit GUID
> * @pc_guid: GUID for telemetry region to read PKGC blocker info
> * @pkgc_ltr_blocker_offset: Offset to PKGC LTR blockers in telemetry region
> @@ -512,9 +513,9 @@ enum pmc_index {
> * @resume: Function to perform platform specific resume
> * @init: Function to perform platform specific init action
> * @sub_req: Function to achieve low power mode substate requirements
> + * @ssram_hidden: Flag to indicate whether SSRAM is hidden
I'd elaborate this as some of the SSRAM devices are hidden in this case.
IMO, the "Flag to indicate whether" is pretty pointless, the C type
information pretty much indicates it's a flag.
> */
> struct pmc_dev_info {
> - u8 pci_func;
> u32 *dmu_guids;
> u32 pc_guid;
> u32 pkgc_ltr_blocker_offset;
> @@ -528,6 +529,7 @@ struct pmc_dev_info {
> int (*resume)(struct pmc_dev *pmcdev);
> int (*init)(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
> int (*sub_req)(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
> + bool ssram_hidden;
> };
>
> extern const struct pmc_bit_map msr_map[];
> diff --git a/drivers/platform/x86/intel/pmc/lnl.c b/drivers/platform/x86/intel/pmc/lnl.c
> index 1cd81ee54dcf8..18f303af328e3 100644
> --- a/drivers/platform/x86/intel/pmc/lnl.c
> +++ b/drivers/platform/x86/intel/pmc/lnl.c
> @@ -571,7 +571,6 @@ static int lnl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
> }
>
> struct pmc_dev_info lnl_pmc_dev = {
> - .pci_func = 2,
> .regmap_list = lnl_pmc_info_list,
> .map = &lnl_socm_reg_map,
> .sub_req_show = &pmc_core_substate_req_regs_fops,
> @@ -579,4 +578,5 @@ struct pmc_dev_info lnl_pmc_dev = {
> .resume = lnl_resume,
> .init = lnl_core_init,
> .sub_req = pmc_core_pmt_get_lpm_req,
> + .ssram_hidden = true,
> };
> diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
> index 57508cbf9cd42..193ebbe584023 100644
> --- a/drivers/platform/x86/intel/pmc/mtl.c
> +++ b/drivers/platform/x86/intel/pmc/mtl.c
> @@ -994,7 +994,6 @@ static int mtl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
>
> static u32 MTL_PMT_DMU_GUIDS[] = {MTL_PMT_DMU_GUID, 0x0};
> struct pmc_dev_info mtl_pmc_dev = {
> - .pci_func = 2,
> .dmu_guids = MTL_PMT_DMU_GUIDS,
> .regmap_list = mtl_pmc_info_list,
> .map = &mtl_socm_reg_map,
> @@ -1003,4 +1002,5 @@ struct pmc_dev_info mtl_pmc_dev = {
> .resume = mtl_resume,
> .init = mtl_core_init,
> .sub_req = pmc_core_pmt_get_lpm_req,
> + .ssram_hidden = true,
> };
> diff --git a/drivers/platform/x86/intel/pmc/ptl.c b/drivers/platform/x86/intel/pmc/ptl.c
> index 1f48e2bbc699f..6c68772e738c8 100644
> --- a/drivers/platform/x86/intel/pmc/ptl.c
> +++ b/drivers/platform/x86/intel/pmc/ptl.c
> @@ -569,7 +569,6 @@ static int ptl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
> }
>
> struct pmc_dev_info ptl_pmc_dev = {
> - .pci_func = 2,
> .regmap_list = ptl_pmc_info_list,
> .map = &ptl_pcdp_reg_map,
> .sub_req_show = &pmc_core_substate_blk_req_fops,
> @@ -577,4 +576,5 @@ struct pmc_dev_info ptl_pmc_dev = {
> .resume = ptl_resume,
> .init = ptl_core_init,
> .sub_req = pmc_core_pmt_get_blk_sub_req,
> + .ssram_hidden = true,
> };
> diff --git a/drivers/platform/x86/intel/pmc/wcl.c b/drivers/platform/x86/intel/pmc/wcl.c
> index a45707e6364f2..b55069945e9e7 100644
> --- a/drivers/platform/x86/intel/pmc/wcl.c
> +++ b/drivers/platform/x86/intel/pmc/wcl.c
> @@ -493,7 +493,6 @@ static int wcl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
> }
>
> struct pmc_dev_info wcl_pmc_dev = {
> - .pci_func = 2,
> .regmap_list = wcl_pmc_info_list,
> .map = &wcl_pcdn_reg_map,
> .sub_req_show = &pmc_core_substate_blk_req_fops,
> @@ -501,4 +500,5 @@ struct pmc_dev_info wcl_pmc_dev = {
> .resume = wcl_resume,
> .init = wcl_core_init,
> .sub_req = pmc_core_pmt_get_blk_sub_req,
> + .ssram_hidden = true,
> };
>
--
i.
next prev parent reply other threads:[~2026-03-20 10:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-02 22:32 [PATCH 0/6] Enable NVL support in intel_pmc_core Xi Pardee
2026-03-02 22:32 ` [PATCH 1/6] platform/x86/intel/pmc: Enable PkgC LTR blocking counter Xi Pardee
2026-03-20 10:25 ` Ilpo Järvinen
2026-03-02 22:32 ` [PATCH 2/6] platform/x86/intel/pmc: Enable Pkgc blocking residency counter Xi Pardee
2026-03-20 10:27 ` Ilpo Järvinen
2026-03-02 22:32 ` [PATCH 3/6] platform/x86/intel/pmc: Use PCI DID for PMC SSRAM device discovery Xi Pardee
2026-03-20 10:45 ` Ilpo Järvinen [this message]
2026-03-02 22:32 ` [PATCH 4/6] platform/x86/intel/pmc: Add support for variable DMU offsets Xi Pardee
2026-03-20 10:50 ` Ilpo Järvinen
2026-03-02 22:32 ` [PATCH 5/6] platform/x86/intel/pmc: Retrieve PMC info only for available PMCs Xi Pardee
2026-03-20 10:58 ` Ilpo Järvinen
2026-03-02 22:32 ` [PATCH 6/6] platform/x86/intel/pmc: Add Nova Lake support to intel_pmc_core driver Xi Pardee
2026-03-20 11:08 ` Ilpo Järvinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=60626f08-0214-6748-5a64-31d88d545fc4@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=david.e.box@linux.intel.com \
--cc=irenic.rajneesh@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=xi.pardee@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox