[PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks

[Ruhl, Michael J]

>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen at linux.intel.com>
>Sent: Thursday, July 11, 2024 7:17 AM
>To: Ruhl, Michael J <michael.j.ruhl at intel.com>
>Cc: intel-xe at lists.freedesktop.org; platform-driver-x86 at vger.kernel.org;
>david.e.box at linux.intel.com; Brost, Matthew <matthew.brost at intel.com>
>Subject: Re: [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks
>
>On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
>
>> From: "David E. Box" <david.e.box at linux.intel.com>
>>
>> Some PMT providers require device specific actions before their telemetry
>> can be read. Provide assignable PMT read callbacks to allow providers to
>> perform those actions.
>>
>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
>> Signed-off-by: David E. Box <david.e.box at linux.intel.com>
>> ---
>>  drivers/platform/x86/intel/vsec.c |  1 +
>>  include/linux/intel_vsec.h        | 14 ++++++++++++++
>>  2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>> index 2b46807f868b..7b5cc9993974 100644
>> --- a/drivers/platform/x86/intel/vsec.c
>> +++ b/drivers/platform/x86/intel/vsec.c
>> @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
>struct intel_vsec_header *he
>>  	intel_vsec_dev->num_resources = header->num_entries;
>>  	intel_vsec_dev->quirks = info->quirks;
>>  	intel_vsec_dev->base_addr = info->base_addr;
>> +	intel_vsec_dev->priv_data = info->priv_data;
>>
>>  	if (header->id == VSEC_ID_SDSI)
>>  		intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
>> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
>> index 1a287541a2f9..4569a55e8645 100644
>> --- a/include/linux/intel_vsec.h
>> +++ b/include/linux/intel_vsec.h
>> @@ -67,6 +67,19 @@ enum intel_vsec_quirks {
>>  	VSEC_QUIRK_EARLY_HW     = BIT(4),
>>  };
>>
>> +/**
>> + * struct pmt_callbacks - Callback infrastructure for PMT devices
>> + * ->read_telem() when specified, called by client driver to access PMT data
>(instead
>> + * of direct copy).
>> + * @args:  pci device info pointer
>
>PCI
>
>> + * @guid:  ID of data to acccss
>> + * @data:  buffer for the data to be copied
>> + * @count: size of buffer
>> + */
>> +struct pmt_callbacks {
>> +	int (*read_telem)(void *args, u32 guid, u64 *data, u32 count);
>
>Can you explain why args is void * and not struct pci_dev *? I was already
>unsure earlier why void * is needed but now you even explicitly documented
>it to be "pci device info pointer".

This was an artifact of a previous attempt.  I was going to use an opaque pointer
to pass the device info, but David pointed out that the PCI device was available
and sufficient.

I have updated to use struct pce_dev.

Thanks!

M

>> +};
>> +
>>  /**
>>   * struct intel_vsec_platform_info - Platform specific data
>>   * @parent:    parent device in the auxbus chain
>> @@ -78,6 +91,7 @@ enum intel_vsec_quirks {
>>  struct intel_vsec_platform_info {
>>  	struct device *parent;
>>  	struct intel_vsec_header **headers;
>> +	void *priv_data;
>>  	unsigned long caps;
>>  	unsigned long quirks;
>>  	u64 base_addr;
>>
>
>--
> i.