* [PATCH 0/4] MFD: intel_pmt: Split OOBMSM from intel_pmt driver @ 2021-06-17 21:54 David E. Box 2021-06-17 21:54 ` [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields David E. Box ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: David E. Box @ 2021-06-17 21:54 UTC (permalink / raw) To: lee.jones, david.e.box, hdegoede, mgross, bhelgaas Cc: linux-kernel, platform-driver-x86, linux-pci Most of the devices in the intel_pmt driver are PMT only devices. However, the Out of Band Management Services Module (OOBMSM) can also be used access non-PMT devices. In order to better support this without the confusion of a dependency on MFD_INTEL_PMT, this patch set rewrites the intel_pmt driver to split the functionality into 3 components: 1. intel_pmt - driver for PMT only devices 2. intel-oobmsm - driver for devices using the OOBMSM IP which can support multiple types of capabilities, including PMT 3. intel-extended-cap - symbols to handle adding platform device for both drivers Additionally, this patch set provides additional DVSEC macros in the PCI header as well as support for reading capabilities from a VSEC structure. Patch 1 - Adds PCI defines for DVSEC registers Patch 2 - Removes OOBMSM from intel_pmt and creates intel-extended-cap.c support the creation of platform devices for capabilities from both intel_pmt and intel-oobmsm (Patch 3) Patch 2 - Creates a separate driver for OOBMSM Patch 3 - Adds support for reading capabilities from PCIe VSEC structures For submission through MFD branch. David E. Box (4): PCI: Add #defines for accessing PCIE DVSEC fields MFD: intel_pmt: Remove OOBMSM device for placement in own driver MFD: Add the Intel Out of Band Management Services Module (OOBMSM) driver MFD: intel-extended-cap: Add support for PCIe VSEC structures MAINTAINERS | 2 + drivers/mfd/Kconfig | 15 ++ drivers/mfd/Makefile | 2 + drivers/mfd/intel_extended_caps.c | 267 +++++++++++++++++++++ drivers/mfd/intel_extended_caps.h | 40 +++ drivers/mfd/intel_oobmsm.c | 61 +++++ drivers/mfd/intel_pmt.c | 198 ++------------- drivers/platform/x86/Kconfig | 4 +- drivers/platform/x86/intel_pmt_crashlog.c | 2 +- drivers/platform/x86/intel_pmt_telemetry.c | 2 +- include/uapi/linux/pci_regs.h | 4 + 11 files changed, 409 insertions(+), 188 deletions(-) create mode 100644 drivers/mfd/intel_extended_caps.c create mode 100644 drivers/mfd/intel_extended_caps.h create mode 100644 drivers/mfd/intel_oobmsm.c -- 2.25.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields 2021-06-17 21:54 [PATCH 0/4] MFD: intel_pmt: Split OOBMSM from intel_pmt driver David E. Box @ 2021-06-17 21:54 ` David E. Box 2021-06-22 15:04 ` Bjorn Helgaas 2021-06-17 21:54 ` [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device David E. Box ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: David E. Box @ 2021-06-17 21:54 UTC (permalink / raw) To: lee.jones, david.e.box, hdegoede, mgross, bhelgaas Cc: linux-kernel, platform-driver-x86, linux-pci Add #defines for accessing Vendor ID, Revision, Length, and ID offsets in the Designated Vendor Specific Extended Capability (DVSEC). Defined in PCIe r5.0, sec 7.9.6. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- include/uapi/linux/pci_regs.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index e709ae8235e7..57ee51f19283 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -1080,7 +1080,11 @@ /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */ #define PCI_DVSEC_HEADER1 0x4 /* Designated Vendor-Specific Header1 */ +#define PCI_DVSEC_HEADER1_VID(x) ((x) & 0xffff) +#define PCI_DVSEC_HEADER1_REV(x) (((x) >> 16) & 0xf) +#define PCI_DVSEC_HEADER1_LEN(x) (((x) >> 20) & 0xfff) #define PCI_DVSEC_HEADER2 0x8 /* Designated Vendor-Specific Header2 */ +#define PCI_DVSEC_HEADER2_ID(x) ((x) & 0xffff) /* Data Link Feature */ #define PCI_DLF_CAP 0x04 /* Capabilities Register */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields 2021-06-17 21:54 ` [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields David E. Box @ 2021-06-22 15:04 ` Bjorn Helgaas 0 siblings, 0 replies; 20+ messages in thread From: Bjorn Helgaas @ 2021-06-22 15:04 UTC (permalink / raw) To: David E. Box Cc: lee.jones, hdegoede, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Dan Williams, Jonathan Cameron [+cc Dan, Jonathan] On Thu, Jun 17, 2021 at 02:54:05PM -0700, David E. Box wrote: > Add #defines for accessing Vendor ID, Revision, Length, and ID offsets > in the Designated Vendor Specific Extended Capability (DVSEC). Defined > in PCIe r5.0, sec 7.9.6. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> I don't have time right now to really look at the intel_extended_caps.c patch [1], but I wonder if there's anything there that could be abstracted and shared with CXL, etc? If not, no worries. [1] https://lore.kernel.org/r/20210617215408.1412409-5-david.e.box@linux.intel.com > --- > include/uapi/linux/pci_regs.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index e709ae8235e7..57ee51f19283 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -1080,7 +1080,11 @@ > > /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */ > #define PCI_DVSEC_HEADER1 0x4 /* Designated Vendor-Specific Header1 */ > +#define PCI_DVSEC_HEADER1_VID(x) ((x) & 0xffff) > +#define PCI_DVSEC_HEADER1_REV(x) (((x) >> 16) & 0xf) > +#define PCI_DVSEC_HEADER1_LEN(x) (((x) >> 20) & 0xfff) > #define PCI_DVSEC_HEADER2 0x8 /* Designated Vendor-Specific Header2 */ > +#define PCI_DVSEC_HEADER2_ID(x) ((x) & 0xffff) > > /* Data Link Feature */ > #define PCI_DLF_CAP 0x04 /* Capabilities Register */ > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-06-17 21:54 [PATCH 0/4] MFD: intel_pmt: Split OOBMSM from intel_pmt driver David E. Box 2021-06-17 21:54 ` [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields David E. Box @ 2021-06-17 21:54 ` David E. Box 2021-06-30 10:15 ` Lee Jones 2021-06-17 21:54 ` [PATCH 3/4] MFD: Intel Out of Band Management Services Module (OOBMSM) driver David E. Box 2021-06-17 21:54 ` [PATCH 4/4] MFD: intel-extended-cap: Add support for PCIe VSEC structures David E. Box 3 siblings, 1 reply; 20+ messages in thread From: David E. Box @ 2021-06-17 21:54 UTC (permalink / raw) To: lee.jones, david.e.box, hdegoede, mgross, bhelgaas Cc: linux-kernel, platform-driver-x86, linux-pci Unlike the other devices in intel_pmt, the Out of Band Management Services Module (OOBMSM) is actually not a PMT dedicated device. It can also be used to describe non-PMT capabilities. Like PMT, these capabilities are also enumerated using PCIe Vendor Specific registers in config space. In order to better support these devices without the confusion of a dependency on MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it can be later placed in its own driver. Since much of the same code will be used by intel_pmt and the new driver, create a new file with symbols to be used by both. While performing this split we need to also handle the creation of platform devices for the non-PMT capabilities. Currently PMT devices are named by their capability (e.g. pmt_telemetry). Instead, generically name them by their capability ID (e.g. intel_extnd_cap_2). This allows the IDs to be created automatically. However, to ensure that unsupported devices aren't created, use an allow list to specify supported capabilities. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- MAINTAINERS | 1 + drivers/mfd/Kconfig | 4 + drivers/mfd/Makefile | 1 + drivers/mfd/intel_extended_caps.c | 208 +++++++++++++++++++++ drivers/mfd/intel_extended_caps.h | 40 ++++ drivers/mfd/intel_pmt.c | 198 ++------------------ drivers/platform/x86/intel_pmt_crashlog.c | 2 +- drivers/platform/x86/intel_pmt_telemetry.c | 2 +- 8 files changed, 270 insertions(+), 186 deletions(-) create mode 100644 drivers/mfd/intel_extended_caps.c create mode 100644 drivers/mfd/intel_extended_caps.h diff --git a/MAINTAINERS b/MAINTAINERS index bc0ceef87b73..ebdc2a0f794b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9355,6 +9355,7 @@ F: include/linux/mfd/intel_soc_pmic* INTEL PMT DRIVER M: "David E. Box" <david.e.box@linux.intel.com> S: Maintained +F: drivers/mfd/intel_extended_cap.c F: drivers/mfd/intel_pmt.c F: drivers/platform/x86/intel_pmt_* diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 5c7f2b100191..4dde8e223a9e 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -673,10 +673,14 @@ config MFD_INTEL_PMC_BXT Register and P-unit access. In addition this creates devices for iTCO watchdog and telemetry that are part of the PMC. +config MFD_INTEL_EXTENDED_CAPS + tristate + config MFD_INTEL_PMT tristate "Intel Platform Monitoring Technology (PMT) support" depends on PCI select MFD_CORE + select MFD_INTEL_EXTENDED_CAPS help The Intel Platform Monitoring Technology (PMT) is an interface that provides access to hardware monitor registers. This driver supports diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 4f6d2b8a5f76..7fa35399ec76 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -209,6 +209,7 @@ obj-$(CONFIG_MFD_AT91_USART) += at91-usart.o obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o obj-$(CONFIG_MFD_ATMEL_SMC) += atmel-smc.o +obj-$(CONFIG_MFD_INTEL_EXTENDED_CAPS) += intel_extended_caps.o obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o diff --git a/drivers/mfd/intel_extended_caps.c b/drivers/mfd/intel_extended_caps.c new file mode 100644 index 000000000000..89cf1ae6f65b --- /dev/null +++ b/drivers/mfd/intel_extended_caps.c @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Intel Extended Capabilities module + * + * Copyright (c) 2021, Intel Corporation. + * All Rights Reserved. + * + * Author: David E. Box <david.e.box@linux.intel.com> + */ + +#include <linux/bits.h> +#include <linux/mfd/core.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/platform_device.h> +#include <linux/types.h> + +#include "intel_extended_caps.h" + +/* Intel DVSEC capability vendor space offsets */ +#define INTEL_DVSEC_ENTRIES 0xA +#define INTEL_DVSEC_SIZE 0xB +#define INTEL_DVSEC_TABLE 0xC +#define INTEL_DVSEC_TABLE_BAR(x) ((x) & GENMASK(2, 0)) +#define INTEL_DVSEC_TABLE_OFFSET(x) ((x) & GENMASK(31, 3)) + +/* Intel Extended Features */ +#define INTEL_EXT_CAP_ID_TELEMETRY 2 +#define INTEL_EXT_CAP_ID_WATCHER 3 +#define INTEL_EXT_CAP_ID_CRASHLOG 4 + +#define INTEL_EXT_CAP_PREFIX "intel_extnd_cap" +#define FEATURE_ID_NAME_LENGTH 25 + +static int intel_ext_cap_allow_list[] = { + INTEL_EXT_CAP_ID_TELEMETRY, + INTEL_EXT_CAP_ID_WATCHER, + INTEL_EXT_CAP_ID_CRASHLOG, +}; + +static bool intel_ext_cap_allowed(u16 id) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(intel_ext_cap_allow_list); i++) + if (intel_ext_cap_allow_list[i] == id) + return true; + + return false; +} + +static bool intel_ext_cap_disabled(u16 id, unsigned long quirks) +{ + switch (id) { + case INTEL_EXT_CAP_ID_WATCHER: + return !!(quirks & EXT_CAP_QUIRK_NO_WATCHER); + + case INTEL_EXT_CAP_ID_CRASHLOG: + return !!(quirks & EXT_CAP_QUIRK_NO_CRASHLOG); + + default: + return false; + } +} + +static int intel_ext_cap_add_dev(struct pci_dev *pdev, struct intel_ext_cap_header *header, + unsigned long quirks) +{ + struct device *dev = &pdev->dev; + struct resource *res, *tmp; + struct mfd_cell *cell; + char feature_id_name[FEATURE_ID_NAME_LENGTH]; + int count = header->num_entries; + int size = header->entry_size; + int id = header->id; + int i; + + if (!intel_ext_cap_allowed(id)) + return -EINVAL; + + if (intel_ext_cap_disabled(id, quirks)) + return -EINVAL; + + snprintf(feature_id_name, sizeof(feature_id_name), "%s_%d", INTEL_EXT_CAP_PREFIX, id); + + if (!header->num_entries) { + dev_err(dev, "Invalid 0 entry count for %s header\n", feature_id_name); + return -EINVAL; + } + + if (!header->entry_size) { + dev_err(dev, "Invalid 0 entry size for %s header\n", feature_id_name); + return -EINVAL; + } + + cell = devm_kzalloc(dev, sizeof(*cell), GFP_KERNEL); + if (!cell) + return -ENOMEM; + + res = devm_kcalloc(dev, count, sizeof(*res), GFP_KERNEL); + if (!res) + return -ENOMEM; + + if (quirks & EXT_CAP_QUIRK_TABLE_SHIFT) + header->offset >>= 3; + + /* + * The DVSEC contains the starting offset and count for a block of + * discovery tables, each providing access to monitoring facilities for + * a section of the device. Create a resource list of these tables to + * provide to the driver. + */ + for (i = 0, tmp = res; i < count; i++, tmp++) { + tmp->start = pdev->resource[header->tbir].start + + header->offset + i * (size * sizeof(u32)); + tmp->end = tmp->start + (size * sizeof(u32)) - 1; + tmp->flags = IORESOURCE_MEM; + } + + cell->resources = res; + cell->num_resources = count; + cell->name = feature_id_name; + + return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cell, 1, NULL, 0, NULL); +} + +int intel_ext_cap_probe(struct pci_dev *pdev, struct intel_ext_cap_platform_info *info) +{ + unsigned long quirks = 0; + bool found_devices = false; + int ret, pos; + + if (info) + quirks = info->quirks; + + if (info && (info->quirks & EXT_CAP_QUIRK_NO_DVSEC)) { + struct intel_ext_cap_header **header; + + header = info->capabilities; + while (*header) { + ret = intel_ext_cap_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; + + header++; + } + } else { + /* Find DVSEC features */ + pos = 0; + do { + struct intel_ext_cap_header header; + u32 table, hdr; + u16 vid; + + pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC); + if (!pos) + break; + + pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER1, &hdr); + vid = PCI_DVSEC_HEADER1_VID(hdr); + if (vid != PCI_VENDOR_ID_INTEL) + continue; + + /* Support only revision 1 */ + header.rev = PCI_DVSEC_HEADER1_REV(hdr); + if (header.rev != 1) { + dev_warn(&pdev->dev, "Unsupported DVSEC revision %d\n", + header.rev); + continue; + } + + header.length = PCI_DVSEC_HEADER1_LEN(hdr); + + 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); + + pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2, &hdr); + header.id = PCI_DVSEC_HEADER2_ID(hdr); + + ret = intel_ext_cap_add_dev(pdev, &header, quirks); + if (ret) + continue; + + found_devices = true; + } while (true); + } + + if (!found_devices) + return -ENODEV; + + return 0; +} +EXPORT_SYMBOL_GPL(intel_ext_cap_probe); + +MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>"); +MODULE_DESCRIPTION("Intel Extended Capability Core driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/mfd/intel_extended_caps.h b/drivers/mfd/intel_extended_caps.h new file mode 100644 index 000000000000..c6722d4cb5e3 --- /dev/null +++ b/drivers/mfd/intel_extended_caps.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef INTEL_EXTENDED_CAPS_H +#define INTEL_EXTENDED_CAPS_H + +/* Intel Extended Features */ +#define INTEL_EXT_CAP_ID_TELEMETRY 2 +#define INTEL_EXT_CAP_ID_WATCHER 3 +#define INTEL_EXT_CAP_ID_CRASHLOG 4 + +struct intel_ext_cap_header { + u8 rev; + u16 length; + u16 id; + u8 num_entries; + u8 entry_size; + u8 tbir; + u32 offset; +}; + +enum intel_ext_cap_quirks { + /* Watcher capability not supported */ + EXT_CAP_QUIRK_NO_WATCHER = BIT(0), + + /* Crashlog capability not supported */ + EXT_CAP_QUIRK_NO_CRASHLOG = BIT(1), + + /* Use shift instead of mask to read discovery table offset */ + EXT_CAP_QUIRK_TABLE_SHIFT = BIT(2), + + /* DVSEC not present (provided in driver data) */ + EXT_CAP_QUIRK_NO_DVSEC = BIT(3), +}; + +struct intel_ext_cap_platform_info { + unsigned long quirks; + struct intel_ext_cap_header **capabilities; +}; + +int intel_ext_cap_probe(struct pci_dev *pdev, struct intel_ext_cap_platform_info *info); +#endif diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c index dd7eb614c28e..f5205d6b5c47 100644 --- a/drivers/mfd/intel_pmt.c +++ b/drivers/mfd/intel_pmt.c @@ -8,64 +8,19 @@ * Author: David E. Box <david.e.box@linux.intel.com> */ -#include <linux/bits.h> -#include <linux/kernel.h> -#include <linux/mfd/core.h> #include <linux/module.h> #include <linux/pci.h> -#include <linux/platform_device.h> -#include <linux/pm.h> #include <linux/pm_runtime.h> -#include <linux/types.h> -/* Intel DVSEC capability vendor space offsets */ -#define INTEL_DVSEC_ENTRIES 0xA -#define INTEL_DVSEC_SIZE 0xB -#define INTEL_DVSEC_TABLE 0xC -#define INTEL_DVSEC_TABLE_BAR(x) ((x) & GENMASK(2, 0)) -#define INTEL_DVSEC_TABLE_OFFSET(x) ((x) & GENMASK(31, 3)) -#define INTEL_DVSEC_ENTRY_SIZE 4 +#include "intel_extended_caps.h" -/* PMT capabilities */ -#define DVSEC_INTEL_ID_TELEMETRY 2 -#define DVSEC_INTEL_ID_WATCHER 3 -#define DVSEC_INTEL_ID_CRASHLOG 4 - -struct intel_dvsec_header { - u16 length; - u16 id; - u8 num_entries; - u8 entry_size; - u8 tbir; - u32 offset; -}; - -enum pmt_quirks { - /* Watcher capability not supported */ - PMT_QUIRK_NO_WATCHER = BIT(0), - - /* Crashlog capability not supported */ - PMT_QUIRK_NO_CRASHLOG = BIT(1), - - /* 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 = { - .quirks = PMT_QUIRK_NO_WATCHER | PMT_QUIRK_NO_CRASHLOG | - PMT_QUIRK_TABLE_SHIFT, +static const struct intel_ext_cap_platform_info tgl_info = { + .quirks = EXT_CAP_QUIRK_NO_WATCHER | EXT_CAP_QUIRK_NO_CRASHLOG | + EXT_CAP_QUIRK_TABLE_SHIFT, }; /* DG1 Platform with DVSEC quirk*/ -static struct intel_dvsec_header dg1_telemetry = { +static struct intel_ext_cap_header dg1_telemetry = { .length = 0x10, .id = 2, .num_entries = 1, @@ -74,154 +29,31 @@ static struct intel_dvsec_header dg1_telemetry = { .offset = 0x466000, }; -static struct intel_dvsec_header *dg1_capabilities[] = { +static struct intel_ext_cap_header *dg1_capabilities[] = { &dg1_telemetry, NULL }; -static const struct pmt_platform_info dg1_info = { - .quirks = PMT_QUIRK_NO_DVSEC, +static const struct intel_ext_cap_platform_info dg1_info = { + .quirks = EXT_CAP_QUIRK_NO_DVSEC, .capabilities = dg1_capabilities, }; -static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header, - unsigned long quirks) -{ - struct device *dev = &pdev->dev; - struct resource *res, *tmp; - struct mfd_cell *cell; - const char *name; - int count = header->num_entries; - int size = header->entry_size; - int id = header->id; - int i; - - switch (id) { - case DVSEC_INTEL_ID_TELEMETRY: - name = "pmt_telemetry"; - break; - case DVSEC_INTEL_ID_WATCHER: - if (quirks & PMT_QUIRK_NO_WATCHER) { - dev_info(dev, "Watcher not supported\n"); - 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 -EINVAL; - } - name = "pmt_crashlog"; - break; - default: - return -EINVAL; - } - - if (!header->num_entries || !header->entry_size) { - dev_err(dev, "Invalid count or size for %s header\n", name); - return -EINVAL; - } - - cell = devm_kzalloc(dev, sizeof(*cell), GFP_KERNEL); - if (!cell) - return -ENOMEM; - - res = devm_kcalloc(dev, count, sizeof(*res), GFP_KERNEL); - if (!res) - return -ENOMEM; - - if (quirks & PMT_QUIRK_TABLE_SHIFT) - header->offset >>= 3; - - /* - * The PMT DVSEC contains the starting offset and count for a block of - * discovery tables, each providing access to monitoring facilities for - * a section of the device. Create a resource list of these tables to - * provide to the driver. - */ - for (i = 0, tmp = res; i < count; i++, tmp++) { - tmp->start = pdev->resource[header->tbir].start + - header->offset + i * (size << 2); - tmp->end = tmp->start + (size << 2) - 1; - tmp->flags = IORESOURCE_MEM; - } - - cell->resources = res; - cell->num_resources = count; - cell->name = name; - - return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cell, 1, NULL, 0, - NULL); -} static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { - struct pmt_platform_info *info; - unsigned long quirks = 0; - bool found_devices = false; - int ret, pos = 0; + struct intel_ext_cap_platform_info *info; + int ret; ret = pcim_enable_device(pdev); if (ret) return ret; - info = (struct pmt_platform_info *)id->driver_data; - - if (info) - quirks = info->quirks; - - if (info && (info->quirks & PMT_QUIRK_NO_DVSEC)) { - struct intel_dvsec_header **header; - - 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; - - ++header; - } - } else { - do { - struct intel_dvsec_header header; - u32 table; - u16 vid; + info = (struct intel_ext_cap_platform_info *)id->driver_data; - 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; + ret = intel_ext_cap_probe(pdev, info); + if (ret) + return ret; pm_runtime_put(&pdev->dev); pm_runtime_allow(&pdev->dev); @@ -237,12 +69,10 @@ 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_crashlog.c b/drivers/platform/x86/intel_pmt_crashlog.c index 92d315a16cfd..6662454af5bf 100644 --- a/drivers/platform/x86/intel_pmt_crashlog.c +++ b/drivers/platform/x86/intel_pmt_crashlog.c @@ -17,7 +17,7 @@ #include "intel_pmt_class.h" -#define DRV_NAME "pmt_crashlog" +#define DRV_NAME "intel_extnd_cap_4" /* Crashlog discovery header types */ #define CRASH_TYPE_OOBMSM 1 diff --git a/drivers/platform/x86/intel_pmt_telemetry.c b/drivers/platform/x86/intel_pmt_telemetry.c index 9b95ef050457..f281d1d71058 100644 --- a/drivers/platform/x86/intel_pmt_telemetry.c +++ b/drivers/platform/x86/intel_pmt_telemetry.c @@ -17,7 +17,7 @@ #include "intel_pmt_class.h" -#define TELEM_DEV_NAME "pmt_telemetry" +#define TELEM_DEV_NAME "intel_extnd_cap_2" #define TELEM_SIZE_OFFSET 0x0 #define TELEM_GUID_OFFSET 0x4 -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-06-17 21:54 ` [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device David E. Box @ 2021-06-30 10:15 ` Lee Jones 2021-06-30 21:11 ` David E. Box 0 siblings, 1 reply; 20+ messages in thread From: Lee Jones @ 2021-06-30 10:15 UTC (permalink / raw) To: David E. Box Cc: hdegoede, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Andy Shevchenko On Thu, 17 Jun 2021, David E. Box wrote: > Unlike the other devices in intel_pmt, the Out of Band Management Services > Module (OOBMSM) is actually not a PMT dedicated device. It can also be used > to describe non-PMT capabilities. Like PMT, these capabilities are also > enumerated using PCIe Vendor Specific registers in config space. In order > to better support these devices without the confusion of a dependency on > MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it can be > later placed in its own driver. Since much of the same code will be used by > intel_pmt and the new driver, create a new file with symbols to be used by > both. > > While performing this split we need to also handle the creation of platform > devices for the non-PMT capabilities. Currently PMT devices are named by > their capability (e.g. pmt_telemetry). Instead, generically name them by > their capability ID (e.g. intel_extnd_cap_2). This allows the IDs to be > created automatically. However, to ensure that unsupported devices aren't > created, use an allow list to specify supported capabilities. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > --- > MAINTAINERS | 1 + > drivers/mfd/Kconfig | 4 + > drivers/mfd/Makefile | 1 + > drivers/mfd/intel_extended_caps.c | 208 +++++++++++++++++++++ Please consider moving this <whatever this is> out to either drivers/pci or drivers/platform/x86. I suggest Andy should also be on Cc. > drivers/mfd/intel_extended_caps.h | 40 ++++ > drivers/mfd/intel_pmt.c | 198 ++------------------ > drivers/platform/x86/intel_pmt_crashlog.c | 2 +- > drivers/platform/x86/intel_pmt_telemetry.c | 2 +- > 8 files changed, 270 insertions(+), 186 deletions(-) > create mode 100644 drivers/mfd/intel_extended_caps.c > create mode 100644 drivers/mfd/intel_extended_caps.h -- 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] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-06-30 10:15 ` Lee Jones @ 2021-06-30 21:11 ` David E. Box 2021-07-01 8:01 ` Lee Jones ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: David E. Box @ 2021-06-30 21:11 UTC (permalink / raw) To: Lee Jones Cc: hdegoede, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Andy Shevchenko On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > On Thu, 17 Jun 2021, David E. Box wrote: > > > Unlike the other devices in intel_pmt, the Out of Band Management > > Services > > Module (OOBMSM) is actually not a PMT dedicated device. It can also > > be used > > to describe non-PMT capabilities. Like PMT, these capabilities are > > also > > enumerated using PCIe Vendor Specific registers in config space. In > > order > > to better support these devices without the confusion of a > > dependency on > > MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it > > can be > > later placed in its own driver. Since much of the same code will be > > used by > > intel_pmt and the new driver, create a new file with symbols to be > > used by > > both. > > > > While performing this split we need to also handle the creation of > > platform > > devices for the non-PMT capabilities. Currently PMT devices are > > named by > > their capability (e.g. pmt_telemetry). Instead, generically name > > them by > > their capability ID (e.g. intel_extnd_cap_2). This allows the IDs > > to be > > created automatically. However, to ensure that unsupported devices > > aren't > > created, use an allow list to specify supported capabilities. > > > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > --- > > MAINTAINERS | 1 + > > drivers/mfd/Kconfig | 4 + > > drivers/mfd/Makefile | 1 + > > drivers/mfd/intel_extended_caps.c | 208 > > +++++++++++++++++++++ > > Please consider moving this <whatever this is> out to either > drivers/pci or drivers/platform/x86. None of the cell drivers are in MFD, only the PCI drivers from which the cells are created. I understood that these should be in MFD. But moving it to drivers/platform/x86 would be fine with me. That keeps the code together in the same subsystem. Comment from Hans or Andy? > > I suggest Andy should also be on Cc. > > > drivers/mfd/intel_extended_caps.h | 40 ++++ > > drivers/mfd/intel_pmt.c | 198 ++--------------- > > --- > > drivers/platform/x86/intel_pmt_crashlog.c | 2 +- > > drivers/platform/x86/intel_pmt_telemetry.c | 2 +- > > 8 files changed, 270 insertions(+), 186 deletions(-) > > create mode 100644 drivers/mfd/intel_extended_caps.c > > create mode 100644 drivers/mfd/intel_extended_caps.h > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-06-30 21:11 ` David E. Box @ 2021-07-01 8:01 ` Lee Jones 2021-07-01 8:39 ` Hans de Goede 2021-07-01 9:43 ` Andy Shevchenko 2 siblings, 0 replies; 20+ messages in thread From: Lee Jones @ 2021-07-01 8:01 UTC (permalink / raw) To: David E. Box Cc: hdegoede, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Andy Shevchenko On Wed, 30 Jun 2021, David E. Box wrote: > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > > On Thu, 17 Jun 2021, David E. Box wrote: > > > > > Unlike the other devices in intel_pmt, the Out of Band Management > > > Services > > > Module (OOBMSM) is actually not a PMT dedicated device. It can also > > > be used > > > to describe non-PMT capabilities. Like PMT, these capabilities are > > > also > > > enumerated using PCIe Vendor Specific registers in config space. In > > > order > > > to better support these devices without the confusion of a > > > dependency on > > > MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it > > > can be > > > later placed in its own driver. Since much of the same code will be > > > used by > > > intel_pmt and the new driver, create a new file with symbols to be > > > used by > > > both. > > > > > > While performing this split we need to also handle the creation of > > > platform > > > devices for the non-PMT capabilities. Currently PMT devices are > > > named by > > > their capability (e.g. pmt_telemetry). Instead, generically name > > > them by > > > their capability ID (e.g. intel_extnd_cap_2). This allows the IDs > > > to be > > > created automatically. However, to ensure that unsupported devices > > > aren't > > > created, use an allow list to specify supported capabilities. > > > > > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > > --- > > > MAINTAINERS | 1 + > > > drivers/mfd/Kconfig | 4 + > > > drivers/mfd/Makefile | 1 + > > > drivers/mfd/intel_extended_caps.c | 208 > > > +++++++++++++++++++++ > > > > Please consider moving this <whatever this is> out to either > > drivers/pci or drivers/platform/x86. > > None of the cell drivers are in MFD, only the PCI drivers from which > the cells are created. I understood that these should be in MFD. But > moving it to drivers/platform/x86 would be fine with me. That keeps the > code together in the same subsystem. Comment from Hans or Andy? You don't need to move everything there. If a driver uses the MFD API (like intel_pmt.c does), it can stay. But all of this PCI/hardware/ platform specific capability craziness has no place here AFAICT. > > I suggest Andy should also be on Cc. > > > > > drivers/mfd/intel_extended_caps.h | 40 ++++ > > > drivers/mfd/intel_pmt.c | 198 ++--------------- > > > --- > > > drivers/platform/x86/intel_pmt_crashlog.c | 2 +- > > > drivers/platform/x86/intel_pmt_telemetry.c | 2 +- > > > 8 files changed, 270 insertions(+), 186 deletions(-) > > > create mode 100644 drivers/mfd/intel_extended_caps.c > > > create mode 100644 drivers/mfd/intel_extended_caps.h > > > > -- 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] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-06-30 21:11 ` David E. Box 2021-07-01 8:01 ` Lee Jones @ 2021-07-01 8:39 ` Hans de Goede 2021-07-01 11:23 ` Lee Jones 2021-07-01 9:43 ` Andy Shevchenko 2 siblings, 1 reply; 20+ messages in thread From: Hans de Goede @ 2021-07-01 8:39 UTC (permalink / raw) To: david.e.box, Lee Jones Cc: mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Andy Shevchenko Hi, On 6/30/21 11:11 PM, David E. Box wrote: > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: >> On Thu, 17 Jun 2021, David E. Box wrote: >> >>> Unlike the other devices in intel_pmt, the Out of Band Management >>> Services >>> Module (OOBMSM) is actually not a PMT dedicated device. It can also >>> be used >>> to describe non-PMT capabilities. Like PMT, these capabilities are >>> also >>> enumerated using PCIe Vendor Specific registers in config space. In >>> order >>> to better support these devices without the confusion of a >>> dependency on >>> MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it >>> can be >>> later placed in its own driver. Since much of the same code will be >>> used by >>> intel_pmt and the new driver, create a new file with symbols to be >>> used by >>> both. >>> >>> While performing this split we need to also handle the creation of >>> platform >>> devices for the non-PMT capabilities. Currently PMT devices are >>> named by >>> their capability (e.g. pmt_telemetry). Instead, generically name >>> them by >>> their capability ID (e.g. intel_extnd_cap_2). This allows the IDs >>> to be >>> created automatically. However, to ensure that unsupported devices >>> aren't >>> created, use an allow list to specify supported capabilities. >>> >>> Signed-off-by: David E. Box <david.e.box@linux.intel.com> >>> --- >>> MAINTAINERS | 1 + >>> drivers/mfd/Kconfig | 4 + >>> drivers/mfd/Makefile | 1 + >>> drivers/mfd/intel_extended_caps.c | 208 >>> +++++++++++++++++++++ >> >> Please consider moving this <whatever this is> out to either >> drivers/pci or drivers/platform/x86. > > None of the cell drivers are in MFD, only the PCI drivers from which > the cells are created. I understood that these should be in MFD. But > moving it to drivers/platform/x86 would be fine with me. That keeps the > code together in the same subsystem. Comment from Hans or Andy? I'm fine with moving everything to drivers/platform/x86, but AFAIK usually the actual code which has the MFD cells and creates the child devices usually lives under drivers/mfd Regards, Hans > >> >> I suggest Andy should also be on Cc. >> >>> drivers/mfd/intel_extended_caps.h | 40 ++++ >>> drivers/mfd/intel_pmt.c | 198 ++--------------- >>> --- >>> drivers/platform/x86/intel_pmt_crashlog.c | 2 +- >>> drivers/platform/x86/intel_pmt_telemetry.c | 2 +- >>> 8 files changed, 270 insertions(+), 186 deletions(-) >>> create mode 100644 drivers/mfd/intel_extended_caps.c >>> create mode 100644 drivers/mfd/intel_extended_caps.h >> > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-07-01 8:39 ` Hans de Goede @ 2021-07-01 11:23 ` Lee Jones [not found] ` <CAHp75Vfn6GKSj6USUPEWiPdhWRYcJbirqhU6aOeB4gruekmocg@mail.gmail.com> 2021-07-01 22:41 ` David E. Box 0 siblings, 2 replies; 20+ messages in thread From: Lee Jones @ 2021-07-01 11:23 UTC (permalink / raw) To: Hans de Goede Cc: david.e.box, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Andy Shevchenko On Thu, 01 Jul 2021, Hans de Goede wrote: > Hi, > > On 6/30/21 11:11 PM, David E. Box wrote: > > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > >> On Thu, 17 Jun 2021, David E. Box wrote: > >> > >>> Unlike the other devices in intel_pmt, the Out of Band Management > >>> Services > >>> Module (OOBMSM) is actually not a PMT dedicated device. It can also > >>> be used > >>> to describe non-PMT capabilities. Like PMT, these capabilities are > >>> also > >>> enumerated using PCIe Vendor Specific registers in config space. In > >>> order > >>> to better support these devices without the confusion of a > >>> dependency on > >>> MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it > >>> can be > >>> later placed in its own driver. Since much of the same code will be > >>> used by > >>> intel_pmt and the new driver, create a new file with symbols to be > >>> used by > >>> both. > >>> > >>> While performing this split we need to also handle the creation of > >>> platform > >>> devices for the non-PMT capabilities. Currently PMT devices are > >>> named by > >>> their capability (e.g. pmt_telemetry). Instead, generically name > >>> them by > >>> their capability ID (e.g. intel_extnd_cap_2). This allows the IDs > >>> to be > >>> created automatically. However, to ensure that unsupported devices > >>> aren't > >>> created, use an allow list to specify supported capabilities. > >>> > >>> Signed-off-by: David E. Box <david.e.box@linux.intel.com> > >>> --- > >>> MAINTAINERS | 1 + > >>> drivers/mfd/Kconfig | 4 + > >>> drivers/mfd/Makefile | 1 + > >>> drivers/mfd/intel_extended_caps.c | 208 > >>> +++++++++++++++++++++ > >> > >> Please consider moving this <whatever this is> out to either > >> drivers/pci or drivers/platform/x86. > > > > None of the cell drivers are in MFD, only the PCI drivers from which > > the cells are created. I understood that these should be in MFD. But > > moving it to drivers/platform/x86 would be fine with me. That keeps the > > code together in the same subsystem. Comment from Hans or Andy? > > I'm fine with moving everything to drivers/platform/x86, but AFAIK > usually the actual code which has the MFD cells and creates the > child devices usually lives under drivers/mfd Correct. It must. No MFD API users outside of drivers/mfd 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] 20+ messages in thread
[parent not found: <CAHp75Vfn6GKSj6USUPEWiPdhWRYcJbirqhU6aOeB4gruekmocg@mail.gmail.com>]
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device [not found] ` <CAHp75Vfn6GKSj6USUPEWiPdhWRYcJbirqhU6aOeB4gruekmocg@mail.gmail.com> @ 2021-07-01 12:06 ` Lee Jones [not found] ` <CAHp75VdmnRJKSBZ8dmU=7XsGOZ-wX6EpZhtC3X6JEE0mz-UJNg@mail.gmail.com> 0 siblings, 1 reply; 20+ messages in thread From: Lee Jones @ 2021-07-01 12:06 UTC (permalink / raw) To: Andy Shevchenko Cc: Hans de Goede, david.e.box, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci On Thu, 01 Jul 2021, Andy Shevchenko wrote: > On Thursday, July 1, 2021, Lee Jones <lee.jones@linaro.org> wrote: > > > On Thu, 01 Jul 2021, Hans de Goede wrote: > > > > > Hi, > > > > > > On 6/30/21 11:11 PM, David E. Box wrote: > > > > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > > > >> On Thu, 17 Jun 2021, David E. Box wrote: > > > >> > > > >>> Unlike the other devices in intel_pmt, the Out of Band Management > > > >>> Services > > > >>> Module (OOBMSM) is actually not a PMT dedicated device. It can also > > > >>> be used > > > >>> to describe non-PMT capabilities. Like PMT, these capabilities are > > > >>> also > > > >>> enumerated using PCIe Vendor Specific registers in config space. In > > > >>> order > > > >>> to better support these devices without the confusion of a > > > >>> dependency on > > > >>> MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it > > > >>> can be > > > >>> later placed in its own driver. Since much of the same code will be > > > >>> used by > > > >>> intel_pmt and the new driver, create a new file with symbols to be > > > >>> used by > > > >>> both. > > > >>> > > > >>> While performing this split we need to also handle the creation of > > > >>> platform > > > >>> devices for the non-PMT capabilities. Currently PMT devices are > > > >>> named by > > > >>> their capability (e.g. pmt_telemetry). Instead, generically name > > > >>> them by > > > >>> their capability ID (e.g. intel_extnd_cap_2). This allows the IDs > > > >>> to be > > > >>> created automatically. However, to ensure that unsupported devices > > > >>> aren't > > > >>> created, use an allow list to specify supported capabilities. > > > >>> > > > >>> Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > > >>> --- > > > >>> MAINTAINERS | 1 + > > > >>> drivers/mfd/Kconfig | 4 + > > > >>> drivers/mfd/Makefile | 1 + > > > >>> drivers/mfd/intel_extended_caps.c | 208 > > > >>> +++++++++++++++++++++ > > > >> > > > >> Please consider moving this <whatever this is> out to either > > > >> drivers/pci or drivers/platform/x86. > > > > > > > > None of the cell drivers are in MFD, only the PCI drivers from which > > > > the cells are created. I understood that these should be in MFD. But > > > > moving it to drivers/platform/x86 would be fine with me. That keeps the > > > > code together in the same subsystem. Comment from Hans or Andy? > > > > > > I'm fine with moving everything to drivers/platform/x86, but AFAIK > > > usually the actual code which has the MFD cells and creates the > > > child devices usually lives under drivers/mfd > > > > Correct. It must. > > It’s definitely not the first time you are talking about, but it may be the > first time I asked why it’s not enforced overall. Last time I have checked > it was like 5-7 MFD uses outside the MFD folder. Are you going to fix that? Because I can't NACK patches that weren't sent to me. :) I'll probably look into 'fixing' it when I get some free time. > > No MFD API users outside of drivers/mfd 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] 20+ messages in thread
[parent not found: <CAHp75VdmnRJKSBZ8dmU=7XsGOZ-wX6EpZhtC3X6JEE0mz-UJNg@mail.gmail.com>]
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device [not found] ` <CAHp75VdmnRJKSBZ8dmU=7XsGOZ-wX6EpZhtC3X6JEE0mz-UJNg@mail.gmail.com> @ 2021-07-01 12:26 ` Lee Jones 0 siblings, 0 replies; 20+ messages in thread From: Lee Jones @ 2021-07-01 12:26 UTC (permalink / raw) To: Andy Shevchenko Cc: Hans de Goede, david.e.box, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci On Thu, 01 Jul 2021, Andy Shevchenko wrote: > On Thursday, July 1, 2021, Lee Jones <lee.jones@linaro.org> wrote: > > > On Thu, 01 Jul 2021, Andy Shevchenko wrote: > > > > > On Thursday, July 1, 2021, Lee Jones <lee.jones@linaro.org> wrote: > > > > > > > On Thu, 01 Jul 2021, Hans de Goede wrote: > > > > > > > > > Hi, > > > > > > > > > > On 6/30/21 11:11 PM, David E. Box wrote: > > > > > > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > > > > > >> On Thu, 17 Jun 2021, David E. Box wrote: > > > > > >> > > > > > >>> Unlike the other devices in intel_pmt, the Out of Band Management > > > > > >>> Services > > > > > >>> Module (OOBMSM) is actually not a PMT dedicated device. It can > > also > > > > > >>> be used > > > > > >>> to describe non-PMT capabilities. Like PMT, these capabilities > > are > > > > > >>> also > > > > > >>> enumerated using PCIe Vendor Specific registers in config space. > > In > > > > > >>> order > > > > > >>> to better support these devices without the confusion of a > > > > > >>> dependency on > > > > > >>> MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so that it > > > > > >>> can be > > > > > >>> later placed in its own driver. Since much of the same code will > > be > > > > > >>> used by > > > > > >>> intel_pmt and the new driver, create a new file with symbols to > > be > > > > > >>> used by > > > > > >>> both. > > > > > >>> > > > > > >>> While performing this split we need to also handle the creation > > of > > > > > >>> platform > > > > > >>> devices for the non-PMT capabilities. Currently PMT devices are > > > > > >>> named by > > > > > >>> their capability (e.g. pmt_telemetry). Instead, generically name > > > > > >>> them by > > > > > >>> their capability ID (e.g. intel_extnd_cap_2). This allows the IDs > > > > > >>> to be > > > > > >>> created automatically. However, to ensure that unsupported > > devices > > > > > >>> aren't > > > > > >>> created, use an allow list to specify supported capabilities. > > > > > >>> > > > > > >>> Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > > > > >>> --- > > > > > >>> MAINTAINERS | 1 + > > > > > >>> drivers/mfd/Kconfig | 4 + > > > > > >>> drivers/mfd/Makefile | 1 + > > > > > >>> drivers/mfd/intel_extended_caps.c | 208 > > > > > >>> +++++++++++++++++++++ > > > > > >> > > > > > >> Please consider moving this <whatever this is> out to either > > > > > >> drivers/pci or drivers/platform/x86. > > > > > > > > > > > > None of the cell drivers are in MFD, only the PCI drivers from > > which > > > > > > the cells are created. I understood that these should be in MFD. > > But > > > > > > moving it to drivers/platform/x86 would be fine with me. That > > keeps the > > > > > > code together in the same subsystem. Comment from Hans or Andy? > > > > > > > > > > I'm fine with moving everything to drivers/platform/x86, but AFAIK > > > > > usually the actual code which has the MFD cells and creates the > > > > > child devices usually lives under drivers/mfd > > > > > > > > Correct. It must. > > > > > > It’s definitely not the first time you are talking about, but it may be > > the > > > first time I asked why it’s not enforced overall. Last time I have > > checked > > > it was like 5-7 MFD uses outside the MFD folder. Are you going to fix > > that? > > > > Because I can't NACK patches that weren't sent to me. :) > > > > > Hint: you may add regexp match to the maintainers database and you will see > them more often Good idea. I'll add it to my TODO. -- 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] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-07-01 11:23 ` Lee Jones [not found] ` <CAHp75Vfn6GKSj6USUPEWiPdhWRYcJbirqhU6aOeB4gruekmocg@mail.gmail.com> @ 2021-07-01 22:41 ` David E. Box 1 sibling, 0 replies; 20+ messages in thread From: David E. Box @ 2021-07-01 22:41 UTC (permalink / raw) To: Lee Jones, Hans de Goede Cc: mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci, Andy Shevchenko On Thu, 2021-07-01 at 12:23 +0100, Lee Jones wrote: > On Thu, 01 Jul 2021, Hans de Goede wrote: > > > Hi, > > > > On 6/30/21 11:11 PM, David E. Box wrote: > > > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > > > > On Thu, 17 Jun 2021, David E. Box wrote: > > > > > > > > > Unlike the other devices in intel_pmt, the Out of Band > > > > > Management > > > > > Services > > > > > Module (OOBMSM) is actually not a PMT dedicated device. It > > > > > can also > > > > > be used > > > > > to describe non-PMT capabilities. Like PMT, these > > > > > capabilities are > > > > > also > > > > > enumerated using PCIe Vendor Specific registers in config > > > > > space. In > > > > > order > > > > > to better support these devices without the confusion of a > > > > > dependency on > > > > > MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so > > > > > that it > > > > > can be > > > > > later placed in its own driver. Since much of the same code > > > > > will be > > > > > used by > > > > > intel_pmt and the new driver, create a new file with symbols > > > > > to be > > > > > used by > > > > > both. > > > > > > > > > > While performing this split we need to also handle the > > > > > creation of > > > > > platform > > > > > devices for the non-PMT capabilities. Currently PMT devices > > > > > are > > > > > named by > > > > > their capability (e.g. pmt_telemetry). Instead, generically > > > > > name > > > > > them by > > > > > their capability ID (e.g. intel_extnd_cap_2). This allows the > > > > > IDs > > > > > to be > > > > > created automatically. However, to ensure that unsupported > > > > > devices > > > > > aren't > > > > > created, use an allow list to specify supported capabilities. > > > > > > > > > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > > > > --- > > > > > MAINTAINERS | 1 + > > > > > drivers/mfd/Kconfig | 4 + > > > > > drivers/mfd/Makefile | 1 + > > > > > drivers/mfd/intel_extended_caps.c | 208 > > > > > +++++++++++++++++++++ > > > > > > > > Please consider moving this <whatever this is> out to either > > > > drivers/pci or drivers/platform/x86. > > > > > > None of the cell drivers are in MFD, only the PCI drivers from > > > which > > > the cells are created. I understood that these should be in MFD. > > > But > > > moving it to drivers/platform/x86 would be fine with me. That > > > keeps the > > > code together in the same subsystem. Comment from Hans or Andy? > > > > I'm fine with moving everything to drivers/platform/x86, but AFAIK > > usually the actual code which has the MFD cells and creates the > > child devices usually lives under drivers/mfd > > Correct. It must. > > No MFD API users outside of drivers/mfd please. > No problem. But these patches are not child device drivers. They take the existing intel_pmt MFD code and split it from the device driver (similar to how intel-lpss core code is split from the acpi and pci bus drivers). There are 2 drivers now, PMT-only and OOBMSM, that use a common MFD API. This is why they all reside in MFD in this patchset. But I could move the API callers to platform/x86. But I'd like feedback on whether this split is even needed. I'm trying to manage the fact that one of the devices in intel_pmt will now need support for new, non-PMT, child devices. So there would be a mismatch between what the driver and Kconfig are named vs what it actually supports. I considered adding all the new cells to intel_pmt and renaming the driver to something more generic, but I understand this will be messy for OSVs managing Kconfig options. Thanks. David ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device 2021-06-30 21:11 ` David E. Box 2021-07-01 8:01 ` Lee Jones 2021-07-01 8:39 ` Hans de Goede @ 2021-07-01 9:43 ` Andy Shevchenko 2 siblings, 0 replies; 20+ messages in thread From: Andy Shevchenko @ 2021-07-01 9:43 UTC (permalink / raw) To: David Box Cc: Lee Jones, Hans de Goede, Mark Gross, Bjorn Helgaas, Linux Kernel Mailing List, Platform Driver, linux-pci On Thu, Jul 1, 2021 at 12:11 AM David E. Box <david.e.box@linux.intel.com> wrote: > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote: > > On Thu, 17 Jun 2021, David E. Box wrote: ... > > Please consider moving this <whatever this is> out to either > > drivers/pci or drivers/platform/x86. > > None of the cell drivers are in MFD, only the PCI drivers from which > the cells are created. I understood that these should be in MFD. But > moving it to drivers/platform/x86 would be fine with me. That keeps the > code together in the same subsystem. Comment from Hans or Andy? I'm fine with the PDx86 location, but please consider moving PMT stuff under drivers/platform/x86/intel/pmt/. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/4] MFD: Intel Out of Band Management Services Module (OOBMSM) driver 2021-06-17 21:54 [PATCH 0/4] MFD: intel_pmt: Split OOBMSM from intel_pmt driver David E. Box 2021-06-17 21:54 ` [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields David E. Box 2021-06-17 21:54 ` [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device David E. Box @ 2021-06-17 21:54 ` David E. Box 2021-06-30 10:17 ` Lee Jones 2021-06-17 21:54 ` [PATCH 4/4] MFD: intel-extended-cap: Add support for PCIe VSEC structures David E. Box 3 siblings, 1 reply; 20+ messages in thread From: David E. Box @ 2021-06-17 21:54 UTC (permalink / raw) To: lee.jones, david.e.box, hdegoede, mgross, bhelgaas Cc: linux-kernel, platform-driver-x86, linux-pci The Intel Out of Band Management Services Module (OOBMSM) is a device that provides access to Intel capabilities described in PCIE vendor specific extended capability registers (both VSEC and DVSEC). These capabilities include features like Intel Platform Monitoring Technology as well as others that are not supported by the intel_pmt driver. Add a driver for creating platform devices for these capabilities coming from OOBMSM. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- MAINTAINERS | 1 + drivers/mfd/Kconfig | 11 +++++++ drivers/mfd/Makefile | 1 + drivers/mfd/intel_oobmsm.c | 61 ++++++++++++++++++++++++++++++++++++ drivers/platform/x86/Kconfig | 4 +-- 5 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 drivers/mfd/intel_oobmsm.c diff --git a/MAINTAINERS b/MAINTAINERS index ebdc2a0f794b..0961e3f89497 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9356,6 +9356,7 @@ INTEL PMT DRIVER M: "David E. Box" <david.e.box@linux.intel.com> S: Maintained F: drivers/mfd/intel_extended_cap.c +F: drivers/mfd/intel_oobmsm.c F: drivers/mfd/intel_pmt.c F: drivers/platform/x86/intel_pmt_* diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 4dde8e223a9e..269312de2666 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -687,6 +687,17 @@ config MFD_INTEL_PMT Telemetry, Watcher, and Crashlog PMT capabilities/devices for platforms starting from Tiger Lake. +config MFD_INTEL_OOBMSM + tristate "Intel Out Of Band Management Services Module (OOBMSM) support" + depends on PCI + select MFD_INTEL_EXTENDED_CAPS + help + The Intel Out of Band Management Service Module driver is used to + enumerate auxiliary platform features described in both Vendor + Specific and Designated Vendor Specific PCIe config space. Supported + features include Intel Platform Monitoring Technology (PMT) as well + as other non-PMT capabilities. + config MFD_IPAQ_MICRO bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" depends on SA1100_H3100 || SA1100_H3600 diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 7fa35399ec76..50fa38810bbd 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -213,6 +213,7 @@ obj-$(CONFIG_MFD_INTEL_EXTENDED_CAPS) += intel_extended_caps.o obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o +obj-$(CONFIG_MFD_INTEL_OOBMSM) += intel_oobmsm.o obj-$(CONFIG_MFD_INTEL_PMC_BXT) += intel_pmc_bxt.o obj-$(CONFIG_MFD_INTEL_PMT) += intel_pmt.o obj-$(CONFIG_MFD_PALMAS) += palmas.o diff --git a/drivers/mfd/intel_oobmsm.c b/drivers/mfd/intel_oobmsm.c new file mode 100644 index 000000000000..c66532f11c29 --- /dev/null +++ b/drivers/mfd/intel_oobmsm.c @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Intel Out of Band Management Services Module driver + * + * Copyright (c) 2021, Intel Corporation. + * All Rights Reserved. + * + * Author: David E. Box <david.e.box@linux.intel.com> + */ + +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/pm_runtime.h> + +#include "intel_extended_caps.h" + +static int intel_oobmsm_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) +{ + struct intel_ext_cap_platform_info *info; + int ret; + + ret = pcim_enable_device(pdev); + if (ret) + return ret; + + info = (struct intel_ext_cap_platform_info *)id->driver_data; + + ret = intel_ext_cap_probe(pdev, info); + if (ret) + return ret; + + pm_runtime_put(&pdev->dev); + pm_runtime_allow(&pdev->dev); + + return 0; +} + +static void intel_oobmsm_pci_remove(struct pci_dev *pdev) +{ + pm_runtime_forbid(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); +} + +#define PCI_DEVICE_ID_INTEL_PMT_OOBMSM 0x09a7 +static const struct pci_device_id intel_oobmsm_pci_ids[] = { + { PCI_DEVICE_DATA(INTEL, PMT_OOBMSM, NULL) }, + { } +}; +MODULE_DEVICE_TABLE(pci, intel_oobmsm_pci_ids); + +static struct pci_driver intel_oobmsm_pci_driver = { + .name = "intel-oobmsm", + .id_table = intel_oobmsm_pci_ids, + .probe = intel_oobmsm_pci_probe, + .remove = intel_oobmsm_pci_remove, +}; +module_pci_driver(intel_oobmsm_pci_driver); + +MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>"); +MODULE_DESCRIPTION("Intel Out of Band Management Services Module driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 60592fb88e7a..4dd3af9f848e 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1226,7 +1226,7 @@ config INTEL_PMT_CLASS config INTEL_PMT_TELEMETRY tristate "Intel Platform Monitoring Technology (PMT) Telemetry driver" - depends on MFD_INTEL_PMT + depends on MFD_INTEL_PMT || MFD_INTEL_OOBMSM select INTEL_PMT_CLASS help The Intel Platform Monitory Technology (PMT) Telemetry driver provides @@ -1238,7 +1238,7 @@ config INTEL_PMT_TELEMETRY config INTEL_PMT_CRASHLOG tristate "Intel Platform Monitoring Technology (PMT) Crashlog driver" - depends on MFD_INTEL_PMT + depends on MFD_INTEL_PMT || MFD_INTEL_OOBMSM select INTEL_PMT_CLASS help The Intel Platform Monitoring Technology (PMT) crashlog driver provides -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] MFD: Intel Out of Band Management Services Module (OOBMSM) driver 2021-06-17 21:54 ` [PATCH 3/4] MFD: Intel Out of Band Management Services Module (OOBMSM) driver David E. Box @ 2021-06-30 10:17 ` Lee Jones 0 siblings, 0 replies; 20+ messages in thread From: Lee Jones @ 2021-06-30 10:17 UTC (permalink / raw) To: David E. Box Cc: hdegoede, mgross, bhelgaas, linux-kernel, platform-driver-x86, linux-pci On Thu, 17 Jun 2021, David E. Box wrote: > The Intel Out of Band Management Services Module (OOBMSM) is a device > that provides access to Intel capabilities described in PCIE vendor > specific extended capability registers (both VSEC and DVSEC). These > capabilities include features like Intel Platform Monitoring Technology > as well as others that are not supported by the intel_pmt driver. Add a > driver for creating platform devices for these capabilities coming from > OOBMSM. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > --- > MAINTAINERS | 1 + > drivers/mfd/Kconfig | 11 +++++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/intel_oobmsm.c | 61 ++++++++++++++++++++++++++++++++++++ > drivers/platform/x86/Kconfig | 4 +-- > 5 files changed, 76 insertions(+), 2 deletions(-) > create mode 100644 drivers/mfd/intel_oobmsm.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ebdc2a0f794b..0961e3f89497 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9356,6 +9356,7 @@ INTEL PMT DRIVER > M: "David E. Box" <david.e.box@linux.intel.com> > S: Maintained > F: drivers/mfd/intel_extended_cap.c > +F: drivers/mfd/intel_oobmsm.c > F: drivers/mfd/intel_pmt.c > F: drivers/platform/x86/intel_pmt_* > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 4dde8e223a9e..269312de2666 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -687,6 +687,17 @@ config MFD_INTEL_PMT > Telemetry, Watcher, and Crashlog PMT capabilities/devices for > platforms starting from Tiger Lake. > > +config MFD_INTEL_OOBMSM > + tristate "Intel Out Of Band Management Services Module (OOBMSM) support" > + depends on PCI > + select MFD_INTEL_EXTENDED_CAPS > + help > + The Intel Out of Band Management Service Module driver is used to > + enumerate auxiliary platform features described in both Vendor > + Specific and Designated Vendor Specific PCIe config space. Supported > + features include Intel Platform Monitoring Technology (PMT) as well > + as other non-PMT capabilities. > + > config MFD_IPAQ_MICRO > bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" > depends on SA1100_H3100 || SA1100_H3600 > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 7fa35399ec76..50fa38810bbd 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -213,6 +213,7 @@ obj-$(CONFIG_MFD_INTEL_EXTENDED_CAPS) += intel_extended_caps.o > obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o > obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o > obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o > +obj-$(CONFIG_MFD_INTEL_OOBMSM) += intel_oobmsm.o > obj-$(CONFIG_MFD_INTEL_PMC_BXT) += intel_pmc_bxt.o > obj-$(CONFIG_MFD_INTEL_PMT) += intel_pmt.o > obj-$(CONFIG_MFD_PALMAS) += palmas.o > diff --git a/drivers/mfd/intel_oobmsm.c b/drivers/mfd/intel_oobmsm.c > new file mode 100644 > index 000000000000..c66532f11c29 > --- /dev/null > +++ b/drivers/mfd/intel_oobmsm.c > @@ -0,0 +1,61 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Intel Out of Band Management Services Module driver > + * > + * Copyright (c) 2021, Intel Corporation. > + * All Rights Reserved. > + * > + * Author: David E. Box <david.e.box@linux.intel.com> > + */ > + > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/pm_runtime.h> This doesn't appear to have anything to do with MFD? > +#include "intel_extended_caps.h" > + > +static int intel_oobmsm_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > +{ > + struct intel_ext_cap_platform_info *info; > + int ret; > + > + ret = pcim_enable_device(pdev); > + if (ret) > + return ret; > + > + info = (struct intel_ext_cap_platform_info *)id->driver_data; > + > + ret = intel_ext_cap_probe(pdev, info); > + if (ret) > + return ret; > + > + pm_runtime_put(&pdev->dev); > + pm_runtime_allow(&pdev->dev); > + > + return 0; > +} > + > +static void intel_oobmsm_pci_remove(struct pci_dev *pdev) > +{ > + pm_runtime_forbid(&pdev->dev); > + pm_runtime_get_sync(&pdev->dev); > +} > + > +#define PCI_DEVICE_ID_INTEL_PMT_OOBMSM 0x09a7 > +static const struct pci_device_id intel_oobmsm_pci_ids[] = { > + { PCI_DEVICE_DATA(INTEL, PMT_OOBMSM, NULL) }, > + { } > +}; > +MODULE_DEVICE_TABLE(pci, intel_oobmsm_pci_ids); > + > +static struct pci_driver intel_oobmsm_pci_driver = { > + .name = "intel-oobmsm", > + .id_table = intel_oobmsm_pci_ids, > + .probe = intel_oobmsm_pci_probe, > + .remove = intel_oobmsm_pci_remove, > +}; > +module_pci_driver(intel_oobmsm_pci_driver); > + > +MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>"); > +MODULE_DESCRIPTION("Intel Out of Band Management Services Module driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 60592fb88e7a..4dd3af9f848e 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1226,7 +1226,7 @@ config INTEL_PMT_CLASS > > config INTEL_PMT_TELEMETRY > tristate "Intel Platform Monitoring Technology (PMT) Telemetry driver" > - depends on MFD_INTEL_PMT > + depends on MFD_INTEL_PMT || MFD_INTEL_OOBMSM > select INTEL_PMT_CLASS > help > The Intel Platform Monitory Technology (PMT) Telemetry driver provides > @@ -1238,7 +1238,7 @@ config INTEL_PMT_TELEMETRY > > config INTEL_PMT_CRASHLOG > tristate "Intel Platform Monitoring Technology (PMT) Crashlog driver" > - depends on MFD_INTEL_PMT > + depends on MFD_INTEL_PMT || MFD_INTEL_OOBMSM > select INTEL_PMT_CLASS > help > The Intel Platform Monitoring Technology (PMT) crashlog driver provides -- 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] 20+ messages in thread
* [PATCH 4/4] MFD: intel-extended-cap: Add support for PCIe VSEC structures 2021-06-17 21:54 [PATCH 0/4] MFD: intel_pmt: Split OOBMSM from intel_pmt driver David E. Box ` (2 preceding siblings ...) 2021-06-17 21:54 ` [PATCH 3/4] MFD: Intel Out of Band Management Services Module (OOBMSM) driver David E. Box @ 2021-06-17 21:54 ` David E. Box 3 siblings, 0 replies; 20+ messages in thread From: David E. Box @ 2021-06-17 21:54 UTC (permalink / raw) To: lee.jones, david.e.box, hdegoede, mgross, bhelgaas Cc: linux-kernel, platform-driver-x86, linux-pci Adds support for discovering Intel extended capability features from Vendor Specific Extended Capability (VSEC) registers in PCIe config space. While doing so place the existing DVSEC and new VSEC code in separate functions. Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- drivers/mfd/intel_extended_caps.c | 159 ++++++++++++++++++++---------- 1 file changed, 109 insertions(+), 50 deletions(-) diff --git a/drivers/mfd/intel_extended_caps.c b/drivers/mfd/intel_extended_caps.c index 89cf1ae6f65b..9b60defda856 100644 --- a/drivers/mfd/intel_extended_caps.c +++ b/drivers/mfd/intel_extended_caps.c @@ -105,7 +105,7 @@ static int intel_ext_cap_add_dev(struct pci_dev *pdev, struct intel_ext_cap_head header->offset >>= 3; /* - * The DVSEC contains the starting offset and count for a block of + * The DVSEC/VSEC contains the starting offset and count for a block of * discovery tables, each providing access to monitoring facilities for * a section of the device. Create a resource list of these tables to * provide to the driver. @@ -124,11 +124,112 @@ static int intel_ext_cap_add_dev(struct pci_dev *pdev, struct intel_ext_cap_head return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cell, 1, NULL, 0, NULL); } +static bool intel_ext_cap_walk_dvsec(struct pci_dev *pdev, unsigned long quirks) +{ + int count = 0; + int pos = 0; + + do { + struct intel_ext_cap_header header; + u32 table, hdr; + u16 vid; + int ret; + + pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC); + if (!pos) + break; + + pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER1, &hdr); + vid = PCI_DVSEC_HEADER1_VID(hdr); + if (vid != PCI_VENDOR_ID_INTEL) + continue; + + /* Support only revision 1 */ + header.rev = PCI_DVSEC_HEADER1_REV(hdr); + if (header.rev != 1) { + dev_warn(&pdev->dev, "Unsupported DVSEC revision %d\n", + header.rev); + continue; + } + + header.length = PCI_DVSEC_HEADER1_LEN(hdr); + + 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); + + pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2, &hdr); + header.id = PCI_DVSEC_HEADER2_ID(hdr); + + ret = intel_ext_cap_add_dev(pdev, &header, quirks); + if (ret) + continue; + + count++; + } while (true); + + return count; +} + +static bool intel_ext_cap_walk_vsec(struct pci_dev *pdev, unsigned long quirks) +{ + int count = 0; + int pos = 0; + + do { + struct intel_ext_cap_header header; + u32 table, hdr; + int ret; + + pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_VNDR); + if (!pos) + break; + + pci_read_config_dword(pdev, pos + PCI_VNDR_HEADER, &hdr); + + /* Support only revision 1 */ + header.rev = PCI_VNDR_HEADER_REV(hdr); + if (header.rev != 1) { + dev_warn(&pdev->dev, "Unsupported VSEC revision %d\n", + header.rev); + continue; + } + + header.id = PCI_VNDR_HEADER_ID(hdr); + header.length = PCI_VNDR_HEADER_LEN(hdr); + + /* entry, size, and table offset are the same as DVSEC */ + 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 = intel_ext_cap_add_dev(pdev, &header, quirks); + if (ret) + continue; + + count++; + } while (true); + + return count; +} + int intel_ext_cap_probe(struct pci_dev *pdev, struct intel_ext_cap_platform_info *info) { unsigned long quirks = 0; - bool found_devices = false; - int ret, pos; + int device_count = 0; + int ret; if (info) quirks = info->quirks; @@ -144,59 +245,17 @@ int intel_ext_cap_probe(struct pci_dev *pdev, struct intel_ext_cap_platform_info "Failed to add device for DVSEC id %d\n", (*header)->id); else - found_devices = true; + device_count++; header++; } - } else { - /* Find DVSEC features */ - pos = 0; - do { - struct intel_ext_cap_header header; - u32 table, hdr; - u16 vid; - - pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC); - if (!pos) - break; - - pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER1, &hdr); - vid = PCI_DVSEC_HEADER1_VID(hdr); - if (vid != PCI_VENDOR_ID_INTEL) - continue; - - /* Support only revision 1 */ - header.rev = PCI_DVSEC_HEADER1_REV(hdr); - if (header.rev != 1) { - dev_warn(&pdev->dev, "Unsupported DVSEC revision %d\n", - header.rev); - continue; - } - - header.length = PCI_DVSEC_HEADER1_LEN(hdr); - - 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); - - pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2, &hdr); - header.id = PCI_DVSEC_HEADER2_ID(hdr); - - ret = intel_ext_cap_add_dev(pdev, &header, quirks); - if (ret) - continue; - found_devices = true; - } while (true); + } else { + device_count += intel_ext_cap_walk_dvsec(pdev, quirks); + device_count += intel_ext_cap_walk_vsec(pdev, quirks); } - if (!found_devices) + if (!device_count) return -ENODEV; return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 0/4] Auxiliary bus driver support for Intel PCIe VSEC/DVSEC @ 2021-11-20 23:17 David E. Box 2021-11-20 23:17 ` [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields David E. Box 0 siblings, 1 reply; 20+ messages in thread From: David E. Box @ 2021-11-20 23:17 UTC (permalink / raw) To: lee.jones, hdegoede, david.e.box, bhelgaas, gregkh, andriy.shevchenko, srinivas.pandruvada, mgross Cc: linux-kernel, platform-driver-x86, linux-pci This series makes changes to the current intel_pmt driver to give it broader support for Intel defined PCIe VSEC and DVSEC features. It moves the implementation from MFD to the auxiliary bus and creates a generic framework for enumerating the extended capabilities. It also adds support for a new VSEC, Software Defined Silicon (SDSi). David E. Box (4): PCI: Add #defines for accessing PCIe DVSEC fields driver core: auxiliary bus: Add driver data helpers platform/x86/intel: Move intel_pmt from MFD to Auxiliary Bus platform/x86: Add Intel Software Defined Silicon driver .../ABI/testing/sysfs-driver-intel_sdsi | 75 +++ MAINTAINERS | 17 +- drivers/mfd/Kconfig | 10 - drivers/mfd/Makefile | 1 - drivers/mfd/intel_pmt.c | 261 -------- drivers/platform/x86/intel/Kconfig | 23 + drivers/platform/x86/intel/Makefile | 4 + drivers/platform/x86/intel/pmt/Kconfig | 4 +- drivers/platform/x86/intel/pmt/class.c | 21 +- drivers/platform/x86/intel/pmt/class.h | 5 +- drivers/platform/x86/intel/pmt/crashlog.c | 47 +- drivers/platform/x86/intel/pmt/telemetry.c | 46 +- drivers/platform/x86/intel/sdsi.c | 571 ++++++++++++++++++ drivers/platform/x86/intel/vsec.c | 418 +++++++++++++ drivers/platform/x86/intel/vsec.h | 43 ++ include/linux/auxiliary_bus.h | 10 + include/uapi/linux/pci_regs.h | 4 + 17 files changed, 1225 insertions(+), 335 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-intel_sdsi delete mode 100644 drivers/mfd/intel_pmt.c create mode 100644 drivers/platform/x86/intel/sdsi.c create mode 100644 drivers/platform/x86/intel/vsec.c create mode 100644 drivers/platform/x86/intel/vsec.h -- 2.25.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields 2021-11-20 23:17 [PATCH 0/4] Auxiliary bus driver support for Intel PCIe VSEC/DVSEC David E. Box @ 2021-11-20 23:17 ` David E. Box 2021-11-21 12:24 ` Greg KH 0 siblings, 1 reply; 20+ messages in thread From: David E. Box @ 2021-11-20 23:17 UTC (permalink / raw) To: lee.jones, hdegoede, david.e.box, bhelgaas, gregkh, andriy.shevchenko, srinivas.pandruvada, mgross Cc: linux-kernel, platform-driver-x86, linux-pci Add #defines for accessing Vendor ID, Revision, Length, and ID offsets in the Designated Vendor Specific Extended Capability (DVSEC). Defined in PCIe r5.0, sec 7.9.6. Signed-off-by: David E. Box <david.e.box@linux.intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- include/uapi/linux/pci_regs.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index ff6ccbc6efe9..318f3f1f9e92 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -1086,7 +1086,11 @@ /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */ #define PCI_DVSEC_HEADER1 0x4 /* Designated Vendor-Specific Header1 */ +#define PCI_DVSEC_HEADER1_VID(x) ((x) & 0xffff) +#define PCI_DVSEC_HEADER1_REV(x) (((x) >> 16) & 0xf) +#define PCI_DVSEC_HEADER1_LEN(x) (((x) >> 20) & 0xfff) #define PCI_DVSEC_HEADER2 0x8 /* Designated Vendor-Specific Header2 */ +#define PCI_DVSEC_HEADER2_ID(x) ((x) & 0xffff) /* Data Link Feature */ #define PCI_DLF_CAP 0x04 /* Capabilities Register */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields 2021-11-20 23:17 ` [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields David E. Box @ 2021-11-21 12:24 ` Greg KH 2021-11-21 15:48 ` David E. Box 2021-11-22 18:28 ` Bjorn Helgaas 0 siblings, 2 replies; 20+ messages in thread From: Greg KH @ 2021-11-21 12:24 UTC (permalink / raw) To: David E. Box Cc: lee.jones, hdegoede, bhelgaas, andriy.shevchenko, srinivas.pandruvada, mgross, linux-kernel, platform-driver-x86, linux-pci On Sat, Nov 20, 2021 at 03:17:02PM -0800, David E. Box wrote: > Add #defines for accessing Vendor ID, Revision, Length, and ID offsets > in the Designated Vendor Specific Extended Capability (DVSEC). Defined > in PCIe r5.0, sec 7.9.6. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > include/uapi/linux/pci_regs.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index ff6ccbc6efe9..318f3f1f9e92 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -1086,7 +1086,11 @@ > > /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */ > #define PCI_DVSEC_HEADER1 0x4 /* Designated Vendor-Specific Header1 */ > +#define PCI_DVSEC_HEADER1_VID(x) ((x) & 0xffff) > +#define PCI_DVSEC_HEADER1_REV(x) (((x) >> 16) & 0xf) > +#define PCI_DVSEC_HEADER1_LEN(x) (((x) >> 20) & 0xfff) > #define PCI_DVSEC_HEADER2 0x8 /* Designated Vendor-Specific Header2 */ > +#define PCI_DVSEC_HEADER2_ID(x) ((x) & 0xffff) Why does userspace need to have these defines? What userspace tool is going to use these? thanks, greg k-h ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields 2021-11-21 12:24 ` Greg KH @ 2021-11-21 15:48 ` David E. Box 2021-11-22 18:28 ` Bjorn Helgaas 1 sibling, 0 replies; 20+ messages in thread From: David E. Box @ 2021-11-21 15:48 UTC (permalink / raw) To: Greg KH Cc: lee.jones, hdegoede, bhelgaas, andriy.shevchenko, srinivas.pandruvada, mgross, linux-kernel, platform-driver-x86, linux-pci On Sun, 2021-11-21 at 13:24 +0100, Greg KH wrote: > On Sat, Nov 20, 2021 at 03:17:02PM -0800, David E. Box wrote: > > Add #defines for accessing Vendor ID, Revision, Length, and ID offsets > > in the Designated Vendor Specific Extended Capability (DVSEC). Defined > > in PCIe r5.0, sec 7.9.6. > > > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > include/uapi/linux/pci_regs.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > > index ff6ccbc6efe9..318f3f1f9e92 100644 > > --- a/include/uapi/linux/pci_regs.h > > +++ b/include/uapi/linux/pci_regs.h > > @@ -1086,7 +1086,11 @@ > > > > /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */ > > #define PCI_DVSEC_HEADER1 0x4 /* Designated Vendor-Specific > > Header1 */ > > +#define PCI_DVSEC_HEADER1_VID(x) ((x) & 0xffff) > > +#define PCI_DVSEC_HEADER1_REV(x) (((x) >> 16) & 0xf) > > +#define PCI_DVSEC_HEADER1_LEN(x) (((x) >> 20) & 0xfff) > > #define PCI_DVSEC_HEADER2 0x8 /* Designated Vendor-Specific > > Header2 */ > > +#define PCI_DVSEC_HEADER2_ID(x) ((x) & 0xffff) > > Why does userspace need to have these defines? What userspace tool is > going to use these? These headers should useful for userspace tools that access PCI devices. DVSEC is also used by CXL so they should come in handy for tools accesses those devices. > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields 2021-11-21 12:24 ` Greg KH 2021-11-21 15:48 ` David E. Box @ 2021-11-22 18:28 ` Bjorn Helgaas 1 sibling, 0 replies; 20+ messages in thread From: Bjorn Helgaas @ 2021-11-22 18:28 UTC (permalink / raw) To: Greg KH Cc: David E. Box, lee.jones, hdegoede, bhelgaas, andriy.shevchenko, srinivas.pandruvada, mgross, linux-kernel, platform-driver-x86, linux-pci On Sun, Nov 21, 2021 at 01:24:09PM +0100, Greg KH wrote: > On Sat, Nov 20, 2021 at 03:17:02PM -0800, David E. Box wrote: > > Add #defines for accessing Vendor ID, Revision, Length, and ID offsets > > in the Designated Vendor Specific Extended Capability (DVSEC). Defined > > in PCIe r5.0, sec 7.9.6. > > > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > include/uapi/linux/pci_regs.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > > index ff6ccbc6efe9..318f3f1f9e92 100644 > > --- a/include/uapi/linux/pci_regs.h > > +++ b/include/uapi/linux/pci_regs.h > > @@ -1086,7 +1086,11 @@ > > > > /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */ > > #define PCI_DVSEC_HEADER1 0x4 /* Designated Vendor-Specific Header1 */ > > +#define PCI_DVSEC_HEADER1_VID(x) ((x) & 0xffff) > > +#define PCI_DVSEC_HEADER1_REV(x) (((x) >> 16) & 0xf) > > +#define PCI_DVSEC_HEADER1_LEN(x) (((x) >> 20) & 0xfff) > > #define PCI_DVSEC_HEADER2 0x8 /* Designated Vendor-Specific Header2 */ > > +#define PCI_DVSEC_HEADER2_ID(x) ((x) & 0xffff) > > Why does userspace need to have these defines? What userspace tool is > going to use these? This is pretty typical of other content in pci_regs.h. There's lots of existing content that may not be directly useful to userspace, but it's there because it's convenient to have definitions prescribed by the spec all together in one place. They probably shouldn't go in drivers/pci/pci.h because by definition this vendor-specific stuff will be used by vendor-specific drivers but not by the PCI core. I guess these *could* go in include/linux/pci.h, but that wouldn't be my first choice because there's nothing similar there. Bjorn ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-11-22 18:28 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-17 21:54 [PATCH 0/4] MFD: intel_pmt: Split OOBMSM from intel_pmt driver David E. Box 2021-06-17 21:54 ` [PATCH 1/4] PCI: Add #defines for accessing PCIE DVSEC fields David E. Box 2021-06-22 15:04 ` Bjorn Helgaas 2021-06-17 21:54 ` [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device David E. Box 2021-06-30 10:15 ` Lee Jones 2021-06-30 21:11 ` David E. Box 2021-07-01 8:01 ` Lee Jones 2021-07-01 8:39 ` Hans de Goede 2021-07-01 11:23 ` Lee Jones [not found] ` <CAHp75Vfn6GKSj6USUPEWiPdhWRYcJbirqhU6aOeB4gruekmocg@mail.gmail.com> 2021-07-01 12:06 ` Lee Jones [not found] ` <CAHp75VdmnRJKSBZ8dmU=7XsGOZ-wX6EpZhtC3X6JEE0mz-UJNg@mail.gmail.com> 2021-07-01 12:26 ` Lee Jones 2021-07-01 22:41 ` David E. Box 2021-07-01 9:43 ` Andy Shevchenko 2021-06-17 21:54 ` [PATCH 3/4] MFD: Intel Out of Band Management Services Module (OOBMSM) driver David E. Box 2021-06-30 10:17 ` Lee Jones 2021-06-17 21:54 ` [PATCH 4/4] MFD: intel-extended-cap: Add support for PCIe VSEC structures David E. Box 2021-11-20 23:17 [PATCH 0/4] Auxiliary bus driver support for Intel PCIe VSEC/DVSEC David E. Box 2021-11-20 23:17 ` [PATCH 1/4] PCI: Add #defines for accessing PCIe DVSEC fields David E. Box 2021-11-21 12:24 ` Greg KH 2021-11-21 15:48 ` David E. Box 2021-11-22 18:28 ` Bjorn Helgaas
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).