From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1947159AbcHRNfZ (ORCPT ); Thu, 18 Aug 2016 09:35:25 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35796 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1946004AbcHRNfX (ORCPT ); Thu, 18 Aug 2016 09:35:23 -0400 Date: Thu, 18 Aug 2016 15:35:18 +0200 From: Ingo Molnar To: Andy Shevchenko Cc: Thomas Gleixner , "H. Peter Anvin" , x86@kernel.org, Ingo Molnar , linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 1/1] x86/platform/intel-mid: Run PWRMU command immediately Message-ID: <20160818133518.GA31105@gmail.com> References: <1471514875-61798-1-git-send-email-andriy.shevchenko@linux.intel.com> <20160818105209.GB30771@gmail.com> <1471519152.4887.162.camel@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1471519152.4887.162.camel@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Andy Shevchenko wrote: > On Thu, 2016-08-18 at 12:52 +0200, Ingo Molnar wrote: > > * Andy Shevchenko wrote: > > > > > > > > On some firmwares we have to tell how exactly we want the command to > > > be run. > > > The default case for now is to run it immediately. > > > > > > Signed-off-by: Andy Shevchenko > > > --- > > >  arch/x86/platform/intel-mid/pwr.c | 6 +++++- > > >  1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/x86/platform/intel-mid/pwr.c > > > b/arch/x86/platform/intel-mid/pwr.c > > > index c901a34..0548741 100644 > > > --- a/arch/x86/platform/intel-mid/pwr.c > > > +++ b/arch/x86/platform/intel-mid/pwr.c > > > @@ -44,6 +44,10 @@ > > >  /* Bits in PM_CMD */ > > >  #define PM_CMD_CMD(x) ((x) << 0) > > >  #define PM_CMD_IOC (1 << 8) > > > +#define PM_CMD_CM_NOP (0 << 9) > > > +#define PM_CMD_CM_IMMEDIATE (1 << 9) > > > +#define PM_CMD_CM_DELAY (2 << 9) > > > +#define PM_CMD_CM_TRIGGER (3 << 9) > > >  #define PM_CMD_D3cold (1 << 21) > > >   > > >  /* List of commands */ > > > @@ -137,7 +141,7 @@ static int mid_pwr_wait(struct mid_pwr *pwr) > > >   > > >  static int mid_pwr_wait_for_cmd(struct mid_pwr *pwr, u8 cmd) > > >  { > > > - writel(PM_CMD_CMD(cmd), pwr->regs + PM_CMD); > > > + writel(PM_CMD_CMD(cmd) | PM_CMD_CM_IMMEDIATE, pwr->regs + > > > PM_CMD); > > >   return mid_pwr_wait(pwr); > > >  } > > > > Does this fix a bug? If yes then please also add that to the > > changelog: what are  > > the symptoms of the bug - how does a user notice, etc. > > Unfortunately I have no firmware (I have knowledge of) to test this. On > the board I have, i.e. Intel Edison, everything works either way. On the > other hand the official BSP code has magic number 0x2201 to set, where > bits [15:13] indeed has no meaning to firmware, but the rest is > meaningful. So, I could conclude it *might* fix a bug. > > [15:13] MODE_ID > Numeric ID associated with the given mode from an OSPM perspective. > Value not interpreted by firmware. Upon successful completion of this > command, this value should be reflected in the PM_STS.MODE_ID field > > Taking above to the consideration what would you advise me? "This appears to be a safer approach based on the documentation." is good enough justification, IMHO. So if you update the changelog with this information it's fine to me! Thanks, Ingo