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

[Nico Huber (Code Review)]

Hello Benjamin Doron,

I'd like you to do a code review. Please visit

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

to review the following change.

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

Revert "soc/intel/jasperlake: Disable PAVP UPD"

This reverts commit 69589294c205b616e80cafbbfb0b33e105a75386.

No reason was given why this should deviate from the other platforms
and the author can't explain it.

Change-Id: I2e8d6f9bd4ebba69b6f7cdd9a1c5d08aaf2e798f
Signed-off-by: Nico Huber 
---
M src/soc/intel/jasperlake/fsp_params.c
1 file changed, 0 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/46044/1

diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c
index cdd088e..d2e07e9 100644
--- a/src/soc/intel/jasperlake/fsp_params.c
+++ b/src/soc/intel/jasperlake/fsp_params.c
@@ -207,9 +207,6 @@
    	params->XdciEnable = 0;
    }

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

[Benjamin Doron (Code Review)]

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

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

Patch Set 1: Code-Review+1

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46044 )

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

Patch Set 1: Code-Review+2

All platforms (including TGL and probably JSL, too) set it to 1=Enable per default. I see no reason to disable it statically. Thus, +2

[Angel Pons (Code Review)]

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

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

Patch Set 1: Code-Review+2

[Angel Pons (Code Review)]

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

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

Patch Set 1:

CB:42745 might be a better way to handle this.

[Nico Huber (Code Review)]

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

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

Patch Set 1:
> CB:42745 might be a better way to handle this.

This change here is necessary so I don't have to further bikeshed
the commit message of CB:42745 (which somehow still doesn't say
that it changes the default, so I thought we could as well make
it a clean revert).

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46044 )

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

Revert "soc/intel/jasperlake: Disable PAVP UPD"

This reverts commit 69589294c205b616e80cafbbfb0b33e105a75386.

No reason was given why this should deviate from the other platforms
and the author can't explain it.

Change-Id: I2e8d6f9bd4ebba69b6f7cdd9a1c5d08aaf2e798f
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46044
Tested-by: build bot (Jenkins) 
Reviewed-by: Benjamin Doron 
Reviewed-by: Michael Niewöhner 
Reviewed-by: Angel Pons 
---
M src/soc/intel/jasperlake/fsp_params.c
1 file changed, 0 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Michael Niewöhner: Looks good to me, approved
  Benjamin Doron: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c
index cdd088e..d2e07e9 100644
--- a/src/soc/intel/jasperlake/fsp_params.c
+++ b/src/soc/intel/jasperlake/fsp_params.c
@@ -207,9 +207,6 @@
    	params->XdciEnable = 0;
    }

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

[Nico Huber (Code Review)]

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

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

Patch Set 2:
> > CB:42745 might be a better way to handle this.
>
> This change here is necessary so I don't have to further bikeshed
> the commit message of CB:42745 (which somehow still doesn't say
> that it changes the default, so I thought we could as well make
> it a clean revert).

Heh, now that I look at it again, it preserves the default but
doesn't mention that. But still, a revert is much better than a
deviating Kconfig default that nobody can explain.