Re: [PATCH 1/6] platform/x86/intel/pmc: Enable PkgC LTR blocking counter

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

On 3/20/26 03:25, Ilpo Järvinen wrote:
> On Mon, 2 Mar 2026, Xi Pardee wrote:
>
>> Enable the Package C-state LTR blocking counter in the PMT telemetry
>> region. This counter records how many times any Package C-state entry
>> is blocked for the specified reasons.
>>
>> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
>> ---
>>   drivers/platform/x86/intel/pmc/core.c | 77 ++++++++++++++++++++++-----
>>   drivers/platform/x86/intel/pmc/core.h | 15 +++++-
>>   2 files changed, 79 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
>> index 02b303418d185..bf95a1f2ba428 100644
>> --- a/drivers/platform/x86/intel/pmc/core.c
>> +++ b/drivers/platform/x86/intel/pmc/core.c
>> @@ -1071,6 +1071,28 @@ static int pmc_core_die_c6_us_show(struct seq_file *s, void *unused)
>>   }
>>   DEFINE_SHOW_ATTRIBUTE(pmc_core_die_c6_us);
>>   
>> +static int pmc_core_pkgc_ltr_blocker_show(struct seq_file *s, void *unused)
>> +{
>> +	struct pmc_dev *pmcdev = s->private;
>> +	const char **pkgc_ltr_blocker_counters;
>> +	u32 counter, offset;
>> +	unsigned int i;
>> +	int ret;
>> +
>> +	offset = pmcdev->pkgc_ltr_blocker_offset;
>> +	pkgc_ltr_blocker_counters = pmcdev->pkgc_ltr_blocker_counters;
>> +	for (i = 0; pkgc_ltr_blocker_counters[i]; i++, offset++) {
>> +		ret = pmt_telem_read32(pmcdev->pc_ep, offset,
>> +				       &counter, 1);
> Fits easily to one line. Though, I'm not sure if offset variable improves
> things, more like it makes this more complex than it need to be. To me it
> would look more straightforward to do just:
>
> 		ret = pmt_telem_read32(pmcdev->pc_ep,
> 				       pmcdev->pkgc_ltr_blocker_offset + i,
> 				       ...);

Will change in next version.

>> +		if (ret)
>> +			return ret;
>> +		seq_printf(s, "%-30s %-30u\n", pkgc_ltr_blocker_counters[i], counter);
>> +	}
>> +
>> +	return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(pmc_core_pkgc_ltr_blocker);
>> +
>>   static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
>>   {
>>   	struct pmc_dev *pmcdev = s->private;
>> @@ -1322,7 +1344,7 @@ static struct telem_endpoint *pmc_core_register_endpoint(struct pci_dev *pcidev,
>>   	return ERR_PTR(-ENODEV);
>>   }
>>   
>> -void pmc_core_punit_pmt_init(struct pmc_dev *pmcdev, u32 *guids)
>> +void pmc_core_punit_pmt_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
>>   {
>>   	struct telem_endpoint *ep;
>>   	struct pci_dev *pcidev;
>> @@ -1333,17 +1355,35 @@ void pmc_core_punit_pmt_init(struct pmc_dev *pmcdev, u32 *guids)
>>   		return;
>>   	}
>>   
>> -	ep = pmc_core_register_endpoint(pcidev, guids);
>> -	pci_dev_put(pcidev);
>> -	if (IS_ERR(ep)) {
>> -		dev_err(&pmcdev->pdev->dev,
>> -			"pmc_core: couldn't get DMU telem endpoint %ld",
>> -			PTR_ERR(ep));
>> -		return;
>> +	if (pmc_dev_info->dmu_guids) {
>> +		ep = pmc_core_register_endpoint(pcidev, pmc_dev_info->dmu_guids);
>> +		if (IS_ERR(ep)) {
>> +			dev_err(&pmcdev->pdev->dev,
>> +				"pmc_core: couldn't get DMU telem endpoint %ld",
>> +				PTR_ERR(ep));
>> +			goto release_dev;
>> +		}
>> +
>> +		pmcdev->punit_ep = ep;
>> +		pmcdev->die_c6_offset = MTL_PMT_DMU_DIE_C6_OFFSET;
>> +	}
>> +
>> +	if (pmc_dev_info->pc_guid) {
>> +		ep = pmt_telem_find_and_register_endpoint(pcidev, pmc_dev_info->pc_guid, 0);
>> +		if (IS_ERR(ep)) {
>> +			dev_err(&pmcdev->pdev->dev,
>> +				"pmc_core: couldn't get Package C-state telem endpoint %ld",
>> +				PTR_ERR(ep));
>> +			goto release_dev;
>> +		}
>> +
>> +		pmcdev->pc_ep = ep;
>> +		pmcdev->pkgc_ltr_blocker_counters = pmc_dev_info->pkgc_ltr_blocker_counters;
>> +		pmcdev->pkgc_ltr_blocker_offset = pmc_dev_info->pkgc_ltr_blocker_offset;
>>   	}
>>   
>> -	pmcdev->punit_ep = ep;
>> -	pmcdev->die_c6_offset = MTL_PMT_DMU_DIE_C6_OFFSET;
>> +release_dev:
>> +	pci_dev_put(pcidev);
> Use __free(pci_dev_put) instead of complicating the code flow.
> Please remember to place the variable declaration at the place of
> assignment (no = NULL; assignments with __free()). You may want to do the
> __free() conversion in own patch preceeding this.

Thanks!

Will change and add another patch to convert to use __free() in next 
version.

Xi