Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1726362 > unrolled thread
| Started by | sathyanarayanan.kuppuswamy@linux.intel.com |
|---|---|
| First post | 2017-09-05 07:40 +0200 |
| Last post | 2017-09-05 07:50 +0200 |
| Articles | 2 — 1 participant |
Back to article view | Back to linux.kernel
[RFC v3 0/7] PMC/PUNIT IPC driver cleanup sathyanarayanan.kuppuswamy@linux.intel.com - 2017-09-05 07:40 +0200
[RFC v3 3/7] platform/x86: intel_pmc_ipc: Use regmap calls for GCR updates sathyanarayanan.kuppuswamy@linux.intel.com - 2017-09-05 07:50 +0200
| From | sathyanarayanan.kuppuswamy@linux.intel.com |
|---|---|
| Date | 2017-09-05 07:40 +0200 |
| Subject | [RFC v3 0/7] PMC/PUNIT IPC driver cleanup |
| Message-ID | <umeKB-73c-3@gated-at.bofh.it> |
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Hi All,
Currently intel_pmc_ipc.c, intel_punit_ipc.c, intel_scu_ipc.c drivers implements the same IPC features.
This code duplication could be avoided if we implement the IPC driver as a generic library and let custom
device drivers use API provided by generic driver. This patchset mainly addresses this issue.
Along with above code duplication issue, This patchset also addresses following issues in intel_pmc_ipc and
intel_punit_ipc drivers.
1. Intel_pmc_ipc.c driver does not use any resource managed (devm_*) calls.
2. In Intel_pmc_ipc.c driver, dependent devices like PUNIT, Telemetry and iTCO are created manually and uses lot of redundant buffer code.
3. Global variable is used to store the IPC device structure and it is used across all functions in intel_pmc_ipc.c and intel_punit_ipc.c.
More info on Intel IPC device library:
-------------------------------------
A generic Intel IPC class driver has been implemented and all common IPC helper functions has been moved to this driver. It exposes APIs to create IPC device channel, send raw IPC command and simple IPC commands. It also creates device attribute to send IPC command from user space.
API for creating a new IPC channel device is,
struct intel_ipc_dev *devm_intel_ipc_dev_create(struct device *dev, const char *devname, struct intel_ipc_dev_cfg *cfg, struct intel_ipc_dev_ops *ops)
The IPC channel drivers (PUNIT/PMC/SCU) when creating a new device can configure their device params like register mapping, irq, irq-mode, channel type,etc using intel_ipc_dev_cfg and intel_ipc_dev_ops arguments. After a new IPC channel device is created, IPC users can use the generic APIs to make IPC calls.
For example, after using this new model, IPC call to PMC device will look like,
pmc_ipc_dev = intel_ipc_dev_get(INTEL_PMC_IPC_DEV);
ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, (u32 *)ipc_in, 1, NULL, 0, 0, 0);
I am still testing the driver in different products. But posted it to get some early comments. I also welcome any PMC/PUNIT driver users to check these patches in their product.
Changes since v2:
* Refactored intel_scu_ipc.c to use generic IPC device APIs.
* Fixed intel_pmc_ipc.c to use pcim_* device managed functions.
Changes since v1:
* Merged devm_* changes in pmc_plat_probe and pmc_pci_probe functions into a
single patch.
* Addressed Andy's comment about keeping the library generic by not implementing
the low level reg access calls in intel_ipc_dev.c. This version will start using
the regmap pointer provided by channel drivers instead of fixed memory map.
* Removed custom IPC APIs in intel_pmc_ipc.c and intel_punit_ipc.c.
* Cleaned up IPC driver users to use APIs provided by generic library (intel_ipc_dev.c).
Kuppuswamy Sathyanarayanan (7):
platform/x86: intel_pmc_ipc: Use devm_* calls in driver probe function
platform/x86: intel_pmc_ipc: Use MFD framework to create dependent
devices
platform/x86: intel_pmc_ipc: Use regmap calls for GCR updates
platform: x86: Add generic Intel IPC driver
platform/x86: intel_punit_ipc: Use generic intel ipc device calls
platform/x86: intel_pmc_ipc: Use generic Intel IPC device calls
platform/x86: intel_scu_ipc: Use generic Intel IPC device calls
arch/x86/include/asm/intel_pmc_ipc.h | 37 +-
arch/x86/include/asm/intel_punit_ipc.h | 125 ++--
arch/x86/include/asm/intel_scu_ipc.h | 23 +-
arch/x86/platform/intel-mid/intel-mid.c | 12 +-
drivers/mfd/intel_soc_pmic_bxtwc.c | 18 +-
drivers/platform/x86/Kconfig | 11 +
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel_ipc_dev.c | 539 ++++++++++++++
drivers/platform/x86/intel_pmc_ipc.c | 948 +++++++++---------------
drivers/platform/x86/intel_punit_ipc.c | 297 +++-----
drivers/platform/x86/intel_scu_ipc.c | 483 +++++-------
drivers/platform/x86/intel_telemetry_pltdrv.c | 210 +++---
drivers/rtc/rtc-mrst.c | 15 +-
drivers/watchdog/intel-mid_wdt.c | 12 +-
drivers/watchdog/intel_scu_watchdog.c | 17 +-
include/linux/mfd/intel_soc_pmic.h | 2 +
include/linux/platform_data/x86/intel_ipc_dev.h | 187 +++++
17 files changed, 1617 insertions(+), 1320 deletions(-)
create mode 100644 drivers/platform/x86/intel_ipc_dev.c
create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
--
2.7.4
[toc] | [next] | [standalone]
| From | sathyanarayanan.kuppuswamy@linux.intel.com |
|---|---|
| Date | 2017-09-05 07:50 +0200 |
| Subject | [RFC v3 3/7] platform/x86: intel_pmc_ipc: Use regmap calls for GCR updates |
| Message-ID | <umeUh-76p-1@gated-at.bofh.it> |
| In reply to | #1726362 |
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 refactoring the
current GCR read/write/update functions with regmap APIs.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_pmc_ipc.c | 115 ++++++++++++-----------------------
2 files changed, 40 insertions(+), 76 deletions(-)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b8795..45f4e79 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1054,6 +1054,7 @@ config PVPANIC
config INTEL_PMC_IPC
tristate "Intel PMC IPC Driver"
depends on ACPI
+ select REGMAP_MMIO
---help---
This driver provides support for PMC control on some Intel platforms.
The PMC is an ARC processor which defines IPC commands for communication
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 021dcf6..40a25f8 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -31,9 +31,11 @@
#include <linux/atomic.h>
#include <linux/notifier.h>
#include <linux/suspend.h>
+#include <linux/spinlock.h>
#include <linux/acpi.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/mfd/core.h>
+#include <linux/regmap.h>
#include <asm/intel_pmc_ipc.h>
@@ -125,7 +127,7 @@ static struct intel_pmc_ipc_dev {
/* gcr */
void __iomem *gcr_mem_base;
- bool has_gcr_regs;
+ struct regmap *gcr_regs;
/* Telemetry */
u8 telem_res_inval;
@@ -150,6 +152,14 @@ static char *ipc_err_sources[] = {
"Unsigned kernel",
};
+static struct regmap_config gcr_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .fast_io = true,
+ .max_register = PLAT_RESOURCE_GCR_SIZE,
+};
+
/* Prevent concurrent calls to the PMC */
static DEFINE_MUTEX(ipclock);
@@ -183,21 +193,6 @@ static inline u32 ipc_data_readl(u32 offset)
return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
}
-static inline u64 gcr_data_readq(u32 offset)
-{
- return readq(ipcdev.gcr_mem_base + offset);
-}
-
-static inline int is_gcr_valid(u32 offset)
-{
- if (!ipcdev.has_gcr_regs)
- return -EACCES;
-
- if (offset > PLAT_RESOURCE_GCR_SIZE)
- return -EINVAL;
-
- return 0;
-}
/**
* intel_pmc_gcr_read() - Read PMC GCR register
@@ -210,21 +205,10 @@ static inline int is_gcr_valid(u32 offset)
*/
int intel_pmc_gcr_read(u32 offset, u32 *data)
{
- int ret;
-
- mutex_lock(&ipclock);
-
- ret = is_gcr_valid(offset);
- if (ret < 0) {
- mutex_unlock(&ipclock);
- return ret;
- }
-
- *data = readl(ipcdev.gcr_mem_base + offset);
-
- mutex_unlock(&ipclock);
+ if (!ipcdev.gcr_regs)
+ return -EACCES;
- return 0;
+ return regmap_read(ipcdev.gcr_regs, offset, data);
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_read);
@@ -240,21 +224,10 @@ EXPORT_SYMBOL_GPL(intel_pmc_gcr_read);
*/
int intel_pmc_gcr_write(u32 offset, u32 data)
{
- int ret;
-
- mutex_lock(&ipclock);
-
- ret = is_gcr_valid(offset);
- if (ret < 0) {
- mutex_unlock(&ipclock);
- return ret;
- }
-
- writel(data, ipcdev.gcr_mem_base + offset);
-
- mutex_unlock(&ipclock);
+ if (!ipcdev.gcr_regs)
+ return -EACCES;
- return 0;
+ return regmap_write(ipcdev.gcr_regs, offset, data);
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_write);
@@ -271,33 +244,10 @@ EXPORT_SYMBOL_GPL(intel_pmc_gcr_write);
*/
int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
{
- u32 new_val;
- int ret = 0;
-
- mutex_lock(&ipclock);
-
- ret = is_gcr_valid(offset);
- if (ret < 0)
- goto gcr_ipc_unlock;
-
- new_val = readl(ipcdev.gcr_mem_base + offset);
-
- new_val &= ~mask;
- new_val |= val & mask;
-
- writel(new_val, ipcdev.gcr_mem_base + offset);
-
- new_val = readl(ipcdev.gcr_mem_base + offset);
-
- /* check whether the bit update is successful */
- if ((new_val & mask) != (val & mask)) {
- ret = -EIO;
- goto gcr_ipc_unlock;
- }
+ if (!ipcdev.gcr_regs)
+ return -EACCES;
-gcr_ipc_unlock:
- mutex_unlock(&ipclock);
- return ret;
+ return regmap_update_bits(ipcdev.gcr_regs, offset, mask, val);
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_update);
@@ -776,16 +726,24 @@ static int ipc_plat_get_res(struct platform_device *pdev)
int intel_pmc_s0ix_counter_read(u64 *data)
{
u64 deep, shlw;
+ int ret;
- if (!ipcdev.has_gcr_regs)
+ if (!ipcdev.gcr_regs)
return -EACCES;
- deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_REG);
- shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_REG);
+ ret = regmap_bulk_read(ipcdev.gcr_regs, PMC_GCR_TELEM_DEEP_S0IX_REG,
+ &deep, 2);
+ if (ret)
+ return ret;
+
+ ret = regmap_bulk_read(ipcdev.gcr_regs, PMC_GCR_TELEM_SHLW_S0IX_REG,
+ &shlw, 2);
+ if (ret)
+ return ret;
*data = S0IX_RESIDENCY_IN_USECS(deep, shlw);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(intel_pmc_s0ix_counter_read);
@@ -817,6 +775,13 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
+ ipcdev.gcr_regs = devm_regmap_init_mmio_clk(ipcdev.dev, NULL,
+ ipcdev.gcr_mem_base, &gcr_regmap_config);
+ if (IS_ERR(ipcdev.gcr_regs)) {
+ dev_err(ipcdev.dev, "gcr_regs regmap init failed\n");
+ return PTR_ERR(ipcdev.gcr_regs);;
+ }
+
ret = ipc_create_pmc_devices(pdev);
if (ret) {
dev_err(&pdev->dev, "Failed to create pmc devices\n");
@@ -836,8 +801,6 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
- ipcdev.has_gcr_regs = true;
-
return 0;
}
--
2.7.4
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web