RE: [PATCH v2 2/4] platform/x86: intel_telemetry: Fix suspend stats

[Chakravarty, Souvik K]

On Fri, November 24, 2017 at 2:55 AM, Andy Shevchenko 
 wrote:
> On Tue, Nov 21, 2017 at 4:36 PM, Souvik Kumar Chakravarty
>  wrote:
> > Suspend stats are not reported consistently due to a limitation in the
> > PMC firmware. This limitation causes a delay in updating the s0ix
> > counters and residencies in the telemetry log upon s0ix exit. As a
> > consequence, reading these counters from the suspend-exit notifier may 
> > result
> in zero read.
> >
> > This patch fixes this issue by cross-verifying the s0ix residencies
> > from the GCR TELEM registers in case the counters are not incremented
> > in the telemetry log after suspend.
> >
> > This fixes https://bugzilla.kernel.org/show_bug.cgi?id=197833
> >
> > We also remove unnecessary 'static' qualifiers from local variables.
> >
> > Reported-and-tested-by: Rajneesh Bhardwaj
> > 
> > Signed-off-by: Souvik Kumar Chakravarty
> > 
> 
> > -   static u32 suspend_shlw_ctr_exit, suspend_deep_ctr_exit;
> > -   static u64 suspend_shlw_res_exit, suspend_deep_res_exit;
> > struct telemetry_debugfs_conf *conf = debugfs_conf;
> > +   u32 suspend_shlw_ctr_exit, suspend_deep_ctr_exit;
> > +   u64 suspend_shlw_res_exit, suspend_deep_res_exit;
> > int ret, index;
> 
> > +   if (suspend_shlw_ctr_exit == suspend_shlw_ctr_temp &&
> > +   suspend_deep_ctr_exit == suspend_deep_ctr_temp) {
> 
> kbuildbot is absolutely right. How this code is supposed to work? It's flaky.

suspend_shlw_ctr_exit & suspend_deep_ctr_exit have already been initialized 
before this comparison (comparing the counters before and after sleep).
I will explicitly initialize them so that kbuildbot does not complain.
> 
> Please, redesign this approach.
> 
> --
> With Best Regards,
> Andy Shevchenko

RE: [PATCH v2 2/4] platform/x86: intel_telemetry: Fix suspend stats

[Chakravarty, Souvik K]

On Fri, November 24, 2017 at 2:55 AM, Andy Shevchenko 
 wrote:
> On Tue, Nov 21, 2017 at 4:36 PM, Souvik Kumar Chakravarty
>  wrote:
> > Suspend stats are not reported consistently due to a limitation in the
> > PMC firmware. This limitation causes a delay in updating the s0ix
> > counters and residencies in the telemetry log upon s0ix exit. As a
> > consequence, reading these counters from the suspend-exit notifier may 
> > result
> in zero read.
> >
> > This patch fixes this issue by cross-verifying the s0ix residencies
> > from the GCR TELEM registers in case the counters are not incremented
> > in the telemetry log after suspend.
> >
> > This fixes https://bugzilla.kernel.org/show_bug.cgi?id=197833
> >
> > We also remove unnecessary 'static' qualifiers from local variables.
> >
> > Reported-and-tested-by: Rajneesh Bhardwaj
> > 
> > Signed-off-by: Souvik Kumar Chakravarty
> > 
> 
> > -   static u32 suspend_shlw_ctr_exit, suspend_deep_ctr_exit;
> > -   static u64 suspend_shlw_res_exit, suspend_deep_res_exit;
> > struct telemetry_debugfs_conf *conf = debugfs_conf;
> > +   u32 suspend_shlw_ctr_exit, suspend_deep_ctr_exit;
> > +   u64 suspend_shlw_res_exit, suspend_deep_res_exit;
> > int ret, index;
> 
> > +   if (suspend_shlw_ctr_exit == suspend_shlw_ctr_temp &&
> > +   suspend_deep_ctr_exit == suspend_deep_ctr_temp) {
> 
> kbuildbot is absolutely right. How this code is supposed to work? It's flaky.

suspend_shlw_ctr_exit & suspend_deep_ctr_exit have already been initialized 
before this comparison (comparing the counters before and after sleep).
I will explicitly initialize them so that kbuildbot does not complain.
> 
> Please, redesign this approach.
> 
> --
> With Best Regards,
> Andy Shevchenko

RE: [PATCH v1 0/5] platform/x86: intel_telemetry: Fix counters and formatting

[Chakravarty, Souvik K]

On Fri, Nov 17, 2017 at 7:07 PM, Andy Shevchenko  
wrote:
> On Thu, Nov 16, 2017 at 12:18 PM, Souvik Kumar Chakravarty
>  wrote:
> > This patchset fixes
> > https://bugzilla.kernel.org/show_bug.cgi?id=197833, and other issues
> > related to telemetry counters. It also cleans up formatting and removes
> redundanant code.
> >
> > It is rebased on top of the TESTING branch.
> 
> Overall looks not so bad, please address my comments.

Thanks for the comments Andy. I will re-spin another set.
> 
> >
> > Souvik Kumar Chakravarty (5):
> >   platform/x86: intel_pmc_ipc: Fix register names
> >   platform/x86: intel_telemetry: Fix suspend stats
> >   platform/x86: intel_telemetry: Fix local variables
> >   platform/x86: intel_telemetry: Remove redundancies
> >   platform/x86: intel_telemetry: Improve S0ix logs
> >
> >  arch/x86/include/asm/intel_pmc_ipc.h   |  10 ++-
> >  drivers/platform/x86/intel_pmc_ipc.c   |   4 +-
> >  drivers/platform/x86/intel_telemetry_debugfs.c | 101
> > +
> >  3 files changed, 59 insertions(+), 56 deletions(-)
> >
> > --
> > 2.7.4
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

RE: [PATCH v1 0/5] platform/x86: intel_telemetry: Fix counters and formatting

[Chakravarty, Souvik K]

On Fri, Nov 17, 2017 at 7:07 PM, Andy Shevchenko  
wrote:
> On Thu, Nov 16, 2017 at 12:18 PM, Souvik Kumar Chakravarty
>  wrote:
> > This patchset fixes
> > https://bugzilla.kernel.org/show_bug.cgi?id=197833, and other issues
> > related to telemetry counters. It also cleans up formatting and removes
> redundanant code.
> >
> > It is rebased on top of the TESTING branch.
> 
> Overall looks not so bad, please address my comments.

Thanks for the comments Andy. I will re-spin another set.
> 
> >
> > Souvik Kumar Chakravarty (5):
> >   platform/x86: intel_pmc_ipc: Fix register names
> >   platform/x86: intel_telemetry: Fix suspend stats
> >   platform/x86: intel_telemetry: Fix local variables
> >   platform/x86: intel_telemetry: Remove redundancies
> >   platform/x86: intel_telemetry: Improve S0ix logs
> >
> >  arch/x86/include/asm/intel_pmc_ipc.h   |  10 ++-
> >  drivers/platform/x86/intel_pmc_ipc.c   |   4 +-
> >  drivers/platform/x86/intel_telemetry_debugfs.c | 101
> > +
> >  3 files changed, 59 insertions(+), 56 deletions(-)
> >
> > --
> > 2.7.4
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

RE: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver

[Chakravarty, Souvik K]

On October 11, 2017 3:39 AM, Kuppuswamy Sathyanarayanan wrote:
> Hi,
> 
> 
> On 10/08/2017 09:53 PM, Chakravarty, Souvik K wrote:
> >> From: sathyanarayanan.kuppusw...@linux.intel.com
> >> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> >> Sent: Sunday, October 8, 2017 3:50 AM
> >> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> >> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> >> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> >> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org;
> >> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>
> >> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> >> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >> Subject: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver
> >>
> >> From: Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >>
> >> Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c
> >> redundantly implements the same IPC features and has lot of code
> >> duplication between them. This driver addresses this issue by
> >> grouping the common IPC functionalities under the same driver.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >> ---
> >>   drivers/platform/x86/Kconfig|   8 +
> >>   drivers/platform/x86/Makefile   |   1 +
> >>   drivers/platform/x86/intel_ipc_dev.c| 576
> >> 
> >>   include/linux/platform_data/x86/intel_ipc_dev.h | 206 +
> >>   4 files changed, 791 insertions(+)
> >>   create mode 100644 drivers/platform/x86/intel_ipc_dev.c
> >>   create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
> >>
> >> Changes since v4:
> >>   * None
> >>
> >> Changes since v3:
> >>   * Fixed NULL pointer exception in intel_ipc_dev_get().
> >>   * Fixed error in check for duplicate intel_ipc_dev.
> >>   * Added custom interrupt handler support.
> >>   * Used char array for error string conversion.
> >>   * Added put dev support.
> >>   * Added devm_* variant of intel_ipc_dev_get().
> >>
> >> Changes since v2:
> >>   * Added ipc_dev_cmd API support.
> >>
> >> diff --git a/drivers/platform/x86/Kconfig
> >> b/drivers/platform/x86/Kconfig index da2d9ba..724ee696 100644
> >> --- a/drivers/platform/x86/Kconfig
> >> +++ b/drivers/platform/x86/Kconfig
> >> @@ -1153,6 +1153,14 @@ config SILEAD_DMI
> >>  with the OS-image for the device. This option supplies the missing
> >>  information. Enable this for x86 tablets with Silead touchscreens.
> >>
> >> +config INTEL_IPC_DEV
> >> +  bool "Intel IPC Device Driver"
> >> +  depends on X86_64
> >> +  ---help---
> >> +This driver implements core features of Intel IPC device. Devices
> >> +like PMC, SCU, PUNIT, etc can use interfaces provided by this
> >> +driver to implement IPC protocol of their respective device.
> >> +
> >>   endif # X86_PLATFORM_DEVICES
> >>
> >>   config PMC_ATOM
> >> diff --git a/drivers/platform/x86/Makefile
> >> b/drivers/platform/x86/Makefile index 2b315d0..99a1af1 100644
> >> --- a/drivers/platform/x86/Makefile
> >> +++ b/drivers/platform/x86/Makefile
> >> @@ -84,3 +84,4 @@ obj-$(CONFIG_PMC_ATOM)   +=
> >> pmc_atom.o
> >>   obj-$(CONFIG_MLX_PLATFORM)   += mlx-platform.o
> >>   obj-$(CONFIG_MLX_CPLD_PLATFORM)  += mlxcpld-hotplug.o
> >>   obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
> >> +obj-$(CONFIG_INTEL_IPC_DEV)   += intel_ipc_dev.o
> >> diff --git a/drivers/platform/x86/intel_ipc_dev.c
> >> b/drivers/platform/x86/intel_ipc_dev.c
> >> new file mode 100644
> >> index 000..f55ddec
> >> --- /dev/null
> >> +++ b/drivers/platform/x86/intel_ipc_dev.c
> >> @@ -0,0 +1,576 @@
> >> +/*
> >> + * intel_ipc_dev.c: Intel IPC device class driver
> >> + *
> >> + * (C) Copyright 2017 Intel Corporation
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the ter

RE: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver

[Chakravarty, Souvik K]

On October 11, 2017 3:39 AM, Kuppuswamy Sathyanarayanan wrote:
> Hi,
> 
> 
> On 10/08/2017 09:53 PM, Chakravarty, Souvik K wrote:
> >> From: sathyanarayanan.kuppusw...@linux.intel.com
> >> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> >> Sent: Sunday, October 8, 2017 3:50 AM
> >> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> >> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> >> ; h...@zytor.com; dvh...@infradead.org;
> >> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org;
> >> Chakravarty, Souvik K 
> >> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> >> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> >> 
> >> Subject: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver
> >>
> >> From: Kuppuswamy Sathyanarayanan
> >> 
> >>
> >> Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c
> >> redundantly implements the same IPC features and has lot of code
> >> duplication between them. This driver addresses this issue by
> >> grouping the common IPC functionalities under the same driver.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan
> >> 
> >> ---
> >>   drivers/platform/x86/Kconfig|   8 +
> >>   drivers/platform/x86/Makefile   |   1 +
> >>   drivers/platform/x86/intel_ipc_dev.c| 576
> >> 
> >>   include/linux/platform_data/x86/intel_ipc_dev.h | 206 +
> >>   4 files changed, 791 insertions(+)
> >>   create mode 100644 drivers/platform/x86/intel_ipc_dev.c
> >>   create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
> >>
> >> Changes since v4:
> >>   * None
> >>
> >> Changes since v3:
> >>   * Fixed NULL pointer exception in intel_ipc_dev_get().
> >>   * Fixed error in check for duplicate intel_ipc_dev.
> >>   * Added custom interrupt handler support.
> >>   * Used char array for error string conversion.
> >>   * Added put dev support.
> >>   * Added devm_* variant of intel_ipc_dev_get().
> >>
> >> Changes since v2:
> >>   * Added ipc_dev_cmd API support.
> >>
> >> diff --git a/drivers/platform/x86/Kconfig
> >> b/drivers/platform/x86/Kconfig index da2d9ba..724ee696 100644
> >> --- a/drivers/platform/x86/Kconfig
> >> +++ b/drivers/platform/x86/Kconfig
> >> @@ -1153,6 +1153,14 @@ config SILEAD_DMI
> >>  with the OS-image for the device. This option supplies the missing
> >>  information. Enable this for x86 tablets with Silead touchscreens.
> >>
> >> +config INTEL_IPC_DEV
> >> +  bool "Intel IPC Device Driver"
> >> +  depends on X86_64
> >> +  ---help---
> >> +This driver implements core features of Intel IPC device. Devices
> >> +like PMC, SCU, PUNIT, etc can use interfaces provided by this
> >> +driver to implement IPC protocol of their respective device.
> >> +
> >>   endif # X86_PLATFORM_DEVICES
> >>
> >>   config PMC_ATOM
> >> diff --git a/drivers/platform/x86/Makefile
> >> b/drivers/platform/x86/Makefile index 2b315d0..99a1af1 100644
> >> --- a/drivers/platform/x86/Makefile
> >> +++ b/drivers/platform/x86/Makefile
> >> @@ -84,3 +84,4 @@ obj-$(CONFIG_PMC_ATOM)   +=
> >> pmc_atom.o
> >>   obj-$(CONFIG_MLX_PLATFORM)   += mlx-platform.o
> >>   obj-$(CONFIG_MLX_CPLD_PLATFORM)  += mlxcpld-hotplug.o
> >>   obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
> >> +obj-$(CONFIG_INTEL_IPC_DEV)   += intel_ipc_dev.o
> >> diff --git a/drivers/platform/x86/intel_ipc_dev.c
> >> b/drivers/platform/x86/intel_ipc_dev.c
> >> new file mode 100644
> >> index 000..f55ddec
> >> --- /dev/null
> >> +++ b/drivers/platform/x86/intel_ipc_dev.c
> >> @@ -0,0 +1,576 @@
> >> +/*
> >> + * intel_ipc_dev.c: Intel IPC device class driver
> >> + *
> >> + * (C) Copyright 2017 Intel Corporation
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * as published by the Free Software Foundation; version 2
> >> + * of the License.
> >> + *
> >> + */
> >>

RE: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc device calls

[Chakravarty, Souvik K]

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of sathyanarayanan kuppuswamy
> Sent: Wednesday, October 11, 2017 3:59 AM
> To: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>;
> a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com
> Subject: Re: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc
> device calls
> 
> 
> 
> On 10/08/2017 10:07 PM, Chakravarty, Souvik K wrote:
> >> From: sathyanarayanan.kuppusw...@linux.intel.com
> >> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> >> Sent: Sunday, October 8, 2017 3:50 AM
> >> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> >> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> >> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> >> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org;
> >> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>
> >> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> >> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >> Subject: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic
> >> intel ipc device calls
> >>
> >> From: Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >>
> >> Removed redundant IPC helper functions and refactored the driver to
> >> use APIs provided by generic IPC driver. This patch also cleans-up
> >> PUNIT IPC user
> >> drivers(intel_telemetry_pltdrv.c) to use APIs provided by generic IPC
> driver.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >> ---
> >>   arch/x86/include/asm/intel_punit_ipc.h| 125 +--
> >>   drivers/platform/x86/Kconfig  |   1 +
> >>   drivers/platform/x86/intel_punit_ipc.c| 303 
> >> ++
> >>   drivers/platform/x86/intel_telemetry_pltdrv.c |  97 +
> >>   4 files changed, 223 insertions(+), 303 deletions(-)
> >>
> >> Changes since v4:
> >>   * None
> >>
> >> Changes since v2:
> >>   * Added unique name to PUNIT BIOS, GTD, & ISP regmaps.
> >>   * Added intel_ipc_dev_put() support.
> >>
> >> Changes since v1:
> >>   * Removed custom APIs.
> >>   * Cleaned up PUNIT IPC user drivers to use APIs provided by generic
> >> IPC driver.
> >>
> >> diff --git a/arch/x86/include/asm/intel_punit_ipc.h
> >> b/arch/x86/include/asm/intel_punit_ipc.h
> >> index 201eb9d..cf1630c 100644
> >> --- a/arch/x86/include/asm/intel_punit_ipc.h
> >> +++ b/arch/x86/include/asm/intel_punit_ipc.h
> >> @@ -1,10 +1,8 @@
> >>   #ifndef _ASM_X86_INTEL_PUNIT_IPC_H_
> >>   #define  _ASM_X86_INTEL_PUNIT_IPC_H_
> >>
> >> -/*
> >> - * Three types of 8bit P-Unit IPC commands are supported,
> >> - * bit[7:6]: [00]: BIOS; [01]: GTD; [10]: ISPD.
> >> - */
> >> +#include 
> >> +
> >>   typedef enum {
> >>BIOS_IPC = 0,
> >>GTDRIVER_IPC,
> >> @@ -12,61 +10,60 @@ typedef enum {
> >>RESERVED_IPC,
> >>   } IPC_TYPE;
> >>
> >> -#define IPC_TYPE_OFFSET   6
> >> -#define IPC_PUNIT_BIOS_CMD_BASE   (BIOS_IPC <<
> >> IPC_TYPE_OFFSET)
> >> -#define IPC_PUNIT_GTD_CMD_BASE(GTDDRIVER_IPC <<
> >> IPC_TYPE_OFFSET)
> >> -#define IPC_PUNIT_ISPD_CMD_BASE   (ISPDRIVER_IPC <<
> >> IPC_TYPE_OFFSET)
> >> -#define IPC_PUNIT_CMD_TYPE_MASK   (RESERVED_IPC <<
> >> IPC_TYPE_OFFSET)
> >> +#define PUNIT_BIOS_IPC_DEV"punit_bios_ipc"
> >> +#define PUNIT_GTD_IPC_DEV "punit_gtd_ipc"
> >> +#define PUNIT_ISP_IPC_DEV "punit_isp_ipc"
> >> +#define PUNIT_PARAM_LEN  

