From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751803AbdDCByD (ORCPT ); Sun, 2 Apr 2017 21:54:03 -0400 Received: from mail-yb0-f170.google.com ([209.85.213.170]:35033 "EHLO mail-yb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbdDCByB (ORCPT ); Sun, 2 Apr 2017 21:54:01 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170331133728.GA23725@rajaneesh-OptiPlex-9010> <4e66c12e4576c56c675d049af8b063d3ccd65d17.1491002056.git.sathyanarayanan.kuppuswamy@linux.intel.com> From: Sathyanarayanan Kuppuswamy Natarajan Date: Sun, 2 Apr 2017 18:53:59 -0700 Message-ID: Subject: Re: [PATCH v4 4/5] platform/x86: intel_pmc_ipc: Fix iTCO GCS memory mapping failure To: Andy Shevchenko Cc: Kuppuswamy Sathyanarayanan , Andy Shevchenko , Zha Qipeng , "dvhart@infradead.org" , Guenter Roeck , Wim Van Sebroeck , 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 Hi Andy, On Sun, Apr 2, 2017 at 7:10 AM, Andy Shevchenko wrote: > On Sat, Apr 1, 2017 at 2:27 AM, Kuppuswamy Sathyanarayanan > wrote: >> iTCO watchdog driver need access to PMC_CFG GCR register to modify > > iTCO_wdt or use above in the rest of the series. > > So, choose one and use it everywhere in your patch series. will go with iTCO_wdt. > >> the no reboot 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. > > >> +static int update_noreboot_bit(bool status) >> +{ >> + if (status) > >> + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, BIT(4), >> + BIT(4)); > > BIT(4) is a magic. Moreover, it's used as mask and value here, you > might introduce temporary variables for better understanding what is > what. > As an example you may look at drivers/acpi/acpi_lpss.c (code related > to IOSF MBI interaction). I will create macros with some meaningful name to explain what BIT(4) represents. > >> + else > > Redundant. > > > -- > With Best Regards, > Andy Shevchenko -- -- Sathya From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-yb0-f170.google.com ([209.85.213.170]:35033 "EHLO mail-yb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbdDCByB (ORCPT ); Sun, 2 Apr 2017 21:54:01 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170331133728.GA23725@rajaneesh-OptiPlex-9010> <4e66c12e4576c56c675d049af8b063d3ccd65d17.1491002056.git.sathyanarayanan.kuppuswamy@linux.intel.com> From: Sathyanarayanan Kuppuswamy Natarajan Date: Sun, 2 Apr 2017 18:53:59 -0700 Message-ID: Subject: Re: [PATCH v4 4/5] platform/x86: intel_pmc_ipc: Fix iTCO GCS memory mapping failure To: Andy Shevchenko Cc: Kuppuswamy Sathyanarayanan , Andy Shevchenko , Zha Qipeng , "dvhart@infradead.org" , Guenter Roeck , Wim Van Sebroeck , 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 Hi Andy, On Sun, Apr 2, 2017 at 7:10 AM, Andy Shevchenko wrote: > On Sat, Apr 1, 2017 at 2:27 AM, Kuppuswamy Sathyanarayanan > wrote: >> iTCO watchdog driver need access to PMC_CFG GCR register to modify > > iTCO_wdt or use above in the rest of the series. > > So, choose one and use it everywhere in your patch series. will go with iTCO_wdt. > >> the no reboot 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. > > >> +static int update_noreboot_bit(bool status) >> +{ >> + if (status) > >> + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, BIT(4), >> + BIT(4)); > > BIT(4) is a magic. Moreover, it's used as mask and value here, you > might introduce temporary variables for better understanding what is > what. > As an example you may look at drivers/acpi/acpi_lpss.c (code related > to IOSF MBI interaction). I will create macros with some meaningful name to explain what BIT(4) represents. > >> + else > > Redundant. > > > -- > With Best Regards, > Andy Shevchenko -- -- Sathya From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sathyanarayanan Kuppuswamy Natarajan Subject: Re: [PATCH v4 4/5] platform/x86: intel_pmc_ipc: Fix iTCO GCS memory mapping failure Date: Sun, 2 Apr 2017 18:53:59 -0700 Message-ID: References: <20170331133728.GA23725@rajaneesh-OptiPlex-9010> <4e66c12e4576c56c675d049af8b063d3ccd65d17.1491002056.git.sathyanarayanan.kuppuswamy@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-yb0-f170.google.com ([209.85.213.170]:35033 "EHLO mail-yb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbdDCByB (ORCPT ); Sun, 2 Apr 2017 21:54:01 -0400 In-Reply-To: Sender: platform-driver-x86-owner@vger.kernel.org List-ID: To: Andy Shevchenko Cc: Kuppuswamy Sathyanarayanan , Andy Shevchenko , Zha Qipeng , "dvhart@infradead.org" , Guenter Roeck , Wim Van Sebroeck , David Box , Rajneesh Bhardwaj , Platform Driver , "linux-kernel@vger.kernel.org" , linux-watchdog@vger.kernel.org Hi Andy, On Sun, Apr 2, 2017 at 7:10 AM, Andy Shevchenko wrote: > On Sat, Apr 1, 2017 at 2:27 AM, Kuppuswamy Sathyanarayanan > wrote: >> iTCO watchdog driver need access to PMC_CFG GCR register to modify > > iTCO_wdt or use above in the rest of the series. > > So, choose one and use it everywhere in your patch series. will go with iTCO_wdt. > >> the no reboot 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. > > >> +static int update_noreboot_bit(bool status) >> +{ >> + if (status) > >> + return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, BIT(4), >> + BIT(4)); > > BIT(4) is a magic. Moreover, it's used as mask and value here, you > might introduce temporary variables for better understanding what is > what. > As an example you may look at drivers/acpi/acpi_lpss.c (code related > to IOSF MBI interaction). I will create macros with some meaningful name to explain what BIT(4) represents. > >> + else > > Redundant. > > > -- > With Best Regards, > Andy Shevchenko --