x86/platform/intel-mid: Run PWRMU command immediately
Subsystems (2)
PATCH 1/1 x86/platform/intel-mid: Run PWRMU command immediately
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.
--- arch/x86/platform/intel-mid/pwr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
arch/x86/platform/intel-mid/pwr.c +5 -1
@@ -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); }
Signature · 1 line
2.8.1
Quoted 24 lines · Andy Shevchenko
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 <[email protected]>--- arch/x86/platform/intel-mid/pwr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/intel-mid/pwr.c +4
-0@@ -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 */
arch/x86/platform/intel-mid/pwr.c static int mid_pwr_wait(struct mid_pwr *pwr) · line 146
Quoted 7 lines
arch/x86/platform/intel-mid/pwr.c +1
-1@@ -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.
Thanks,
Ingo
Quoted 33 lines · Ingo Molnar
Quoted 31 lines · Andy Shevchenko
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 <[email protected]>---
arch/x86/platform/intel-mid/pwr.c | 6 +++++-1 file changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/intel-mid/pwr.c +4
-0@@ -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 */arch/x86/platform/intel-mid/pwr.c +0
-0@@ -137,7 +141,7 @@ static int mid_pwr_wait(struct mid_pwr *pwr)
static intmid_pwr_wait_for_cmd(struct mid_pwr *pwr, u8 cmd){arch/x86/platform/intel-mid/pwr.c +0
-1-writel (PM_CMD_CMD (cmd), pwr->regs +PM_CMD );
Quoted 8 lines
Quoted 4 lines
arch/x86/platform/intel-mid/pwr.c +1
-0+ writel(PM_CMD_CMD(cmd) | PM_CMD_CM_IMMEDIATE, pwr->regs +
PM_CMD);
returnmid_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?
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Quoted 49 lines · Andy Shevchenko
Quoted 33 lines · Ingo Molnar
Quoted 31 lines · Andy Shevchenko
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 <[email protected]>---
arch/x86/platform/intel-mid/pwr.c | 6 +++++-1 file changed, 5 insertions(+), 1 deletion(-)arch/x86/platform/intel-mid/pwr.c +4
-0@@ -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 */arch/x86/platform/intel-mid/pwr.c +0
-0@@ -137,7 +141,7 @@ static int mid_pwr_wait(struct mid_pwr *pwr)
static intmid_pwr_wait_for_cmd(struct mid_pwr *pwr, u8 cmd){arch/x86/platform/intel-mid/pwr.c +0
-1-writel (PM_CMD_CMD (cmd), pwr->regs +PM_CMD );Quoted 8 lines
Quoted 4 lines
arch/x86/platform/intel-mid/pwr.c +1
-0+ writel(PM_CMD_CMD(cmd) | PM_CMD_CM_IMMEDIATE, pwr->regs +
PM_CMD);
returnmid_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
0x2201to 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.
Quoted 6 lines
[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 fieldTaking 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