RE: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc device calls

[Chakravarty, Souvik K]

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of sathyanarayanan kuppuswamy
> Sent: Wednesday, October 11, 2017 3:59 AM
> To: Chakravarty, Souvik K ;
> a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com
> Subject: Re: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc
> device calls
> 
> 
> 
> On 10/08/2017 10:07 PM, Chakravarty, Souvik K wrote:
> >> From: sathyanarayanan.kuppusw...@linux.intel.com
> >> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> >> Sent: Sunday, October 8, 2017 3:50 AM
> >> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> >> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> >> ; h...@zytor.com; dvh...@infradead.org;
> >> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org;
> >> Chakravarty, Souvik K 
> >> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> >> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> >> 
> >> Subject: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic
> >> intel ipc device calls
> >>
> >> From: Kuppuswamy Sathyanarayanan
> >> 
> >>
> >> Removed redundant IPC helper functions and refactored the driver to
> >> use APIs provided by generic IPC driver. This patch also cleans-up
> >> PUNIT IPC user
> >> drivers(intel_telemetry_pltdrv.c) to use APIs provided by generic IPC
> driver.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan
> >> 
> >> ---
> >>   arch/x86/include/asm/intel_punit_ipc.h| 125 +--
> >>   drivers/platform/x86/Kconfig  |   1 +
> >>   drivers/platform/x86/intel_punit_ipc.c| 303 
> >> ++
> >>   drivers/platform/x86/intel_telemetry_pltdrv.c |  97 +
> >>   4 files changed, 223 insertions(+), 303 deletions(-)
> >>
> >> Changes since v4:
> >>   * None
> >>
> >> Changes since v2:
> >>   * Added unique name to PUNIT BIOS, GTD, & ISP regmaps.
> >>   * Added intel_ipc_dev_put() support.
> >>
> >> Changes since v1:
> >>   * Removed custom APIs.
> >>   * Cleaned up PUNIT IPC user drivers to use APIs provided by generic
> >> IPC driver.
> >>
> >> diff --git a/arch/x86/include/asm/intel_punit_ipc.h
> >> b/arch/x86/include/asm/intel_punit_ipc.h
> >> index 201eb9d..cf1630c 100644
> >> --- a/arch/x86/include/asm/intel_punit_ipc.h
> >> +++ b/arch/x86/include/asm/intel_punit_ipc.h
> >> @@ -1,10 +1,8 @@
> >>   #ifndef _ASM_X86_INTEL_PUNIT_IPC_H_
> >>   #define  _ASM_X86_INTEL_PUNIT_IPC_H_
> >>
> >> -/*
> >> - * Three types of 8bit P-Unit IPC commands are supported,
> >> - * bit[7:6]: [00]: BIOS; [01]: GTD; [10]: ISPD.
> >> - */
> >> +#include 
> >> +
> >>   typedef enum {
> >>BIOS_IPC = 0,
> >>GTDRIVER_IPC,
> >> @@ -12,61 +10,60 @@ typedef enum {
> >>RESERVED_IPC,
> >>   } IPC_TYPE;
> >>
> >> -#define IPC_TYPE_OFFSET   6
> >> -#define IPC_PUNIT_BIOS_CMD_BASE   (BIOS_IPC <<
> >> IPC_TYPE_OFFSET)
> >> -#define IPC_PUNIT_GTD_CMD_BASE(GTDDRIVER_IPC <<
> >> IPC_TYPE_OFFSET)
> >> -#define IPC_PUNIT_ISPD_CMD_BASE   (ISPDRIVER_IPC <<
> >> IPC_TYPE_OFFSET)
> >> -#define IPC_PUNIT_CMD_TYPE_MASK   (RESERVED_IPC <<
> >> IPC_TYPE_OFFSET)
> >> +#define PUNIT_BIOS_IPC_DEV"punit_bios_ipc"
> >> +#define PUNIT_GTD_IPC_DEV "punit_gtd_ipc"
> >> +#define PUNIT_ISP_IPC_DEV "punit_isp_ipc"
> >> +#define PUNIT_PARAM_LEN   3
> >>
> >>   /* BIOS => Pcode commands */
> >> -#define IPC_PUNIT_BIOS_ZERO
>   (IPC_PUNIT_BIOS_CMD_BASE
> >> | 0x00)
> >> -#define IPC_PUNIT_BIOS_VR_INTERFACE
> >>(IPC_PUNIT_BIOS_CMD_BA

RE: [RFC v5 7/8] platform/x86: intel_pmc_ipc: Use generic Intel IPC device calls

[Chakravarty, Souvik K]

> From: sathyanarayanan.kuppusw...@linux.intel.com
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Sunday, October 8, 2017 3:50 AM
> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org; Chakravarty,
> Souvik K <souvik.k.chakrava...@intel.com>
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> Subject: [RFC v5 7/8] platform/x86: intel_pmc_ipc: Use generic Intel IPC
> device calls
> 
> From: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> 
> Removed redundant IPC helper functions and refactored the driver to use
> generic IPC device driver APIs. Also, cleaned up the driver to minimize the
> usage of global variable ipcdev by propogating the struct intel_pmc_ipc_dev
> pointer or by getting it from device private data.
> 
> This patch also cleans-up PMC IPC user drivers(intel_telemetry_pltdrv.c,
> intel_soc_pmic_bxtwc.c) to use APIs provided by generic IPC driver.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> ---
>  arch/x86/include/asm/intel_pmc_ipc.h  |  37 +--
>  drivers/mfd/intel_soc_pmic_bxtwc.c|  21 +-
>  drivers/platform/x86/intel_pmc_ipc.c  | 393 
> ++
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 119 
>  include/linux/mfd/intel_soc_pmic.h|   2 +
>  5 files changed, 238 insertions(+), 334 deletions(-)
> 
> Changes since v4:
>  * None
> 
> Changes since v3:
>  * Added unique name to PMC regmaps.
>  * Added support to clear interrupt bit.
>  * Added intel_ipc_dev_put() support.
> 
> Changes since v1:
>  * Removed custom APIs.
>  * Cleaned up PMC IPC user drivers to use APIs provided by generic
>IPC driver.
> 
> diff --git a/arch/x86/include/asm/intel_pmc_ipc.h
> b/arch/x86/include/asm/intel_pmc_ipc.h
> index fac89eb..9fc7c3c 100644
> --- a/arch/x86/include/asm/intel_pmc_ipc.h
> +++ b/arch/x86/include/asm/intel_pmc_ipc.h
> @@ -1,10 +1,15 @@
>  #ifndef _ASM_X86_INTEL_PMC_IPC_H_
>  #define  _ASM_X86_INTEL_PMC_IPC_H_
> 
> +#include 
> +
> +#define INTEL_PMC_IPC_DEV"intel_pmc_ipc"
> +#define PMC_PARAM_LEN2
> +
>  /* Commands */
>  #define PMC_IPC_PMIC_ACCESS  0xFF
> -#define  PMC_IPC_PMIC_ACCESS_READ0x0
> -#define  PMC_IPC_PMIC_ACCESS_WRITE   0x1
> +#define  PMC_IPC_PMIC_ACCESS_READ0x0
> +#define  PMC_IPC_PMIC_ACCESS_WRITE   0x1
>  #define PMC_IPC_USB_PWR_CTRL 0xF0
>  #define PMC_IPC_PMIC_BLACKLIST_SEL   0xEF
>  #define PMC_IPC_PHY_CONFIG   0xEE
> @@ -28,13 +33,14 @@
>  #define PMC_GCR_TELEM_DEEP_S0IX_REG  0x78
>  #define PMC_GCR_TELEM_SHLW_S0IX_REG  0x80
> 
> +static inline void pmc_cmd_init(u32 *cmd, u32 param1, u32 param2) {
> + cmd[0] = param1;
> + cmd[1] = param2;
> +}
> +
>  #if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> 
> -int intel_pmc_ipc_simple_command(int cmd, int sub); -int
> intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
> - u32 *out, u32 outlen, u32 dptr, u32 sptr);
> -int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> - u32 *out, u32 outlen);
>  int intel_pmc_s0ix_counter_read(u64 *data);  int intel_pmc_gcr_read(u32
> offset, u32 *data);  int intel_pmc_gcr_write(u32 offset, u32 data); @@ -42,23
> +48,6 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val);
> 
>  #else
> 
> -static inline int intel_pmc_ipc_simple_command(int cmd, int sub) -{
> - return -EINVAL;
> -}
> -
> -static inline int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32
> inlen,
> - u32 *out, u32 outlen, u32 dptr, u32 sptr)
> -{
> - return -EINVAL;
> -}
> -
> -static inline int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32
> inlen,
> - u32 *out, u32 outlen)
> -{
> - return -EINVAL;
> -}
> -
>  static inline int intel_pmc_s0ix_counter_read(u64 *data)  {
>   return -EINVAL;
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c
> b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index 15bc052..f9df9e7 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -271,6 +271,8 @@ static int regmap_ipc_byte_reg_read(void *context,
> unsig

RE: [RFC v5 7/8] platform/x86: intel_pmc_ipc: Use generic Intel IPC device calls

[Chakravarty, Souvik K]

> From: sathyanarayanan.kuppusw...@linux.intel.com
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Sunday, October 8, 2017 3:50 AM
> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org; Chakravarty,
> Souvik K 
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> 
> Subject: [RFC v5 7/8] platform/x86: intel_pmc_ipc: Use generic Intel IPC
> device calls
> 
> From: Kuppuswamy Sathyanarayanan
> 
> 
> Removed redundant IPC helper functions and refactored the driver to use
> generic IPC device driver APIs. Also, cleaned up the driver to minimize the
> usage of global variable ipcdev by propogating the struct intel_pmc_ipc_dev
> pointer or by getting it from device private data.
> 
> This patch also cleans-up PMC IPC user drivers(intel_telemetry_pltdrv.c,
> intel_soc_pmic_bxtwc.c) to use APIs provided by generic IPC driver.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> 
> ---
>  arch/x86/include/asm/intel_pmc_ipc.h  |  37 +--
>  drivers/mfd/intel_soc_pmic_bxtwc.c|  21 +-
>  drivers/platform/x86/intel_pmc_ipc.c  | 393 
> ++
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 119 
>  include/linux/mfd/intel_soc_pmic.h|   2 +
>  5 files changed, 238 insertions(+), 334 deletions(-)
> 
> Changes since v4:
>  * None
> 
> Changes since v3:
>  * Added unique name to PMC regmaps.
>  * Added support to clear interrupt bit.
>  * Added intel_ipc_dev_put() support.
> 
> Changes since v1:
>  * Removed custom APIs.
>  * Cleaned up PMC IPC user drivers to use APIs provided by generic
>IPC driver.
> 
> diff --git a/arch/x86/include/asm/intel_pmc_ipc.h
> b/arch/x86/include/asm/intel_pmc_ipc.h
> index fac89eb..9fc7c3c 100644
> --- a/arch/x86/include/asm/intel_pmc_ipc.h
> +++ b/arch/x86/include/asm/intel_pmc_ipc.h
> @@ -1,10 +1,15 @@
>  #ifndef _ASM_X86_INTEL_PMC_IPC_H_
>  #define  _ASM_X86_INTEL_PMC_IPC_H_
> 
> +#include 
> +
> +#define INTEL_PMC_IPC_DEV"intel_pmc_ipc"
> +#define PMC_PARAM_LEN2
> +
>  /* Commands */
>  #define PMC_IPC_PMIC_ACCESS  0xFF
> -#define  PMC_IPC_PMIC_ACCESS_READ0x0
> -#define  PMC_IPC_PMIC_ACCESS_WRITE   0x1
> +#define  PMC_IPC_PMIC_ACCESS_READ0x0
> +#define  PMC_IPC_PMIC_ACCESS_WRITE   0x1
>  #define PMC_IPC_USB_PWR_CTRL 0xF0
>  #define PMC_IPC_PMIC_BLACKLIST_SEL   0xEF
>  #define PMC_IPC_PHY_CONFIG   0xEE
> @@ -28,13 +33,14 @@
>  #define PMC_GCR_TELEM_DEEP_S0IX_REG  0x78
>  #define PMC_GCR_TELEM_SHLW_S0IX_REG  0x80
> 
> +static inline void pmc_cmd_init(u32 *cmd, u32 param1, u32 param2) {
> + cmd[0] = param1;
> + cmd[1] = param2;
> +}
> +
>  #if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> 
> -int intel_pmc_ipc_simple_command(int cmd, int sub); -int
> intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
> - u32 *out, u32 outlen, u32 dptr, u32 sptr);
> -int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> - u32 *out, u32 outlen);
>  int intel_pmc_s0ix_counter_read(u64 *data);  int intel_pmc_gcr_read(u32
> offset, u32 *data);  int intel_pmc_gcr_write(u32 offset, u32 data); @@ -42,23
> +48,6 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val);
> 
>  #else
> 
> -static inline int intel_pmc_ipc_simple_command(int cmd, int sub) -{
> - return -EINVAL;
> -}
> -
> -static inline int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32
> inlen,
> - u32 *out, u32 outlen, u32 dptr, u32 sptr)
> -{
> - return -EINVAL;
> -}
> -
> -static inline int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32
> inlen,
> - u32 *out, u32 outlen)
> -{
> - return -EINVAL;
> -}
> -
>  static inline int intel_pmc_s0ix_counter_read(u64 *data)  {
>   return -EINVAL;
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c
> b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index 15bc052..f9df9e7 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -271,6 +271,8 @@ static int regmap_ipc_byte_reg_read(void *context,
> unsigned int reg,
>   u8 ipc_in[2];
>   u8 ipc_out[4];
>   struct intel_soc_pmic *pmic = context;
> + u32 cmd[PMC_PARAM_LEN] = {PMC_IPC_PMIC_ACCESS,
> + PMC_IPC_PMIC_ACCESS_READ};

RE: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc device calls

[Chakravarty, Souvik K]

> From: sathyanarayanan.kuppusw...@linux.intel.com
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Sunday, October 8, 2017 3:50 AM
> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org; Chakravarty,
> Souvik K <souvik.k.chakrava...@intel.com>
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> Subject: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc
> device calls
> 
> From: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> 
> Removed redundant IPC helper functions and refactored the driver to use
> APIs provided by generic IPC driver. This patch also cleans-up PUNIT IPC user
> drivers(intel_telemetry_pltdrv.c) to use APIs provided by generic IPC driver.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> ---
>  arch/x86/include/asm/intel_punit_ipc.h| 125 +--
>  drivers/platform/x86/Kconfig  |   1 +
>  drivers/platform/x86/intel_punit_ipc.c| 303 
> ++
>  drivers/platform/x86/intel_telemetry_pltdrv.c |  97 +
>  4 files changed, 223 insertions(+), 303 deletions(-)
> 
> Changes since v4:
>  * None
> 
> Changes since v2:
>  * Added unique name to PUNIT BIOS, GTD, & ISP regmaps.
>  * Added intel_ipc_dev_put() support.
> 
> Changes since v1:
>  * Removed custom APIs.
>  * Cleaned up PUNIT IPC user drivers to use APIs provided by generic
>IPC driver.
> 
> diff --git a/arch/x86/include/asm/intel_punit_ipc.h
> b/arch/x86/include/asm/intel_punit_ipc.h
> index 201eb9d..cf1630c 100644
> --- a/arch/x86/include/asm/intel_punit_ipc.h
> +++ b/arch/x86/include/asm/intel_punit_ipc.h
> @@ -1,10 +1,8 @@
>  #ifndef _ASM_X86_INTEL_PUNIT_IPC_H_
>  #define  _ASM_X86_INTEL_PUNIT_IPC_H_
> 
> -/*
> - * Three types of 8bit P-Unit IPC commands are supported,
> - * bit[7:6]: [00]: BIOS; [01]: GTD; [10]: ISPD.
> - */
> +#include 
> +
>  typedef enum {
>   BIOS_IPC = 0,
>   GTDRIVER_IPC,
> @@ -12,61 +10,60 @@ typedef enum {
>   RESERVED_IPC,
>  } IPC_TYPE;
> 
> -#define IPC_TYPE_OFFSET  6
> -#define IPC_PUNIT_BIOS_CMD_BASE  (BIOS_IPC <<
> IPC_TYPE_OFFSET)
> -#define IPC_PUNIT_GTD_CMD_BASE   (GTDDRIVER_IPC <<
> IPC_TYPE_OFFSET)
> -#define IPC_PUNIT_ISPD_CMD_BASE  (ISPDRIVER_IPC <<
> IPC_TYPE_OFFSET)
> -#define IPC_PUNIT_CMD_TYPE_MASK  (RESERVED_IPC <<
> IPC_TYPE_OFFSET)
> +#define PUNIT_BIOS_IPC_DEV   "punit_bios_ipc"
> +#define PUNIT_GTD_IPC_DEV"punit_gtd_ipc"
> +#define PUNIT_ISP_IPC_DEV"punit_isp_ipc"
> +#define PUNIT_PARAM_LEN  3
> 
>  /* BIOS => Pcode commands */
> -#define IPC_PUNIT_BIOS_ZERO  (IPC_PUNIT_BIOS_CMD_BASE
> | 0x00)
> -#define IPC_PUNIT_BIOS_VR_INTERFACE
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x01)
> -#define IPC_PUNIT_BIOS_READ_PCS
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x02)
> -#define IPC_PUNIT_BIOS_WRITE_PCS (IPC_PUNIT_BIOS_CMD_BASE
> | 0x03)
> -#define IPC_PUNIT_BIOS_READ_PCU_CONFIG
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x04)
> -#define IPC_PUNIT_BIOS_WRITE_PCU_CONFIG
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x05)
> -#define IPC_PUNIT_BIOS_READ_PL1_SETTING
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x06)
> -#define IPC_PUNIT_BIOS_WRITE_PL1_SETTING (IPC_PUNIT_BIOS_CMD_BASE
> | 0x07)
> -#define IPC_PUNIT_BIOS_TRIGGER_VDD_RAM
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x08)
> -#define IPC_PUNIT_BIOS_READ_TELE_INFO
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x09)
> -#define IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0a)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0b)
> -#define IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0c)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0d)
> -#define IPC_PUNIT_BIOS_READ_TELE_TRACE
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0e)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0f)
> -#define IPC_PUNIT_BIOS_READ_TELE_EVENT
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x10)
> -#define IPC_PU

