Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

EricR Lai (Code Review)

22 Jan 2020 22 Jan '20

11:47 p.m.

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC.

BUG=b:1746900

Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 --- M src/ec/google/wilco/commands.c M src/ec/google/wilco/commands.h 2 files changed, 41 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/38526/1

diff --git a/src/ec/google/wilco/commands.c b/src/ec/google/wilco/commands.c index 626f9dd..636136b 100644 --- a/src/ec/google/wilco/commands.c +++ b/src/ec/google/wilco/commands.c @@ -224,3 +224,28 @@ wilco_ec_mailbox(WILCO_EC_MSG_DEFAULT, KB_ERR_CODE, &err_code, 1, NULL, 0); } + +/* + * EC CPU ID data struct + * MBOX[2] = 0xFF + * MBOX[3] = CPUID_Low + * MBOX[4] = CPUID_Mid + * MBOX[5] = CPUID_High + * MBOX[6] = CPU_Core + * MBOX[7] = GPU_Core + * MBOX[8] = Reserved +*/ +int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores,uint8_t gpu_cores) +{ + uint8_t cpu_id[7] = {0}, i; + cpu_id[0] = 0xff; + for (i = 1; i < 4; i++) { + cpu_id[i] = cpuid & 0xff; + cpuid = cpuid >> 8; + } + cpu_id[4] = cpu_cores; + cpu_id[5] = gpu_cores; + + return wilco_ec_mailbox(WILCO_EC_MSG_DEFAULT, KB_CPU_ID, cpu_id, + ARRAY_SIZE(cpu_id),NULL, 0); +} diff --git a/src/ec/google/wilco/commands.h b/src/ec/google/wilco/commands.h index 9a18580..bfdd111 100644 --- a/src/ec/google/wilco/commands.h +++ b/src/ec/google/wilco/commands.h @@ -52,6 +52,8 @@ KB_BIOS_PROGRESS = 0xc2, /* Inform the EC that a fatal error occurred */ KB_ERR_CODE = 0x7b, + /* Set CPU ID */ + KB_CPU_ID = 0xbf, };

enum ec_ram_addr { @@ -337,4 +339,18 @@ */ void wilco_ec_save_post_code(uint8_t post_code);

+/** + * wilco_ec_set_cpuid + * + * Set CPU ID to EC. + * + * @cpuid: read CPU ID from cpu_eax(1) + * @cpu_cores: cores of CPU + * @gpu_cores: cores of GPU + * + * Returns 0 if EC command was successful + * Returns -1 if EC command failed + */ +int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores,uint8_t gpu_cores); + #endif /* EC_GOOGLE_WILCO_COMMANDS_H */

Attachments:

attachment.htm (text/html — 5.9 KB)

Show replies by date

build bot (Jenkins) (Code Review)

22 Jan 22 Jan

11:47 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... File src/ec/google/wilco/commands.h:

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... PS1, Line 354: int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores,uint8_t gpu_cores); space required after that ',' (ctx:VxV)

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... PS1, Line 238: int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores,uint8_t gpu_cores) space required after that ',' (ctx:VxV)

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... PS1, Line 250: ARRAY_SIZE(cpu_id),NULL, 0); space required after that ',' (ctx:VxV)

EricR Lai (Code Review)

11:48 p.m.

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 1:

Please help check the wilco command implement correct or not:)

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 1 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-CC: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 23 Jan 2020 06:48:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

EricR Lai (Code Review)

11:50 p.m.

Hello Mathew King, Duncan Laurie,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38526

to look at the new patch set (#2).

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC.

BUG=b:1746900

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/38526/2

EricR Lai (Code Review)

11:51 p.m.

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... File src/ec/google/wilco/commands.h:

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... PS1, Line 354: int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores,uint8_t gpu_cores);

...

space required after that ',' (ctx:VxV)

Done

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... PS1, Line 238: int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores,uint8_t gpu_cores)

...

space required after that ',' (ctx:VxV)

Done

https://review.coreboot.org/c/coreboot/+/38526/1/src/ec/google/wilco/command... PS1, Line 250: ARRAY_SIZE(cpu_id),NULL, 0);

...

space required after that ',' (ctx:VxV)

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 2 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 23 Jan 2020 06:51:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: comment

EricR Lai (Code Review)

11:52 p.m.

