From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755322AbdDFViE (ORCPT ); Thu, 6 Apr 2017 17:38:04 -0400 Received: from mail-qk0-f182.google.com ([209.85.220.182]:36110 "EHLO mail-qk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752194AbdDFVhz (ORCPT ); Thu, 6 Apr 2017 17:37:55 -0400 MIME-Version: 1.0 In-Reply-To: References: <463a23bb1f44098dd0fe506346bae71282009e05.1491428146.git.sathyanarayanan.kuppuswamy@linux.intel.com> <3e7ca77c-2be5-5f89-f51f-faaca9f2d9ac@linux.intel.com> From: Andy Shevchenko Date: Fri, 7 Apr 2017 00:37:54 +0300 Message-ID: Subject: Re: [PATCH v6 5/6] platform/x86: intel_pmc_ipc: Fix iTCO_wdt GCS memory mapping failure 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 On Thu, Apr 6, 2017 at 1:54 AM, Kuppuswamy Sathyanarayanan wrote: > iTCO_wdt driver need access to PMC_CFG GCR register to modify the > noreboot setting. Currently, this is done by passing PMC_CFG reg > address as memory resource to watchdog driver and allowing it directly > modify the PMC_CFG register. But currently PMC driver also has > requirement to memory map the entire GCR register space in this driver. > This causes mem request failure in watchdog driver. So this patch fixes > this issue by adding API to update noreboot flag and passes them > to watchdog driver via platform data. > +/* PMC_CFG_REG bit masks */ > +#define PMC_CFG_NO_REBOOT_MASK BIT(4) > +#define PMC_CFG_NO_REBOOT_ENABLE BIT(4) > +#define PMC_CFG_NO_REBOOT_DISABLE 0 I would go for those like (1 << 4) and (0 << 4) which for my opinion is better to get a picture. > +static int update_no_reboot_bit(void *priv, bool set) > +{ > + if (set) > + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, > + PMC_CFG_NO_REBOOT_MASK, > + PMC_CFG_NO_REBOOT_ENABLE); > + > + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, > + PMC_CFG_NO_REBOOT_MASK, > + PMC_CFG_NO_REBOOT_DISABLE); u32 value = set ? PMC_CFG_NO_REBOOT_ENABLE : PMC_CFG_NO_REBOOT_DISABLE; (also you may consider to use just *_EN and *_DIS) return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, PMC_CFG_NO_REBOOT_MASK, value); > +} -- With Best Regards, Andy Shevchenko From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-qk0-f182.google.com ([209.85.220.182]:36110 "EHLO mail-qk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752194AbdDFVhz (ORCPT ); Thu, 6 Apr 2017 17:37:55 -0400 MIME-Version: 1.0 In-Reply-To: References: <463a23bb1f44098dd0fe506346bae71282009e05.1491428146.git.sathyanarayanan.kuppuswamy@linux.intel.com> <3e7ca77c-2be5-5f89-f51f-faaca9f2d9ac@linux.intel.com> From: Andy Shevchenko Date: Fri, 7 Apr 2017 00:37:54 +0300 Message-ID: Subject: Re: [PATCH v6 5/6] platform/x86: intel_pmc_ipc: Fix iTCO_wdt GCS memory mapping failure 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-watchdog-owner@vger.kernel.org List-Id: linux-watchdog@vger.kernel.org On Thu, Apr 6, 2017 at 1:54 AM, Kuppuswamy Sathyanarayanan wrote: > iTCO_wdt driver need access to PMC_CFG GCR register to modify the > noreboot setting. Currently, this is done by passing PMC_CFG reg > address as memory resource to watchdog driver and allowing it directly > modify the PMC_CFG register. But currently PMC driver also has > requirement to memory map the entire GCR register space in this driver. > This causes mem request failure in watchdog driver. So this patch fixes > this issue by adding API to update noreboot flag and passes them > to watchdog driver via platform data. > +/* PMC_CFG_REG bit masks */ > +#define PMC_CFG_NO_REBOOT_MASK BIT(4) > +#define PMC_CFG_NO_REBOOT_ENABLE BIT(4) > +#define PMC_CFG_NO_REBOOT_DISABLE 0 I would go for those like (1 << 4) and (0 << 4) which for my opinion is better to get a picture. > +static int update_no_reboot_bit(void *priv, bool set) > +{ > + if (set) > + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, > + PMC_CFG_NO_REBOOT_MASK, > + PMC_CFG_NO_REBOOT_ENABLE); > + > + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, > + PMC_CFG_NO_REBOOT_MASK, > + PMC_CFG_NO_REBOOT_DISABLE); u32 value = set ? PMC_CFG_NO_REBOOT_ENABLE : PMC_CFG_NO_REBOOT_DISABLE; (also you may consider to use just *_EN and *_DIS) return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, PMC_CFG_NO_REBOOT_MASK, value); > +} -- With Best Regards, Andy Shevchenko