RE: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc device calls

[Chakravarty, Souvik K]

> From: sathyanarayanan.kuppusw...@linux.intel.com
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Sunday, October 8, 2017 3:50 AM
> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org; Chakravarty,
> Souvik K 
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> 
> Subject: [RFC v5 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc
> device calls
> 
> From: Kuppuswamy Sathyanarayanan
> 
> 
> Removed redundant IPC helper functions and refactored the driver to use
> APIs provided by generic IPC driver. This patch also cleans-up PUNIT IPC user
> drivers(intel_telemetry_pltdrv.c) to use APIs provided by generic IPC driver.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> 
> ---
>  arch/x86/include/asm/intel_punit_ipc.h| 125 +--
>  drivers/platform/x86/Kconfig  |   1 +
>  drivers/platform/x86/intel_punit_ipc.c| 303 
> ++
>  drivers/platform/x86/intel_telemetry_pltdrv.c |  97 +
>  4 files changed, 223 insertions(+), 303 deletions(-)
> 
> Changes since v4:
>  * None
> 
> Changes since v2:
>  * Added unique name to PUNIT BIOS, GTD, & ISP regmaps.
>  * Added intel_ipc_dev_put() support.
> 
> Changes since v1:
>  * Removed custom APIs.
>  * Cleaned up PUNIT IPC user drivers to use APIs provided by generic
>IPC driver.
> 
> diff --git a/arch/x86/include/asm/intel_punit_ipc.h
> b/arch/x86/include/asm/intel_punit_ipc.h
> index 201eb9d..cf1630c 100644
> --- a/arch/x86/include/asm/intel_punit_ipc.h
> +++ b/arch/x86/include/asm/intel_punit_ipc.h
> @@ -1,10 +1,8 @@
>  #ifndef _ASM_X86_INTEL_PUNIT_IPC_H_
>  #define  _ASM_X86_INTEL_PUNIT_IPC_H_
> 
> -/*
> - * Three types of 8bit P-Unit IPC commands are supported,
> - * bit[7:6]: [00]: BIOS; [01]: GTD; [10]: ISPD.
> - */
> +#include 
> +
>  typedef enum {
>   BIOS_IPC = 0,
>   GTDRIVER_IPC,
> @@ -12,61 +10,60 @@ typedef enum {
>   RESERVED_IPC,
>  } IPC_TYPE;
> 
> -#define IPC_TYPE_OFFSET  6
> -#define IPC_PUNIT_BIOS_CMD_BASE  (BIOS_IPC <<
> IPC_TYPE_OFFSET)
> -#define IPC_PUNIT_GTD_CMD_BASE   (GTDDRIVER_IPC <<
> IPC_TYPE_OFFSET)
> -#define IPC_PUNIT_ISPD_CMD_BASE  (ISPDRIVER_IPC <<
> IPC_TYPE_OFFSET)
> -#define IPC_PUNIT_CMD_TYPE_MASK  (RESERVED_IPC <<
> IPC_TYPE_OFFSET)
> +#define PUNIT_BIOS_IPC_DEV   "punit_bios_ipc"
> +#define PUNIT_GTD_IPC_DEV"punit_gtd_ipc"
> +#define PUNIT_ISP_IPC_DEV"punit_isp_ipc"
> +#define PUNIT_PARAM_LEN  3
> 
>  /* BIOS => Pcode commands */
> -#define IPC_PUNIT_BIOS_ZERO  (IPC_PUNIT_BIOS_CMD_BASE
> | 0x00)
> -#define IPC_PUNIT_BIOS_VR_INTERFACE
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x01)
> -#define IPC_PUNIT_BIOS_READ_PCS
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x02)
> -#define IPC_PUNIT_BIOS_WRITE_PCS (IPC_PUNIT_BIOS_CMD_BASE
> | 0x03)
> -#define IPC_PUNIT_BIOS_READ_PCU_CONFIG
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x04)
> -#define IPC_PUNIT_BIOS_WRITE_PCU_CONFIG
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x05)
> -#define IPC_PUNIT_BIOS_READ_PL1_SETTING
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x06)
> -#define IPC_PUNIT_BIOS_WRITE_PL1_SETTING (IPC_PUNIT_BIOS_CMD_BASE
> | 0x07)
> -#define IPC_PUNIT_BIOS_TRIGGER_VDD_RAM
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x08)
> -#define IPC_PUNIT_BIOS_READ_TELE_INFO
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x09)
> -#define IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0a)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0b)
> -#define IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0c)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0d)
> -#define IPC_PUNIT_BIOS_READ_TELE_TRACE
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0e)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x0f)
> -#define IPC_PUNIT_BIOS_READ_TELE_EVENT
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x10)
> -#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x11)
> -#define IPC_PUNIT_BIOS_READ_MODULE_TEMP
>   (IPC_PUNIT_BIOS_CMD_BASE | 0x12)
> -#define IPC_PUNIT_BIOS_RESERVED

RE: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver

[Chakravarty, Souvik K]

> From: sathyanarayanan.kuppusw...@linux.intel.com
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Sunday, October 8, 2017 3:50 AM
> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org; Chakravarty,
> Souvik K <souvik.k.chakrava...@intel.com>
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> Subject: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver
> 
> From: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> 
> Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c redundantly
> implements the same IPC features and has lot of code duplication between
> them. This driver addresses this issue by grouping the common IPC
> functionalities under the same driver.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppusw...@linux.intel.com>
> ---
>  drivers/platform/x86/Kconfig|   8 +
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/intel_ipc_dev.c| 576
> 
>  include/linux/platform_data/x86/intel_ipc_dev.h | 206 +
>  4 files changed, 791 insertions(+)
>  create mode 100644 drivers/platform/x86/intel_ipc_dev.c
>  create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
> 
> Changes since v4:
>  * None
> 
> Changes since v3:
>  * Fixed NULL pointer exception in intel_ipc_dev_get().
>  * Fixed error in check for duplicate intel_ipc_dev.
>  * Added custom interrupt handler support.
>  * Used char array for error string conversion.
>  * Added put dev support.
>  * Added devm_* variant of intel_ipc_dev_get().
> 
> Changes since v2:
>  * Added ipc_dev_cmd API support.
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index da2d9ba..724ee696 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1153,6 +1153,14 @@ config SILEAD_DMI
> with the OS-image for the device. This option supplies the missing
> information. Enable this for x86 tablets with Silead touchscreens.
> 
> +config INTEL_IPC_DEV
> + bool "Intel IPC Device Driver"
> + depends on X86_64
> + ---help---
> +   This driver implements core features of Intel IPC device. Devices
> +   like PMC, SCU, PUNIT, etc can use interfaces provided by this
> +   driver to implement IPC protocol of their respective device.
> +
>  endif # X86_PLATFORM_DEVICES
> 
>  config PMC_ATOM
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2b315d0..99a1af1 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -84,3 +84,4 @@ obj-$(CONFIG_PMC_ATOM)  +=
> pmc_atom.o
>  obj-$(CONFIG_MLX_PLATFORM)   += mlx-platform.o
>  obj-$(CONFIG_MLX_CPLD_PLATFORM)  += mlxcpld-hotplug.o
>  obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
> +obj-$(CONFIG_INTEL_IPC_DEV)  += intel_ipc_dev.o
> diff --git a/drivers/platform/x86/intel_ipc_dev.c
> b/drivers/platform/x86/intel_ipc_dev.c
> new file mode 100644
> index 000..f55ddec
> --- /dev/null
> +++ b/drivers/platform/x86/intel_ipc_dev.c
> @@ -0,0 +1,576 @@
> +/*
> + * intel_ipc_dev.c: Intel IPC device class driver
> + *
> + * (C) Copyright 2017 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* mutex to sync different ipc devices in same channel */ static struct
> +mutex channel_lock[IPC_CHANNEL_MAX];
> +
> +static char *ipc_err_sources[] = {
> + [IPC_DEV_ERR_NONE] =
> + "No error",
> + [IPC_DEV_ERR_CMD_NOT_SUPPORTED] =
> + "Command not-supported/Invalid",
> + [IPC_DEV_ERR_CMD_NOT_SERVICED] =
> + "Command not-serviced/Invalid param",
> + [IPC_DEV_ERR_UNABLE_TO_SERVICE] =
> + "Unable-to-service/Cmd-timeout",
> + [IPC_DEV_ERR_CMD_INVA

RE: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver

[Chakravarty, Souvik K]

> From: sathyanarayanan.kuppusw...@linux.intel.com
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Sunday, October 8, 2017 3:50 AM
> To: a.zu...@towertech.it; x...@kernel.org; w...@iguana.be;
> mi...@redhat.com; alexandre.bell...@free-electrons.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; lee.jo...@linaro.org; a...@infradead.org; Chakravarty,
> Souvik K 
> Cc: linux-...@vger.kernel.org; linux-watch...@vger.kernel.org; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> 
> Subject: [RFC v5 4/8] platform: x86: Add generic Intel IPC driver
> 
> From: Kuppuswamy Sathyanarayanan
> 
> 
> Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c redundantly
> implements the same IPC features and has lot of code duplication between
> them. This driver addresses this issue by grouping the common IPC
> functionalities under the same driver.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> 
> ---
>  drivers/platform/x86/Kconfig|   8 +
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/intel_ipc_dev.c| 576
> 
>  include/linux/platform_data/x86/intel_ipc_dev.h | 206 +
>  4 files changed, 791 insertions(+)
>  create mode 100644 drivers/platform/x86/intel_ipc_dev.c
>  create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
> 
> Changes since v4:
>  * None
> 
> Changes since v3:
>  * Fixed NULL pointer exception in intel_ipc_dev_get().
>  * Fixed error in check for duplicate intel_ipc_dev.
>  * Added custom interrupt handler support.
>  * Used char array for error string conversion.
>  * Added put dev support.
>  * Added devm_* variant of intel_ipc_dev_get().
> 
> Changes since v2:
>  * Added ipc_dev_cmd API support.
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index da2d9ba..724ee696 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1153,6 +1153,14 @@ config SILEAD_DMI
> with the OS-image for the device. This option supplies the missing
> information. Enable this for x86 tablets with Silead touchscreens.
> 
> +config INTEL_IPC_DEV
> + bool "Intel IPC Device Driver"
> + depends on X86_64
> + ---help---
> +   This driver implements core features of Intel IPC device. Devices
> +   like PMC, SCU, PUNIT, etc can use interfaces provided by this
> +   driver to implement IPC protocol of their respective device.
> +
>  endif # X86_PLATFORM_DEVICES
> 
>  config PMC_ATOM
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2b315d0..99a1af1 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -84,3 +84,4 @@ obj-$(CONFIG_PMC_ATOM)  +=
> pmc_atom.o
>  obj-$(CONFIG_MLX_PLATFORM)   += mlx-platform.o
>  obj-$(CONFIG_MLX_CPLD_PLATFORM)  += mlxcpld-hotplug.o
>  obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
> +obj-$(CONFIG_INTEL_IPC_DEV)  += intel_ipc_dev.o
> diff --git a/drivers/platform/x86/intel_ipc_dev.c
> b/drivers/platform/x86/intel_ipc_dev.c
> new file mode 100644
> index 000..f55ddec
> --- /dev/null
> +++ b/drivers/platform/x86/intel_ipc_dev.c
> @@ -0,0 +1,576 @@
> +/*
> + * intel_ipc_dev.c: Intel IPC device class driver
> + *
> + * (C) Copyright 2017 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* mutex to sync different ipc devices in same channel */ static struct
> +mutex channel_lock[IPC_CHANNEL_MAX];
> +
> +static char *ipc_err_sources[] = {
> + [IPC_DEV_ERR_NONE] =
> + "No error",
> + [IPC_DEV_ERR_CMD_NOT_SUPPORTED] =
> + "Command not-supported/Invalid",
> + [IPC_DEV_ERR_CMD_NOT_SERVICED] =
> + "Command not-serviced/Invalid param",
> + [IPC_DEV_ERR_UNABLE_TO_SERVICE] =
> + "Unable-to-service/Cmd-timeout",
> + [IPC_DEV_ERR_CMD_INVALID] =
> + "Command-invalid/Cmd-locked",
> + [IPC_DEV_ERR_CMD_FAILED] =
> + "Command-failed/Invalid-VR-id",
> + [IPC_DEV_ERR_EMSECURITY] =
> + "Invalid 

RE: [PATCH v2] drivers:platform/x86:Use ARRAY_SIZE() for the size calculation of the array.

[Chakravarty, Souvik K]

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Bjørn Mork
> Sent: Wednesday, September 13, 2017 3:32 PM
> To: Allen Pais <allen.l...@gmail.com>
> Cc: linux-kernel@vger.kernel.org; Chakravarty, Souvik K
> <souvik.k.chakrava...@intel.com>; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v2] drivers:platform/x86:Use ARRAY_SIZE() for the size
> calculation of the array.
> 
> Allen Pais <allen.l...@gmail.com> writes:
> 
> > Signed-off-by: Allen Pais <allen.l...@gmail.com>
> > ---
> >  drivers/platform/x86/intel_telemetry_debugfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> > b/drivers/platform/x86/intel_telemetry_debugfs.c
> > index d4fc42b..6b63b3d 100644
> > --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> > +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> > @@ -76,7 +76,7 @@
> >  #define TELEM_IOSS_DX_D0IX_EVTS25
> >  #define TELEM_IOSS_PG_EVTS 30
> >
> > -#define TELEM_EVT_LEN(x) (sizeof(x)/sizeof((x)[0]))
> > +#define TELEM_EVT_LEN(x) ARRAY_SIZE(x)
> >
> >  #define TELEM_DEBUGFS_CPU(model, data) \
> > { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned
> long)}
> 
> What's the point?  Why not simply replace all instances of TELEM_EVT_LEN
> with ARRAY_SIZE?
I was about to say this too, although I should have probably said this in the 
first spin itself.

> 
> 
> 
> Bjørn

RE: [PATCH v2] drivers:platform/x86:Use ARRAY_SIZE() for the size calculation of the array.

[Chakravarty, Souvik K]

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Bjørn Mork
> Sent: Wednesday, September 13, 2017 3:32 PM
> To: Allen Pais 
> Cc: linux-kernel@vger.kernel.org; Chakravarty, Souvik K
> ; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v2] drivers:platform/x86:Use ARRAY_SIZE() for the size
> calculation of the array.
> 
> Allen Pais  writes:
> 
> > Signed-off-by: Allen Pais 
> > ---
> >  drivers/platform/x86/intel_telemetry_debugfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> > b/drivers/platform/x86/intel_telemetry_debugfs.c
> > index d4fc42b..6b63b3d 100644
> > --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> > +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> > @@ -76,7 +76,7 @@
> >  #define TELEM_IOSS_DX_D0IX_EVTS25
> >  #define TELEM_IOSS_PG_EVTS 30
> >
> > -#define TELEM_EVT_LEN(x) (sizeof(x)/sizeof((x)[0]))
> > +#define TELEM_EVT_LEN(x) ARRAY_SIZE(x)
> >
> >  #define TELEM_DEBUGFS_CPU(model, data) \
> > { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned
> long)}
> 
> What's the point?  Why not simply replace all instances of TELEM_EVT_LEN
> with ARRAY_SIZE?
I was about to say this too, although I should have probably said this in the 
first spin itself.