Hello Mathew King, Duncan Laurie, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38526

to look at the new patch set (#3).

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC.

BUG=b:148126144

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/38526/3

Paul Menzel (Code Review)

23 Jan 23 Jan

8:50 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 3:

(4 comments)

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG@7 PS3, Line 7: add ec One space.

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG@10 PS3, Line 10: EC. Fits on one line.

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG@11 PS3, Line 11: What does the EC do with that?

https://review.coreboot.org/c/coreboot/+/38526/3/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/3/src/ec/google/wilco/command... PS3, Line 240: {0} Spaces around 0?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 3 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 15:50:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

EricR Lai (Code Review)

9:26 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Hello Mathew King, Duncan Laurie, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38526

to look at the new patch set (#4).

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC. EC will according to different CPU to set different power table.

BUG=b:148126144

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/38526/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

EricR Lai (Code Review)

9:27 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG@7 PS3, Line 7: add ec

...

One space.

Done

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG@10 PS3, Line 10: EC.

...

Fits on one line.

Done

https://review.coreboot.org/c/coreboot/+/38526/3//COMMIT_MSG@11 PS3, Line 11:

...

What does the EC do with that?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 16:27:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Mathew King (Code Review)

10 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4: Code-Review+2

(1 comment)

LGTM as long as 3 bytes of cpuid is intentional.

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 231: * MBOX[3] = CPUID_Low : * MBOX[4] = CPUID_Mid : * MBOX[5] = CPUID_High Why not all 4 bytes?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 17:00:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

EricR Lai (Code Review)

10:29 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 231: * MBOX[3] = CPUID_Low : * MBOX[4] = CPUID_Mid : * MBOX[5] = CPUID_High

...

Why not all 4 bytes?

This is EC spec, I don't know why. You can check in issue tracker.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 17:29:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Mathew King mathewk@chromium.org Gerrit-MessageType: comment

EricR Lai (Code Review)

10:41 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38526/3/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/3/src/ec/google/wilco/command... PS3, Line 240: {0}

...

Spaces around 0?

I checked many drivers, there no space around 0 as initial value.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 17:41:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

EricR Lai (Code Review)

11:05 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 231: * MBOX[3] = CPUID_Low : * MBOX[4] = CPUID_Mid : * MBOX[5] = CPUID_High

...

This is EC spec, I don't know why. You can check in issue tracker.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 18:05:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: EricR Lai ericr_lai@compal.corp-partner.google.com Comment-In-Reply-To: Mathew King mathewk@chromium.org Gerrit-MessageType: comment

Duncan Laurie (Code Review)

1:05 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4:

(2 comments)

Just minor style notes.

Are you planning to call this from wilco_ec_init() or from the mainboard/soc code?

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 237: */ space before * looks like it would align the comment. but this could be just a gerrit ui issue.

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 240: uint8_t cpu_id[7] = {0}, i; can you add a newline after the declaration and before the code?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 23 Jan 2020 20:05:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

EricR Lai (Code Review)

7:32 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 4:

...

Patch Set 4:

(2 comments)

Just minor style notes.

Are you planning to call this from wilco_ec_init() or from the mainboard/soc code?

If there no common way to get cpu cores, I will put into board level. I am checking with Intel. If we can get cpu cores from command, we can put into wilco_ec_init.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 4 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 24 Jan 2020 02:32:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

EricR Lai (Code Review)

8:16 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Hello Mathew King, Duncan Laurie, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38526

to look at the new patch set (#5).

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC. EC will according to different CPU to set different power table.

BUG=b:148126144

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/38526/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 5 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

EricR Lai (Code Review)

8:17 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 5:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... File src/ec/google/wilco/commands.c:

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 231: * MBOX[3] = CPUID_Low : * MBOX[4] = CPUID_Mid : * MBOX[5] = CPUID_High

...

Ack

Done

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 237: */

...

space before * looks like it would align the comment. but this could be just a gerrit ui issue.

Done

https://review.coreboot.org/c/coreboot/+/38526/4/src/ec/google/wilco/command... PS4, Line 240: uint8_t cpu_id[7] = {0}, i;

...

can you add a newline after the declaration and before the code?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 5 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 24 Jan 2020 03:17:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: EricR Lai ericr_lai@compal.corp-partner.google.com Comment-In-Reply-To: Mathew King mathewk@chromium.org Comment-In-Reply-To: Duncan Laurie dlaurie@chromium.org Gerrit-MessageType: comment

EricR Lai (Code Review)

8:18 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Hello Mathew King, Duncan Laurie, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38526

to look at the new patch set (#6).

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC. EC will according to different CPU to set different power table.

BUG=b:148126144

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/38526/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 6 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Duncan Laurie (Code Review)

8:36 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 6: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 6 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 24 Jan 2020 03:36:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Mathew King (Code Review)

24 Jan 24 Jan

12:39 p.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

Patch Set 6: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 6 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 24 Jan 2020 19:39:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

27 Jan 27 Jan

12:43 a.m.

New subject: Change in coreboot[master]: ec/google/wilco: add ec command set cpu id

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38526 )

Change subject: ec/google/wilco: add ec command set cpu id ......................................................................

ec/google/wilco: add ec command set cpu id

Add new mailbox command support. Set CPU ID and cores to EC. EC will according to different CPU to set different power table.

BUG=b:148126144

Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38526 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Mathew King mathewk@chromium.org --- M src/ec/google/wilco/commands.c M src/ec/google/wilco/commands.h 2 files changed, 42 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved Mathew King: Looks good to me, approved

diff --git a/src/ec/google/wilco/commands.c b/src/ec/google/wilco/commands.c index 626f9dd..791141e 100644 --- a/src/ec/google/wilco/commands.c +++ b/src/ec/google/wilco/commands.c @@ -224,3 +224,29 @@ wilco_ec_mailbox(WILCO_EC_MSG_DEFAULT, KB_ERR_CODE, &err_code, 1, NULL, 0); } + +/* + * EC CPU ID data struct + * MBOX[2] = 0xFF + * MBOX[3] = CPUID_Low + * MBOX[4] = CPUID_Mid + * MBOX[5] = CPUID_High + * MBOX[6] = CPU_Core + * MBOX[7] = GPU_Core + * MBOX[8] = Reserved + */ +int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores, uint8_t gpu_cores) +{ + uint8_t cpu_id[7] = {0}, i; + + cpu_id[0] = 0xff; + for (i = 1; i < 4; i++) { + cpu_id[i] = cpuid & 0xff; + cpuid = cpuid >> 8; + } + cpu_id[4] = cpu_cores; + cpu_id[5] = gpu_cores; + + return wilco_ec_mailbox(WILCO_EC_MSG_DEFAULT, KB_CPU_ID, cpu_id, + ARRAY_SIZE(cpu_id), NULL, 0); +} diff --git a/src/ec/google/wilco/commands.h b/src/ec/google/wilco/commands.h index 9a18580..3d2ae46 100644 --- a/src/ec/google/wilco/commands.h +++ b/src/ec/google/wilco/commands.h @@ -52,6 +52,8 @@ KB_BIOS_PROGRESS = 0xc2, /* Inform the EC that a fatal error occurred */ KB_ERR_CODE = 0x7b, + /* Set CPU ID */ + KB_CPU_ID = 0xbf, };

enum ec_ram_addr { @@ -337,4 +339,18 @@ */ void wilco_ec_save_post_code(uint8_t post_code);

+/** + * wilco_ec_set_cpuid + * + * Set CPU ID to EC. + * + * @cpuid: read CPU ID from cpu_eax(1) + * @cpu_cores: cores of CPU + * @gpu_cores: cores of GPU + * + * Returns 0 if EC command was successful + * Returns -1 if EC command failed + */ +int wilco_ec_set_cpuid(uint32_t cpuid, uint8_t cpu_cores, uint8_t gpu_cores); + #endif /* EC_GOOGLE_WILCO_COMMANDS_H */

-- To view, visit https://review.coreboot.org/c/coreboot/+/38526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I135d2421d2106934be996a1780786f6bb0bf6b34 Gerrit-Change-Number: 38526 Gerrit-PatchSet: 7 Gerrit-Owner: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org Gerrit-Reviewer: EricR Lai ericr_lai@compal.corp-partner.google.com Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: John Su john_su@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1933

days inactive

1937

days old

coreboot-gerrit@coreboot.org

20 comments

6 participants

tags (0)

participants (6)

build bot (Jenkins) (Code Review)
Duncan Laurie (Code Review)
EricR Lai (Code Review)
Mathew King (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)