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

From: Andy Shevchenko
Date: Thu May 26 2016 - 06:52:16 EST


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