> 
> 
> 
> Bjørn

RE: [PATCH] drivers:platform/x86:Use ARRAY_SIZE() for the size calculation of the array.

[Chakravarty, Souvik K]

Hi Allen,

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Allen Pais
> Sent: Wednesday, September 13, 2017 1:33 PM
> To: linux-kernel@vger.kernel.org
> Cc: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; platform-
> driver-...@vger.kernel.org; Allen Pais <allen.l...@gmail.com>
> Subject: [PATCH] drivers:platform/x86:Use ARRAY_SIZE() for the size
> calculation of the array.
> 
> Signed-off-by: Allen Pais <allen.l...@gmail.com>
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index d4fc42b..3ccc7ac 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -76,7 +76,7 @@
>  #define TELEM_IOSS_DX_D0IX_EVTS  25
>  #define TELEM_IOSS_PG_EVTS   30
> 
> -#define TELEM_EVT_LEN(x) (sizeof(x)/sizeof((x)[0]))
> +#define TELEM_EVT_LEN(x) (ARRAY_SIZE(x))
The outer parenthesis seems redundant to me.

> 
>  #define TELEM_DEBUGFS_CPU(model, data) \
>   { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned
> long)}
> --
> 2.7.4

BR,
Souvik

RE: [PATCH] drivers:platform/x86:Use ARRAY_SIZE() for the size calculation of the array.

[Chakravarty, Souvik K]

Hi Allen,

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Allen Pais
> Sent: Wednesday, September 13, 2017 1:33 PM
> To: linux-kernel@vger.kernel.org
> Cc: Chakravarty, Souvik K ; platform-
> driver-...@vger.kernel.org; Allen Pais 
> Subject: [PATCH] drivers:platform/x86:Use ARRAY_SIZE() for the size
> calculation of the array.
> 
> Signed-off-by: Allen Pais 
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index d4fc42b..3ccc7ac 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -76,7 +76,7 @@
>  #define TELEM_IOSS_DX_D0IX_EVTS  25
>  #define TELEM_IOSS_PG_EVTS   30
> 
> -#define TELEM_EVT_LEN(x) (sizeof(x)/sizeof((x)[0]))
> +#define TELEM_EVT_LEN(x) (ARRAY_SIZE(x))
The outer parenthesis seems redundant to me.

> 
>  #define TELEM_DEBUGFS_CPU(model, data) \
>   { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned
> long)}
> --
> 2.7.4

BR,
Souvik

RE: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices

[Chakravarty, Souvik K]

