]> git.proxmox.com Git - mirror_ubuntu-bionic-kernel.git/commitdiff
platform/x86: Revert intel_pmc_ipc: Use MFD framework to create dependent devices
authorAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Mon, 13 Nov 2017 12:50:06 +0000 (14:50 +0200)
committerAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Mon, 13 Nov 2017 12:57:20 +0000 (14:57 +0200)
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 <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
drivers/platform/x86/intel_pmc_ipc.c

index e36144c337cda03e5ad73b1f80ff49685a197adb..e03fa31446ca83b107c220e438db53db38c44e80 100644 (file)
@@ -20,7 +20,6 @@
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/device.h>
-#include <linux/mfd/core.h>
 #include <linux/pm.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
@@ -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
 
 /*
 #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;
        }