From 73ed298b0605a81d8cbf212df887f7b2fe65a61b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 13 Nov 2017 14:50:06 +0200 Subject: [PATCH] platform/x86: Revert intel_pmc_ipc: Use MFD framework to create dependent devices Heikki discovered a runtime issue with this patch. Taking into consideration we have no time to test any fix right now, revert the commit 43aaf4f03f063b12bcba2f8b800fdec85e2acc75. Reported-by: Heikki Krogerus Signed-off-by: Andy Shevchenko --- drivers/platform/x86/intel_pmc_ipc.c | 398 +++++++++++++++++---------- 1 file changed, 259 insertions(+), 139 deletions(-) diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c index e36144c337cd..e03fa31446ca 100644 --- a/drivers/platform/x86/intel_pmc_ipc.c +++ b/drivers/platform/x86/intel_pmc_ipc.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -89,7 +88,6 @@ #define PLAT_RESOURCE_ISP_IFACE_INDEX 5 #define PLAT_RESOURCE_GTD_DATA_INDEX 6 #define PLAT_RESOURCE_GTD_IFACE_INDEX 7 -#define PLAT_RESOURCE_MEM_MAX_INDEX 8 #define PLAT_RESOURCE_ACPI_IO_INDEX 0 /* @@ -108,6 +106,8 @@ #define TELEM_SSRAM_SIZE 240 #define TELEM_PMC_SSRAM_OFFSET 0x1B00 #define TELEM_PUNIT_SSRAM_OFFSET 0x1A00 +#define TCO_PMC_OFFSET 0x8 +#define TCO_PMC_SIZE 0x4 /* PMC register bit definitions */ @@ -124,10 +124,26 @@ static struct intel_pmc_ipc_dev { int cmd; struct completion cmd_complete; + /* The following PMC BARs share the same ACPI device with the IPC */ + resource_size_t acpi_io_base; + int acpi_io_size; + struct platform_device *tco_dev; + /* gcr */ void __iomem *gcr_mem_base; bool has_gcr_regs; spinlock_t gcr_lock; + + /* punit */ + struct platform_device *punit_dev; + + /* Telemetry */ + resource_size_t telem_pmc_ssram_base; + resource_size_t telem_punit_ssram_base; + int telem_pmc_ssram_size; + int telem_punit_ssram_size; + u8 telem_res_inval; + struct platform_device *telemetry_dev; } ipcdev; static char *ipc_err_sources[] = { @@ -492,7 +508,7 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) ret = devm_request_irq(&pdev->dev, pdev->irq, ioc, 0, "intel_pmc_ipc", pmc); if (ret) { - dev_err(&pdev->dev, "Failed to request IRQ\n"); + dev_err(&pdev->dev, "Failed to request irq\n"); return ret; } @@ -577,6 +593,44 @@ static const struct attribute_group intel_ipc_group = { .attrs = intel_ipc_attrs, }; +static struct resource punit_res_array[] = { + /* Punit BIOS */ + { + .flags = IORESOURCE_MEM, + }, + { + .flags = IORESOURCE_MEM, + }, + /* Punit ISP */ + { + .flags = IORESOURCE_MEM, + }, + { + .flags = IORESOURCE_MEM, + }, + /* Punit GTD */ + { + .flags = IORESOURCE_MEM, + }, + { + .flags = IORESOURCE_MEM, + }, +}; + +#define TCO_RESOURCE_ACPI_IO 0 +#define TCO_RESOURCE_SMI_EN_IO 1 +#define TCO_RESOURCE_GCR_MEM 2 +static struct resource tco_res[] = { + /* ACPI - TCO */ + { + .flags = IORESOURCE_IO, + }, + /* ACPI - SMI */ + { + .flags = IORESOURCE_IO, + }, +}; + static struct itco_wdt_platform_data tco_info = { .name = "Apollo Lake SoC", .version = 5, @@ -584,177 +638,234 @@ static struct itco_wdt_platform_data tco_info = { .update_no_reboot_bit = update_no_reboot_bit, }; -static int ipc_create_punit_device(struct platform_device *pdev) +#define TELEMETRY_RESOURCE_PUNIT_SSRAM 0 +#define TELEMETRY_RESOURCE_PMC_SSRAM 1 +static struct resource telemetry_res[] = { + /*Telemetry*/ + { + .flags = IORESOURCE_MEM, + }, + { + .flags = IORESOURCE_MEM, + }, +}; + +static int ipc_create_punit_device(void) { - struct resource punit_res[PLAT_RESOURCE_MEM_MAX_INDEX]; - struct mfd_cell punit_cell; - struct resource *res; - int mindex, pindex = 0; - - for (mindex = 0; mindex <= PLAT_RESOURCE_MEM_MAX_INDEX; mindex++) { - - res = platform_get_resource(pdev, IORESOURCE_MEM, mindex); - - switch (mindex) { - /* Get PUNIT resources */ - case PLAT_RESOURCE_BIOS_DATA_INDEX: - case PLAT_RESOURCE_BIOS_IFACE_INDEX: - /* BIOS resources are required, so return error if not - * available - */ - if (!res) { - dev_err(&pdev->dev, - "Failed to get PUNIT MEM resource %d\n", - pindex); - return -ENXIO; - } - case PLAT_RESOURCE_ISP_DATA_INDEX: - case PLAT_RESOURCE_ISP_IFACE_INDEX: - case PLAT_RESOURCE_GTD_DATA_INDEX: - case PLAT_RESOURCE_GTD_IFACE_INDEX: - /* if valid resource found, copy the resource to PUNIT - * resource - */ - if (res) - memcpy(&punit_res[pindex], res, sizeof(*res)); - punit_res[pindex].flags = IORESOURCE_MEM; - dev_dbg(&pdev->dev, "PUNIT memory res: %pR\n", - &punit_res[pindex]); - pindex++; - break; + struct platform_device *pdev; + const struct platform_device_info pdevinfo = { + .parent = ipcdev.dev, + .name = PUNIT_DEVICE_NAME, + .id = -1, + .res = punit_res_array, + .num_res = ARRAY_SIZE(punit_res_array), }; - } - /* Create PUNIT IPC MFD cell */ - punit_cell.name = PUNIT_DEVICE_NAME; - punit_cell.num_resources = ARRAY_SIZE(punit_res); - punit_cell.resources = punit_res; - punit_cell.ignore_resource_conflicts = 1; + pdev = platform_device_register_full(&pdevinfo); + if (IS_ERR(pdev)) + return PTR_ERR(pdev); - return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, - &punit_cell, 1, NULL, 0, NULL); + ipcdev.punit_dev = pdev; + + return 0; } -static int ipc_create_wdt_device(struct platform_device *pdev) +static int ipc_create_tco_device(void) { - static struct resource wdt_ipc_res[2]; + struct platform_device *pdev; struct resource *res; - static struct mfd_cell wdt_cell; - - /* If we have ACPI based watchdog use that instead, othewise create - * a MFD cell for iTCO watchdog - */ - if (acpi_has_watchdog()) - return 0; + const struct platform_device_info pdevinfo = { + .parent = ipcdev.dev, + .name = TCO_DEVICE_NAME, + .id = -1, + .res = tco_res, + .num_res = ARRAY_SIZE(tco_res), + .data = &tco_info, + .size_data = sizeof(tco_info), + }; - /* Get iTCO watchdog resources */ - res = platform_get_resource(pdev, IORESOURCE_IO, - PLAT_RESOURCE_ACPI_IO_INDEX); - if (!res) { - dev_err(&pdev->dev, "Failed to get WDT resource\n"); - return -ENXIO; - } + res = tco_res + TCO_RESOURCE_ACPI_IO; + res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET; + res->end = res->start + TCO_REGS_SIZE - 1; - wdt_ipc_res[0].start = res->start + TCO_BASE_OFFSET; - wdt_ipc_res[0].end = res->start + - TCO_BASE_OFFSET + TCO_REGS_SIZE - 1; - wdt_ipc_res[0].flags = IORESOURCE_IO; - wdt_ipc_res[1].start = res->start + SMI_EN_OFFSET; - wdt_ipc_res[1].end = res->start + - SMI_EN_OFFSET + SMI_EN_SIZE - 1; - wdt_ipc_res[1].flags = IORESOURCE_IO; + res = tco_res + TCO_RESOURCE_SMI_EN_IO; + res->start = ipcdev.acpi_io_base + SMI_EN_OFFSET; + res->end = res->start + SMI_EN_SIZE - 1; - dev_dbg(&pdev->dev, "watchdog res 0: %pR\n", &wdt_ipc_res[0]); - dev_dbg(&pdev->dev, "watchdog res 1: %pR\n", &wdt_ipc_res[1]); + pdev = platform_device_register_full(&pdevinfo); + if (IS_ERR(pdev)) + return PTR_ERR(pdev); - wdt_cell.name = TCO_DEVICE_NAME; - wdt_cell.platform_data = &tco_info; - wdt_cell.pdata_size = sizeof(tco_info); - wdt_cell.num_resources = ARRAY_SIZE(wdt_ipc_res); - wdt_cell.resources = wdt_ipc_res; - wdt_cell.ignore_resource_conflicts = 1; + ipcdev.tco_dev = pdev; - return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, - &wdt_cell, 1, NULL, 0, NULL); + return 0; } -static int ipc_create_telemetry_device(struct platform_device *pdev) +static int ipc_create_telemetry_device(void) { - struct resource telemetry_ipc_res[2]; - struct mfd_cell telemetry_cell; + struct platform_device *pdev; struct resource *res; + const struct platform_device_info pdevinfo = { + .parent = ipcdev.dev, + .name = TELEMETRY_DEVICE_NAME, + .id = -1, + .res = telemetry_res, + .num_res = ARRAY_SIZE(telemetry_res), + }; - /* Get telemetry resources */ - res = platform_get_resource(pdev, IORESOURCE_MEM, - PLAT_RESOURCE_TELEM_SSRAM_INDEX); - if (!res) { - dev_err(&pdev->dev, "Failed to get telemetry resource\n"); - return -ENXIO; - } + res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM; + res->start = ipcdev.telem_punit_ssram_base; + res->end = res->start + ipcdev.telem_punit_ssram_size - 1; - telemetry_ipc_res[0].start = res->start + TELEM_PUNIT_SSRAM_OFFSET; - telemetry_ipc_res[0].end = res->start + - TELEM_PUNIT_SSRAM_OFFSET + TELEM_SSRAM_SIZE - 1; - telemetry_ipc_res[0].flags = IORESOURCE_MEM; - telemetry_ipc_res[1].start = res->start + TELEM_PMC_SSRAM_OFFSET; - telemetry_ipc_res[1].end = res->start + - TELEM_PMC_SSRAM_OFFSET + TELEM_SSRAM_SIZE - 1; - telemetry_ipc_res[1].flags = IORESOURCE_MEM; + res = telemetry_res + TELEMETRY_RESOURCE_PMC_SSRAM; + res->start = ipcdev.telem_pmc_ssram_base; + res->end = res->start + ipcdev.telem_pmc_ssram_size - 1; - dev_dbg(&pdev->dev, "Telemetry res 0: %pR\n", &telemetry_ipc_res[0]); - dev_dbg(&pdev->dev, "Telemetry res 1: %pR\n", &telemetry_ipc_res[1]); + pdev = platform_device_register_full(&pdevinfo); + if (IS_ERR(pdev)) + return PTR_ERR(pdev); - telemetry_cell.name = TELEMETRY_DEVICE_NAME; - telemetry_cell.num_resources = ARRAY_SIZE(telemetry_ipc_res); - telemetry_cell.resources = telemetry_ipc_res; - telemetry_cell.ignore_resource_conflicts = 1; + ipcdev.telemetry_dev = pdev; - return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, - &telemetry_cell, 1, NULL, 0, NULL); + return 0; } -static int ipc_create_pmc_devices(struct platform_device *pdev) +static int ipc_create_pmc_devices(void) { int ret; - ret = ipc_create_punit_device(pdev); - if (ret < 0) - return ret; + /* If we have ACPI based watchdog use that instead */ + if (!acpi_has_watchdog()) { + ret = ipc_create_tco_device(); + if (ret) { + dev_err(ipcdev.dev, "Failed to add tco platform device\n"); + return ret; + } + } - ret = ipc_create_wdt_device(pdev); - if (ret < 0) - return ret; + ret = ipc_create_punit_device(); + if (ret) { + dev_err(ipcdev.dev, "Failed to add punit platform device\n"); + platform_device_unregister(ipcdev.tco_dev); + } - ret = ipc_create_telemetry_device(pdev); - if (ret < 0) - return ret; + if (!ipcdev.telem_res_inval) { + ret = ipc_create_telemetry_device(); + if (ret) + dev_warn(ipcdev.dev, + "Failed to add telemetry platform device\n"); + } - return 0; + return ret; } static int ipc_plat_get_res(struct platform_device *pdev) { - struct resource *res; + struct resource *res, *punit_res; void __iomem *addr; + int size; + + res = platform_get_resource(pdev, IORESOURCE_IO, + PLAT_RESOURCE_ACPI_IO_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get io resource\n"); + return -ENXIO; + } + size = resource_size(res); + ipcdev.acpi_io_base = res->start; + ipcdev.acpi_io_size = size; + dev_info(&pdev->dev, "io res: %pR\n", res); - /* Get IPC resources */ + punit_res = punit_res_array; + /* This is index 0 to cover BIOS data register */ res = platform_get_resource(pdev, IORESOURCE_MEM, - PLAT_RESOURCE_IPC_INDEX); + PLAT_RESOURCE_BIOS_DATA_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get res of punit BIOS data\n"); + return -ENXIO; + } + *punit_res = *res; + dev_info(&pdev->dev, "punit BIOS data res: %pR\n", res); + + /* This is index 1 to cover BIOS interface register */ + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_BIOS_IFACE_INDEX); if (!res) { - dev_err(&pdev->dev, "Failed to get IPC resources\n"); + dev_err(&pdev->dev, "Failed to get res of punit BIOS iface\n"); return -ENXIO; } + *++punit_res = *res; + dev_info(&pdev->dev, "punit BIOS interface res: %pR\n", res); + + /* This is index 2 to cover ISP data register, optional */ + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_ISP_DATA_INDEX); + ++punit_res; + if (res) { + *punit_res = *res; + dev_info(&pdev->dev, "punit ISP data res: %pR\n", res); + } + + /* This is index 3 to cover ISP interface register, optional */ + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_ISP_IFACE_INDEX); + ++punit_res; + if (res) { + *punit_res = *res; + dev_info(&pdev->dev, "punit ISP interface res: %pR\n", res); + } - res->end = res->start + - PLAT_RESOURCE_IPC_SIZE + PLAT_RESOURCE_GCR_SIZE - 1; + /* This is index 4 to cover GTD data register, optional */ + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_GTD_DATA_INDEX); + ++punit_res; + if (res) { + *punit_res = *res; + dev_info(&pdev->dev, "punit GTD data res: %pR\n", res); + } + + /* This is index 5 to cover GTD interface register, optional */ + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_GTD_IFACE_INDEX); + ++punit_res; + if (res) { + *punit_res = *res; + dev_info(&pdev->dev, "punit GTD interface res: %pR\n", res); + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_IPC_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get ipc resource\n"); + return -ENXIO; + } + size = PLAT_RESOURCE_IPC_SIZE + PLAT_RESOURCE_GCR_SIZE; + res->end = res->start + size - 1; addr = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(addr)) return PTR_ERR(addr); ipcdev.ipc_base = addr; + ipcdev.gcr_mem_base = addr + PLAT_RESOURCE_GCR_OFFSET; - dev_dbg(&pdev->dev, "PMC IPC resource %pR\n", res); + dev_info(&pdev->dev, "ipc res: %pR\n", res); + + ipcdev.telem_res_inval = 0; + res = platform_get_resource(pdev, IORESOURCE_MEM, + PLAT_RESOURCE_TELEM_SSRAM_INDEX); + if (!res) { + dev_err(&pdev->dev, "Failed to get telemetry ssram resource\n"); + ipcdev.telem_res_inval = 1; + } else { + ipcdev.telem_punit_ssram_base = res->start + + TELEM_PUNIT_SSRAM_OFFSET; + ipcdev.telem_punit_ssram_size = TELEM_SSRAM_SIZE; + ipcdev.telem_pmc_ssram_base = res->start + + TELEM_PMC_SSRAM_OFFSET; + ipcdev.telem_pmc_ssram_size = TELEM_SSRAM_SIZE; + dev_info(&pdev->dev, "telemetry ssram res: %pR\n", res); + } return 0; } @@ -800,7 +911,7 @@ static int ipc_plat_probe(struct platform_device *pdev) ipcdev.irq = platform_get_irq(pdev, 0); if (ipcdev.irq < 0) { - dev_err(&pdev->dev, "Failed to get IRQ\n"); + dev_err(&pdev->dev, "Failed to get irq\n"); return -EINVAL; } @@ -810,38 +921,47 @@ static int ipc_plat_probe(struct platform_device *pdev) return ret; } - ret = ipc_create_pmc_devices(pdev); + ret = ipc_create_pmc_devices(); if (ret) { - dev_err(&pdev->dev, "Failed to create PMC devices\n"); + dev_err(&pdev->dev, "Failed to create pmc devices\n"); return ret; } - ret = devm_request_irq(&pdev->dev, ipcdev.irq, ioc, IRQF_NO_SUSPEND, - "intel_pmc_ipc", &ipcdev); - if (ret) { - dev_err(&pdev->dev, "Failed to request IRQ\n"); - return ret; + if (devm_request_irq(&pdev->dev, ipcdev.irq, ioc, IRQF_NO_SUSPEND, + "intel_pmc_ipc", &ipcdev)) { + dev_err(&pdev->dev, "Failed to request irq\n"); + ret = -EBUSY; + goto err_irq; } ret = sysfs_create_group(&pdev->dev.kobj, &intel_ipc_group); if (ret) { dev_err(&pdev->dev, "Failed to create sysfs group %d\n", ret); - devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev); - return ret; + goto err_sys; } ipcdev.has_gcr_regs = true; return 0; +err_sys: + devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev); +err_irq: + platform_device_unregister(ipcdev.tco_dev); + platform_device_unregister(ipcdev.punit_dev); + platform_device_unregister(ipcdev.telemetry_dev); + + return ret; } static int ipc_plat_remove(struct platform_device *pdev) { sysfs_remove_group(&pdev->dev.kobj, &intel_ipc_group); devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev); + platform_device_unregister(ipcdev.tco_dev); + platform_device_unregister(ipcdev.punit_dev); + platform_device_unregister(ipcdev.telemetry_dev); ipcdev.dev = NULL; - return 0; } @@ -860,12 +980,12 @@ static int __init intel_pmc_ipc_init(void) ret = platform_driver_register(&ipc_plat_driver); if (ret) { - pr_err("Failed to register PMC IPC platform driver\n"); + pr_err("Failed to register PMC ipc platform driver\n"); return ret; } ret = pci_register_driver(&ipc_pci_driver); if (ret) { - pr_err("Failed to register PMC IPC PCI driver\n"); + pr_err("Failed to register PMC ipc pci driver\n"); platform_driver_unregister(&ipc_plat_driver); return ret; } -- 2.20.1