Change in coreboot[master]: soc/intel/jasperlake: Disable PAVP UPD

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Ronak Kanabar (Code Review)

27 May 2020 27 May '20

3:11 a.m.

Ronak Kanabar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41763 )

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

soc/intel/jasperlake: Disable PAVP UPD

This patch will disable PAVP UPD, which is by default enable in FSP.

BUG=b:155595624 BRANCH=None TEST=Build, boot JSLRVP, Verified UPD values from FSP log

Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/41763/1

diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index 45162f9..311af3e 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -199,6 +199,9 @@ params->XdciEnable = 0; }

+ /* Disable Pavp */ + params->PavpEnable = 0 + /* Provide correct UART number for FSP debug logs */ params->SerialIoDebugUartNumber = CONFIG_UART_FOR_CONSOLE;

Attachments:

attachment.htm (text/html — 2.6 KB)

Show replies by date

Ronak Kanabar (Code Review)

27 May 27 May

3:19 a.m.

Hello V Sowmya, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Maulik V Vaghela, Subrata Banik, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,

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

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

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

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

soc/intel/jasperlake: Disable PAVP UPD

This patch will disable PAVP UPD, which is by default enable in FSP.

BUG=b:155595624 BRANCH=None TEST=Build, boot JSLRVP, Verified UPD values from FSP log

Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/41763/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 2 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Justin TerAvest (Code Review)

12:15 p.m.

Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41763 )

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 2: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 2 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 15:15:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

12:22 p.m.

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

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 2:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/41763/2//COMMIT_MSG@9 PS2, Line 9: enable enabled

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 2 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 27 May 2020 15:22:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

28 May 28 May

2:04 a.m.

Hello V Sowmya, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Maulik V Vaghela, Justin TerAvest, Subrata Banik, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,

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

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

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

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

soc/intel/jasperlake: Disable PAVP UPD

This patch will disable PAVP UPD, which is by default enabled in FSP.

BUG=b:155595624 BRANCH=None TEST=Build, boot JSLRVP, Verified UPD values from FSP log

Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/41763/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 3 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Ronak Kanabar (Code Review)

2:06 a.m.

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41763 )

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 3:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/41763/2//COMMIT_MSG@9 PS2, Line 9: enable

...

enabled

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 3 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 May 2020 05:06:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

3:50 a.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41763 )

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 3: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 3 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 May 2020 06:50:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

6:50 a.m.

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

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

soc/intel/jasperlake: Disable PAVP UPD

This patch will disable PAVP UPD, which is by default enabled in FSP.

BUG=b:155595624 BRANCH=None TEST=Build, boot JSLRVP, Verified UPD values from FSP log

Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41763 Reviewed-by: Karthik Ramasubramanian kramasub@google.com Reviewed-by: Justin TerAvest teravest@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Justin TerAvest: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, approved

diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index eafc374..75e1c64 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -210,6 +210,9 @@ params->XdciEnable = 0; }

+ /* Disable Pavp */ + params->PavpEnable = 0; + /* Provide correct UART number for FSP debug logs */ params->SerialIoDebugUartNumber = CONFIG_UART_FOR_CONSOLE;

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

Nico Huber (Code Review)

29 Jul 29 Jul

11:17 a.m.

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

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 4:

What is the reason for this change. All other platforms leave it enabled why disable it for this one? Also, if it should be globally disabled why not update the FSP binary?

Please always mention the reason for a change in the commit message.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 29 Jul 2020 14:17:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

30 Jul 30 Jul

4:26 a.m.

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41763 )

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 4:

...

Patch Set 4:

What is the reason for this change. All other platforms leave it enabled why disable it for this one? Also, if it should be globally disabled why not update the FSP binary?

Please always mention the reason for a change in the commit message.

In other platforms FSP default is 0. In JSL it is 1 so need to disable it from here.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 30 Jul 2020 07:26:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

6:06 a.m.

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

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 4:

...

Patch Set 4:

...
Patch Set 4:

What is the reason for this change. All other platforms leave it enabled why disable it for this one? Also, if it should be globally disabled why not update the FSP binary?

Please always mention the reason for a change in the commit message.

In other platforms FSP default is 0. In JSL it is 1 so need to disable it from here.

Are you sure about this? A quick grep suggests otherwise:

$ git grep Enable\ PavpEnable -- $(git ls-files src/vendorcode/intel/fsp/*.h) src/vendorcode/intel/fsp/fsp2_0/cannonlake/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable src/vendorcode/intel/fsp/fsp2_0/jasperlake/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable src/vendorcode/intel/fsp/fsp2_0/tigerlake/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable $ cd 3rdparty/fsp/ $ git grep Enable\ PavpEnable -- $(git ls-files *.h) AmberLakeFspBinPkg/Include/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable CoffeeLakeFspBinPkg/Include/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable CometLakeFspBinPkg/CometLake1/Include/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable IceLakeFspBinPkg/Include/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable KabylakeFspBinPkg/Include/FspsUpd.h: Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable

Are all the comments wrong?

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 30 Jul 2020 09:06:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Benjamin Doron (Code Review)

8 Sep 8 Sep

11 p.m.

Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41763 )

Change subject: soc/intel/jasperlake: Disable PAVP UPD ......................................................................

Patch Set 4:

...

Patch Set 4:

...
Patch Set 4:

...
Patch Set 4:

What is the reason for this change. All other platforms leave it enabled why disable it for this one? Also, if it should be globally disabled why not update the FSP binary?

Please always mention the reason for a change in the commit message.

In other platforms FSP default is 0. In JSL it is 1 so need to disable it from here.

Are you sure about this? A quick grep suggests otherwise:
$ git grep Enable\ PavpEnable -- $(git ls-files src/vendorcode/intel/fsp/*.h)
src/vendorcode/intel/fsp/fsp2_0/cannonlake/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
src/vendorcode/intel/fsp/fsp2_0/jasperlake/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
src/vendorcode/intel/fsp/fsp2_0/tigerlake/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
$ cd 3rdparty/fsp/
$ git grep Enable\ PavpEnable -- $(git ls-files *.h)
AmberLakeFspBinPkg/Include/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
CoffeeLakeFspBinPkg/Include/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
CometLakeFspBinPkg/CometLake1/Include/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
IceLakeFspBinPkg/Include/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
KabylakeFspBinPkg/Include/FspsUpd.h:  Enable(Default): Enable PavpEnable, Disable: Disable PavpEnable
Are all the comments wrong?

The comments are correct, according to the BSF files.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e103ad11ae6ffa6b9efe4bf249bbe344bc10a30 Gerrit-Change-Number: 41763 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@chromium.org Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: V Sowmya v.sowmya@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 09 Sep 2020 02:00:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1685

days inactive

1790

days old

coreboot-gerrit@coreboot.org

11 comments

7 participants

tags (0)

participants (7)

Benjamin Doron (Code Review)
Justin TerAvest (Code Review)
Karthik Ramasubramanian (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)
Ronak Kanabar (Code Review)