> -Original Message-
> From: sathyanarayanan kuppuswamy
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Thursday, August 3, 2017 3:29 AM
> To: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>;
> x...@kernel.org; mi...@redhat.com; Zha, Qipeng
> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; a...@infradead.org
> Cc: linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com
> Subject: Re: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework
> to create dependent devices
> 
> 
> 
> On 08/01/2017 10:14 PM, Chakravarty, Souvik K wrote:
> >
> >> -Original Message-
> >> From: platform-driver-x86-ow...@vger.kernel.org
> >> [mailto:platform-driver- x86-ow...@vger.kernel.org] On Behalf Of
> >> sathyanarayanan.kuppusw...@linux.intel.com
> >> Sent: Tuesday, August 1, 2017 11:44 PM
> >> To: x...@kernel.org; mi...@redhat.com; Zha, Qipeng
> >> <qipeng@intel.com>; h...@zytor.com; dvh...@infradead.org;
> >> t...@linutronix.de; a...@infradead.org
> >> Cc: linux-kernel@vger.kernel.org;
> >> platform-driver-...@vger.kernel.org;
> >> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >> Subject: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework
> >> to create dependent devices
> >>
> >> From: Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >>
> >> Currently, we have lot of repetitive code in dependent device
> >> resource allocation and device creation handling code. This logic can
> >> be improved if we use MFD framework for dependent device creation.
> >> This patch adds this support.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan
> >> <sathyanarayanan.kuppusw...@linux.intel.com>
> >> ---
> >>   drivers/platform/x86/intel_pmc_ipc.c | 376
> >> +---
> >> ---
> >>   1 file changed, 141 insertions(+), 235 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/intel_pmc_ipc.c
> >> b/drivers/platform/x86/intel_pmc_ipc.c
> >> index ad0416e..43533ec 100644
> >> --- a/drivers/platform/x86/intel_pmc_ipc.c
> >> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> >> @@ -33,6 +33,7 @@
> >>   #include 
> >>   #include 
> >>   #include 
> >> +#include 
> >>
> >>   #include 
> >>
> >> @@ -105,8 +106,6 @@
> >>   #define TELEM_SSRAM_SIZE 240
> >>   #define TELEM_PMC_SSRAM_OFFSET   0x1B00
> >>   #define TELEM_PUNIT_SSRAM_OFFSET 0x1A00
> >> -#define TCO_PMC_OFFSET0x8
> >> -#define TCO_PMC_SIZE  0x4
> >>
> >>   /* PMC register bit definitions */
> >>
> >> @@ -115,6 +114,13 @@
> >>   #define PMC_CFG_NO_REBOOT_EN (1 << 4)
> >>   #define PMC_CFG_NO_REBOOT_DIS(0 << 4)
> >>
> >> +enum {
> >> +  PMC_IPC_PUNIT_MFD_BLOCK,
> >> +  PMC_IPC_WATCHDOG_MFD_BLOCK,
> >> +  PMC_IPC_TELEMETRY_MFD_BLOCK,
> >> +  PMC_IPC_MAX_MFD_BLOCK
> >> +};
> >> +
> >>   static struct intel_pmc_ipc_dev {
> >>struct device *dev;
> >>void __iomem *ipc_base;
> >> @@ -123,25 +129,12 @@ static struct intel_pmc_ipc_dev {
> >>int cmd;
> >>struct completion cmd_complete;
> >>
> >> -  /* The following PMC BARs share the same ACPI device with the IPC
> >> */
> >> -  resource_size_t acpi_io_base;
> >> -  int acpi_io_size;
> >> -  struct platform_device *tco_dev;
> >> -
> >>/* gcr */
> >>void __iomem *gcr_mem_base;
> >>bool has_gcr_regs;
> >>
> >> -  /* punit */
> >> -  struct platform_device *punit_dev;
> >> -
> >>/* Telemetry */
> >> -  resource_size_t telem_pmc_ssram_base;
> >> -  resource_size_t telem_punit_ssram_base;
> >> -  int telem_pmc_ssram_size;
> >> -  int telem_punit_ssram_size;
> >>u8 telem_res_inval;
> >> -  struct platform_device *telemetry_dev;
> >>   } ipcdev;
> >>
> >>   static char *ipc_err_sources[] = {
> >> @@ -613,7 +606,7 @@ static const struct attribute_group
> >> intel_ipc_group = {
> >>.attrs = intel_ipc_attrs,
> >>   };
> >>
> >> 

RE: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices

[Chakravarty, Souvik K]

> -Original Message-
> From: sathyanarayanan kuppuswamy
> [mailto:sathyanarayanan.kuppusw...@linux.intel.com]
> Sent: Thursday, August 3, 2017 3:29 AM
> To: Chakravarty, Souvik K ;
> x...@kernel.org; mi...@redhat.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; a...@infradead.org
> Cc: linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com
> Subject: Re: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework
> to create dependent devices
> 
> 
> 
> On 08/01/2017 10:14 PM, Chakravarty, Souvik K wrote:
> >
> >> -Original Message-
> >> From: platform-driver-x86-ow...@vger.kernel.org
> >> [mailto:platform-driver- x86-ow...@vger.kernel.org] On Behalf Of
> >> sathyanarayanan.kuppusw...@linux.intel.com
> >> Sent: Tuesday, August 1, 2017 11:44 PM
> >> To: x...@kernel.org; mi...@redhat.com; Zha, Qipeng
> >> ; h...@zytor.com; dvh...@infradead.org;
> >> t...@linutronix.de; a...@infradead.org
> >> Cc: linux-kernel@vger.kernel.org;
> >> platform-driver-...@vger.kernel.org;
> >> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> >> 
> >> Subject: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework
> >> to create dependent devices
> >>
> >> From: Kuppuswamy Sathyanarayanan
> >> 
> >>
> >> Currently, we have lot of repetitive code in dependent device
> >> resource allocation and device creation handling code. This logic can
> >> be improved if we use MFD framework for dependent device creation.
> >> This patch adds this support.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan
> >> 
> >> ---
> >>   drivers/platform/x86/intel_pmc_ipc.c | 376
> >> +---
> >> ---
> >>   1 file changed, 141 insertions(+), 235 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/intel_pmc_ipc.c
> >> b/drivers/platform/x86/intel_pmc_ipc.c
> >> index ad0416e..43533ec 100644
> >> --- a/drivers/platform/x86/intel_pmc_ipc.c
> >> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> >> @@ -33,6 +33,7 @@
> >>   #include 
> >>   #include 
> >>   #include 
> >> +#include 
> >>
> >>   #include 
> >>
> >> @@ -105,8 +106,6 @@
> >>   #define TELEM_SSRAM_SIZE 240
> >>   #define TELEM_PMC_SSRAM_OFFSET   0x1B00
> >>   #define TELEM_PUNIT_SSRAM_OFFSET 0x1A00
> >> -#define TCO_PMC_OFFSET0x8
> >> -#define TCO_PMC_SIZE  0x4
> >>
> >>   /* PMC register bit definitions */
> >>
> >> @@ -115,6 +114,13 @@
> >>   #define PMC_CFG_NO_REBOOT_EN (1 << 4)
> >>   #define PMC_CFG_NO_REBOOT_DIS(0 << 4)
> >>
> >> +enum {
> >> +  PMC_IPC_PUNIT_MFD_BLOCK,
> >> +  PMC_IPC_WATCHDOG_MFD_BLOCK,
> >> +  PMC_IPC_TELEMETRY_MFD_BLOCK,
> >> +  PMC_IPC_MAX_MFD_BLOCK
> >> +};
> >> +
> >>   static struct intel_pmc_ipc_dev {
> >>struct device *dev;
> >>void __iomem *ipc_base;
> >> @@ -123,25 +129,12 @@ static struct intel_pmc_ipc_dev {
> >>int cmd;
> >>struct completion cmd_complete;
> >>
> >> -  /* The following PMC BARs share the same ACPI device with the IPC
> >> */
> >> -  resource_size_t acpi_io_base;
> >> -  int acpi_io_size;
> >> -  struct platform_device *tco_dev;
> >> -
> >>/* gcr */
> >>void __iomem *gcr_mem_base;
> >>bool has_gcr_regs;
> >>
> >> -  /* punit */
> >> -  struct platform_device *punit_dev;
> >> -
> >>/* Telemetry */
> >> -  resource_size_t telem_pmc_ssram_base;
> >> -  resource_size_t telem_punit_ssram_base;
> >> -  int telem_pmc_ssram_size;
> >> -  int telem_punit_ssram_size;
> >>u8 telem_res_inval;
> >> -  struct platform_device *telemetry_dev;
> >>   } ipcdev;
> >>
> >>   static char *ipc_err_sources[] = {
> >> @@ -613,7 +606,7 @@ static const struct attribute_group
> >> intel_ipc_group = {
> >>.attrs = intel_ipc_attrs,
> >>   };
> >>
> >> -static struct resource punit_res_array[] = {
> >> +static struct resource punit_ipc_resources[] = {
> >>/* Punit BIOS */
> >>{
> >>.flags = IORESOURCE_MEM,
> >> @@ -637,10 +

RE: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices

[Chakravarty, Souvik K]

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of
> sathyanarayanan.kuppusw...@linux.intel.com
> Sent: Tuesday, August 1, 2017 11:44 PM
> To: x...@kernel.org; mi...@redhat.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; a...@infradead.org
> Cc: linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> 
> Subject: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to
> create dependent devices
> 
> From: Kuppuswamy Sathyanarayanan
> 
> 
> Currently, we have lot of repetitive code in dependent device resource
> allocation and device creation handling code. This logic can be improved if
> we use MFD framework for dependent device creation. This patch adds this
> support.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> 
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 376 +---
> ---
>  1 file changed, 141 insertions(+), 235 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c
> b/drivers/platform/x86/intel_pmc_ipc.c
> index ad0416e..43533ec 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> @@ -105,8 +106,6 @@
>  #define TELEM_SSRAM_SIZE 240
>  #define TELEM_PMC_SSRAM_OFFSET   0x1B00
>  #define TELEM_PUNIT_SSRAM_OFFSET 0x1A00
> -#define TCO_PMC_OFFSET   0x8
> -#define TCO_PMC_SIZE 0x4
> 
>  /* PMC register bit definitions */
> 
> @@ -115,6 +114,13 @@
>  #define PMC_CFG_NO_REBOOT_EN (1 << 4)
>  #define PMC_CFG_NO_REBOOT_DIS(0 << 4)
> 
> +enum {
> + PMC_IPC_PUNIT_MFD_BLOCK,
> + PMC_IPC_WATCHDOG_MFD_BLOCK,
> + PMC_IPC_TELEMETRY_MFD_BLOCK,
> + PMC_IPC_MAX_MFD_BLOCK
> +};
> +
>  static struct intel_pmc_ipc_dev {
>   struct device *dev;
>   void __iomem *ipc_base;
> @@ -123,25 +129,12 @@ static struct intel_pmc_ipc_dev {
>   int cmd;
>   struct completion cmd_complete;
> 
> - /* The following PMC BARs share the same ACPI device with the IPC
> */
> - resource_size_t acpi_io_base;
> - int acpi_io_size;
> - struct platform_device *tco_dev;
> -
>   /* gcr */
>   void __iomem *gcr_mem_base;
>   bool has_gcr_regs;
> 
> - /* punit */
> - struct platform_device *punit_dev;
> -
>   /* Telemetry */
> - resource_size_t telem_pmc_ssram_base;
> - resource_size_t telem_punit_ssram_base;
> - int telem_pmc_ssram_size;
> - int telem_punit_ssram_size;
>   u8 telem_res_inval;
> - struct platform_device *telemetry_dev;
>  } ipcdev;
> 
>  static char *ipc_err_sources[] = {
> @@ -613,7 +606,7 @@ static const struct attribute_group intel_ipc_group
> = {
>   .attrs = intel_ipc_attrs,
>  };
> 
> -static struct resource punit_res_array[] = {
> +static struct resource punit_ipc_resources[] = {
>   /* Punit BIOS */
>   {
>   .flags = IORESOURCE_MEM,
> @@ -637,10 +630,7 @@ static struct resource punit_res_array[] = {
>   },
>  };
> 
> -#define TCO_RESOURCE_ACPI_IO 0
> -#define TCO_RESOURCE_SMI_EN_IO   1
> -#define TCO_RESOURCE_GCR_MEM 2
> -static struct resource tco_res[] = {
> +static struct resource watchdog_ipc_resources[] = {
>   /* ACPI - TCO */
>   {
>   .flags = IORESOURCE_IO,
> @@ -658,9 +648,7 @@ static struct itco_wdt_platform_data tco_info = {
>   .update_no_reboot_bit = update_no_reboot_bit,  };
> 
> -#define TELEMETRY_RESOURCE_PUNIT_SSRAM   0
> -#define TELEMETRY_RESOURCE_PMC_SSRAM 1
> -static struct resource telemetry_res[] = {
> +static struct resource telemetry_ipc_resources[] = {
>   /*Telemetry*/
>   {
>   .flags = IORESOURCE_MEM,
> @@ -670,224 +658,152 @@ static struct resource telemetry_res[] = {
>   },
>  };
> 
> -static int ipc_create_punit_device(void) -{
> - struct platform_device *pdev;
> - const struct platform_device_info pdevinfo = {
> - .parent = ipcdev.dev,
> - .name = PUNIT_DEVICE_NAME,
> - .id = -1,
> - .res = punit_res_array,
> - .num_res = ARRAY_SIZE(punit_res_array),
> - };
> -
> - pdev = platform_device_register_full();
> - if (IS_ERR(pdev))
> - return PTR_ERR(pdev);
> -
> - ipcdev.punit_dev = pdev;
> -
> - return 0;
> -}
> -
> -static int ipc_create_tco_device(void)
> -{
> - struct platform_device *pdev;
> - struct resource *res;
> - const struct platform_device_info pdevinfo = {
> - .parent = ipcdev.dev,
> - .name = TCO_DEVICE_NAME,
> - 

RE: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices

[Chakravarty, Souvik K]

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of
> sathyanarayanan.kuppusw...@linux.intel.com
> Sent: Tuesday, August 1, 2017 11:44 PM
> To: x...@kernel.org; mi...@redhat.com; Zha, Qipeng
> ; h...@zytor.com; dvh...@infradead.org;
> t...@linutronix.de; a...@infradead.org
> Cc: linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org;
> sathyao...@gmail.com; Kuppuswamy Sathyanarayanan
> 
> Subject: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to
> create dependent devices
> 
> From: Kuppuswamy Sathyanarayanan
> 
> 
> Currently, we have lot of repetitive code in dependent device resource
> allocation and device creation handling code. This logic can be improved if
> we use MFD framework for dependent device creation. This patch adds this
> support.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan
> 
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 376 +---
> ---
>  1 file changed, 141 insertions(+), 235 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c
> b/drivers/platform/x86/intel_pmc_ipc.c
> index ad0416e..43533ec 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> @@ -105,8 +106,6 @@
>  #define TELEM_SSRAM_SIZE 240
>  #define TELEM_PMC_SSRAM_OFFSET   0x1B00
>  #define TELEM_PUNIT_SSRAM_OFFSET 0x1A00
> -#define TCO_PMC_OFFSET   0x8
> -#define TCO_PMC_SIZE 0x4
> 
>  /* PMC register bit definitions */
> 
> @@ -115,6 +114,13 @@
>  #define PMC_CFG_NO_REBOOT_EN (1 << 4)
>  #define PMC_CFG_NO_REBOOT_DIS(0 << 4)
> 
> +enum {
> + PMC_IPC_PUNIT_MFD_BLOCK,
> + PMC_IPC_WATCHDOG_MFD_BLOCK,
> + PMC_IPC_TELEMETRY_MFD_BLOCK,
> + PMC_IPC_MAX_MFD_BLOCK
> +};
> +
>  static struct intel_pmc_ipc_dev {
>   struct device *dev;
>   void __iomem *ipc_base;
> @@ -123,25 +129,12 @@ static struct intel_pmc_ipc_dev {
>   int cmd;
>   struct completion cmd_complete;
> 
> - /* The following PMC BARs share the same ACPI device with the IPC
> */
> - resource_size_t acpi_io_base;
> - int acpi_io_size;
> - struct platform_device *tco_dev;
> -
>   /* gcr */
>   void __iomem *gcr_mem_base;
>   bool has_gcr_regs;
> 
> - /* punit */
> - struct platform_device *punit_dev;
> -
>   /* Telemetry */
> - resource_size_t telem_pmc_ssram_base;
> - resource_size_t telem_punit_ssram_base;
> - int telem_pmc_ssram_size;
> - int telem_punit_ssram_size;
>   u8 telem_res_inval;
> - struct platform_device *telemetry_dev;
>  } ipcdev;
> 
>  static char *ipc_err_sources[] = {
> @@ -613,7 +606,7 @@ static const struct attribute_group intel_ipc_group
> = {
>   .attrs = intel_ipc_attrs,
>  };
> 
> -static struct resource punit_res_array[] = {
> +static struct resource punit_ipc_resources[] = {
>   /* Punit BIOS */
>   {
>   .flags = IORESOURCE_MEM,
> @@ -637,10 +630,7 @@ static struct resource punit_res_array[] = {
>   },
>  };
> 
> -#define TCO_RESOURCE_ACPI_IO 0
> -#define TCO_RESOURCE_SMI_EN_IO   1
> -#define TCO_RESOURCE_GCR_MEM 2
> -static struct resource tco_res[] = {
> +static struct resource watchdog_ipc_resources[] = {
>   /* ACPI - TCO */
>   {
>   .flags = IORESOURCE_IO,
> @@ -658,9 +648,7 @@ static struct itco_wdt_platform_data tco_info = {
>   .update_no_reboot_bit = update_no_reboot_bit,  };
> 
> -#define TELEMETRY_RESOURCE_PUNIT_SSRAM   0
> -#define TELEMETRY_RESOURCE_PMC_SSRAM 1
> -static struct resource telemetry_res[] = {
> +static struct resource telemetry_ipc_resources[] = {
>   /*Telemetry*/
>   {
>   .flags = IORESOURCE_MEM,
> @@ -670,224 +658,152 @@ static struct resource telemetry_res[] = {
>   },
>  };
> 
> -static int ipc_create_punit_device(void) -{
> - struct platform_device *pdev;
> - const struct platform_device_info pdevinfo = {
> - .parent = ipcdev.dev,
> - .name = PUNIT_DEVICE_NAME,
> - .id = -1,
> - .res = punit_res_array,
> - .num_res = ARRAY_SIZE(punit_res_array),
> - };
> -
> - pdev = platform_device_register_full();
> - if (IS_ERR(pdev))
> - return PTR_ERR(pdev);
> -
> - ipcdev.punit_dev = pdev;
> -
> - return 0;
> -}
> -
> -static int ipc_create_tco_device(void)
> -{
> - struct platform_device *pdev;
> - struct resource *res;
> - const struct platform_device_info pdevinfo = {
> - .parent = ipcdev.dev,
> - .name = TCO_DEVICE_NAME,
> - .id = -1,
> - .res = tco_res,
> - .num_res = ARRAY_SIZE(tco_res),
> - .data = _info,
> - .size_data = 

RE: [PATCH v2 1/2] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

Reviewed-by: Souvik K Chakravarty <souvik.k.chakrava...@intel.com>

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Thursday, July 20, 2017 7:51 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhard...@intel.com>
> Subject: [PATCH v2 1/2] platform/x86: Add GLK PSS Event Table
> 
> Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
> events remain same. This patch adds the updated PSS event table to enable
> Telemetry driver on Gemini Lake.
> 
> Signed-off-by: Shanth Murthy <shanth.mur...@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> ---
> Changes in v2:
>  * Dropped "Add Audio domain PG status events" from series.
> 
>  drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
> drivers/platform/x86/intel_telemetry_pltdrv.c  | 35
> ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index cd21df982abd..d4fc42b4cbeb 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf
> telem_apl_debugfs_conf = {
> 
>  static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
>   TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT,
> telem_apl_debugfs_conf),
> + TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE,
> +telem_apl_debugfs_conf),
>   {}
>  };
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6ebdbd2b04fc..6393b3b1d5a6 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -153,6 +153,30 @@ static struct telemetry_evtmap
>   {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
>  };
> 
> +static struct telemetry_evtmap
> +
>   telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_E
> VENTS] = {
> + {"IA_CORE0_C6_RES", 0x0400},
> + {"IA_CORE0_C6_CTR", 0x},
> + {"IA_MODULE0_C7_RES",   0x0410},
> + {"IA_MODULE0_C7_CTR",   0x000C},
> + {"IA_C0_RES",   0x0805},
> + {"PCS_LTR", 0x2801},
> + {"PSTATES", 0x2802},
> + {"SOC_S0I3_RES",0x0407},
> + {"SOC_S0I3_CTR",0x0008},
> + {"PCS_S0I3_CTR",0x0007},
> + {"PCS_C1E_RES", 0x0414},
> + {"PCS_IDLE_STATUS", 0x2806},
> + {"IA_PERF_LIMITS",  0x280B},
> + {"GT_PERF_LIMITS",  0x280C},
> + {"PCS_WAKEUP_S0IX_CTR", 0x0025},
> + {"PCS_IDLE_BLOCKED",0x2C00},
> + {"PCS_S0IX_BLOCKED",0x2C01},
> + {"PCS_S0IX_WAKE_REASONS",   0x2C02},
> + {"PCS_LTR_BLOCKING",0x2C03},
> + {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
> +};
> +
>  /* APL specific Data */
>  static struct telemetry_plt_config telem_apl_config = {
>   .pss_config = {
> @@ -163,8 +187,19 @@ static struct telemetry_plt_config
> telem_apl_config = {
>   },
>  };
> 
> +/* GLK specific Data */
> +static struct telemetry_plt_config telem_glk_config = {
> + .pss_config = {
> + .telem_evts = telemetry_glk_pss_default_events,
> + },
> + .ioss_config = {
> + .telem_evts = telemetry_apl_ioss_default_events,
> + },
> +};
> +
>  static const struct x86_cpu_id telemetry_cpu_ids[] = {
>   TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
> + TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
>   {}
>  };
> 
> --
> 2.7.4

RE: [PATCH v2 1/2] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

Reviewed-by: Souvik K Chakravarty 

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Thursday, July 20, 2017 7:51 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth ;
> Chakravarty, Souvik K ; Bhardwaj,
> Rajneesh 
> Subject: [PATCH v2 1/2] platform/x86: Add GLK PSS Event Table
> 
> Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
> events remain same. This patch adds the updated PSS event table to enable
> Telemetry driver on Gemini Lake.
> 
> Signed-off-by: Shanth Murthy 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
> Changes in v2:
>  * Dropped "Add Audio domain PG status events" from series.
> 
>  drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
> drivers/platform/x86/intel_telemetry_pltdrv.c  | 35
> ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index cd21df982abd..d4fc42b4cbeb 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf
> telem_apl_debugfs_conf = {
> 
>  static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
>   TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT,
> telem_apl_debugfs_conf),
> + TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE,
> +telem_apl_debugfs_conf),
>   {}
>  };
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6ebdbd2b04fc..6393b3b1d5a6 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -153,6 +153,30 @@ static struct telemetry_evtmap
>   {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
>  };
> 
> +static struct telemetry_evtmap
> +
>   telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_E
> VENTS] = {
> + {"IA_CORE0_C6_RES", 0x0400},
> + {"IA_CORE0_C6_CTR", 0x},
> + {"IA_MODULE0_C7_RES",   0x0410},
> + {"IA_MODULE0_C7_CTR",   0x000C},
> + {"IA_C0_RES",   0x0805},
> + {"PCS_LTR", 0x2801},
> + {"PSTATES", 0x2802},
> + {"SOC_S0I3_RES",0x0407},
> + {"SOC_S0I3_CTR",0x0008},
> + {"PCS_S0I3_CTR",0x0007},
> + {"PCS_C1E_RES", 0x0414},
> + {"PCS_IDLE_STATUS", 0x2806},
> + {"IA_PERF_LIMITS",  0x280B},
> + {"GT_PERF_LIMITS",  0x280C},
> + {"PCS_WAKEUP_S0IX_CTR", 0x0025},
> + {"PCS_IDLE_BLOCKED",0x2C00},
> + {"PCS_S0IX_BLOCKED",0x2C01},
> + {"PCS_S0IX_WAKE_REASONS",   0x2C02},
> + {"PCS_LTR_BLOCKING",0x2C03},
> + {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
> +};
> +
>  /* APL specific Data */
>  static struct telemetry_plt_config telem_apl_config = {
>   .pss_config = {
> @@ -163,8 +187,19 @@ static struct telemetry_plt_config
> telem_apl_config = {
>   },
>  };
> 
> +/* GLK specific Data */
> +static struct telemetry_plt_config telem_glk_config = {
> + .pss_config = {
> + .telem_evts = telemetry_glk_pss_default_events,
> + },
> + .ioss_config = {
> + .telem_evts = telemetry_apl_ioss_default_events,
> + },
> +};
> +
>  static const struct x86_cpu_id telemetry_cpu_ids[] = {
>   TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
> + TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
>   {}
>  };
> 
> --
> 2.7.4

RE: [PATCH v2 2/2] Telemetry: remove redundant macro definition

[Chakravarty, Souvik K]

Just missed the email from Darren.

Reviewed-by: Souvik K Chakravarty <souvik.k.chakrava...@intel.com>

> -Original Message-
> From: Chakravarty, Souvik K
> Sent: Friday, July 21, 2017 8:45 AM
> To: 'Rajneesh Bhardwaj' <rajneesh.bhard...@intel.com>; platform-driver-
> x...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> Bhardwaj, Rajneesh <rajneesh.bhard...@intel.com>
> Subject: RE: [PATCH v2 2/2] Telemetry: remove redundant macro definition
> 
> Both set of two looks good. +1.
> 
> > -Original Message-
> > From: platform-driver-x86-ow...@vger.kernel.org
> > [mailto:platform-driver- x86-ow...@vger.kernel.org] On Behalf Of
> > Rajneesh Bhardwaj
> > Sent: Thursday, July 20, 2017 7:51 PM
> > To: platform-driver-...@vger.kernel.org
> > Cc: dvh...@infradead.org; a...@infradead.org; linux-
> > ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> > Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> > Rajneesh <rajneesh.bhard...@intel.com>
> > Subject: [PATCH v2 2/2] Telemetry: remove redundant macro definition
> >
> > Telemetry driver includes intel_telemetry.h which defines
> > TELEM_MAX_OS_ALLOCATED_EVENTS already.
> >
> > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> > ---
> >  drivers/platform/x86/intel_telemetry_pltdrv.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> > b/drivers/platform/x86/intel_telemetry_pltdrv.c
> > index 6393b3b1d5a6..e0424d5a795a 100644
> > --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> > +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> > @@ -46,7 +46,6 @@
> >  #define TELEM_SAMPLING_DEFAULT_PERIOD  0xD
> >
> >  #define TELEM_MAX_EVENTS_SRAM  28
> > -#define TELEM_MAX_OS_ALLOCATED_EVENTS  20
> >  #define TELEM_SSRAM_STARTTIME_OFFSET   8
> >  #define TELEM_SSRAM_EVTLOG_OFFSET  16
> >
> > --
> > 2.7.4

RE: [PATCH v2 2/2] Telemetry: remove redundant macro definition

[Chakravarty, Souvik K]

Just missed the email from Darren.

Reviewed-by: Souvik K Chakravarty 

> -Original Message-
> From: Chakravarty, Souvik K
> Sent: Friday, July 21, 2017 8:45 AM
> To: 'Rajneesh Bhardwaj' ; platform-driver-
> x...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth ;
> Bhardwaj, Rajneesh 
> Subject: RE: [PATCH v2 2/2] Telemetry: remove redundant macro definition
> 
> Both set of two looks good. +1.
> 
> > -Original Message-
> > From: platform-driver-x86-ow...@vger.kernel.org
> > [mailto:platform-driver- x86-ow...@vger.kernel.org] On Behalf Of
> > Rajneesh Bhardwaj
> > Sent: Thursday, July 20, 2017 7:51 PM
> > To: platform-driver-...@vger.kernel.org
> > Cc: dvh...@infradead.org; a...@infradead.org; linux-
> > ker...@vger.kernel.org; Murthy, Shanth ;
> > Chakravarty, Souvik K ; Bhardwaj,
> > Rajneesh 
> > Subject: [PATCH v2 2/2] Telemetry: remove redundant macro definition
> >
> > Telemetry driver includes intel_telemetry.h which defines
> > TELEM_MAX_OS_ALLOCATED_EVENTS already.
> >
> > Signed-off-by: Rajneesh Bhardwaj 
> > ---
> >  drivers/platform/x86/intel_telemetry_pltdrv.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> > b/drivers/platform/x86/intel_telemetry_pltdrv.c
> > index 6393b3b1d5a6..e0424d5a795a 100644
> > --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> > +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> > @@ -46,7 +46,6 @@
> >  #define TELEM_SAMPLING_DEFAULT_PERIOD  0xD
> >
> >  #define TELEM_MAX_EVENTS_SRAM  28
> > -#define TELEM_MAX_OS_ALLOCATED_EVENTS  20
> >  #define TELEM_SSRAM_STARTTIME_OFFSET   8
> >  #define TELEM_SSRAM_EVTLOG_OFFSET  16
> >
> > --
> > 2.7.4

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Friday, July 21, 2017 2:36 AM
> To: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>
> Cc: Bhardwaj, Rajneesh <rajneesh.bhard...@intel.com>; platform-driver-
> x...@vger.kernel.org; a...@infradead.org; linux-kernel@vger.kernel.org;
> Murthy, Shanth <shanth.mur...@intel.com>
> Subject: Re: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> On Mon, Jul 17, 2017 at 05:18:54AM +, Chakravarty, Souvik K wrote:
> > +1 From me.
> 
> Souvik, as the listed maintainer for this driver, what I require from you is a
> complete Acked-by or a Reviewed-by line, preferably the latter, and
> preferably after an actual review with comments. +1 has no semantic
> meaning in the Linux kernel development process, and one line approvals,
> especially from the same company, have little incremental value.
> 
> For details, please see:
> Documentation/process/5.Posting.rst
> Documentation/submitting-patches.rst

Thanks Darren...my mistake :(
Will do.

> 
> Thanks,
> 
> --
> Darren Hart
> VMware Open Source Technology Center

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Friday, July 21, 2017 2:36 AM
> To: Chakravarty, Souvik K 
> Cc: Bhardwaj, Rajneesh ; platform-driver-
> x...@vger.kernel.org; a...@infradead.org; linux-kernel@vger.kernel.org;
> Murthy, Shanth 
> Subject: Re: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> On Mon, Jul 17, 2017 at 05:18:54AM +, Chakravarty, Souvik K wrote:
> > +1 From me.
> 
> Souvik, as the listed maintainer for this driver, what I require from you is a
> complete Acked-by or a Reviewed-by line, preferably the latter, and
> preferably after an actual review with comments. +1 has no semantic
> meaning in the Linux kernel development process, and one line approvals,
> especially from the same company, have little incremental value.
> 
> For details, please see:
> Documentation/process/5.Posting.rst
> Documentation/submitting-patches.rst

Thanks Darren...my mistake :(
Will do.

> 
> Thanks,
> 
> --
> Darren Hart
> VMware Open Source Technology Center

RE: [PATCH v2 2/2] Telemetry: remove redundant macro definition

[Chakravarty, Souvik K]

Both set of two looks good. +1.

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Rajneesh Bhardwaj
> Sent: Thursday, July 20, 2017 7:51 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhard...@intel.com>
> Subject: [PATCH v2 2/2] Telemetry: remove redundant macro definition
> 
> Telemetry driver includes intel_telemetry.h which defines
> TELEM_MAX_OS_ALLOCATED_EVENTS already.
> 
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> ---
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6393b3b1d5a6..e0424d5a795a 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -46,7 +46,6 @@
>  #define TELEM_SAMPLING_DEFAULT_PERIOD0xD
> 
>  #define TELEM_MAX_EVENTS_SRAM28
> -#define TELEM_MAX_OS_ALLOCATED_EVENTS20
>  #define TELEM_SSRAM_STARTTIME_OFFSET 8
>  #define TELEM_SSRAM_EVTLOG_OFFSET16
> 
> --
> 2.7.4

RE: [PATCH v2 2/2] Telemetry: remove redundant macro definition

[Chakravarty, Souvik K]

Both set of two looks good. +1.

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Rajneesh Bhardwaj
> Sent: Thursday, July 20, 2017 7:51 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth ;
> Chakravarty, Souvik K ; Bhardwaj,
> Rajneesh 
> Subject: [PATCH v2 2/2] Telemetry: remove redundant macro definition
> 
> Telemetry driver includes intel_telemetry.h which defines
> TELEM_MAX_OS_ALLOCATED_EVENTS already.
> 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6393b3b1d5a6..e0424d5a795a 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -46,7 +46,6 @@
>  #define TELEM_SAMPLING_DEFAULT_PERIOD0xD
> 
>  #define TELEM_MAX_EVENTS_SRAM28
> -#define TELEM_MAX_OS_ALLOCATED_EVENTS20
>  #define TELEM_SSRAM_STARTTIME_OFFSET 8
>  #define TELEM_SSRAM_EVTLOG_OFFSET16
> 
> --
> 2.7.4

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Wednesday, July 19, 2017 7:08 PM
> To: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>
> Cc: platform-driver-...@vger.kernel.org; dvh...@infradead.org;
> a...@infradead.org; linux-kernel@vger.kernel.org; Murthy, Shanth
> <shanth.mur...@intel.com>
> Subject: Re: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> On Mon, Jul 17, 2017 at 10:48:30AM +0530, Chakravarty, Souvik K wrote:
> >
> >
> > > -Original Message-
> > > From: Bhardwaj, Rajneesh
> > > Sent: Friday, July 14, 2017 5:24 PM
> > > To: platform-driver-...@vger.kernel.org
> > > Cc: dvh...@infradead.org; a...@infradead.org; linux-
> > > ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> > > Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> > > Rajneesh <rajneesh.bhard...@intel.com>
> > > Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> > >
> > > From: "Murthy, Shanth" <shanth.mur...@intel.com>
> > >
> > > This patch adds events to ioss telemetry to read the power gating
> > > status for the audio domain.
> > >
> > > Signed-off-by: Shanth Murthy <shanth.mur...@intel.com>
> > > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> > > ---
> > >  arch/x86/include/asm/intel_telemetry.h| 2 +-
> > >  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/intel_telemetry.h
> > > b/arch/x86/include/asm/intel_telemetry.h
> > > index 85029b58d0cd..4eeae0a4f9a2 100644
> > > --- a/arch/x86/include/asm/intel_telemetry.h
> > > +++ b/arch/x86/include/asm/intel_telemetry.h
> > > @@ -17,7 +17,7 @@
> > >  #define INTEL_TELEMETRY_H
> > >
> > >  #define TELEM_MAX_EVENTS_SRAM28
> > > -#define TELEM_MAX_OS_ALLOCATED_EVENTS20
> > > +#define TELEM_MAX_OS_ALLOCATED_EVENTS25
> >
> > This is something that should not be done without testing. There was an
> understanding to not use more than 20 counters in OS, so that tools like
> SoCWatch could use the rest 8 without total reconfiguration of the events.
> With this change we are not leaving much for tools and it may break
> SoCwatch/IMON or other tools dependent on using just 8.
> > The better way for this would be for the userspace tool to configure the
> extra 5 events via add_config() call and not increase the OS usage ceiling.
> >
> > Or you could add some more sysfs apis for get/set events and do it via a
> daemon/script.
> >
> > 
> 
> Thanks for the review and the feedback Souvik. Ideally user space tools
> should not put a restriction such as this one. It would be good to document
> such details.
Theoretically yes. But many of the tools have no maintaince budget/owner. 

> 
> Is there a plan to support more than 20 events in telemetry driver? If not
> then we can probably think of removing few events and add audio related
> events maintaining total count of 20?

Sure...why not.

> 
> For now i will respin the patches and drop this one.

Thanks. 
I will also add support for an 'add' sysfs entry so that you can configure it 
easily and not have to fiddle with the default set.

> 
> --
> Best Regards,
> Rajneesh

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Wednesday, July 19, 2017 7:08 PM
> To: Chakravarty, Souvik K 
> Cc: platform-driver-...@vger.kernel.org; dvh...@infradead.org;
> a...@infradead.org; linux-kernel@vger.kernel.org; Murthy, Shanth
> 
> Subject: Re: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> On Mon, Jul 17, 2017 at 10:48:30AM +0530, Chakravarty, Souvik K wrote:
> >
> >
> > > -Original Message-
> > > From: Bhardwaj, Rajneesh
> > > Sent: Friday, July 14, 2017 5:24 PM
> > > To: platform-driver-...@vger.kernel.org
> > > Cc: dvh...@infradead.org; a...@infradead.org; linux-
> > > ker...@vger.kernel.org; Murthy, Shanth ;
> > > Chakravarty, Souvik K ; Bhardwaj,
> > > Rajneesh 
> > > Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> > >
> > > From: "Murthy, Shanth" 
> > >
> > > This patch adds events to ioss telemetry to read the power gating
> > > status for the audio domain.
> > >
> > > Signed-off-by: Shanth Murthy 
> > > Signed-off-by: Rajneesh Bhardwaj 
> > > ---
> > >  arch/x86/include/asm/intel_telemetry.h| 2 +-
> > >  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/intel_telemetry.h
> > > b/arch/x86/include/asm/intel_telemetry.h
> > > index 85029b58d0cd..4eeae0a4f9a2 100644
> > > --- a/arch/x86/include/asm/intel_telemetry.h
> > > +++ b/arch/x86/include/asm/intel_telemetry.h
> > > @@ -17,7 +17,7 @@
> > >  #define INTEL_TELEMETRY_H
> > >
> > >  #define TELEM_MAX_EVENTS_SRAM28
> > > -#define TELEM_MAX_OS_ALLOCATED_EVENTS20
> > > +#define TELEM_MAX_OS_ALLOCATED_EVENTS25
> >
> > This is something that should not be done without testing. There was an
> understanding to not use more than 20 counters in OS, so that tools like
> SoCWatch could use the rest 8 without total reconfiguration of the events.
> With this change we are not leaving much for tools and it may break
> SoCwatch/IMON or other tools dependent on using just 8.
> > The better way for this would be for the userspace tool to configure the
> extra 5 events via add_config() call and not increase the OS usage ceiling.
> >
> > Or you could add some more sysfs apis for get/set events and do it via a
> daemon/script.
> >
> > 
> 
> Thanks for the review and the feedback Souvik. Ideally user space tools
> should not put a restriction such as this one. It would be good to document
> such details.
Theoretically yes. But many of the tools have no maintaince budget/owner. 

> 
> Is there a plan to support more than 20 events in telemetry driver? If not
> then we can probably think of removing few events and add audio related
> events maintaining total count of 20?

Sure...why not.

> 
> For now i will respin the patches and drop this one.

Thanks. 
I will also add support for an 'add' sysfs entry so that you can configure it 
easily and not have to fiddle with the default set.

> 
> --
> Best Regards,
> Rajneesh

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

+1 From me.

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Rajneesh Bhardwaj
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhard...@intel.com>
> Subject: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
> events remain same. This patch adds the updated PSS event table to enable
> Telemetry driver on Gemini Lake.
> 
> Signed-off-by: Shanth Murthy <shanth.mur...@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
> drivers/platform/x86/intel_telemetry_pltdrv.c  | 35
> ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index 4cc2f4ea0a25..a0e4344b2eec 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf
> telem_apl_debugfs_conf = {
> 
>  static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
>   TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT,
> telem_apl_debugfs_conf),
> + TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE,
> +telem_apl_debugfs_conf),
>   {}
>  };
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6ebdbd2b04fc..6393b3b1d5a6 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -153,6 +153,30 @@ static struct telemetry_evtmap
>   {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
>  };
> 
> +static struct telemetry_evtmap
> +
>   telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_E
> VENTS] = {
> + {"IA_CORE0_C6_RES", 0x0400},
> + {"IA_CORE0_C6_CTR", 0x},
> + {"IA_MODULE0_C7_RES",   0x0410},
> + {"IA_MODULE0_C7_CTR",   0x000C},
> + {"IA_C0_RES",   0x0805},
> + {"PCS_LTR", 0x2801},
> + {"PSTATES", 0x2802},
> + {"SOC_S0I3_RES",0x0407},
> + {"SOC_S0I3_CTR",0x0008},
> + {"PCS_S0I3_CTR",0x0007},
> + {"PCS_C1E_RES", 0x0414},
> + {"PCS_IDLE_STATUS", 0x2806},
> + {"IA_PERF_LIMITS",  0x280B},
> + {"GT_PERF_LIMITS",  0x280C},
> + {"PCS_WAKEUP_S0IX_CTR", 0x0025},
> + {"PCS_IDLE_BLOCKED",0x2C00},
> + {"PCS_S0IX_BLOCKED",0x2C01},
> + {"PCS_S0IX_WAKE_REASONS",   0x2C02},
> + {"PCS_LTR_BLOCKING",0x2C03},
> + {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
> +};
> +
>  /* APL specific Data */
>  static struct telemetry_plt_config telem_apl_config = {
>   .pss_config = {
> @@ -163,8 +187,19 @@ static struct telemetry_plt_config
> telem_apl_config = {
>   },
>  };
> 
> +/* GLK specific Data */
> +static struct telemetry_plt_config telem_glk_config = {
> + .pss_config = {
> + .telem_evts = telemetry_glk_pss_default_events,
> + },
> + .ioss_config = {
> + .telem_evts = telemetry_apl_ioss_default_events,
> + },
> +};
> +
>  static const struct x86_cpu_id telemetry_cpu_ids[] = {
>   TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
> + TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
>   {}
>  };
> 
> --
> 2.7.4

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

+1 From me.

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Rajneesh Bhardwaj
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth ;
> Chakravarty, Souvik K ; Bhardwaj,
> Rajneesh 
> Subject: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
> events remain same. This patch adds the updated PSS event table to enable
> Telemetry driver on Gemini Lake.
> 
> Signed-off-by: Shanth Murthy 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
> drivers/platform/x86/intel_telemetry_pltdrv.c  | 35
> ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index 4cc2f4ea0a25..a0e4344b2eec 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf
> telem_apl_debugfs_conf = {
> 
>  static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
>   TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT,
> telem_apl_debugfs_conf),
> + TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE,
> +telem_apl_debugfs_conf),
>   {}
>  };
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6ebdbd2b04fc..6393b3b1d5a6 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -153,6 +153,30 @@ static struct telemetry_evtmap
>   {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
>  };
> 
> +static struct telemetry_evtmap
> +
>   telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_E
> VENTS] = {
> + {"IA_CORE0_C6_RES", 0x0400},
> + {"IA_CORE0_C6_CTR", 0x},
> + {"IA_MODULE0_C7_RES",   0x0410},
> + {"IA_MODULE0_C7_CTR",   0x000C},
> + {"IA_C0_RES",   0x0805},
> + {"PCS_LTR", 0x2801},
> + {"PSTATES", 0x2802},
> + {"SOC_S0I3_RES",0x0407},
> + {"SOC_S0I3_CTR",0x0008},
> + {"PCS_S0I3_CTR",0x0007},
> + {"PCS_C1E_RES", 0x0414},
> + {"PCS_IDLE_STATUS", 0x2806},
> + {"IA_PERF_LIMITS",  0x280B},
> + {"GT_PERF_LIMITS",  0x280C},
> + {"PCS_WAKEUP_S0IX_CTR", 0x0025},
> + {"PCS_IDLE_BLOCKED",0x2C00},
> + {"PCS_S0IX_BLOCKED",0x2C01},
> + {"PCS_S0IX_WAKE_REASONS",   0x2C02},
> + {"PCS_LTR_BLOCKING",0x2C03},
> + {"PC2_AND_MEM_SHALLOW_IDLE_RES",0x1D40},
> +};
> +
>  /* APL specific Data */
>  static struct telemetry_plt_config telem_apl_config = {
>   .pss_config = {
> @@ -163,8 +187,19 @@ static struct telemetry_plt_config
> telem_apl_config = {
>   },
>  };
> 
> +/* GLK specific Data */
> +static struct telemetry_plt_config telem_glk_config = {
> + .pss_config = {
> + .telem_evts = telemetry_glk_pss_default_events,
> + },
> + .ioss_config = {
> + .telem_evts = telemetry_apl_ioss_default_events,
> + },
> +};
> +
>  static const struct x86_cpu_id telemetry_cpu_ids[] = {
>   TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
> + TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
>   {}
>  };
> 
> --
> 2.7.4

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhard...@intel.com>
> Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> From: "Murthy, Shanth" <shanth.mur...@intel.com>
> 
> This patch adds events to ioss telemetry to read the power gating status for
> the audio domain.
> 
> Signed-off-by: Shanth Murthy <shanth.mur...@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> ---
>  arch/x86/include/asm/intel_telemetry.h| 2 +-
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/intel_telemetry.h
> b/arch/x86/include/asm/intel_telemetry.h
> index 85029b58d0cd..4eeae0a4f9a2 100644
> --- a/arch/x86/include/asm/intel_telemetry.h
> +++ b/arch/x86/include/asm/intel_telemetry.h
> @@ -17,7 +17,7 @@
>  #define INTEL_TELEMETRY_H
> 
>  #define TELEM_MAX_EVENTS_SRAM28
> -#define TELEM_MAX_OS_ALLOCATED_EVENTS20
> +#define TELEM_MAX_OS_ALLOCATED_EVENTS25

This is something that should not be done without testing. There was an 
understanding to not use more than 20 counters in OS, so that tools like 
SoCWatch could use the rest 8 without total reconfiguration of the events. With 
this change we are not leaving much for tools and it may break SoCwatch/IMON or 
other tools dependent on using just 8. 
The better way for this would be for the userspace tool to configure the extra 
5 events via add_config() call and not increase the OS usage ceiling.

Or you could add some more sysfs apis for get/set events and do it via a 
daemon/script.

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth ;
> Chakravarty, Souvik K ; Bhardwaj,
> Rajneesh 
> Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> From: "Murthy, Shanth" 
> 
> This patch adds events to ioss telemetry to read the power gating status for
> the audio domain.
> 
> Signed-off-by: Shanth Murthy 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
>  arch/x86/include/asm/intel_telemetry.h| 2 +-
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/intel_telemetry.h
> b/arch/x86/include/asm/intel_telemetry.h
> index 85029b58d0cd..4eeae0a4f9a2 100644
> --- a/arch/x86/include/asm/intel_telemetry.h
> +++ b/arch/x86/include/asm/intel_telemetry.h
> @@ -17,7 +17,7 @@
>  #define INTEL_TELEMETRY_H
> 
>  #define TELEM_MAX_EVENTS_SRAM28
> -#define TELEM_MAX_OS_ALLOCATED_EVENTS20
> +#define TELEM_MAX_OS_ALLOCATED_EVENTS25

This is something that should not be done without testing. There was an 
understanding to not use more than 20 counters in OS, so that tools like 
SoCWatch could use the rest 8 without total reconfiguration of the events. With 
this change we are not leaving much for tools and it may break SoCwatch/IMON or 
other tools dependent on using just 8. 
The better way for this would be for the userspace tool to configure the extra 
5 events via add_config() call and not increase the OS usage ceiling.

Or you could add some more sysfs apis for get/set events and do it via a 
daemon/script.

RE: [PATCH] x86/intel_telemetry: Add debugfs entry for S0ix residency

[Chakravarty, Souvik K]

Seems to be an useful data to provide. 
+1 from me.

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Thursday, June 22, 2017 7:12 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth <shanth.mur...@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhard...@intel.com>
> Subject: [PATCH] x86/intel_telemetry: Add debugfs entry for S0ix residency
> 
> This adds a debugfs consumer for the exported kernel API
> intel_pmc_read_s0ix_residency. This debugfs entry reads S0ix residency
> directly from the PMC hardware counters.
> 
> TEST:
> - echo freeze > /sys/power/state
> - Wake the system, read the S0ix residency i.e.
>   cat /sys/kernel/debug/telemetry/s0ix_residency_usec
> 
> Signed-off-by: Shanth Murthy <shanth.mur...@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c | 26
> ++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index efc0140..2eacb16 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -713,6 +713,24 @@ static int telem_soc_state_open(struct inode
> *inode, struct file *file)
>   .release= single_release,
>  };
> 
> +static int telem_s0ix_res_get(void *data, u64 *val) {
> + u64 s0ix_total_res = 0;
> + int ret;
> +
> + ret = intel_pmc_s0ix_counter_read(_total_res);
> + if (ret) {
> + pr_err("Failed to read S0ix residency");
> + return ret;
> + }
> +
> + *val = s0ix_total_res;
> +
> + return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(telem_s0ix_fops, telem_s0ix_res_get, NULL,
> +"%llu\n");
> +
>  static int telem_pss_trc_verb_show(struct seq_file *s, void *unused)  {
>   u32 verbosity;
> @@ -994,6 +1012,14 @@ static int __init telemetry_debugfs_init(void)
>   goto out;
>   }
> 
> + f = debugfs_create_file("s0ix_residency_usec", S_IFREG | S_IRUGO,
> + debugfs_conf->telemetry_dbg_dir,
> + NULL, _s0ix_fops);
> + if (!f) {
> + pr_err("s0ix_residency_us debugfs register failed\n");
> + goto out;
> + }
> +
>   f = debugfs_create_file("pss_trace_verbosity", S_IFREG | S_IRUGO,
>   debugfs_conf->telemetry_dbg_dir, NULL,
>   _pss_trc_verb_ops);
> --
> 1.9.1

RE: [PATCH] x86/intel_telemetry: Add debugfs entry for S0ix residency

[Chakravarty, Souvik K]

Seems to be an useful data to provide. 
+1 from me.

> -Original Message-
> From: Bhardwaj, Rajneesh
> Sent: Thursday, June 22, 2017 7:12 PM
> To: platform-driver-...@vger.kernel.org
> Cc: dvh...@infradead.org; a...@infradead.org; linux-
> ker...@vger.kernel.org; Murthy, Shanth ;
> Chakravarty, Souvik K ; Bhardwaj,
> Rajneesh 
> Subject: [PATCH] x86/intel_telemetry: Add debugfs entry for S0ix residency
> 
> This adds a debugfs consumer for the exported kernel API
> intel_pmc_read_s0ix_residency. This debugfs entry reads S0ix residency
> directly from the PMC hardware counters.
> 
> TEST:
> - echo freeze > /sys/power/state
> - Wake the system, read the S0ix residency i.e.
>   cat /sys/kernel/debug/telemetry/s0ix_residency_usec
> 
> Signed-off-by: Shanth Murthy 
> Signed-off-by: Rajneesh Bhardwaj 
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c | 26
> ++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index efc0140..2eacb16 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -713,6 +713,24 @@ static int telem_soc_state_open(struct inode
> *inode, struct file *file)
>   .release= single_release,
>  };
> 
> +static int telem_s0ix_res_get(void *data, u64 *val) {
> + u64 s0ix_total_res = 0;
> + int ret;
> +
> + ret = intel_pmc_s0ix_counter_read(_total_res);
> + if (ret) {
> + pr_err("Failed to read S0ix residency");
> + return ret;
> + }
> +
> + *val = s0ix_total_res;
> +
> + return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(telem_s0ix_fops, telem_s0ix_res_get, NULL,
> +"%llu\n");
> +
>  static int telem_pss_trc_verb_show(struct seq_file *s, void *unused)  {
>   u32 verbosity;
> @@ -994,6 +1012,14 @@ static int __init telemetry_debugfs_init(void)
>   goto out;
>   }
> 
> + f = debugfs_create_file("s0ix_residency_usec", S_IFREG | S_IRUGO,
> + debugfs_conf->telemetry_dbg_dir,
> + NULL, _s0ix_fops);
> + if (!f) {
> + pr_err("s0ix_residency_us debugfs register failed\n");
> + goto out;
> + }
> +
>   f = debugfs_create_file("pss_trace_verbosity", S_IFREG | S_IRUGO,
>   debugfs_conf->telemetry_dbg_dir, NULL,
>   _pss_trc_verb_ops);
> --
> 1.9.1

RE: [PATCH v1 1/1] intel_telemetry_debugfs: fix oops found while load/unload module test

[Chakravarty, Souvik K]

Thanks for catching this.

BR,
Souvik

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of priyalee.kushw...@intel.com
> Sent: Saturday, May 27, 2017 8:48 PM
> To: dvh...@infradead.org; Chakravarty, Souvik K
> <souvik.k.chakrava...@intel.com>; a...@infradead.org
> Cc: linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org;
> Kushwaha, Priyalee <priyalee.kushw...@intel.com>
> Subject: [PATCH v1 1/1] intel_telemetry_debugfs: fix oops found while
> load/unload module test
> 
> From: Priyalee Kushwaha <priyalee.kushw...@intel.com>
> 
> This fix oops found while testing load/unload test of
> intel_telemetry_debugfs module. Module_init uses register_pm_notifier
> for PM callbacks, but unregister_pm_notifier was missing from
> module_exit.
> 
>  [ 97.481860] BUG: unable to handle kernel paging request at
> a006f010  [ 97.489742] IP:
> blocking_notifier_chain_register+0x3a/0xa0
>  [ 97.495898] PGD 2e0a067
>  [ 97.495899] PUD 2e0b063
>  [ 97.498737] PMD 179e29067
>  [ 97.501573] PTE 0
> 
>  [ 97.508423] Oops:  1 PREEMPT SMP
>  [ 97.512724] Modules linked in: intel_telemetry_debugfs intel_rapl
> gpio_keys dwc3 udc_core intel_telemetry_pltdrv intel_punit_ipc
> intel_telemetry_core rtc_cmos efivars x86_pkg_temp_thermal iwlwifi
> snd_hda_codec_hdmi soc_button_array btusb cfg80211 btrtl mei_me
> hci_uart btbcm mei btintel i915 bluetooth intel_pmc_ipc snd_hda_intel
> spi_pxa2xx_platform snd_hda_codec dwc3_pci snd_hda_core tpm_tis
> tpm_tis_core tpm efivarfs  [ 97.558453] CPU: 0 PID: 889 Comm: modprobe
> Not tainted 4.11.0-rc6-intel-dev-bkc #1  [ 97.566950] Hardware name: Intel
> Corp. Joule DVT3/SDS, BIOS GTPP181A.X64.0143.B30.1701132137
> 01/13/2017  [ 97.577518] task: 8801793a21c0 task.stack:
> 8801793f  [ 97.584162] RIP:
> 0010:blocking_notifier_chain_register+0x3a/0xa0
>  [ 97.590903] RSP: 0018:8801793f3c58 EFLAGS: 00010286  [
> 97.596802] RAX: a006f000 RBX: 81e3ea20 RCX:
>   [ 97.604812] RDX: 880179eaf210 RSI:
> a0131000 RDI: 81e3ea20  [ 97.612821] RBP: 8801793f3c68
> R08: 0006 R09: 005c  [ 97.620847] R10:
>  R11: 0006 R12: a0131000  [
> 97.628855] R13:  R14: 880176e35f48 R15:
> 8801793f3ea8  [ 97.636865] FS: 7f7eeba07700()
> GS:88017fc0() knlGS:  [ 97.645948] CS:
> 0010 DS:  ES:  CR0: 80050033  [ 97.652423] CR2:
> a006f010 CR3: 0001775ef000 CR4: 003406f0  [
> 97.660423] Call Trace:
>  [ 97.663166] ? 0xa0031000
>  [ 97.666885] register_pm_notifier+0x18/0x20  [ 97.671581]
> telemetry_debugfs_init+0x92/0x1000
> 
> Signed-off-by: Priyalee Kushwaha <priyalee.kushw...@intel.com>
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index ef29f18..0f93975 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -966,8 +966,12 @@ static int __init telemetry_debugfs_init(void)
> #endif /* CONFIG_PM_SLEEP */
> 
>   debugfs_conf->telemetry_dbg_dir =
> debugfs_create_dir("telemetry", NULL);
> - if (!debugfs_conf->telemetry_dbg_dir)
> + if (!debugfs_conf->telemetry_dbg_dir) { #ifdef CONFIG_PM_SLEEP
> + unregister_pm_notifier(_notifier);
> +#endif /* CONFIG_PM_SLEEP */
>   return -ENOMEM;
> + }
> 
>   f = debugfs_create_file("pss_info", S_IFREG | S_IRUGO,
>   debugfs_conf->telemetry_dbg_dir, NULL,
> @@ -1014,6 +1018,9 @@ static int __init telemetry_debugfs_init(void)
>  out:
>   debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir);
>   debugfs_conf->telemetry_dbg_dir = NULL;
> +#ifdef CONFIG_PM_SLEEP
> + unregister_pm_notifier(_notifier);
> +#endif /* CONFIG_PM_SLEEP */
> 
>   return err;
>  }
> @@ -1022,6 +1029,9 @@ static void __exit telemetry_debugfs_exit(void)
> {
>   debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir);
>   debugfs_conf->telemetry_dbg_dir = NULL;
> +#ifdef CONFIG_PM_SLEEP
> + unregister_pm_notifier(_notifier);
> +#endif /* CONFIG_PM_SLEEP */
>  }
> 
>  late_initcall(telemetry_debugfs_init);
> --
> 2.10.0

RE: [PATCH v1 1/1] intel_telemetry_debugfs: fix oops found while load/unload module test

[Chakravarty, Souvik K]

Thanks for catching this.

BR,
Souvik

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of priyalee.kushw...@intel.com
> Sent: Saturday, May 27, 2017 8:48 PM
> To: dvh...@infradead.org; Chakravarty, Souvik K
> ; a...@infradead.org
> Cc: linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org;
> Kushwaha, Priyalee 
> Subject: [PATCH v1 1/1] intel_telemetry_debugfs: fix oops found while
> load/unload module test
> 
> From: Priyalee Kushwaha 
> 
> This fix oops found while testing load/unload test of
> intel_telemetry_debugfs module. Module_init uses register_pm_notifier
> for PM callbacks, but unregister_pm_notifier was missing from
> module_exit.
> 
>  [ 97.481860] BUG: unable to handle kernel paging request at
> a006f010  [ 97.489742] IP:
> blocking_notifier_chain_register+0x3a/0xa0
>  [ 97.495898] PGD 2e0a067
>  [ 97.495899] PUD 2e0b063
>  [ 97.498737] PMD 179e29067
>  [ 97.501573] PTE 0
> 
>  [ 97.508423] Oops:  1 PREEMPT SMP
>  [ 97.512724] Modules linked in: intel_telemetry_debugfs intel_rapl
> gpio_keys dwc3 udc_core intel_telemetry_pltdrv intel_punit_ipc
> intel_telemetry_core rtc_cmos efivars x86_pkg_temp_thermal iwlwifi
> snd_hda_codec_hdmi soc_button_array btusb cfg80211 btrtl mei_me
> hci_uart btbcm mei btintel i915 bluetooth intel_pmc_ipc snd_hda_intel
> spi_pxa2xx_platform snd_hda_codec dwc3_pci snd_hda_core tpm_tis
> tpm_tis_core tpm efivarfs  [ 97.558453] CPU: 0 PID: 889 Comm: modprobe
> Not tainted 4.11.0-rc6-intel-dev-bkc #1  [ 97.566950] Hardware name: Intel
> Corp. Joule DVT3/SDS, BIOS GTPP181A.X64.0143.B30.1701132137
> 01/13/2017  [ 97.577518] task: 8801793a21c0 task.stack:
> 8801793f  [ 97.584162] RIP:
> 0010:blocking_notifier_chain_register+0x3a/0xa0
>  [ 97.590903] RSP: 0018:8801793f3c58 EFLAGS: 00010286  [
> 97.596802] RAX: a006f000 RBX: 81e3ea20 RCX:
>   [ 97.604812] RDX: 880179eaf210 RSI:
> a0131000 RDI: 81e3ea20  [ 97.612821] RBP: 8801793f3c68
> R08: 0006 R09: 005c  [ 97.620847] R10:
>  R11: 0006 R12: a0131000  [
> 97.628855] R13:  R14: 880176e35f48 R15:
> 8801793f3ea8  [ 97.636865] FS: 7f7eeba07700()
> GS:88017fc0() knlGS:  [ 97.645948] CS:
> 0010 DS:  ES:  CR0: 80050033  [ 97.652423] CR2:
> a006f010 CR3: 0001775ef000 CR4: 003406f0  [
> 97.660423] Call Trace:
>  [ 97.663166] ? 0xa0031000
>  [ 97.666885] register_pm_notifier+0x18/0x20  [ 97.671581]
> telemetry_debugfs_init+0x92/0x1000
> 
> Signed-off-by: Priyalee Kushwaha 
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index ef29f18..0f93975 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -966,8 +966,12 @@ static int __init telemetry_debugfs_init(void)
> #endif /* CONFIG_PM_SLEEP */
> 
>   debugfs_conf->telemetry_dbg_dir =
> debugfs_create_dir("telemetry", NULL);
> - if (!debugfs_conf->telemetry_dbg_dir)
> + if (!debugfs_conf->telemetry_dbg_dir) { #ifdef CONFIG_PM_SLEEP
> + unregister_pm_notifier(_notifier);
> +#endif /* CONFIG_PM_SLEEP */
>   return -ENOMEM;
> + }
> 
>   f = debugfs_create_file("pss_info", S_IFREG | S_IRUGO,
>   debugfs_conf->telemetry_dbg_dir, NULL,
> @@ -1014,6 +1018,9 @@ static int __init telemetry_debugfs_init(void)
>  out:
>   debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir);
>   debugfs_conf->telemetry_dbg_dir = NULL;
> +#ifdef CONFIG_PM_SLEEP
> + unregister_pm_notifier(_notifier);
> +#endif /* CONFIG_PM_SLEEP */
> 
>   return err;
>  }
> @@ -1022,6 +1029,9 @@ static void __exit telemetry_debugfs_exit(void)
> {
>   debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir);
>   debugfs_conf->telemetry_dbg_dir = NULL;
> +#ifdef CONFIG_PM_SLEEP
> + unregister_pm_notifier(_notifier);
> +#endif /* CONFIG_PM_SLEEP */
>  }
> 
>  late_initcall(telemetry_debugfs_init);
> --
> 2.10.0

RE: [PATCH] platform:x86 Remove Monitor MWAIT feature dependency

[Chakravarty, Souvik K]

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Wednesday, June 29, 2016 1:54 AM
> To: Yu, Ong Hock <ong.hock...@intel.com>
> Cc: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; platform-driver-
> x...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] platform:x86 Remove Monitor MWAIT feature
> dependency
> 
> On Thu, Jun 23, 2016 at 02:59:58PM +0800, ong.hock...@intel.com wrote:
> > From: "Yu, Ong Hock" <ong.hock...@intel.com>
> >
> > Telemetry capability does not depend on Monitor MWAIT feature.
> >
> > Signed-off-by: Yu, Ong Hock <ong.hock...@intel.com>
Acked-by: Souvik Kumar Chakravarty <souvik.k.chakrava...@intel.com>

> 
> Souvik, any objection?
Seems reasonable, since I have not heard anything break from the Chrome guys as 
well.

> 
> > ---
> >  drivers/platform/x86/intel_telemetry_debugfs.c | 2 +-
> > drivers/platform/x86/intel_telemetry_pltdrv.c  | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >

RE: [PATCH] platform:x86 Remove Monitor MWAIT feature dependency

[Chakravarty, Souvik K]

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Wednesday, June 29, 2016 1:54 AM
> To: Yu, Ong Hock 
> Cc: Chakravarty, Souvik K ; platform-driver-
> x...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] platform:x86 Remove Monitor MWAIT feature
> dependency
> 
> On Thu, Jun 23, 2016 at 02:59:58PM +0800, ong.hock...@intel.com wrote:
> > From: "Yu, Ong Hock" 
> >
> > Telemetry capability does not depend on Monitor MWAIT feature.
> >
> > Signed-off-by: Yu, Ong Hock 
Acked-by: Souvik Kumar Chakravarty 

> 
> Souvik, any objection?
Seems reasonable, since I have not heard anything break from the Chrome guys as 
well.

> 
> > ---
> >  drivers/platform/x86/intel_telemetry_debugfs.c | 2 +-
> > drivers/platform/x86/intel_telemetry_pltdrv.c  | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >

RE: [patch] intel_telemetry_pltdrv: silence an unintialized variable warning

[Chakravarty, Souvik K]

Looks good...thanks Dan.

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Friday, April 15, 2016 8:16 PM
> To: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>
> Cc: Darren Hart <dvh...@infradead.org>; platform-driver-
> x...@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> janit...@vger.kernel.org
> Subject: [patch] intel_telemetry_pltdrv: silence an unintialized variable
> warning
> 
> Presumably "pss_period" and "ioss_period" can't both be zero, but this
> function is never called so we can't infer that using static analysis alone.
> 
> Let's silence the warning by setting "ret" to zero.
> 
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 397119f..781bd10 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -659,7 +659,7 @@ static int telemetry_plt_update_events(struct
> telemetry_evtconfig pss_evtconfig,  static int
> telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)  {
>   u32 telem_ctrl = 0;
> - int ret;
> + int ret = 0;
> 
>   mutex_lock(&(telm_conf->telem_lock));
>   if (ioss_period) {

RE: [patch] intel_telemetry_pltdrv: silence an unintialized variable warning

[Chakravarty, Souvik K]

Looks good...thanks Dan.

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Friday, April 15, 2016 8:16 PM
> To: Chakravarty, Souvik K 
> Cc: Darren Hart ; platform-driver-
> x...@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> janit...@vger.kernel.org
> Subject: [patch] intel_telemetry_pltdrv: silence an unintialized variable
> warning
> 
> Presumably "pss_period" and "ioss_period" can't both be zero, but this
> function is never called so we can't infer that using static analysis alone.
> 
> Let's silence the warning by setting "ret" to zero.
> 
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 397119f..781bd10 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -659,7 +659,7 @@ static int telemetry_plt_update_events(struct
> telemetry_evtconfig pss_evtconfig,  static int
> telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)  {
>   u32 telem_ctrl = 0;
> - int ret;
> + int ret = 0;
> 
>   mutex_lock(&(telm_conf->telem_lock));
>   if (ioss_period) {

RE: [PATCH] platform:x86 decouple telemetry driver from the optional IPC resources

[Chakravarty, Souvik K]

> -Original Message-
> From: Zha, Qipeng
> Sent: Monday, April 11, 2016 7:34 AM
> To: Darren Hart <dvh...@infradead.org>; Aubrey Li
> <aubrey...@linux.intel.com>; Chakravarty, Souvik K
> <souvik.k.chakrava...@intel.com>
> Cc: platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] platform:x86 decouple telemetry driver from the
> optional IPC resources
> 
> >> Currently the optional IPC resources prevent telemetry driver from
> >> probing if these resources are not in ACPI table. This patch
> >> decouples telemetry driver from these optional resources, so that
> >> telemetry driver has dependency only on the necessary ACPI resources.
> >>
> >> Signed-off-by: Aubrey Li <aubrey...@linux.intel.com>
> 
> >Given the impact to their recent contributions, I'm looking for reviews from
> Qipeng and Souvik before I merge this.
> 
> >Qipeng, as the listed maintainer for these two files, I particularly need to
> hear from you.
> 
> >Thanks,
> 
> Hi Darren,  I think this is a reasonable solution for such issue.
> Thanks Aubrey for this fixing patch.

We went over this internally once. So OK from my POV.

RE: [PATCH] platform:x86 decouple telemetry driver from the optional IPC resources

[Chakravarty, Souvik K]

> -Original Message-
> From: Zha, Qipeng
> Sent: Monday, April 11, 2016 7:34 AM
> To: Darren Hart ; Aubrey Li
> ; Chakravarty, Souvik K
> 
> Cc: platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] platform:x86 decouple telemetry driver from the
> optional IPC resources
> 
> >> Currently the optional IPC resources prevent telemetry driver from
> >> probing if these resources are not in ACPI table. This patch
> >> decouples telemetry driver from these optional resources, so that
> >> telemetry driver has dependency only on the necessary ACPI resources.
> >>
> >> Signed-off-by: Aubrey Li 
> 
> >Given the impact to their recent contributions, I'm looking for reviews from
> Qipeng and Souvik before I merge this.
> 
> >Qipeng, as the listed maintainer for these two files, I particularly need to
> hear from you.
> 
> >Thanks,
> 
> Hi Darren,  I think this is a reasonable solution for such issue.
> Thanks Aubrey for this fixing patch.

We went over this internally once. So OK from my POV.

RE: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs interfaces

[Chakravarty, Souvik K]

Or rather I will send this in as a fix.

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Chakravarty, Souvik K
> Sent: Thursday, January 21, 2016 8:21 AM
> To: Darren Hart ; Joe Perches 
> Cc: platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs
> interfaces
> 
> 
> 
> > -Original Message-
> > From: Darren Hart [mailto:dvh...@infradead.org]
> > Sent: Thursday, January 21, 2016 5:05 AM
> > To: Joe Perches 
> > Cc: Chakravarty, Souvik K ; platform-
> > driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs
> > interfaces
> >
> > On Tue, Jan 19, 2016 at 11:00:46PM -0800, Joe Perches wrote:
> > > On Wed, 2016-01-20 at 11:34 +0530, Souvik Kumar Chakravarty wrote:
> > > > This implements debugfs interfaces for reading the telemetry
> > > > samples from SSRAM and configuring firmware trace verbosity.
> > > []
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > []
> > > > @@ -5698,6 +5698,7 @@ S:Maintained
> > > >  F: drivers/platform/x86/intel_telemetry_core.c
> > > >  F: arch/x86/include/asm/intel_telemetry.h
> > > >  F: drivers/platform/x86/intel_telemetry_pltdrv.c
> > > > +F: drivers/platform/x86/intel_telemetry_debugfs.c
> > >
> > > Maybe most of these should be deleted and this should be a single
> > > entry:
> > >
> > > F:drivers/platform/x86/intel_telemetry*
> >
> > Indeed, thanks for catching that Joe.
> 
> Seems fair. I can do a respin.
> >
> > --
> > Darren Hart
> > Intel Open Source Technology Center
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86"
> in the body of a message to majord...@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs interfaces

[Chakravarty, Souvik K]

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Thursday, January 21, 2016 5:05 AM
> To: Joe Perches 
> Cc: Chakravarty, Souvik K ; platform-
> driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs
> interfaces
> 
> On Tue, Jan 19, 2016 at 11:00:46PM -0800, Joe Perches wrote:
> > On Wed, 2016-01-20 at 11:34 +0530, Souvik Kumar Chakravarty wrote:
> > > This implements debugfs interfaces for reading the telemetry samples
> > > from SSRAM and configuring firmware trace verbosity.
> > []
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > []
> > > @@ -5698,6 +5698,7 @@ S:  Maintained
> > >  F:   drivers/platform/x86/intel_telemetry_core.c
> > >  F:   arch/x86/include/asm/intel_telemetry.h
> > >  F:   drivers/platform/x86/intel_telemetry_pltdrv.c
> > > +F:   drivers/platform/x86/intel_telemetry_debugfs.c
> >
> > Maybe most of these should be deleted and this should be a single
> > entry:
> >
> > F:  drivers/platform/x86/intel_telemetry*
> 
> Indeed, thanks for catching that Joe.

Seems fair. I can do a respin.
> 
> --
> Darren Hart
> Intel Open Source Technology Center

RE: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs interfaces

[Chakravarty, Souvik K]

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Thursday, January 21, 2016 5:05 AM
> To: Joe Perches <j...@perches.com>
> Cc: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; platform-
> driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs
> interfaces
> 
> On Tue, Jan 19, 2016 at 11:00:46PM -0800, Joe Perches wrote:
> > On Wed, 2016-01-20 at 11:34 +0530, Souvik Kumar Chakravarty wrote:
> > > This implements debugfs interfaces for reading the telemetry samples
> > > from SSRAM and configuring firmware trace verbosity.
> > []
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > []
> > > @@ -5698,6 +5698,7 @@ S:  Maintained
> > >  F:   drivers/platform/x86/intel_telemetry_core.c
> > >  F:   arch/x86/include/asm/intel_telemetry.h
> > >  F:   drivers/platform/x86/intel_telemetry_pltdrv.c
> > > +F:   drivers/platform/x86/intel_telemetry_debugfs.c
> >
> > Maybe most of these should be deleted and this should be a single
> > entry:
> >
> > F:  drivers/platform/x86/intel_telemetry*
> 
> Indeed, thanks for catching that Joe.

Seems fair. I can do a respin.
> 
> --
> Darren Hart
> Intel Open Source Technology Center

RE: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs interfaces

[Chakravarty, Souvik K]

Or rather I will send this in as a fix.

> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-
> x86-ow...@vger.kernel.org] On Behalf Of Chakravarty, Souvik K
> Sent: Thursday, January 21, 2016 8:21 AM
> To: Darren Hart <dvh...@infradead.org>; Joe Perches <j...@perches.com>
> Cc: platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs
> interfaces
> 
> 
> 
> > -Original Message-
> > From: Darren Hart [mailto:dvh...@infradead.org]
> > Sent: Thursday, January 21, 2016 5:05 AM
> > To: Joe Perches <j...@perches.com>
> > Cc: Chakravarty, Souvik K <souvik.k.chakrava...@intel.com>; platform-
> > driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 4/4] platform:x86: Add Intel Telemetry Debugfs
> > interfaces
> >
> > On Tue, Jan 19, 2016 at 11:00:46PM -0800, Joe Perches wrote:
> > > On Wed, 2016-01-20 at 11:34 +0530, Souvik Kumar Chakravarty wrote:
> > > > This implements debugfs interfaces for reading the telemetry
> > > > samples from SSRAM and configuring firmware trace verbosity.
> > > []
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > []
> > > > @@ -5698,6 +5698,7 @@ S:Maintained
> > > >  F: drivers/platform/x86/intel_telemetry_core.c
> > > >  F: arch/x86/include/asm/intel_telemetry.h
> > > >  F: drivers/platform/x86/intel_telemetry_pltdrv.c
> > > > +F: drivers/platform/x86/intel_telemetry_debugfs.c
> > >
> > > Maybe most of these should be deleted and this should be a single
> > > entry:
> > >
> > > F:drivers/platform/x86/intel_telemetry*
> >
> > Indeed, thanks for catching that Joe.
> 
> Seems fair. I can do a respin.
> >
> > --
> > Darren Hart
> > Intel Open Source Technology Center
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86"
> in the body of a message to majord...@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html