Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754180AbdDDNvz (ORCPT ); Tue, 4 Apr 2017 09:51:55 -0400 Received: from mail-qk0-f195.google.com ([209.85.220.195]:35639 "EHLO mail-qk0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752202AbdDDNvx (ORCPT ); Tue, 4 Apr 2017 09:51:53 -0400 MIME-Version: 1.0 In-Reply-To: <8aad93502f17d05367588ea0c17b01b450a1385a.1491264643.git.sathyanarayanan.kuppuswamy@linux.intel.com> References: <8aad93502f17d05367588ea0c17b01b450a1385a.1491264643.git.sathyanarayanan.kuppuswamy@linux.intel.com> From: Andy Shevchenko Date: Tue, 4 Apr 2017 16:51:51 +0300 Message-ID: Subject: Re: [PATCH v5 6/6] platform/x86: intel_pmc_ipc: use gcr mem base for S0ix counter read To: Kuppuswamy Sathyanarayanan Cc: Andy Shevchenko , Zha Qipeng , "dvhart@infradead.org" , Guenter Roeck , Wim Van Sebroeck , Sathyanarayanan Kuppuswamy Natarajan , David Box , Rajneesh Bhardwaj , Platform Driver , "linux-kernel@vger.kernel.org" , linux-watchdog@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3331 Lines: 100 On Tue, Apr 4, 2017 at 3:24 AM, Kuppuswamy Sathyanarayanan wrote: > To maintain the uniformity in accessing GCR registers, this patch > modifies the S0ix counter read function to use GCR address base > instead of ipc address base. > This one looks good. --- 8< --- 8< --- Overall, I do not like this spreading interface where we have more and more functions which use some global variable without clear understanding of device / initialization dependencies. Better approach would be something using opaque pointers and API which takes it as an input. I didn't investigate yet about something similar to UCLASS_SYSCON (this is nice model of dependency used in U-Boot) in Linux kernel, it would be cool to use it. So, the priority number #2 (number #1 is to fix interrupt handling for Whiskey Cove) is to refactor this all mess. Like we agreed it would be last series I'm going to apply without refactoring done. --- 8< --- 8< --- > Signed-off-by: Kuppuswamy Sathyanarayanan > Reviewed-by: Rajneesh Bhardwaj > Tested-by: Shanth Murthy > --- > arch/x86/include/asm/intel_pmc_ipc.h | 2 ++ > drivers/platform/x86/intel_pmc_ipc.c | 10 +++------- > 2 files changed, 5 insertions(+), 7 deletions(-) > > Changes since v4: > * Rebased on top of latest changes. > > Changes since v3: > * Rebased on top of latest changes. > > diff --git a/arch/x86/include/asm/intel_pmc_ipc.h b/arch/x86/include/asm/intel_pmc_ipc.h > index 8402efe..fac89eb 100644 > --- a/arch/x86/include/asm/intel_pmc_ipc.h > +++ b/arch/x86/include/asm/intel_pmc_ipc.h > @@ -25,6 +25,8 @@ > > /* GCR reg offsets from gcr base*/ > #define PMC_GCR_PMC_CFG_REG 0x08 > +#define PMC_GCR_TELEM_DEEP_S0IX_REG 0x78 > +#define PMC_GCR_TELEM_SHLW_S0IX_REG 0x80 > > #if IS_ENABLED(CONFIG_INTEL_PMC_IPC) > > diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c > index 3d0d6f17..b61f569 100644 > --- a/drivers/platform/x86/intel_pmc_ipc.c > +++ b/drivers/platform/x86/intel_pmc_ipc.c > @@ -57,10 +57,6 @@ > #define IPC_WRITE_BUFFER 0x80 > #define IPC_READ_BUFFER 0x90 > > -/* PMC Global Control Registers */ > -#define GCR_TELEM_DEEP_S0IX_OFFSET 0x1078 > -#define GCR_TELEM_SHLW_S0IX_OFFSET 0x1080 > - > /* Residency with clock rate at 19.2MHz to usecs */ > #define S0IX_RESIDENCY_IN_USECS(d, s) \ > ({ \ > @@ -203,7 +199,7 @@ static inline u32 ipc_data_readl(u32 offset) > > static inline u64 gcr_data_readq(u32 offset) > { > - return readq(ipcdev.ipc_base + offset); > + return readq(ipcdev.gcr_mem_base + offset); > } > > static inline int is_gcr_valid(u32 offset) > @@ -906,8 +902,8 @@ int intel_pmc_s0ix_counter_read(u64 *data) > if (!ipcdev.has_gcr_regs) > return -EACCES; > > - deep = gcr_data_readq(GCR_TELEM_DEEP_S0IX_OFFSET); > - shlw = gcr_data_readq(GCR_TELEM_SHLW_S0IX_OFFSET); > + deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_REG); > + shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_REG); > > *data = S0IX_RESIDENCY_IN_USECS(deep, shlw); > > -- > 2.7.4 > -- With Best Regards, Andy Shevchenko