| Message ID | e70046c8d49173840f8ee7218e8b2288301f8e67.1507414288.git.sathyanarayanan.kuppuswamy@linux.intel.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PMC/PUNIT IPC driver cleanup | expand |
On Sun, Oct 8, 2017 at 1:19 AM, <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > Currently, update_no_reboot_bit() function implemented in this driver > uses mutex_lock() to protect its register updates. But this function is > called with in atomic context in iTCO_wdt_start() and iTCO_wdt_stop() > functions in iTCO_wdt.c driver, which in turn causes "sleeping into > atomic context" issue. This patch fixes this issue by replacing the > mutex_lock() with spin_lock() to protect the GCR read/write/update APIs. > > Fixes: 9d855d4 ("platform/x86: intel_pmc_ipc: Fix iTCO_wdt GCS memory mapping failure") > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kupuswamy@linux.intel.com> > * Rebased this patch on top of Andy's review branch. Oh, what I asked you is to use vanilla kernel as a base. Please, be sure (you assured me, though it's not true) that it's applied against vanilla (or our fixes branch) and send just this one patch separately. No need to resend v5 right now.
Hi Andy, On 10/8/2017 11:38 AM, Andy Shevchenko wrote: > On Sun, Oct 8, 2017 at 1:19 AM, > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> Currently, update_no_reboot_bit() function implemented in this driver >> uses mutex_lock() to protect its register updates. But this function is >> called with in atomic context in iTCO_wdt_start() and iTCO_wdt_stop() >> functions in iTCO_wdt.c driver, which in turn causes "sleeping into >> atomic context" issue. This patch fixes this issue by replacing the >> mutex_lock() with spin_lock() to protect the GCR read/write/update APIs. >> >> Fixes: 9d855d4 ("platform/x86: intel_pmc_ipc: Fix iTCO_wdt GCS memory mapping failure") >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kupuswamy@linux.intel.com> >> * Rebased this patch on top of Andy's review branch. > Oh, what I asked you is to use vanilla kernel as a base. > Please, be sure (you assured me, though it's not true) I did test this patch on top of 4.14-rc3, but I have included another patch (""platform/x86: intel_pmc_ipc: Use devm_* calls in driver probe function") from your review branch before testing. I assumed that your will be pushing this patch along with devm_* fixes patch (since you already reviewed it), So I re-based them together on top of 4.14-rc3. Sorry, it looks like my assumption is incorrect. > that it's > applied against vanilla (or our fixes branch) and send just this one > patch separately. I will send it separately now. > > No need to resend v5 right now. >
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c index 751b121..e03fa314 100644 --- a/drivers/platform/x86/intel_pmc_ipc.c +++ b/drivers/platform/x86/intel_pmc_ipc.c @@ -33,6 +33,7 @@ #include <linux/suspend.h> #include <linux/acpi.h> #include <linux/io-64-nonatomic-lo-hi.h> +#include <linux/spinlock.h> #include <asm/intel_pmc_ipc.h> @@ -131,6 +132,7 @@ static struct intel_pmc_ipc_dev { /* gcr */ void __iomem *gcr_mem_base; bool has_gcr_regs; + spinlock_t gcr_lock; /* punit */ struct platform_device *punit_dev; @@ -225,17 +227,17 @@ int intel_pmc_gcr_read(u32 offset, u32 *data) { int ret; - mutex_lock(&ipclock); + spin_lock(&ipcdev.gcr_lock); ret = is_gcr_valid(offset); if (ret < 0) { - mutex_unlock(&ipclock); + spin_unlock(&ipcdev.gcr_lock); return ret; } *data = readl(ipcdev.gcr_mem_base + offset); - mutex_unlock(&ipclock); + spin_unlock(&ipcdev.gcr_lock); return 0; } @@ -255,17 +257,17 @@ int intel_pmc_gcr_write(u32 offset, u32 data) { int ret; - mutex_lock(&ipclock); + spin_lock(&ipcdev.gcr_lock); ret = is_gcr_valid(offset); if (ret < 0) { - mutex_unlock(&ipclock); + spin_unlock(&ipcdev.gcr_lock); return ret; } writel(data, ipcdev.gcr_mem_base + offset); - mutex_unlock(&ipclock); + spin_unlock(&ipcdev.gcr_lock); return 0; } @@ -287,7 +289,7 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val) u32 new_val; int ret = 0; - mutex_lock(&ipclock); + spin_lock(&ipcdev.gcr_lock); ret = is_gcr_valid(offset); if (ret < 0) @@ -309,7 +311,7 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val) } gcr_ipc_unlock: - mutex_unlock(&ipclock); + spin_unlock(&ipcdev.gcr_lock); return ret; } EXPORT_SYMBOL_GPL(intel_pmc_gcr_update); @@ -489,6 +491,8 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) pmc->irq_mode = IPC_TRIGGER_MODE_IRQ; + spin_lock_init(&ipcdev.gcr_lock); + ret = pcim_enable_device(pdev); if (ret) return ret; @@ -903,6 +907,7 @@ static int ipc_plat_probe(struct platform_device *pdev) ipcdev.dev = &pdev->dev; ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ; init_completion(&ipcdev.cmd_complete); + spin_lock_init(&ipcdev.gcr_lock); ipcdev.irq = platform_get_irq(pdev, 0); if (ipcdev.irq < 0) {