Re: [PATCH v7] platform:x86: Add PMC Driver for Intel Core SoC

[Andy Shevchenko]

On Thu, 2016-05-26 at 14:41 +0530, Rajneesh Bhardwaj wrote:
> This patch adds the Power Management Controller driver as a PCI driver
> for Intel Core SoC architecture.
> 
> This driver can utilize debugging capabilities and supported features
> as exposed by the Power Management Controller.
> 
> Please refer to the below specification for more details on PMC
> features.
> http://www.intel.in/content/www/in/en/chipsets/100-series-chipset-data
> sheet-vol-2.html
> 
> The current version of this driver exposes SLP_S0_RESIDENCY counter.
> This counter can be used for detecting fragile SLP_S0 signal related
> failures and take corrective actions when PCH SLP_S0 signal is not
> asserted after kernel freeze as part of suspend to idle flow
> (echo freeze > /sys/power/state).
> 
> Intel Platform Controller Hub (PCH) asserts SLP_S0 signal when it
> detects favorable conditions to enter its low power mode. As a
> pre-requisite the SoC should be in deepest possible Package C-State
> and devices should be in low power mode. For example, on Skylake SoC
> the deepest Package C-State is Package C10 or PC10. Suspend to idle
> flow generally leads to PC10 state but PC10 state may not be
> sufficient
> for realizing the platform wide power potential which SLP_S0 signal
> assertion can provide.
> 
> SLP_S0 signal is often connected to the Embedded Controller (EC) and
> the
> Power Management IC (PMIC) for other platform power management related
> optimizations.
> 
> In general, SLP_S0 assertion == PC10 + PCH low power mode + ModPhy
> Lanes
> power gated + PLL Idle.
> 
> As part of this driver, a mechanism to read the SLP_S0_RESIDENCY is
> exposed
> as an API and also debugfs features are added to indicate SLP_S0
> signal
> assertion residency in microseconds.
> 
> echo freeze > /sys/power/state
> wake the system
> cat /sys/kernel/debug/pmc_core/slp_s0_residency_usec
> 
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx>
> Signed-off-by: Vishwanath Somayaji <vishwanath.somayaji@xxxxxxxxx>

Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> ---
> Changes in v7:
> Â* Using builtin_pci_driver instead of device_initcall to fix build
> warning
> Â* Deleted .remove line from struct intel_pmc_core_driver
> 
> Changes in v6:
> Â* Removed module specific code:
> 	- removed module.h, .remove callback.
> 	- replaced module_pci_driver with device_initcall.
> 	- removed MODULE_*
> Â* Address style related review comments.
> Â* Updated Authors, MAINTAINERS.
> Â* Removed unused #define from intel_pmc_core.h:
> 	- SPT_PMC_REG_BIT_WIDTH
> 
> Changes in v5:
> Â* Addressed generic review comments for v4.
> Â* Added generic read function "pmc_core_reg_read".
> Â* Split the header file into two:
> 	- arch/x86/include/asm/pmc_core.h => exposes API.
> 	- driver/platform/x86/intel_pmc_core.h => used by the driver.
> Â* Updated signature for intel_pmc_slp_s0_counter_read as slp_s0
> ÂÂÂcounter is a 32 bit counter.
> Â* Updated SOB, Authors and MAINTAINERS.
> Â* Using managed APIs:
> 	- pcim_enable_device
> 	- devm_ioremap_nocache
> Â* Updated probe and remove code.
> 
> Changes in v4:
> Â* Moved the header file to drivers/platform/x86 directory.
> Â* Updated the same in MAINTAINERS.
> Â* Removed 'default y' option from Kconfig.
> Â* Introduced static inline dummy functions for debugfs register
> ÂÂÂand unregister case when CONFIG_DEBUG_FS is not set. Removed
> ÂÂÂ#if IS_ENABLED(CONFIG_DEBUG_FS) check from .probe and .remove
> calls.
> Â* Add CC to LKML (linux-kernel@xxxxxxxxxxxxxxx)
> Â* Earlier review comments till v3 are available at:
> ÂÂÂ- http://www.spinics.net/lists/platform-driver-x86/msg08790.html
> ÂÂÂ- http://www.spinics.net/lists/platform-driver-x86/msg08759.html
> ÂÂÂ- http://www.spinics.net/lists/platform-driver-x86/msg08742.html
> 
> Changes in v3:
> Â* Updated the commit message, added reference to the chipset
> datasheet.
> Â* Renamed the header file to intel_pmc_core.h for consistency.
> Â* Added All rights reserved notification after the Copyright message.
> Â* Improved variable names in the header file. Added SPT prefix.
> Â* Fixed kernel-doc related whitespace and comma issues for struct
> pmc_dev.
> Â* Changed feature_available to bool has_slp_s0_res.
> Â* Enhanced the Kconfig description as per the review suggestions.
> Â* Added error propagating logic to pmc_core_dev_state_show.
> Â* Streamlined intel_pmc_slp_s0_counter_read as per the review
> comments.
> Â* Streamlined the use of #if IS_ENABLED(CONFIG_DEBUG_FS) while
> registering
> ÂÂÂdebugfs in the probe call.
> Â* Removed the *duplicate definition* of pmc_core_debugfs_register.
> Â* Added MAINTAINERS related changes.
> Â* Checkpatch warning due to long URL name in the commit message.
> 
> Changes in v2:
> Â* Fixed inconsistent spaces in the header file.
> Â* Updated commit message.
> Â* Enhanced acronym SKL CPU to Skylake CPUID Signature.
> Â* Put error check on pci_read_config_dword in probe function.
> Â* Changed goto label (disable) to disable_pci.
> Â* Changed x86_match_cpu error handling for consistency.
> 
> ÂMAINTAINERSÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂÂ8 ++
> Âarch/x86/include/asm/pmc_core.hÂÂÂÂÂÂÂ|ÂÂ27 +++++
> Âdrivers/platform/x86/KconfigÂÂÂÂÂÂÂÂÂÂ|ÂÂ12 ++
> Âdrivers/platform/x86/MakefileÂÂÂÂÂÂÂÂÂ|ÂÂÂ1 +
> Âdrivers/platform/x86/intel_pmc_core.c | 200
> ++++++++++++++++++++++++++++++++++
> Âdrivers/platform/x86/intel_pmc_core.h |ÂÂ51 +++++++++
> Â6 files changed, 299 insertions(+)
> Âcreate mode 100644 arch/x86/include/asm/pmc_core.h
> Âcreate mode 100644 drivers/platform/x86/intel_pmc_core.c
> Âcreate mode 100644 drivers/platform/x86/intel_pmc_core.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3302006..db18359 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6096,6 +6096,14 @@ S:	Maintained
> ÂF:	arch/x86/include/asm/intel_telemetry.h
> ÂF:	drivers/platform/x86/intel_telemetry*
> Â
> +INTEL PMC CORE DRIVER
> +M:	Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx>
> +M:	Vishwanath Somayaji <vishwanath.somayaji@xxxxxxxxx>
> +L:	platform-driver-x86@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	arch/x86/include/asm/pmc_core.h
> +F:	drivers/platform/x86/intel_pmc_core*
> +
> ÂIOC3 ETHERNET DRIVER
> ÂM:	Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> ÂL:	linux-mips@xxxxxxxxxxxxxx
> diff --git a/arch/x86/include/asm/pmc_core.h
> b/arch/x86/include/asm/pmc_core.h
> new file mode 100644
> index 0000000..d4855f1
> --- /dev/null
> +++ b/arch/x86/include/asm/pmc_core.h
> @@ -0,0 +1,27 @@
> +/*
> + * Intel Core SoC Power Management Controller Header File
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + * All Rights Reserved.
> + *
> + * Authors: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx>
> + *ÂÂÂÂÂÂÂÂÂÂVishwanath Somayaji <vishwanath.somayaji@xxxxxxxxx>
> + *
> + * 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 _ASM_PMC_CORE_H
> +#define _ASM_PMC_CORE_H
> +
> +/* API to read SLP_S0_RESIDENCY counter */
> +int intel_pmc_slp_s0_counter_read(u32 *data);
> +
> +#endif /* _ASM_PMC_CORE_H */
> diff --git a/drivers/platform/x86/Kconfig
> b/drivers/platform/x86/Kconfig
> index ed2004b..c06bb85 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -846,6 +846,18 @@ config INTEL_IMR
> Â
> Â	ÂÂIf you are running on a Galileo/Quark say Y here.
> Â
> +config INTEL_PMC_CORE
> +	bool "Intel PMC Core driver"
> +	depends on X86 && PCI
> +	---help---
> +	ÂÂThe Intel Platform Controller Hub for Intel Core SoCs
> provides access
> +	ÂÂto Power Management Controller registers via a PCI
> interface. This
> +	ÂÂdriver can utilize debugging capabilities and supported
> features as
> +	ÂÂexposed by the Power Management Controller.
> +
> +	ÂÂSupported features:
> +		- SLP_S0_RESIDENCY counter.
> +
> Âconfig IBM_RTL
> Â	tristate "Device driver to enable PRTL support"
> Â	depends on X86 && PCI
> 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..2776bec
> --- /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.
> + * All Rights Reserved.
> + *
> + * Authors: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx>
> + *ÂÂÂÂÂÂÂÂÂÂVishwanath Somayaji <vishwanath.somayaji@xxxxxxxxx>
> + *
> + * 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/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/seq_file.h>
> +
> +#include <asm/cpu_device_id.h>
> +#include <asm/pmc_core.h>
> +
> +#include "intel_pmc_core.h"
> +
> +static struct pmc_dev pmc;
> +
> +static const struct pci_device_id pmc_pci_ids[] = {
> +	{ PCI_VDEVICE(INTEL, SPT_PMC_PCI_DEVICE_ID),
> (kernel_ulong_t)NULL },
> +	{ 0, },
> +};
> +
> +static inline u32 pmc_core_reg_read(struct pmc_dev *pmcdev, int
> reg_offset)
> +{
> +	return readl(pmcdev->regbase + reg_offset);
> +}
> +
> +static inline u32 pmc_core_adjust_slp_s0_step(u32 value)
> +{
> +	return value * SPT_PMC_SLP_S0_RES_COUNTER_STEP;
> +}
> +
> +/**
> + * 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(u32 *data)
> +{
> +	struct pmc_dev *pmcdev = &pmc;
> +	u32 value;
> +
> +	if (!pmcdev->has_slp_s0_res)
> +		return -EACCES;
> +
> +	value = pmc_core_reg_read(pmcdev,
> SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
> +	*data = pmc_core_adjust_slp_s0_step(value);
> +
> +	return 0;
> +}
> +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)
> +{
> +	struct pmc_dev *pmcdev = s->private;
> +	u32 counter_val;
> +
> +	counter_val = pmc_core_reg_read(pmcdev,
> +					SPT_PMC_SLP_S0_RES_COUNTER_OF
> FSET);
> +	seq_printf(s, "%u\n",
> pmc_core_adjust_slp_s0_step(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 *pmcdev)
> +{
> +	debugfs_remove_recursive(pmcdev->dbgfs_dir);
> +}
> +
> +static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
> +{
> +	struct dentry *dir, *file;
> +
> +	dir = debugfs_create_dir("pmc_core", NULL);
> +	if (!dir)
> +		return -ENOMEM;
> +
> +	pmcdev->dbgfs_dir = dir;
> +	file = debugfs_create_file("slp_s0_residency_usec", S_IFREG |
> S_IRUGO,
> +				ÂÂÂdir, pmcdev,
> &pmc_core_dev_state_ops);
> +
> +	if (!file) {
> +		pmc_core_dbgfs_unregister(pmcdev);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static inline int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
> +{
> +	return 0;
> +}
> +
> +static inline void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
> +{
> +}
> +#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}, /* Skylake CPUID Signature */
> +	{ X86_VENDOR_INTEL, 6, 0x5e, X86_FEATURE_MWAIT,
> +		(kernel_ulong_t)NULL}, /* Skylake CPUID Signature */
> +	{}
> +};
> +
> +static int pmc_core_probe(struct pci_dev *dev, const struct
> pci_device_id *id)
> +{
> +	struct device *ptr_dev = &dev->dev;
> +	struct pmc_dev *pmcdev = &pmc;
> +	const struct x86_cpu_id *cpu_id;
> +	int err;
> +
> +	cpu_id = x86_match_cpu(intel_pmc_core_ids);
> +	if (!cpu_id) {
> +		dev_dbg(&dev->dev, "PMC Core: cpuid mismatch.\n");
> +		return -EINVAL;
> +	}
> +
> +	err = pcim_enable_device(dev);
> +	if (err < 0) {
> +		dev_dbg(&dev->dev, "PMC Core: failed to enable Power
> Management Controller.\n");
> +		return err;
> +	}
> +
> +	err = pci_read_config_dword(dev,
> +				ÂÂÂÂSPT_PMC_BASE_ADDR_OFFSET,
> +				ÂÂÂÂ&pmcdev->base_addr);
> +	if (err < 0) {
> +		dev_dbg(&dev->dev, "PMC Core: failed to read PCI
> config space.\n");
> +		return err;
> +	}
> +	dev_dbg(&dev->dev, "PMC Core: PWRMBASE is %#x\n", pmcdev-
> >base_addr);
> +
> +	pmcdev->regbase = devm_ioremap_nocache(ptr_dev,
> +					ÂÂÂÂÂÂpmcdev->base_addr,
> +					ÂÂÂÂÂÂSPT_PMC_MMIO_REG_LEN);
> +	if (!pmcdev->regbase) {
> +		dev_dbg(&dev->dev, "PMC Core: ioremap failed.\n");
> +		return -ENOMEM;
> +	}
> +
> +	err = pmc_core_dbgfs_register(pmcdev);
> +	if (err < 0) {
> +		dev_err(&dev->dev, "PMC Core: debugfs register
> failed.\n");
> +		return err;
> +	}
> +
> +	pmc.has_slp_s0_res = true;
> +	return 0;
> +}
> +
> +static struct pci_driver intel_pmc_core_driver = {
> +	.name = "intel_pmc_core",
> +	.id_table = pmc_pci_ids,
> +	.probe = pmc_core_probe,
> +};
> +
> +builtin_pci_driver(intel_pmc_core_driver);
> diff --git a/drivers/platform/x86/intel_pmc_core.h
> b/drivers/platform/x86/intel_pmc_core.h
> new file mode 100644
> index 0000000..a9dadaf
> --- /dev/null
> +++ b/drivers/platform/x86/intel_pmc_core.h
> @@ -0,0 +1,51 @@
> +/*
> + * Intel Core SoC Power Management Controller Header File
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + * All Rights Reserved.
> + *
> + * Authors: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx>
> + *ÂÂÂÂÂÂÂÂÂÂVishwanath Somayaji <vishwanath.somayaji@xxxxxxxxx>
> + *
> + * 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
> +
> +/* Sunrise Point Power Management Controller PCI Device ID */
> +#define SPT_PMC_PCI_DEVICE_ID			0x9d21
> +#define SPT_PMC_BASE_ADDR_OFFSET		0x48
> +#define SPT_PMC_SLP_S0_RES_COUNTER_OFFSET	0x13c
> +#define SPT_PMC_MMIO_REG_LEN			0x100
> +#define SPT_PMC_SLP_S0_RES_COUNTER_STEP		0x64
> +
> +/**
> + * struct pmc_dev - pmc device structure
> + * @base_addr:		comtains pmc base address
> + * @regbase:		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 *regbase;
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> +	struct dentry *dbgfs_dir;
> +#endif /* CONFIG_DEBUG_FS */
> +	bool has_slp_s0_res;
> +};
> +
> +#endif /* PMC_CORE_H */

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy