linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities
@ 2021-01-28 17:28 David E. Box
  2021-02-02 14:22 ` Hans de Goede
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: David E. Box @ 2021-01-28 17:28 UTC (permalink / raw)
  To: lee.jones, hdegoede; +Cc: David E. Box, linux-kernel

Some products will be available that have PMT capabilities that are not
supported. Remove the warnings in this instance to avoid nuisance messages
and confusion.

Also return an error code for capabilities that are disabled by quirk to
prevent them from keeping the driver loaded if only disabled capabilities
are found.

Fixes: 4f8217d5b0ca ("mfd: Intel Platform Monitoring Technology support")
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 drivers/mfd/intel_pmt.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
index 744b230cdcca..65da2b17a204 100644
--- a/drivers/mfd/intel_pmt.c
+++ b/drivers/mfd/intel_pmt.c
@@ -79,19 +79,18 @@ static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
 	case DVSEC_INTEL_ID_WATCHER:
 		if (quirks & PMT_QUIRK_NO_WATCHER) {
 			dev_info(dev, "Watcher not supported\n");
-			return 0;
+			return -EINVAL;
 		}
 		name = "pmt_watcher";
 		break;
 	case DVSEC_INTEL_ID_CRASHLOG:
 		if (quirks & PMT_QUIRK_NO_CRASHLOG) {
 			dev_info(dev, "Crashlog not supported\n");
-			return 0;
+			return -EINVAL;
 		}
 		name = "pmt_crashlog";
 		break;
 	default:
-		dev_err(dev, "Unrecognized PMT capability: %d\n", id);
 		return -EINVAL;
 	}
 
@@ -174,12 +173,8 @@ static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
 
 		ret = pmt_add_dev(pdev, &header, quirks);
-		if (ret) {
-			dev_warn(&pdev->dev,
-				 "Failed to add device for DVSEC id %d\n",
-				 header.id);
+		if (ret)
 			continue;
-		}
 
 		found_devices = true;
 	} while (true);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities
  2021-01-28 17:28 [PATCH] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
@ 2021-02-02 14:22 ` Hans de Goede
  2021-02-24 20:10 ` [PATCH V2 1/2] " David E. Box
  2021-02-24 20:10 ` [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1 David E. Box
  2 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-02-02 14:22 UTC (permalink / raw)
  To: David E. Box, lee.jones; +Cc: linux-kernel

Hi,

On 1/28/21 6:28 PM, David E. Box wrote:
> Some products will be available that have PMT capabilities that are not
> supported. Remove the warnings in this instance to avoid nuisance messages
> and confusion.
> 
> Also return an error code for capabilities that are disabled by quirk to
> prevent them from keeping the driver loaded if only disabled capabilities
> are found.
> 
> Fixes: 4f8217d5b0ca ("mfd: Intel Platform Monitoring Technology support")
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/mfd/intel_pmt.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
> index 744b230cdcca..65da2b17a204 100644
> --- a/drivers/mfd/intel_pmt.c
> +++ b/drivers/mfd/intel_pmt.c
> @@ -79,19 +79,18 @@ static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
>  	case DVSEC_INTEL_ID_WATCHER:
>  		if (quirks & PMT_QUIRK_NO_WATCHER) {
>  			dev_info(dev, "Watcher not supported\n");
> -			return 0;
> +			return -EINVAL;
>  		}
>  		name = "pmt_watcher";
>  		break;
>  	case DVSEC_INTEL_ID_CRASHLOG:
>  		if (quirks & PMT_QUIRK_NO_CRASHLOG) {
>  			dev_info(dev, "Crashlog not supported\n");
> -			return 0;
> +			return -EINVAL;
>  		}
>  		name = "pmt_crashlog";
>  		break;
>  	default:
> -		dev_err(dev, "Unrecognized PMT capability: %d\n", id);
>  		return -EINVAL;
>  	}
>  
> @@ -174,12 +173,8 @@ static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
>  
>  		ret = pmt_add_dev(pdev, &header, quirks);
> -		if (ret) {
> -			dev_warn(&pdev->dev,
> -				 "Failed to add device for DVSEC id %d\n",
> -				 header.id);
> +		if (ret)
>  			continue;
> -		}
>  
>  		found_devices = true;
>  	} while (true);
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V2 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities
  2021-01-28 17:28 [PATCH] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
  2021-02-02 14:22 ` Hans de Goede
@ 2021-02-24 20:10 ` David E. Box
  2021-02-24 20:22   ` Hans de Goede
  2021-03-09 18:12   ` [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window Lee Jones
  2021-02-24 20:10 ` [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1 David E. Box
  2 siblings, 2 replies; 17+ messages in thread
From: David E. Box @ 2021-02-24 20:10 UTC (permalink / raw)
  To: lee.jones, hdegoede, mgross
  Cc: David E. Box, linux-kernel, platform-driver-x86

Some products will be available that have PMT capabilities that are not
supported. Remove the warnings in this instance to avoid nuisance messages
and confusion.

Also return an error code for capabilities that are disabled by quirk to
prevent them from keeping the driver loaded if only disabled capabilities
are found.

Fixes: 4f8217d5b0ca ("mfd: Intel Platform Monitoring Technology support")
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
For merge in platform-drivers-x86

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:

	- None. Patch 2 added.

 drivers/mfd/intel_pmt.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
index 744b230cdcca..65da2b17a204 100644
--- a/drivers/mfd/intel_pmt.c
+++ b/drivers/mfd/intel_pmt.c
@@ -79,19 +79,18 @@ static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
 	case DVSEC_INTEL_ID_WATCHER:
 		if (quirks & PMT_QUIRK_NO_WATCHER) {
 			dev_info(dev, "Watcher not supported\n");
-			return 0;
+			return -EINVAL;
 		}
 		name = "pmt_watcher";
 		break;
 	case DVSEC_INTEL_ID_CRASHLOG:
 		if (quirks & PMT_QUIRK_NO_CRASHLOG) {
 			dev_info(dev, "Crashlog not supported\n");
-			return 0;
+			return -EINVAL;
 		}
 		name = "pmt_crashlog";
 		break;
 	default:
-		dev_err(dev, "Unrecognized PMT capability: %d\n", id);
 		return -EINVAL;
 	}
 
@@ -174,12 +173,8 @@ static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
 
 		ret = pmt_add_dev(pdev, &header, quirks);
-		if (ret) {
-			dev_warn(&pdev->dev,
-				 "Failed to add device for DVSEC id %d\n",
-				 header.id);
+		if (ret)
 			continue;
-		}
 
 		found_devices = true;
 	} while (true);

base-commit: a7d53dbbc70a81d5781da7fc905b656f41ad2381
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1
  2021-01-28 17:28 [PATCH] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
  2021-02-02 14:22 ` Hans de Goede
  2021-02-24 20:10 ` [PATCH V2 1/2] " David E. Box
@ 2021-02-24 20:10 ` David E. Box
  2021-03-09 16:45   ` Lee Jones
  2 siblings, 1 reply; 17+ messages in thread
From: David E. Box @ 2021-02-24 20:10 UTC (permalink / raw)
  To: lee.jones, hdegoede, mgross
  Cc: David E. Box, linux-kernel, platform-driver-x86

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)) {
+		struct intel_dvsec_header **header;
 
-		pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC);
-		if (!pos)
-			break;
+		header = info->capabilities;
+		while (*header) {
+			ret = pmt_add_dev(pdev, *header, quirks);
+			if (ret)
+				dev_warn(&pdev->dev,
+					 "Failed to add device for DVSEC id %d\n",
+					 (*header)->id);
+			else
+				found_devices = true;
 
-		pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER1, &vid);
-		if (vid != PCI_VENDOR_ID_INTEL)
-			continue;
-
-		pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER2,
-				     &header.id);
-		pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES,
-				     &header.num_entries);
-		pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE,
-				     &header.entry_size);
-		pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE,
-				      &table);
-
-		header.tbir = INTEL_DVSEC_TABLE_BAR(table);
-		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
-
-		ret = pmt_add_dev(pdev, &header, quirks);
-		if (ret)
-			continue;
-
-		found_devices = true;
-	} while (true);
+			++header;
+		}
+	} else {
+		do {
+			struct intel_dvsec_header header;
+			u32 table;
+			u16 vid;
+
+			pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC);
+			if (!pos)
+				break;
+
+			pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER1, &vid);
+			if (vid != PCI_VENDOR_ID_INTEL)
+				continue;
+
+			pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER2,
+					     &header.id);
+			pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES,
+					     &header.num_entries);
+			pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE,
+					     &header.entry_size);
+			pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE,
+					      &table);
+
+			header.tbir = INTEL_DVSEC_TABLE_BAR(table);
+			header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
+
+			ret = pmt_add_dev(pdev, &header, quirks);
+			if (ret)
+				continue;
+
+			found_devices = true;
+		} while (true);
+	}
 
 	if (!found_devices)
 		return -ENODEV;
@@ -195,10 +236,12 @@ static void pmt_pci_remove(struct pci_dev *pdev)
 }
 
 #define PCI_DEVICE_ID_INTEL_PMT_ADL	0x467d
+#define PCI_DEVICE_ID_INTEL_PMT_DG1	0x490e
 #define PCI_DEVICE_ID_INTEL_PMT_OOBMSM	0x09a7
 #define PCI_DEVICE_ID_INTEL_PMT_TGL	0x9a0d
 static const struct pci_device_id pmt_pci_ids[] = {
 	{ PCI_DEVICE_DATA(INTEL, PMT_ADL, &tgl_info) },
+	{ PCI_DEVICE_DATA(INTEL, PMT_DG1, &dg1_info) },
 	{ PCI_DEVICE_DATA(INTEL, PMT_OOBMSM, NULL) },
 	{ PCI_DEVICE_DATA(INTEL, PMT_TGL, &tgl_info) },
 	{ }
diff --git a/drivers/platform/x86/intel_pmt_class.c b/drivers/platform/x86/intel_pmt_class.c
index c8939fba4509..bd940b90c306 100644
--- a/drivers/platform/x86/intel_pmt_class.c
+++ b/drivers/platform/x86/intel_pmt_class.c
@@ -19,6 +19,28 @@
 #define PMT_XA_MAX		INT_MAX
 #define PMT_XA_LIMIT		XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
 
+/*
+ * Early implementations of PMT on client platforms have some
+ * differences from the server platforms (which use the Out Of Band
+ * Management Services Module OOBMSM). This list tracks those
+ * platforms as needed to handle those differences. Newer client
+ * platforms are expected to be fully compatible with server.
+ */
+static const struct pci_device_id pmt_telem_early_client_pci_ids[] = {
+	{ PCI_VDEVICE(INTEL, 0x467d) }, /* ADL */
+	{ PCI_VDEVICE(INTEL, 0x490e) }, /* DG1 */
+	{ PCI_VDEVICE(INTEL, 0x9a0d) }, /* TGL */
+	{ }
+};
+
+bool intel_pmt_is_early_client_hw(struct device *dev)
+{
+	struct pci_dev *parent = to_pci_dev(dev->parent);
+
+	return !!pci_match_id(pmt_telem_early_client_pci_ids, parent);
+}
+EXPORT_SYMBOL_GPL(intel_pmt_is_early_client_hw);
+
 /*
  * sysfs
  */
@@ -147,6 +169,30 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
 		 * base address = end of discovery region + base offset
 		 */
 		entry->base_addr = disc_res->end + 1 + header->base_offset;
+
+		/*
+		 * Some hardware use a different calculation for the base address
+		 * when access_type == ACCESS_LOCAL. On the these systems
+		 * ACCCESS_LOCAL refers to an address in the same BAR as the
+		 * header but at a fixed offset. But as the header address was
+		 * supplied to the driver, we don't know which BAR it was in.
+		 * So search for the bar whose range includes the header address.
+		 */
+		if (intel_pmt_is_early_client_hw(dev)) {
+			int i;
+
+			entry->base_addr = 0;
+			for (i = 0; i < 6; i++)
+				if (disc_res->start >= pci_resource_start(pci_dev, i) &&
+				   (disc_res->start <= pci_resource_end(pci_dev, i))) {
+					entry->base_addr = pci_resource_start(pci_dev, i) +
+							   header->base_offset;
+					break;
+				}
+			if (!entry->base_addr)
+				return -EINVAL;
+		}
+
 		break;
 	case ACCESS_BARID:
 		/*
diff --git a/drivers/platform/x86/intel_pmt_class.h b/drivers/platform/x86/intel_pmt_class.h
index de8f8139ba31..1337019c2873 100644
--- a/drivers/platform/x86/intel_pmt_class.h
+++ b/drivers/platform/x86/intel_pmt_class.h
@@ -44,6 +44,7 @@ struct intel_pmt_namespace {
 				 struct device *dev);
 };
 
+bool intel_pmt_is_early_client_hw(struct device *dev);
 int intel_pmt_dev_create(struct intel_pmt_entry *entry,
 			 struct intel_pmt_namespace *ns,
 			 struct platform_device *pdev, int idx);
diff --git a/drivers/platform/x86/intel_pmt_telemetry.c b/drivers/platform/x86/intel_pmt_telemetry.c
index f8a87614efa4..9b95ef050457 100644
--- a/drivers/platform/x86/intel_pmt_telemetry.c
+++ b/drivers/platform/x86/intel_pmt_telemetry.c
@@ -34,26 +34,6 @@ struct pmt_telem_priv {
 	struct intel_pmt_entry		entry[];
 };
 
-/*
- * Early implementations of PMT on client platforms have some
- * differences from the server platforms (which use the Out Of Band
- * Management Services Module OOBMSM). This list tracks those
- * platforms as needed to handle those differences. Newer client
- * platforms are expected to be fully compatible with server.
- */
-static const struct pci_device_id pmt_telem_early_client_pci_ids[] = {
-	{ PCI_VDEVICE(INTEL, 0x9a0d) }, /* TGL */
-	{ PCI_VDEVICE(INTEL, 0x467d) }, /* ADL */
-	{ }
-};
-
-static bool intel_pmt_is_early_client_hw(struct device *dev)
-{
-	struct pci_dev *parent = to_pci_dev(dev->parent);
-
-	return !!pci_match_id(pmt_telem_early_client_pci_ids, parent);
-}
-
 static bool pmt_telem_region_overlaps(struct intel_pmt_entry *entry,
 				      struct device *dev)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities
  2021-02-24 20:10 ` [PATCH V2 1/2] " David E. Box
@ 2021-02-24 20:22   ` Hans de Goede
  2021-02-24 23:01     ` David E. Box
  2021-03-09 18:12   ` [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window Lee Jones
  1 sibling, 1 reply; 17+ messages in thread
From: Hans de Goede @ 2021-02-24 20:22 UTC (permalink / raw)
  To: David E. Box, lee.jones, mgross; +Cc: linux-kernel, platform-driver-x86

Hi,

On 2/24/21 9:10 PM, David E. Box wrote:
> Some products will be available that have PMT capabilities that are not
> supported. Remove the warnings in this instance to avoid nuisance messages
> and confusion.
> 
> Also return an error code for capabilities that are disabled by quirk to
> prevent them from keeping the driver loaded if only disabled capabilities
> are found.
> 
> Fixes: 4f8217d5b0ca ("mfd: Intel Platform Monitoring Technology support")
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
> For merge in platform-drivers-x86
> 
> 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

Which, assuming you did a git remote update recently is AKA platform-drivers-x86-v5.12-1 .

> Changes from V1:
> 
> 	- None. Patch 2 added.

The series looks good to me, so for the series:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Lee, since both patches touch mfd files (and patch 2 also touches files under drivers/platform/x86)
I think it would be best if you just merge the entire series.

As always I would appreciate a pull-req from you to also pull the changes
into my tree, in case further drivers/platform/x86/intel_pmt* changes
show up during this cycle.

Regards,

Hans






> 
>  drivers/mfd/intel_pmt.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
> index 744b230cdcca..65da2b17a204 100644
> --- a/drivers/mfd/intel_pmt.c
> +++ b/drivers/mfd/intel_pmt.c
> @@ -79,19 +79,18 @@ static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
>  	case DVSEC_INTEL_ID_WATCHER:
>  		if (quirks & PMT_QUIRK_NO_WATCHER) {
>  			dev_info(dev, "Watcher not supported\n");
> -			return 0;
> +			return -EINVAL;
>  		}
>  		name = "pmt_watcher";
>  		break;
>  	case DVSEC_INTEL_ID_CRASHLOG:
>  		if (quirks & PMT_QUIRK_NO_CRASHLOG) {
>  			dev_info(dev, "Crashlog not supported\n");
> -			return 0;
> +			return -EINVAL;
>  		}
>  		name = "pmt_crashlog";
>  		break;
>  	default:
> -		dev_err(dev, "Unrecognized PMT capability: %d\n", id);
>  		return -EINVAL;
>  	}
>  
> @@ -174,12 +173,8 @@ static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
>  
>  		ret = pmt_add_dev(pdev, &header, quirks);
> -		if (ret) {
> -			dev_warn(&pdev->dev,
> -				 "Failed to add device for DVSEC id %d\n",
> -				 header.id);
> +		if (ret)
>  			continue;
> -		}
>  
>  		found_devices = true;
>  	} while (true);
> 
> base-commit: a7d53dbbc70a81d5781da7fc905b656f41ad2381
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities
  2021-02-24 20:22   ` Hans de Goede
@ 2021-02-24 23:01     ` David E. Box
  0 siblings, 0 replies; 17+ messages in thread
From: David E. Box @ 2021-02-24 23:01 UTC (permalink / raw)
  To: Hans de Goede, lee.jones, mgross; +Cc: linux-kernel, platform-driver-x86

On Wed, 2021-02-24 at 21:22 +0100, Hans de Goede wrote:
> Hi,
> 
> On 2/24/21 9:10 PM, David E. Box wrote:
> > Some products will be available that have PMT capabilities that are
> > not
> > supported. Remove the warnings in this instance to avoid nuisance
> > messages
> > and confusion.
> > 
> > Also return an error code for capabilities that are disabled by
> > quirk to
> > prevent them from keeping the driver loaded if only disabled
> > capabilities
> > are found.
> > 
> > Fixes: 4f8217d5b0ca ("mfd: Intel Platform Monitoring Technology
> > support")
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > For merge in platform-drivers-x86
> > 
> > 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
> 
> Which, assuming you did a git remote update recently is AKA platform-
> drivers-x86-v5.12-1 .

That it is.

> 
> > Changes from V1:
> > 
> >         - None. Patch 2 added.
> 
> The series looks good to me, so for the series:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Lee, since both patches touch mfd files (and patch 2 also touches
> files under drivers/platform/x86)
> I think it would be best if you just merge the entire series.

Ack

David

> 
> As always I would appreciate a pull-req from you to also pull the
> changes
> into my tree, in case further drivers/platform/x86/intel_pmt* changes
> show up during this cycle.
> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> > 
> >  drivers/mfd/intel_pmt.c | 11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
> > index 744b230cdcca..65da2b17a204 100644
> > --- a/drivers/mfd/intel_pmt.c
> > +++ b/drivers/mfd/intel_pmt.c
> > @@ -79,19 +79,18 @@ static int pmt_add_dev(struct pci_dev *pdev,
> > struct intel_dvsec_header *header,
> >         case DVSEC_INTEL_ID_WATCHER:
> >                 if (quirks & PMT_QUIRK_NO_WATCHER) {
> >                         dev_info(dev, "Watcher not supported\n");
> > -                       return 0;
> > +                       return -EINVAL;
> >                 }
> >                 name = "pmt_watcher";
> >                 break;
> >         case DVSEC_INTEL_ID_CRASHLOG:
> >                 if (quirks & PMT_QUIRK_NO_CRASHLOG) {
> >                         dev_info(dev, "Crashlog not supported\n");
> > -                       return 0;
> > +                       return -EINVAL;
> >                 }
> >                 name = "pmt_crashlog";
> >                 break;
> >         default:
> > -               dev_err(dev, "Unrecognized PMT capability: %d\n",
> > id);
> >                 return -EINVAL;
> >         }
> >  
> > @@ -174,12 +173,8 @@ static int pmt_pci_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> >                 header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
> >  
> >                 ret = pmt_add_dev(pdev, &header, quirks);
> > -               if (ret) {
> > -                       dev_warn(&pdev->dev,
> > -                                "Failed to add device for DVSEC id
> > %d\n",
> > -                                header.id);
> > +               if (ret)
> >                         continue;
> > -               }
> >  
> >                 found_devices = true;
> >         } while (true);
> > 
> > base-commit: a7d53dbbc70a81d5781da7fc905b656f41ad2381
> > 
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1
  2021-02-24 20:10 ` [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1 David E. Box
@ 2021-03-09 16:45   ` Lee Jones
  2021-03-09 17:27     ` David E. Box
  0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2021-03-09 16:45 UTC (permalink / raw)
  To: David E. Box; +Cc: hdegoede, mgross, linux-kernel, platform-driver-x86

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1
  2021-03-09 16:45   ` Lee Jones
@ 2021-03-09 17:27     ` David E. Box
  2021-03-09 18:13       ` Lee Jones
  0 siblings, 1 reply; 17+ messages in thread
From: David E. Box @ 2021-03-09 17:27 UTC (permalink / raw)
  To: Lee Jones; +Cc: hdegoede, mgross, linux-kernel, platform-driver-x86

On Tue, 2021-03-09 at 16:45 +0000, Lee Jones wrote:
> 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?

Ack

David

> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
  2021-02-24 20:10 ` [PATCH V2 1/2] " David E. Box
  2021-02-24 20:22   ` Hans de Goede
@ 2021-03-09 18:12   ` Lee Jones
  2021-03-09 18:59     ` Hans de Goede
  2021-03-10 10:57     ` [GIT PULL v2] " Lee Jones
  1 sibling, 2 replies; 17+ messages in thread
From: Lee Jones @ 2021-03-09 18:12 UTC (permalink / raw)
  To: David E. Box; +Cc: hdegoede, mgross, linux-kernel, platform-driver-x86

Enjoy!

The following changes since commit fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8:

  Linux 5.12-rc1 (2021-02-28 16:05:19 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-x86-v5.13

for you to fetch changes up to ccafe3126ad3f48ea1cd9ae460c69d1ba879fb65:

  mfd: intel_pmt: Add support for DG1 (2021-03-09 17:05:25 +0000)

----------------------------------------------------------------
Immutable branch between MFD and Platform/X86 due for the v5.13 merge window

----------------------------------------------------------------
David E. Box (2):
      mfd: intel_pmt: Fix nuisance messages and handling of disabled capabilities
      mfd: intel_pmt: Add support for DG1

 drivers/mfd/intel_pmt.c                    | 112 +++++++++++++++++++----------
 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, 122 insertions(+), 57 deletions(-)

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1
  2021-03-09 17:27     ` David E. Box
@ 2021-03-09 18:13       ` Lee Jones
  2021-03-09 19:52         ` [PATCH V3 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
  2021-03-09 19:52         ` [PATCH V3 2/2] MFD: intel_pmt: Add support for DG1 David E. Box
  0 siblings, 2 replies; 17+ messages in thread
From: Lee Jones @ 2021-03-09 18:13 UTC (permalink / raw)
  To: David E. Box; +Cc: hdegoede, mgross, linux-kernel, platform-driver-x86

On Tue, 09 Mar 2021, David E. Box wrote:

> On Tue, 2021-03-09 at 16:45 +0000, Lee Jones wrote:
> > 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?
> 
> Ack

I've applied this.  Can you fix this as a follow-up please?

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
  2021-03-09 18:12   ` [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window Lee Jones
@ 2021-03-09 18:59     ` Hans de Goede
  2021-03-09 19:17       ` Lee Jones
  2021-03-10 10:57     ` [GIT PULL v2] " Lee Jones
  1 sibling, 1 reply; 17+ messages in thread
From: Hans de Goede @ 2021-03-09 18:59 UTC (permalink / raw)
  To: Lee Jones, David E. Box; +Cc: mgross, linux-kernel, platform-driver-x86

Hi Lee,

On 3/9/21 7:12 PM, Lee Jones wrote:
> Enjoy!
> 
> The following changes since commit fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8:
> 
>   Linux 5.12-rc1 (2021-02-28 16:05:19 -0800)

I thought we were supposed to avoid using 5.12-rc1 as a base to avoid people
hitting the swapfile related disk-corruption when bisecting?

See: https://lwn.net/Articles/848431/

So it might be better to redo this branch with 5.12-rc2 as a base ?

Regards,

Hans


> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-x86-v5.13
> 
> for you to fetch changes up to ccafe3126ad3f48ea1cd9ae460c69d1ba879fb65:
> 
>   mfd: intel_pmt: Add support for DG1 (2021-03-09 17:05:25 +0000)
> 
> ----------------------------------------------------------------
> Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
> 
> ----------------------------------------------------------------
> David E. Box (2):
>       mfd: intel_pmt: Fix nuisance messages and handling of disabled capabilities
>       mfd: intel_pmt: Add support for DG1
> 
>  drivers/mfd/intel_pmt.c                    | 112 +++++++++++++++++++----------
>  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, 122 insertions(+), 57 deletions(-)
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
  2021-03-09 18:59     ` Hans de Goede
@ 2021-03-09 19:17       ` Lee Jones
  2021-03-09 20:06         ` Lee Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2021-03-09 19:17 UTC (permalink / raw)
  To: Hans de Goede; +Cc: David E. Box, mgross, linux-kernel, platform-driver-x86

On Tue, 09 Mar 2021, Hans de Goede wrote:

> Hi Lee,
> 
> On 3/9/21 7:12 PM, Lee Jones wrote:
> > Enjoy!
> > 
> > The following changes since commit fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8:
> > 
> >   Linux 5.12-rc1 (2021-02-28 16:05:19 -0800)
> 
> I thought we were supposed to avoid using 5.12-rc1 as a base to avoid people
> hitting the swapfile related disk-corruption when bisecting?
> 
> See: https://lwn.net/Articles/848431/

Interesting.  First I'd heard of it.

> So it might be better to redo this branch with 5.12-rc2 as a base ?

I already have 3 immutable branches based on -rc1.

Will need to look further into this to see what I can do.

Please bear with me.

> > are available in the Git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-x86-v5.13
> > 
> > for you to fetch changes up to ccafe3126ad3f48ea1cd9ae460c69d1ba879fb65:
> > 
> >   mfd: intel_pmt: Add support for DG1 (2021-03-09 17:05:25 +0000)
> > 
> > ----------------------------------------------------------------
> > Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
> > 
> > ----------------------------------------------------------------
> > David E. Box (2):
> >       mfd: intel_pmt: Fix nuisance messages and handling of disabled capabilities
> >       mfd: intel_pmt: Add support for DG1
> > 
> >  drivers/mfd/intel_pmt.c                    | 112 +++++++++++++++++++----------
> >  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, 122 insertions(+), 57 deletions(-)
> > 
> 

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V3 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities
  2021-03-09 18:13       ` Lee Jones
@ 2021-03-09 19:52         ` David E. Box
  2021-03-09 19:52         ` [PATCH V3 2/2] MFD: intel_pmt: Add support for DG1 David E. Box
  1 sibling, 0 replies; 17+ messages in thread
From: David E. Box @ 2021-03-09 19:52 UTC (permalink / raw)
  To: lee.jones, hdegoede, mgross
  Cc: David E. Box, linux-kernel, platform-driver-x86

Some products will be available that have PMT capabilities that are not
supported. Remove the warnings in this instance to avoid nuisance messages
and confusion.

Also return an error code for capabilities that are disabled by quirk to
prevent them from keeping the driver loaded if only disabled capabilities
are found.

Fixes: 4f8217d5b0ca ("mfd: Intel Platform Monitoring Technology support")
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---

No change from V2

 drivers/mfd/intel_pmt.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
index 744b230cdcca..65da2b17a204 100644
--- a/drivers/mfd/intel_pmt.c
+++ b/drivers/mfd/intel_pmt.c
@@ -79,19 +79,18 @@ static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
 	case DVSEC_INTEL_ID_WATCHER:
 		if (quirks & PMT_QUIRK_NO_WATCHER) {
 			dev_info(dev, "Watcher not supported\n");
-			return 0;
+			return -EINVAL;
 		}
 		name = "pmt_watcher";
 		break;
 	case DVSEC_INTEL_ID_CRASHLOG:
 		if (quirks & PMT_QUIRK_NO_CRASHLOG) {
 			dev_info(dev, "Crashlog not supported\n");
-			return 0;
+			return -EINVAL;
 		}
 		name = "pmt_crashlog";
 		break;
 	default:
-		dev_err(dev, "Unrecognized PMT capability: %d\n", id);
 		return -EINVAL;
 	}
 
@@ -174,12 +173,8 @@ static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
 
 		ret = pmt_add_dev(pdev, &header, quirks);
-		if (ret) {
-			dev_warn(&pdev->dev,
-				 "Failed to add device for DVSEC id %d\n",
-				 header.id);
+		if (ret)
 			continue;
-		}
 
 		found_devices = true;
 	} while (true);

base-commit: a38fd8748464831584a19438cbb3082b5a2dab15
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V3 2/2] MFD: intel_pmt: Add support for DG1
  2021-03-09 18:13       ` Lee Jones
  2021-03-09 19:52         ` [PATCH V3 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
@ 2021-03-09 19:52         ` David E. Box
  1 sibling, 0 replies; 17+ messages in thread
From: David E. Box @ 2021-03-09 19:52 UTC (permalink / raw)
  To: lee.jones, hdegoede, mgross
  Cc: David E. Box, linux-kernel, platform-driver-x86

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>
---

Changes from V2:
	- Use declared variable

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..448ae0f6d36f 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 && (quirks & PMT_QUIRK_NO_DVSEC)) {
+		struct intel_dvsec_header **header;
 
-		pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC);
-		if (!pos)
-			break;
+		header = info->capabilities;
+		while (*header) {
+			ret = pmt_add_dev(pdev, *header, quirks);
+			if (ret)
+				dev_warn(&pdev->dev,
+					 "Failed to add device for DVSEC id %d\n",
+					 (*header)->id);
+			else
+				found_devices = true;
 
-		pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER1, &vid);
-		if (vid != PCI_VENDOR_ID_INTEL)
-			continue;
-
-		pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER2,
-				     &header.id);
-		pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES,
-				     &header.num_entries);
-		pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE,
-				     &header.entry_size);
-		pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE,
-				      &table);
-
-		header.tbir = INTEL_DVSEC_TABLE_BAR(table);
-		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
-
-		ret = pmt_add_dev(pdev, &header, quirks);
-		if (ret)
-			continue;
-
-		found_devices = true;
-	} while (true);
+			++header;
+		}
+	} else {
+		do {
+			struct intel_dvsec_header header;
+			u32 table;
+			u16 vid;
+
+			pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC);
+			if (!pos)
+				break;
+
+			pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER1, &vid);
+			if (vid != PCI_VENDOR_ID_INTEL)
+				continue;
+
+			pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER2,
+					     &header.id);
+			pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES,
+					     &header.num_entries);
+			pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE,
+					     &header.entry_size);
+			pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE,
+					      &table);
+
+			header.tbir = INTEL_DVSEC_TABLE_BAR(table);
+			header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
+
+			ret = pmt_add_dev(pdev, &header, quirks);
+			if (ret)
+				continue;
+
+			found_devices = true;
+		} while (true);
+	}
 
 	if (!found_devices)
 		return -ENODEV;
@@ -195,10 +236,12 @@ static void pmt_pci_remove(struct pci_dev *pdev)
 }
 
 #define PCI_DEVICE_ID_INTEL_PMT_ADL	0x467d
+#define PCI_DEVICE_ID_INTEL_PMT_DG1	0x490e
 #define PCI_DEVICE_ID_INTEL_PMT_OOBMSM	0x09a7
 #define PCI_DEVICE_ID_INTEL_PMT_TGL	0x9a0d
 static const struct pci_device_id pmt_pci_ids[] = {
 	{ PCI_DEVICE_DATA(INTEL, PMT_ADL, &tgl_info) },
+	{ PCI_DEVICE_DATA(INTEL, PMT_DG1, &dg1_info) },
 	{ PCI_DEVICE_DATA(INTEL, PMT_OOBMSM, NULL) },
 	{ PCI_DEVICE_DATA(INTEL, PMT_TGL, &tgl_info) },
 	{ }
diff --git a/drivers/platform/x86/intel_pmt_class.c b/drivers/platform/x86/intel_pmt_class.c
index c8939fba4509..228e21f1ce5c 100644
--- a/drivers/platform/x86/intel_pmt_class.c
+++ b/drivers/platform/x86/intel_pmt_class.c
@@ -19,6 +19,28 @@
 #define PMT_XA_MAX		INT_MAX
 #define PMT_XA_LIMIT		XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
 
+/*
+ * Early implementations of PMT on client platforms have some
+ * differences from the server platforms (which use the Out Of Band
+ * Management Services Module OOBMSM). This list tracks those
+ * platforms as needed to handle those differences. Newer client
+ * platforms are expected to be fully compatible with server.
+ */
+static const struct pci_device_id pmt_telem_early_client_pci_ids[] = {
+	{ PCI_VDEVICE(INTEL, 0x467d) }, /* ADL */
+	{ PCI_VDEVICE(INTEL, 0x490e) }, /* DG1 */
+	{ PCI_VDEVICE(INTEL, 0x9a0d) }, /* TGL */
+	{ }
+};
+
+bool intel_pmt_is_early_client_hw(struct device *dev)
+{
+	struct pci_dev *parent = to_pci_dev(dev->parent);
+
+	return !!pci_match_id(pmt_telem_early_client_pci_ids, parent);
+}
+EXPORT_SYMBOL_GPL(intel_pmt_is_early_client_hw);
+
 /*
  * sysfs
  */
@@ -147,6 +169,30 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
 		 * base address = end of discovery region + base offset
 		 */
 		entry->base_addr = disc_res->end + 1 + header->base_offset;
+
+		/*
+		 * Some hardware use a different calculation for the base address
+		 * when access_type == ACCESS_LOCAL. On the these systems
+		 * ACCCESS_LOCAL refers to an address in the same BAR as the
+		 * header but at a fixed offset. But as the header address was
+		 * supplied to the driver, we don't know which BAR it was in.
+		 * So search for the bar whose range includes the header address.
+		 */
+		if (intel_pmt_is_early_client_hw(dev)) {
+			int i;
+
+			entry->base_addr = 0;
+			for (i = 0; i < 6; i++)
+				if (disc_res->start >= pci_resource_start(pci_dev, i) &&
+				   (disc_res->start <= pci_resource_end(pci_dev, i))) {
+					entry->base_addr = pci_resource_start(pci_dev, i) +
+							   header->base_offset;
+					break;
+				}
+			if (!entry->base_addr)
+				return -EINVAL;
+		}
+
 		break;
 	case ACCESS_BARID:
 		/*
diff --git a/drivers/platform/x86/intel_pmt_class.h b/drivers/platform/x86/intel_pmt_class.h
index de8f8139ba31..1337019c2873 100644
--- a/drivers/platform/x86/intel_pmt_class.h
+++ b/drivers/platform/x86/intel_pmt_class.h
@@ -44,6 +44,7 @@ struct intel_pmt_namespace {
 				 struct device *dev);
 };
 
+bool intel_pmt_is_early_client_hw(struct device *dev);
 int intel_pmt_dev_create(struct intel_pmt_entry *entry,
 			 struct intel_pmt_namespace *ns,
 			 struct platform_device *pdev, int idx);
diff --git a/drivers/platform/x86/intel_pmt_telemetry.c b/drivers/platform/x86/intel_pmt_telemetry.c
index f8a87614efa4..9b95ef050457 100644
--- a/drivers/platform/x86/intel_pmt_telemetry.c
+++ b/drivers/platform/x86/intel_pmt_telemetry.c
@@ -34,26 +34,6 @@ struct pmt_telem_priv {
 	struct intel_pmt_entry		entry[];
 };
 
-/*
- * Early implementations of PMT on client platforms have some
- * differences from the server platforms (which use the Out Of Band
- * Management Services Module OOBMSM). This list tracks those
- * platforms as needed to handle those differences. Newer client
- * platforms are expected to be fully compatible with server.
- */
-static const struct pci_device_id pmt_telem_early_client_pci_ids[] = {
-	{ PCI_VDEVICE(INTEL, 0x9a0d) }, /* TGL */
-	{ PCI_VDEVICE(INTEL, 0x467d) }, /* ADL */
-	{ }
-};
-
-static bool intel_pmt_is_early_client_hw(struct device *dev)
-{
-	struct pci_dev *parent = to_pci_dev(dev->parent);
-
-	return !!pci_match_id(pmt_telem_early_client_pci_ids, parent);
-}
-
 static bool pmt_telem_region_overlaps(struct intel_pmt_entry *entry,
 				      struct device *dev)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
  2021-03-09 19:17       ` Lee Jones
@ 2021-03-09 20:06         ` Lee Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2021-03-09 20:06 UTC (permalink / raw)
  To: Hans de Goede; +Cc: David E. Box, mgross, linux-kernel, platform-driver-x86

On Tue, 09 Mar 2021, Lee Jones wrote:

> On Tue, 09 Mar 2021, Hans de Goede wrote:
> 
> > Hi Lee,
> > 
> > On 3/9/21 7:12 PM, Lee Jones wrote:
> > > Enjoy!
> > > 
> > > The following changes since commit fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8:
> > > 
> > >   Linux 5.12-rc1 (2021-02-28 16:05:19 -0800)
> > 
> > I thought we were supposed to avoid using 5.12-rc1 as a base to avoid people
> > hitting the swapfile related disk-corruption when bisecting?
> > 
> > See: https://lwn.net/Articles/848431/
> 
> Interesting.  First I'd heard of it.
> 
> > So it might be better to redo this branch with 5.12-rc2 as a base ?
> 
> I already have 3 immutable branches based on -rc1.
> 
> Will need to look further into this to see what I can do.
> 
> Please bear with me.

Okay, discard this one.  I'll do another first thing.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [GIT PULL v2] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
  2021-03-09 18:12   ` [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window Lee Jones
  2021-03-09 18:59     ` Hans de Goede
@ 2021-03-10 10:57     ` Lee Jones
  2021-03-18 11:02       ` Hans de Goede
  1 sibling, 1 reply; 17+ messages in thread
From: Lee Jones @ 2021-03-10 10:57 UTC (permalink / raw)
  To: David E. Box; +Cc: hdegoede, mgross, linux-kernel, platform-driver-x86

Rebased onto -rc2

The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15:

  Linux 5.12-rc2 (2021-03-05 17:33:41 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-x86-v5.13

for you to fetch changes up to aa47ad3f853ae72c32b7e46dfc8bc2c8dc2dbad7:

  mfd: intel_pmt: Add support for DG1 (2021-03-10 10:48:48 +0000)

----------------------------------------------------------------
Immutable branch between MFD and Platform/x86 due for the v5.13 merge window

----------------------------------------------------------------
David E. Box (2):
      mfd: intel_pmt: Fix nuisance messages and handling of disabled capabilities
      mfd: intel_pmt: Add support for DG1

 drivers/mfd/intel_pmt.c                    | 112 +++++++++++++++++++----------
 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, 122 insertions(+), 57 deletions(-)

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL v2] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window
  2021-03-10 10:57     ` [GIT PULL v2] " Lee Jones
@ 2021-03-18 11:02       ` Hans de Goede
  0 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-03-18 11:02 UTC (permalink / raw)
  To: Lee Jones, David E. Box; +Cc: mgross, linux-kernel, platform-driver-x86

Hi,

On 3/10/21 11:57 AM, Lee Jones wrote:
> Rebased onto -rc2
> 
> The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15:
> 
>   Linux 5.12-rc2 (2021-03-05 17:33:41 -0800)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-platform-x86-v5.13
> 
> for you to fetch changes up to aa47ad3f853ae72c32b7e46dfc8bc2c8dc2dbad7:
> 
>   mfd: intel_pmt: Add support for DG1 (2021-03-10 10:48:48 +0000)
> 
> ----------------------------------------------------------------
> Immutable branch between MFD and Platform/x86 due for the v5.13 merge window

Thank you, I've merged this into my review-hans branch now, so this will
soon be in pdx86/for-next.

Regards,

Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-03-18 11:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28 17:28 [PATCH] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
2021-02-02 14:22 ` Hans de Goede
2021-02-24 20:10 ` [PATCH V2 1/2] " David E. Box
2021-02-24 20:22   ` Hans de Goede
2021-02-24 23:01     ` David E. Box
2021-03-09 18:12   ` [GIT PULL] Immutable branch between MFD and Platform/X86 due for the v5.13 merge window Lee Jones
2021-03-09 18:59     ` Hans de Goede
2021-03-09 19:17       ` Lee Jones
2021-03-09 20:06         ` Lee Jones
2021-03-10 10:57     ` [GIT PULL v2] " Lee Jones
2021-03-18 11:02       ` Hans de Goede
2021-02-24 20:10 ` [PATCH 2/2 V2] MFD: intel_pmt: Add support for DG1 David E. Box
2021-03-09 16:45   ` Lee Jones
2021-03-09 17:27     ` David E. Box
2021-03-09 18:13       ` Lee Jones
2021-03-09 19:52         ` [PATCH V3 1/2] MFD: intel_pmt: Fix nuisance messages and handling of disabled capabilities David E. Box
2021-03-09 19:52         ` [PATCH V3 2/2] MFD: intel_pmt: Add support for DG1 David E. Box

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).