Re: [PATCH 5/6] platform/x86/intel/pmc: Retrieve PMC info only for available PMCs

[Xi Pardee <xi.pardee@linux.intel.com>]

On 3/20/26 03:58, Ilpo Järvinen wrote:
> On Mon, 2 Mar 2026, Xi Pardee wrote:
>
>> Update the Intel PMC Core driver to fetch PMC information only for
>> available PMCs. Previously, the driver attempted to retrieve PMC info
>> even when the corresponding PMC was not present.
>>
>> This change aligns with recent updates to the Intel SSRAM Telemetry
>> driver. Starting with NVL, the SSRAM Telemetry driver is probed for
>> each individual SSRAM device. The prior implementation could not
>> differentiate between an unavailable PMC and one that had not yet
>> completed information retrieval. To resolve this, the PMC Core driver
>> now skips obtaining PMC info for unavailable PMCs.
>>
>> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
>> ---
>>   drivers/platform/x86/intel/pmc/arl.c  |  5 +++++
>>   drivers/platform/x86/intel/pmc/core.c | 19 +++++++++++--------
>>   drivers/platform/x86/intel/pmc/core.h |  2 ++
>>   drivers/platform/x86/intel/pmc/lnl.c  |  3 +++
>>   drivers/platform/x86/intel/pmc/mtl.c  |  3 +++
>>   drivers/platform/x86/intel/pmc/ptl.c  |  3 +++
>>   drivers/platform/x86/intel/pmc/wcl.c  |  3 +++
>>   7 files changed, 30 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
>> index 4d91ee010f6d0..34506542c94da 100644
>> --- a/drivers/platform/x86/intel/pmc/arl.c
>> +++ b/drivers/platform/x86/intel/pmc/arl.c
>> @@ -672,6 +672,9 @@ static struct pmc_info arl_pmc_info_list[] = {
>>   	{}
>>   };
>>   
>> +static u8 arl_pmc_list[] = {PMC_IDX_MAIN, PMC_IDX_IOE, PMC_IDX_PCH, PMC_IDX_MAX};
>> +static u8 arl_h_pmc_list[] = {PMC_IDX_MAIN, PMC_IDX_IOE, PMC_IDX_MAX};
> This seems a bit dangerous approach if somebody forgets to add that
> PMC_IDX_MAX. It would be better to do it with sizeof() for safety).

Will add another field num_pmcs in pmc_dev_info struct to store the 
number of available PMCs.

Will use ARRAY_SIZE() on pmc_list variable to retrieve the number of 
available PMCs and store in num_pmcs variable and check that in 
pmc_core_ssram_get_reg_base().

