| Message ID | c2932168ae25c83f2b2ee6a570274cd0670ba543.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sat, Oct 7, 2017 at 5:33 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") I think it need to be a one line. Does it apply on top of v4.14-rc3 ? Btw, this patch should be first in the series.
Hi, On 10/7/2017 9:13 AM, Andy Shevchenko wrote: > On Sat, Oct 7, 2017 at 5:33 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") > I think it need to be a one line. Fixed it in v5 version. But it crosses 80 char limit. I am not sure whether its alright. > > Does it apply on top of v4.14-rc3 ? Yes. Just checked it now. It applies cleanly on top of v4.14-rc3. But v5 patch set is re-based on top of your review branch. So it has dependency on a patch ( "platform/x86: intel_pmc_ipc: Use devm_* calls in driver probe function") in your review branch. > Btw, this patch should be first in the series. Fixed it in v5 version. > >
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c index c85351e..c68f6a4 100644 --- a/drivers/platform/x86/intel_pmc_ipc.c +++ b/drivers/platform/x86/intel_pmc_ipc.c @@ -34,6 +34,7 @@ #include <linux/acpi.h> #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/mfd/core.h> +#include <linux/spinlock.h> #include <asm/intel_pmc_ipc.h> @@ -126,6 +127,7 @@ static struct intel_pmc_ipc_dev { /* gcr */ void __iomem *gcr_mem_base; bool has_gcr_regs; + spinlock_t gcr_lock; } ipcdev; static char *ipc_err_sources[] = { @@ -209,17 +211,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; } @@ -239,17 +241,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; } @@ -271,7 +273,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) @@ -293,7 +295,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); @@ -471,6 +473,8 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (pmc->dev) return -EBUSY; + spin_lock_init(&ipcdev.gcr_lock); + pmc->irq_mode = IPC_TRIGGER_MODE_IRQ; ret = pcim_enable_device(pdev); @@ -794,6 +798,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) {