Change in flashrom[master]: chipset_enable.c: Enable Tiger Lake U support

List overview All Threads
Download

newer

older

Change in flashrom[master]:...

Edward O'Callaghan (Code Review)

17 Dec 2019 17 Dec '19

12:12 p.m.

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37777 )

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

chipset_enable.c: Enable Tiger Lake U support

Did a basic local test with --flash-name and -r bios and ran strings to see if the bios contained reasonable data.

BUG=b:146089922

Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M chipset_enable.c 1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/37777/1

diff --git a/chipset_enable.c b/chipset_enable.c index b55852c..d5a2f03 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1996,6 +1996,7 @@ {0x8086, 0x9d56, B_S, NT, "Intel", "Kaby Lake Y Premium", enable_flash_pch100}, {0x8086, 0x9d58, B_S, NT, "Intel", "Kaby Lake U Premium", enable_flash_pch100}, {0x8086, 0x9d84, B_S, DEP, "Intel", "Cannon Lake U Premium", enable_flash_pch300}, + {0x8086, 0xa082, B_FS, NT, "Intel", "Tiger Lake U", enable_flash_pch300}, {0x8086, 0xa141, B_S, NT, "Intel", "Sunrise Point Desktop Sample", enable_flash_pch100}, {0x8086, 0xa142, B_S, NT, "Intel", "Sunrise Point Unknown Sample", enable_flash_pch100}, {0x8086, 0xa143, B_S, NT, "Intel", "H110", enable_flash_pch100},

Attachments:

attachment.htm (text/html — 2.9 KB)

Show replies by thread

Furquan Shaikh (Code Review)

25 Dec 25 Dec

10:22 a.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37777 )

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/flashrom/+/37777/1/chipset_enable.c File chipset_enable.c:

https://review.coreboot.org/c/flashrom/+/37777/1/chipset_enable.c@1999 PS1, Line 1999: 0xa082 I don't think this is correct.

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Dec 2019 01:22:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Edward O'Callaghan (Code Review)

26 Dec 26 Dec

12:03 p.m.

Hello build bot (Jenkins),

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

https://review.coreboot.org/c/flashrom/+/37777

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

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

chipset_enable.c: Enable Tiger Lake U support

Did a basic local test with --flash-name and -r bios and ran strings to see if the bios contained reasonable data.

V.2: Fixup missing stashed change that fixed the pid id.

BUG=b:146089922

Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M chipset_enable.c 1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/37777/2

Edward O'Callaghan (Code Review)

12:03 p.m.

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37777 )

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/flashrom/+/37777/1/chipset_enable.c File chipset_enable.c:

https://review.coreboot.org/c/flashrom/+/37777/1/chipset_enable.c@1999 PS1, Line 1999: 0xa082

...

I don't think this is correct.

local change was still stashed, fixed now.

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 26 Dec 2019 03:03:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Angel Pons (Code Review)

4 Jan 4 Jan

11:57 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37777 )

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

Patch Set 2: Code-Review+1

Would be nice if this could be marked as tested (actually, DEP, because it DEPends on IFD/ME settings). I would say being able to reflash coreboot internally is enough of a test. I guess it shouldn't be much of a problem if this machine is a chromebook.

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 04 Jan 2020 14:57:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

5 Jan 5 Jan

7:49 p.m.

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

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/flashrom/+/37777/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/flashrom/+/37777/2//COMMIT_MSG@12 PS2, Line 12: Fixup Fix up

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Sun, 05 Jan 2020 10:49:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

10 Jan 10 Jan

6:23 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37777 )

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/37777/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/flashrom/+/37777/2//COMMIT_MSG@12 PS2, Line 12: Fixup

...

Fix up

IMHO, it's such a minor nit I'd just drop the entire `V.2` line altogether.

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Thu, 09 Jan 2020 21:23:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Nico Huber (Code Review)

17 Jan 17 Jan

9:53 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37777 )

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

Patch Set 2:

(1 comment)

Same as for https://review.coreboot.org/c/flashrom/+/37677, I don't have documentation for this SoC available. So no way to review this. Note that all the occurrences of CHIPSET_300_SERIES_CANNON_POINT in the code would need to be reviewed or tested.

Alternatively to waiting for Intel docs, we could also ask them to add all the details to Documentation/ first. So we'd have something to compare to during review of the code.

https://review.coreboot.org/c/flashrom/+/37777/2/chipset_enable.c File chipset_enable.c:

https://review.coreboot.org/c/flashrom/+/37777/2/chipset_enable.c@1999 PS2, Line 1999: F FWH support is not implemented, AFAIR.

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Fri, 17 Jan 2020 12:53:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Edward O'Callaghan (Code Review)

11 Apr 11 Apr

10:30 a.m.

Hello build bot (Jenkins), Paul Menzel,

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

https://review.coreboot.org/c/flashrom/+/37777

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

Change subject: chipset_enable.c: Enable Tiger Lake U support ......................................................................

chipset_enable.c: Enable Tiger Lake U support

Did a basic local test with --flash-name and -r bios and ran strings to see if the bios contained reasonable data.

V.2: Fixup missing stashed change that fixed the pid id.

BUG=b:146089922

Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M chipset_enable.c 1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/37777/3

-- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan.m.shaikh@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-MessageType: newpatchset

909

days inactive

1755

days old

flashrom-gerrit@flashrom.org

8 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
Edward O'Callaghan (Code Review)
Furquan Shaikh (Code Review)
Nico Huber (Code Review)
Paul Menzel (Code Review)