>> +
>>   #define ARL_NPU_PCI_DEV			0xad1d
>>   #define ARL_GNA_PCI_DEV			0xae4c
>>   #define ARL_H_NPU_PCI_DEV		0x7d1d
>> @@ -721,6 +724,7 @@ 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 = {
>>   	.dmu_guids = ARL_PMT_DMU_GUIDS,
>> +	.pmc_list = arl_pmc_list,
>>   	.regmap_list = arl_pmc_info_list,
>>   	.map = &arl_socs_reg_map,
>>   	.sub_req_show = &pmc_core_substate_req_regs_fops,
>> @@ -735,6 +739,7 @@ struct pmc_dev_info arl_pmc_dev = {
>>   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 = {
>>   	.dmu_guids = ARL_H_PMT_DMU_GUIDS,
>> +	.pmc_list = arl_h_pmc_list,
>>   	.regmap_list = arl_pmc_info_list,
>>   	.map = &mtl_socm_reg_map,
>>   	.sub_req_show = &pmc_core_substate_req_regs_fops,
>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
>> index 7bd0e1eaa32e2..85fff5e3abe0d 100644
>> --- a/drivers/platform/x86/intel/pmc/core.c
>> +++ b/drivers/platform/x86/intel/pmc/core.c
>> @@ -1744,16 +1744,19 @@ static int pmc_core_pmc_add(struct pmc_dev *pmcdev, unsigned int pmc_idx)
>>   	return 0;
>>   }
>>   
>> -static int pmc_core_ssram_get_reg_base(struct pmc_dev *pmcdev)
>> +static int pmc_core_ssram_get_reg_base(struct pmc_dev *pmcdev, u8 *pmc_list)
>>   {
>> +	unsigned int i;
>>   	int ret;
>>   
>> -	ret = pmc_core_pmc_add(pmcdev, PMC_IDX_MAIN);
>> -	if (ret)
>> -		return ret;
>> -
>> -	pmc_core_pmc_add(pmcdev, PMC_IDX_IOE);
>> -	pmc_core_pmc_add(pmcdev, PMC_IDX_PCH);
>> +	for (i = 0; pmc_list[i] != PMC_IDX_MAX; ++i) {
>> +		if (pmc_list[i] == PMC_IDX_MAIN) {
>> +			ret = pmc_core_pmc_add(pmcdev, pmc_list[i]);
>> +			if (ret)
>> +				return ret;
>> +		} else
>> +			pmc_core_pmc_add(pmcdev, pmc_list[i]);
> Isn't this same as:
>
> 		/* Non-MAIN PMCs are allowed to fail */
> 		ret = pmc_core_pmc_add(pmcdev, pmc_list[i]);
> 		if (ret && (pmc_list[i] == PMC_IDX_MAIN))
> 			return ret;
>
>
Will change to this format in next version.

Thanks!

Xi

>> +	}
>>   
>>   	return 0;
>>   }
>> @@ -1775,7 +1778,7 @@ int generic_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
>>   	ssram = pmc_dev_info->regmap_list != NULL;
>>   	if (ssram) {
>>   		pmcdev->regmap_list = pmc_dev_info->regmap_list;
>> -		ret = pmc_core_ssram_get_reg_base(pmcdev);
>> +		ret = pmc_core_ssram_get_reg_base(pmcdev, pmc_dev_info->pmc_list);
>>   		/*
>>   		 * EAGAIN error code indicates Intel PMC SSRAM Telemetry driver
>>   		 * has not finished probe and PMC info is not available yet. Try
>> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
>> index f09791f866223..c4984e44f7b80 100644
>> --- a/drivers/platform/x86/intel/pmc/core.h
>> +++ b/drivers/platform/x86/intel/pmc/core.h
>> @@ -501,6 +501,7 @@ enum pmc_index {
>>    * @pc_guid:		GUID for telemetry region to read PKGC blocker info
>>    * @pkgc_ltr_blocker_offset: Offset to PKGC LTR blockers in telemetry region
>>    * @pkgc_blocker_offset:Offset to PKGC blocker in telemetry region
>> + * @pmc_list:		Index list of available PMC
>>    * @regmap_list:	Pointer to a list of pmc_info structure that could be
>>    *			available for the platform. When set, this field implies
>>    *			SSRAM support.
>> @@ -521,6 +522,7 @@ struct pmc_dev_info {
>>   	u32 pc_guid;
>>   	u32 pkgc_ltr_blocker_offset;
>>   	u32 pkgc_blocker_offset;
>> +	u8 *pmc_list;
>>   	struct pmc_info *regmap_list;
>>   	const struct pmc_reg_map *map;
>>   	const struct file_operations *sub_req_show;
>> diff --git a/drivers/platform/x86/intel/pmc/lnl.c b/drivers/platform/x86/intel/pmc/lnl.c
>> index 18f303af328e3..7b09b59e1326c 100644
>> --- a/drivers/platform/x86/intel/pmc/lnl.c
>> +++ b/drivers/platform/x86/intel/pmc/lnl.c
>> @@ -544,6 +544,8 @@ static struct pmc_info lnl_pmc_info_list[] = {
>>   	{}
>>   };
>>   
>> +static u8 lnl_pmc_list[] = {PMC_IDX_MAIN, PMC_IDX_MAX};
>> +
>>   #define LNL_NPU_PCI_DEV		0x643e
>>   #define LNL_IPU_PCI_DEV		0x645d
>>   
>> @@ -571,6 +573,7 @@ static int lnl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
>>   }
>>   
>>   struct pmc_dev_info lnl_pmc_dev = {
>> +	.pmc_list = lnl_pmc_list,
>>   	.regmap_list = lnl_pmc_info_list,
>>   	.map = &lnl_socm_reg_map,
>>   	.sub_req_show = &pmc_core_substate_req_regs_fops,
>> diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
>> index b724dd8c34dba..6438fca266392 100644
>> --- a/drivers/platform/x86/intel/pmc/mtl.c
>> +++ b/drivers/platform/x86/intel/pmc/mtl.c
>> @@ -965,6 +965,8 @@ static struct pmc_info mtl_pmc_info_list[] = {
>>   	{}
>>   };
>>   
>> +static u8 mtl_pmc_list[] = {PMC_IDX_MAIN, PMC_IDX_IOE, PMC_IDX_MAX};
>> +
>>   #define MTL_GNA_PCI_DEV	0x7e4c
>>   #define MTL_IPU_PCI_DEV	0x7d19
>>   #define MTL_VPU_PCI_DEV	0x7d1d
>> @@ -995,6 +997,7 @@ 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 = {
>>   	.dmu_guids = MTL_PMT_DMU_GUIDS,
>> +	.pmc_list = mtl_pmc_list,
>>   	.regmap_list = mtl_pmc_info_list,
>>   	.map = &mtl_socm_reg_map,
>>   	.sub_req_show = &pmc_core_substate_req_regs_fops,
>> diff --git a/drivers/platform/x86/intel/pmc/ptl.c b/drivers/platform/x86/intel/pmc/ptl.c
>> index 6c68772e738c8..538ca5ae2e9ec 100644
>> --- a/drivers/platform/x86/intel/pmc/ptl.c
>> +++ b/drivers/platform/x86/intel/pmc/ptl.c
>> @@ -543,6 +543,8 @@ static struct pmc_info ptl_pmc_info_list[] = {
>>   	{}
>>   };
>>   
>> +static u8 ptl_pmc_list[] = {PMC_IDX_MAIN, PMC_IDX_MAX};
>> +
>>   #define PTL_NPU_PCI_DEV                0xb03e
>>   #define PTL_IPU_PCI_DEV                0xb05d
>>   
>> @@ -569,6 +571,7 @@ static int ptl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
>>   }
>>   
>>   struct pmc_dev_info ptl_pmc_dev = {
>> +	.pmc_list = ptl_pmc_list,
>>   	.regmap_list = ptl_pmc_info_list,
>>   	.map = &ptl_pcdp_reg_map,
>>   	.sub_req_show = &pmc_core_substate_blk_req_fops,
>> diff --git a/drivers/platform/x86/intel/pmc/wcl.c b/drivers/platform/x86/intel/pmc/wcl.c
>> index b55069945e9e7..429f53f22a89f 100644
>> --- a/drivers/platform/x86/intel/pmc/wcl.c
>> +++ b/drivers/platform/x86/intel/pmc/wcl.c
>> @@ -469,6 +469,8 @@ static struct pmc_info wcl_pmc_info_list[] = {
>>   	{}
>>   };
>>   
>> +static u8 wcl_pmc_list[] = {PMC_IDX_MAIN, PMC_IDX_MAX};
>> +
>>   #define WCL_NPU_PCI_DEV                0xfd3e
>>   
>>   /*
>> @@ -494,6 +496,7 @@ static int wcl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
>>   
>>   struct pmc_dev_info wcl_pmc_dev = {
>>   	.regmap_list = wcl_pmc_info_list,
>> +	.pmc_list = wcl_pmc_list,
>>   	.map = &wcl_pcdn_reg_map,
>>   	.sub_req_show = &pmc_core_substate_blk_req_fops,
>>   	.suspend = cnl_suspend,
>>