On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> From: "David E. Box" <david.e.box@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> ---
> 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".
> +};
> +
> /**
> * 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.
