| Message ID | 1461236605-27245-1-git-send-email-rajneesh.bhardwaj@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, Apr 21, 2016 at 04:33:25PM +0530, Rajneesh Bhardwaj wrote: [...] Just some minor things I've spotted and one error is probably unchecked. [...] > diff --git a/arch/x86/include/asm/pmc_core.h b/arch/x86/include/asm/pmc_core.h > new file mode 100644 > index 0000000..3ea61bf > --- /dev/null > +++ b/arch/x86/include/asm/pmc_core.h > @@ -0,0 +1,53 @@ > +/* > + * Intel Core SOC Power Management Controller Header File > + * > + * Copyright (c) 2016, Intel Corporation. > + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + */ > + > +#ifndef PMC_CORE_H > +#define PMC_CORE_H > + > +/* Skylake Power Management Controller PCI Device ID */ > +#define PCI_DEVICE_ID_SKL_PMC 0x9d21 > +#define PMC_BASE_ADDR_OFFSET 0x48 > +#define PMC_SLP_S0_RESIDENCY_COUNTER 0x13c > +#define PMC_MMIO_REG_LEN 0x100 > +#define PMC_REG_BIT_WIDTH 0x20 > +#define SLP_S0_RESIDENCY_COUNTER_GRANULARITY 0x64 Inconsistent spaces. Maybe just use one space. [...] > diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c > new file mode 100644 > index 0000000..42cee87 > --- /dev/null > +++ b/drivers/platform/x86/intel_pmc_core.c > @@ -0,0 +1,200 @@ > +/* > + * Intel Core SOC Power Management Controller Driver > + * > + * Copyright (c) 2016, Intel Corporation. > + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + */ > + > +#include <linux/init.h> > +#include <linux/pci.h> > +#include <linux/device.h> > +#include <linux/debugfs.h> > +#include <linux/seq_file.h> > +#include <linux/module.h> > +#include <linux/io.h> > +#include <asm/cpu_device_id.h> > +#include <asm/pmc_core.h> > + > +static struct pmc_dev pmc; > + > +static const struct pci_device_id pmc_pci_ids[] = { > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_SKL_PMC), (kernel_ulong_t)NULL }, Minor thing here but maybe just use a simple 0 here instead of (kernel_ulong_t)NULL? In the other places as well. [...] > +static int pmc_core_dbgfs_register(struct pmc_dev *pmc) > +{ > + return 0; /* nothing to register */ > +} > + > +#endif /* CONFIG_DEBUG_FS */ > + > +static const struct x86_cpu_id intel_pmc_core_ids[] = { > + { X86_VENDOR_INTEL, 6, 0x4e, X86_FEATURE_MWAIT, > + (kernel_ulong_t)NULL}, /* SKL CPU*/ > + { X86_VENDOR_INTEL, 6, 0x5e, X86_FEATURE_MWAIT, > + (kernel_ulong_t)NULL}, /* SKL CPU*/ > + {} A space after CPU? SKL? I assume this means skylake so perhaps add the whole name. > +}; > +MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_ids); > + > +static int pmc_core_probe(struct pci_dev *dev, const struct pci_device_id *id) > +{ > + int err; > + const struct x86_cpu_id *cpu_id; > + > + cpu_id = x86_match_cpu(intel_pmc_core_ids); > + if (!cpu_id) > + return -EINVAL; > + > + err = pci_enable_device(dev); > + if (err) { > + dev_err(&dev->dev, "PMC Core: failed to enable Power Management Controller.\n"); > + goto exit; > + } > + > + pci_read_config_dword(dev, PMC_BASE_ADDR_OFFSET, &pmc.base_addr); Shouldn't the return value of this function be checked if an error occured? Also while we are at it maybe the label names could be improved. For example: err_disable_dev; err_return > + err = pmc_core_dbgfs_register(&pmc); > + if (err) { > + dev_err(&dev->dev, "PMC Core: debugfs register failed\n"); > + iounmap(pmc.regmap); > + goto disable; > + } > + > + pmc.feature_available = 1; > + return 0; > + > +disable: > + pci_disable_device(dev); > +exit: > + return err; > +} > + -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 21, 2016 at 03:47:29PM +0300, Giedrius Statkevi?ius wrote: > On Thu, Apr 21, 2016 at 04:33:25PM +0530, Rajneesh Bhardwaj wrote: > [...] > > Just some minor things I've spotted and one error is probably unchecked. > > [...] > > diff --git a/arch/x86/include/asm/pmc_core.h b/arch/x86/include/asm/pmc_core.h > > new file mode 100644 > > index 0000000..3ea61bf > > --- /dev/null > > +++ b/arch/x86/include/asm/pmc_core.h > > @@ -0,0 +1,53 @@ > > +/* > > + * Intel Core SOC Power Management Controller Header File > > + * > > + * Copyright (c) 2016, Intel Corporation. > > + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms and conditions of the GNU General Public License, > > + * version 2, as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope it will be useful, but WITHOUT > > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > > + * more details. > > + * > > + */ > > + > > +#ifndef PMC_CORE_H > > +#define PMC_CORE_H > > + > > +/* Skylake Power Management Controller PCI Device ID */ > > +#define PCI_DEVICE_ID_SKL_PMC 0x9d21 > > +#define PMC_BASE_ADDR_OFFSET 0x48 > > +#define PMC_SLP_S0_RESIDENCY_COUNTER 0x13c > > +#define PMC_MMIO_REG_LEN 0x100 > > +#define PMC_REG_BIT_WIDTH 0x20 > > +#define SLP_S0_RESIDENCY_COUNTER_GRANULARITY 0x64 > > Inconsistent spaces. Maybe just use one space. > > [...] > > > diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c > > new file mode 100644 > > index 0000000..42cee87 > > --- /dev/null > > +++ b/drivers/platform/x86/intel_pmc_core.c > > @@ -0,0 +1,200 @@ > > +/* > > + * Intel Core SOC Power Management Controller Driver > > + * > > + * Copyright (c) 2016, Intel Corporation. > > + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms and conditions of the GNU General Public License, > > + * version 2, as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope it will be useful, but WITHOUT > > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > > + * more details. > > + * > > + */ > > + > > +#include <linux/init.h> > > +#include <linux/pci.h> > > +#include <linux/device.h> > > +#include <linux/debugfs.h> > > +#include <linux/seq_file.h> > > +#include <linux/module.h> > > +#include <linux/io.h> > > +#include <asm/cpu_device_id.h> > > +#include <asm/pmc_core.h> > > + > > +static struct pmc_dev pmc; > > + > > +static const struct pci_device_id pmc_pci_ids[] = { > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_SKL_PMC), (kernel_ulong_t)NULL }, > > Minor thing here but maybe just use a simple 0 here instead of > (kernel_ulong_t)NULL? In the other places as well. > > [...] > Please provide the rational for a request for change. What do you think is wrong with the above? There is precedent in the kernel today for this usage. Is it wrong? Unnecessary? Subjectively ugly? > > +static int pmc_core_dbgfs_register(struct pmc_dev *pmc) > > +{ > > + return 0; /* nothing to register */ > > +} > > + > > +#endif /* CONFIG_DEBUG_FS */ > > + > > +static const struct x86_cpu_id intel_pmc_core_ids[] = { > > + { X86_VENDOR_INTEL, 6, 0x4e, X86_FEATURE_MWAIT, > > + (kernel_ulong_t)NULL}, /* SKL CPU*/ > > + { X86_VENDOR_INTEL, 6, 0x5e, X86_FEATURE_MWAIT, > > + (kernel_ulong_t)NULL}, /* SKL CPU*/ > > + {} > > A space after CPU? SKL? I assume this means skylake so perhaps add the whole > name. > The three letter CPU code is very common throughout the kernel of Intel CPUs. NHM,WSM,SNB,IVB,BYT,HSW,BDW,SKL, etc. Those are becoming more and more difficult to grep for though, so a full expansion now and then would certainly be nice :-) > > +}; > > +MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_ids); > > + > > +static int pmc_core_probe(struct pci_dev *dev, const struct pci_device_id *id) > > +{ > > + int err; > > + const struct x86_cpu_id *cpu_id; > > + > > + cpu_id = x86_match_cpu(intel_pmc_core_ids); > > + if (!cpu_id) > > + return -EINVAL; > > + > > + err = pci_enable_device(dev); > > + if (err) { > > + dev_err(&dev->dev, "PMC Core: failed to enable Power Management Controller.\n"); > > + goto exit; > > + } > > + > > + pci_read_config_dword(dev, PMC_BASE_ADDR_OFFSET, &pmc.base_addr); > > Shouldn't the return value of this function be checked if an error occured? Also Yes please. > while we are at it maybe the label names could be improved. For example: > err_disable_dev; err_return > This is more typical, although "out" or "err_out" if only used in the error case. However, if the label does nothing but return with an error code, be sure to be consistent in it's usage. Don't do "return -EINVAL" and "ret = -EINVAL; goto err_out;" in the same function. Thanks, > > + err = pmc_core_dbgfs_register(&pmc); > > + if (err) { > > + dev_err(&dev->dev, "PMC Core: debugfs register failed\n"); > > + iounmap(pmc.regmap); > > + goto disable; > > + } > > + > > + pmc.feature_available = 1; > > + return 0; > > + > > +disable: > > + pci_disable_device(dev); > > +exit: > > + return err; > > +} > > + >
Thanks for the review , will send the revised patch v2 with all review comments addressed so far. Regards Rajneesh >-----Original Message----- >From: Darren Hart [mailto:dvhart@infradead.org] >Sent: Monday, April 25, 2016 11:41 PM >To: Giedrius Statkevi?ius <giedrius.statkevicius@gmail.com> >Cc: Bhardwaj, Rajneesh <rajneesh.bhardwaj@intel.com>; platform-driver- >x86@vger.kernel.org; Somayaji, Vishwanath ><vishwanath.somayaji@intel.com> >Subject: Re: [PATCH] platform:x86: Add PMC Driver for Intel Core SOC > >On Thu, Apr 21, 2016 at 03:47:29PM +0300, Giedrius Statkevi?ius wrote: >> On Thu, Apr 21, 2016 at 04:33:25PM +0530, Rajneesh Bhardwaj wrote: >> [...] >> >> Just some minor things I've spotted and one error is probably unchecked. >> >> [...] >> > diff --git a/arch/x86/include/asm/pmc_core.h >> > b/arch/x86/include/asm/pmc_core.h new file mode 100644 index >> > 0000000..3ea61bf >> > --- /dev/null >> > +++ b/arch/x86/include/asm/pmc_core.h >> > @@ -0,0 +1,53 @@ >> > +/* >> > + * Intel Core SOC Power Management Controller Header File >> > + * >> > + * Copyright (c) 2016, Intel Corporation. >> > + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) >> > + * >> > + * This program is free software; you can redistribute it and/or >> > +modify it >> > + * under the terms and conditions of the GNU General Public >> > +License, >> > + * version 2, as published by the Free Software Foundation. >> > + * >> > + * This program is distributed in the hope it will be useful, but >> > +WITHOUT >> > + * ANY WARRANTY; without even the implied warranty of >> > +MERCHANTABILITY or >> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public >> > +License for >> > + * more details. >> > + * >> > + */ >> > + >> > +#ifndef PMC_CORE_H >> > +#define PMC_CORE_H >> > + >> > +/* Skylake Power Management Controller PCI Device ID */ >> > +#define PCI_DEVICE_ID_SKL_PMC 0x9d21 >> > +#define PMC_BASE_ADDR_OFFSET 0x48 >> > +#define PMC_SLP_S0_RESIDENCY_COUNTER 0x13c >> > +#define PMC_MMIO_REG_LEN 0x100 >> > +#define PMC_REG_BIT_WIDTH 0x20 >> > +#define SLP_S0_RESIDENCY_COUNTER_GRANULARITY 0x64 >> >> Inconsistent spaces. Maybe just use one space. >> >> [...] >> >> > diff --git a/drivers/platform/x86/intel_pmc_core.c >> > b/drivers/platform/x86/intel_pmc_core.c >> > new file mode 100644 >> > index 0000000..42cee87 >> > --- /dev/null >> > +++ b/drivers/platform/x86/intel_pmc_core.c >> > @@ -0,0 +1,200 @@ >> > +/* >> > + * Intel Core SOC Power Management Controller Driver >> > + * >> > + * Copyright (c) 2016, Intel Corporation. >> > + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) >> > + * >> > + * This program is free software; you can redistribute it and/or >> > +modify it >> > + * under the terms and conditions of the GNU General Public >> > +License, >> > + * version 2, as published by the Free Software Foundation. >> > + * >> > + * This program is distributed in the hope it will be useful, but >> > +WITHOUT >> > + * ANY WARRANTY; without even the implied warranty of >> > +MERCHANTABILITY or >> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public >> > +License for >> > + * more details. >> > + * >> > + */ >> > + >> > +#include <linux/init.h> >> > +#include <linux/pci.h> >> > +#include <linux/device.h> >> > +#include <linux/debugfs.h> >> > +#include <linux/seq_file.h> >> > +#include <linux/module.h> >> > +#include <linux/io.h> >> > +#include <asm/cpu_device_id.h> >> > +#include <asm/pmc_core.h> >> > + >> > +static struct pmc_dev pmc; >> > + >> > +static const struct pci_device_id pmc_pci_ids[] = { >> > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_SKL_PMC), >(kernel_ulong_t)NULL >> > +}, >> >> Minor thing here but maybe just use a simple 0 here instead of >> (kernel_ulong_t)NULL? In the other places as well. >> >> [...] >> > >Please provide the rational for a request for change. What do you think is >wrong with the above? There is precedent in the kernel today for this usage. >Is it wrong? Unnecessary? Subjectively ugly? > >> > +static int pmc_core_dbgfs_register(struct pmc_dev *pmc) { >> > + return 0; /* nothing to register */ } >> > + >> > +#endif /* CONFIG_DEBUG_FS */ >> > + >> > +static const struct x86_cpu_id intel_pmc_core_ids[] = { >> > + { X86_VENDOR_INTEL, 6, 0x4e, X86_FEATURE_MWAIT, >> > + (kernel_ulong_t)NULL}, /* SKL CPU*/ >> > + { X86_VENDOR_INTEL, 6, 0x5e, X86_FEATURE_MWAIT, >> > + (kernel_ulong_t)NULL}, /* SKL CPU*/ >> > + {} >> >> A space after CPU? SKL? I assume this means skylake so perhaps add the >> whole name. >> > >The three letter CPU code is very common throughout the kernel of Intel >CPUs. >NHM,WSM,SNB,IVB,BYT,HSW,BDW,SKL, etc. > >Those are becoming more and more difficult to grep for though, so a full >expansion now and then would certainly be nice :-) > >> > +}; >> > +MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_ids); >> > + >> > +static int pmc_core_probe(struct pci_dev *dev, const struct >> > +pci_device_id *id) { >> > + int err; >> > + const struct x86_cpu_id *cpu_id; >> > + >> > + cpu_id = x86_match_cpu(intel_pmc_core_ids); >> > + if (!cpu_id) >> > + return -EINVAL; >> > + >> > + err = pci_enable_device(dev); >> > + if (err) { >> > + dev_err(&dev->dev, "PMC Core: failed to enable Power >Management Controller.\n"); >> > + goto exit; >> > + } >> > + >> > + pci_read_config_dword(dev, PMC_BASE_ADDR_OFFSET, >&pmc.base_addr); >> >> Shouldn't the return value of this function be checked if an error >> occured? Also > >Yes please. > >> while we are at it maybe the label names could be improved. For example: >> err_disable_dev; err_return >> > >This is more typical, although "out" or "err_out" if only used in the error case. >However, if the label does nothing but return with an error code, be sure to >be consistent in it's usage. Don't do "return -EINVAL" and "ret = -EINVAL; goto >err_out;" in the same function. > > >Thanks, > >> > + err = pmc_core_dbgfs_register(&pmc); >> > + if (err) { >> > + dev_err(&dev->dev, "PMC Core: debugfs register failed\n"); >> > + iounmap(pmc.regmap); >> > + goto disable; >> > + } >> > + >> > + pmc.feature_available = 1; >> > + return 0; >> > + >> > +disable: >> > + pci_disable_device(dev); >> > +exit: >> > + return err; >> > +} >> > + >> > >-- >Darren Hart >Intel Open Source Technology Center
diff --git a/arch/x86/include/asm/pmc_core.h b/arch/x86/include/asm/pmc_core.h new file mode 100644 index 0000000..3ea61bf --- /dev/null +++ b/arch/x86/include/asm/pmc_core.h @@ -0,0 +1,53 @@ +/* + * Intel Core SOC Power Management Controller Header File + * + * Copyright (c) 2016, Intel Corporation. + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + */ + +#ifndef PMC_CORE_H +#define PMC_CORE_H + +/* Skylake Power Management Controller PCI Device ID */ +#define PCI_DEVICE_ID_SKL_PMC 0x9d21 +#define PMC_BASE_ADDR_OFFSET 0x48 +#define PMC_SLP_S0_RESIDENCY_COUNTER 0x13c +#define PMC_MMIO_REG_LEN 0x100 +#define PMC_REG_BIT_WIDTH 0x20 +#define SLP_S0_RESIDENCY_COUNTER_GRANULARITY 0x64 + +/** + * struct pmc_dev - pmc device structure + * @base_addr: comtains pmc base address + * @regmap: pointer to io-remapped memory location + * @dbgfs_dir: path to debug fs interface + * @feature_available: flag to indicate whether, + * the feature is available, + * on a particular platform or not. + * + * pmc_dev contains info about power management controller device. + */ +struct pmc_dev { + u32 base_addr; + void __iomem *regmap; +#if IS_ENABLED(CONFIG_DEBUG_FS) + struct dentry *dbgfs_dir; +#endif /* CONFIG_DEBUG_FS */ + int feature_available; +}; + +int intel_pmc_slp_s0_counter_read(u64 *data); + +#endif + +/* PMC_CORE_H */ diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ed2004b..5db364c 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -821,6 +821,16 @@ config INTEL_IPS functionality. If in doubt, say Y here; it will only load on supported platforms. +config INTEL_PMC_CORE + bool "Intel PMC Core driver" + depends on X86 && PCI + default y + ---help--- + This driver exposes the features provided by Power Management + Controller for Intel Core SOC. Intel Platform Controller Hub + for Intel Core SOCs provides access to Power Management Controller + registers via pci interface. + config INTEL_IMR bool "Intel Isolated Memory Region support" default n diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 448443c..9b11b40 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -69,3 +69,4 @@ obj-$(CONFIG_INTEL_PUNIT_IPC) += intel_punit_ipc.o obj-$(CONFIG_INTEL_TELEMETRY) += intel_telemetry_core.o \ intel_telemetry_pltdrv.o \ intel_telemetry_debugfs.o +obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c new file mode 100644 index 0000000..42cee87 --- /dev/null +++ b/drivers/platform/x86/intel_pmc_core.c @@ -0,0 +1,200 @@ +/* + * Intel Core SOC Power Management Controller Driver + * + * Copyright (c) 2016, Intel Corporation. + * Author: Rajneesh Bhardwaj (rajneesh.bhardwaj@intel.com) + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + */ + +#include <linux/init.h> +#include <linux/pci.h> +#include <linux/device.h> +#include <linux/debugfs.h> +#include <linux/seq_file.h> +#include <linux/module.h> +#include <linux/io.h> +#include <asm/cpu_device_id.h> +#include <asm/pmc_core.h> + +static struct pmc_dev pmc; + +static const struct pci_device_id pmc_pci_ids[] = { + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_SKL_PMC), (kernel_ulong_t)NULL }, + { 0, }, +}; +MODULE_DEVICE_TABLE(pci, pmc_pci_ids); + +/** + * intel_pmc_slp_s0_counter_read() - Read slp_s0 residency. + * @data: Out param that contains current slp_s0 count. + * + * This API currently supports Intel Skylake SOC and Sunrise + * point Platform Controller Hub. Future platform support + * should be added for platforms that support low power modes + * beyond Package C10 state. + * + * SLP_S0_RESIDENCY counter counts in 100 us granularity per + * step hence function populates the multiplied value in out + * parameter @data + * + * Return: an error code or 0 on success. + */ +int intel_pmc_slp_s0_counter_read(u64 *data) +{ + int ret = 0; + + if (pmc.feature_available) { + *data = readl(pmc.regmap + PMC_SLP_S0_RESIDENCY_COUNTER); + *data *= SLP_S0_RESIDENCY_COUNTER_GRANULARITY; + } else { + ret = -EACCES; + } + + return ret; +} +EXPORT_SYMBOL_GPL(intel_pmc_slp_s0_counter_read); + +#if IS_ENABLED(CONFIG_DEBUG_FS) + +static int pmc_core_dev_state_show(struct seq_file *s, void *unused) +{ + u64 counter_val; + int err; + + err = intel_pmc_slp_s0_counter_read(&counter_val); + if (err) { + counter_val = 0; + seq_printf(s, "%lld\n", counter_val); + } else { + seq_printf(s, "%lld\n", counter_val); + } + + return 0; +} + +static int pmc_core_dev_state_open(struct inode *inode, struct file *file) +{ + return single_open(file, pmc_core_dev_state_show, inode->i_private); +} + +static const struct file_operations pmc_core_dev_state_ops = { + .open = pmc_core_dev_state_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static void pmc_core_dbgfs_unregister(struct pmc_dev *pmc) +{ + debugfs_remove_recursive(pmc->dbgfs_dir); +} + +static int pmc_core_dbgfs_register(struct pmc_dev *pmc) +{ + struct dentry *dir, *file; + int ret = 0; + + dir = debugfs_create_dir("pmc_core", NULL); + if (!dir) + return -ENOMEM; + + pmc->dbgfs_dir = dir; + file = debugfs_create_file("slp_s0_residency_usec", S_IFREG | S_IRUGO, + dir, pmc, &pmc_core_dev_state_ops); + + if (!file) { + pmc_core_dbgfs_unregister(pmc); + ret = -ENODEV; + } + return ret; +} + +#else + +static int pmc_core_dbgfs_register(struct pmc_dev *pmc) +{ + return 0; /* nothing to register */ +} + +#endif /* CONFIG_DEBUG_FS */ + +static const struct x86_cpu_id intel_pmc_core_ids[] = { + { X86_VENDOR_INTEL, 6, 0x4e, X86_FEATURE_MWAIT, + (kernel_ulong_t)NULL}, /* SKL CPU*/ + { X86_VENDOR_INTEL, 6, 0x5e, X86_FEATURE_MWAIT, + (kernel_ulong_t)NULL}, /* SKL CPU*/ + {} +}; +MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_ids); + +static int pmc_core_probe(struct pci_dev *dev, const struct pci_device_id *id) +{ + int err; + const struct x86_cpu_id *cpu_id; + + cpu_id = x86_match_cpu(intel_pmc_core_ids); + if (!cpu_id) + return -EINVAL; + + err = pci_enable_device(dev); + if (err) { + dev_err(&dev->dev, "PMC Core: failed to enable Power Management Controller.\n"); + goto exit; + } + + pci_read_config_dword(dev, PMC_BASE_ADDR_OFFSET, &pmc.base_addr); + dev_dbg(&dev->dev, "PMC Core: PWRMBASE is 0x%x\n", pmc.base_addr); + + pmc.regmap = ioremap_nocache(pmc.base_addr, PMC_MMIO_REG_LEN); + if (!pmc.regmap) { + dev_err(&dev->dev, "PMC Core: ioremap failed\n"); + err = -ENOMEM; + goto disable; + } + + err = pmc_core_dbgfs_register(&pmc); + if (err) { + dev_err(&dev->dev, "PMC Core: debugfs register failed\n"); + iounmap(pmc.regmap); + goto disable; + } + + pmc.feature_available = 1; + return 0; + +disable: + pci_disable_device(dev); +exit: + return err; +} + +static void intel_pmc_core_remove(struct pci_dev *pdev) +{ +#if IS_ENABLED(CONFIG_DEBUG_FS) + pmc_core_dbgfs_unregister(&pmc); +#endif + pci_disable_device(pdev); + iounmap(pmc.regmap); +} + +static struct pci_driver intel_pmc_core_driver = { + .name = "intel_pmc_core", + .id_table = pmc_pci_ids, + .probe = pmc_core_probe, + .remove = intel_pmc_core_remove, +}; +module_pci_driver(intel_pmc_core_driver); + +MODULE_AUTHOR("Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>"); +MODULE_AUTHOR("Vishwanath Somayaji <vishwanath.somayaji@intel.com>"); +MODULE_DESCRIPTION("Intel CORE SOC Power Management Controller Interface"); +MODULE_LICENSE("GPL v2");