Re: [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1

From: Lee Jones <lee.jones@linaro.org>
To: "David E. Box" <david.e.box@linux.intel.com>
Cc: hdegoede@redhat.com, mgross@linux.intel.com,
	linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1
Date: Tue, 9 Mar 2021 16:45:05 +0000	[thread overview]
Message-ID: <20210309164505.GS4931@dell> (raw)
In-Reply-To: <20210224201005.1034005-2-david.e.box@linux.intel.com>

On Wed, 24 Feb 2021, David E. Box wrote:

> Adds PMT Telemetry aggregator support for the DG1 graphics PCIe card. The
> device does not have the DVSEC region in its PCI config space so hard
> code the discovery table data in the driver. Also requires a fix for DG1
> in the Telemetry driver for how the ACCESS_TYPE field is used.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> Based on 5.11-rc1 review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
> 
> Changes from V1:
> 
> 	- New patch
> 
>  drivers/mfd/intel_pmt.c                    | 101 +++++++++++++++------
>  drivers/platform/x86/intel_pmt_class.c     |  46 ++++++++++
>  drivers/platform/x86/intel_pmt_class.h     |   1 +
>  drivers/platform/x86/intel_pmt_telemetry.c |  20 ----
>  4 files changed, 119 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
> index 65da2b17a204..dd7eb614c28e 100644
> --- a/drivers/mfd/intel_pmt.c
> +++ b/drivers/mfd/intel_pmt.c
> @@ -49,10 +49,14 @@ enum pmt_quirks {
>  
>  	/* Use shift instead of mask to read discovery table offset */
>  	PMT_QUIRK_TABLE_SHIFT	= BIT(2),
> +
> +	/* DVSEC not present (provided in driver data) */
> +	PMT_QUIRK_NO_DVSEC	= BIT(3),
>  };
>  
>  struct pmt_platform_info {
>  	unsigned long quirks;
> +	struct intel_dvsec_header **capabilities;
>  };
>  
>  static const struct pmt_platform_info tgl_info = {
> @@ -60,6 +64,26 @@ static const struct pmt_platform_info tgl_info = {
>  		  PMT_QUIRK_TABLE_SHIFT,
>  };
>  
> +/* DG1 Platform with DVSEC quirk*/
> +static struct intel_dvsec_header dg1_telemetry = {
> +	.length = 0x10,
> +	.id = 2,
> +	.num_entries = 1,
> +	.entry_size = 3,
> +	.tbir = 0,
> +	.offset = 0x466000,
> +};
> +
> +static struct intel_dvsec_header *dg1_capabilities[] = {
> +	&dg1_telemetry,
> +	NULL
> +};
> +
> +static const struct pmt_platform_info dg1_info = {
> +	.quirks = PMT_QUIRK_NO_DVSEC,
> +	.capabilities = dg1_capabilities,
> +};
> +
>  static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
>  		       unsigned long quirks)
>  {
> @@ -147,37 +171,54 @@ static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (info)
>  		quirks = info->quirks;
>  
> -	do {
> -		struct intel_dvsec_header header;
> -		u32 table;
> -		u16 vid;
> +	if (info && (info->quirks & PMT_QUIRK_NO_DVSEC)) {

Nit: Why not use 'quirks' from a few lines above?

> +		struct intel_dvsec_header **header;

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog