Change in coreboot[master]: soc/intel/xeon_sp: Lock down DMIC

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

soc/intel/xeon_sp: Lock down DMIC

This is required for CBnT.

Change-Id: I290742c163f5f067c8d529ddca8e2d8572ab6e6a
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/xeon_sp/pch.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47449/1

diff --git a/src/soc/intel/xeon_sp/pch.c b/src/soc/intel/xeon_sp/pch.c
index 5427952..2b35223 100644
--- a/src/soc/intel/xeon_sp/pch.c
+++ b/src/soc/intel/xeon_sp/pch.c
@@ -39,6 +39,9 @@
    reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1;
    pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32);
    pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8);
+
+	reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL);
+	pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK);
 }

void bootblock_pch_init(void)

[Arthur Heymans (Code Review)]

Hello Patrick Rudolph,

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

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

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

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

soc/intel/xeon_sp: Lock down DMIC

This is required for CBnT.

Change-Id: I290742c163f5f067c8d529ddca8e2d8572ab6e6a
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/xeon_sp/pch.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47449/2

[Angel Pons (Code Review)]

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

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/47449/2//COMMIT_MSG@7 
PS2, Line 7: DMIC
"DMIC" usually means Digital MICrophone. Maybe use DMI or DMICTL?

[Arthur Heymans (Code Review)]

Hello build bot (Jenkins), Jonathan Zhang, Angel Pons, Patrick Rudolph,

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

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

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

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

soc/intel/xeon_sp: Lock down DMIC

This is required for CBnT.

Change-Id: I290742c163f5f067c8d529ddca8e2d8572ab6e6a
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/xeon_sp/pch.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47449/8

[Paul Menzel (Code Review)]

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

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47449/8//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47449/8//COMMIT_MSG@9 
PS8, Line 9: This is required for CBnT.
Can you please add the reference to the requirement (datasheet, …)?

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

Patch Set 8: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/47449/8//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47449/8//COMMIT_MSG@9 
PS8, Line 9: This is required for CBnT.
> Can you please add the reference to the requirement (datasheet, …)?

That's try and error here.

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMIC
......................................................................

Patch Set 8:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/47449/2//COMMIT_MSG@7 
PS2, Line 7: DMIC
> "DMIC" usually means Digital MICrophone. […]

Ack

Change in coreboot[master]: soc/intel/xeon_sp: Lock down DMICTL

[Christian Walter (Code Review)]

Christian Walter has uploaded a new patch set (#9) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMICTL
......................................................................

soc/intel/xeon_sp: Lock down DMICTL

This is required for CBnT.

Change-Id: I290742c163f5f067c8d529ddca8e2d8572ab6e6a
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/xeon_sp/pch.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47449/9

Change in coreboot[master]: soc/intel/xeon_sp: Lock down DMICTL

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMICTL
......................................................................

soc/intel/xeon_sp: Lock down DMICTL

This is required for CBnT.

Change-Id: I290742c163f5f067c8d529ddca8e2d8572ab6e6a
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47449
Reviewed-by: Christian Walter 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/xeon_sp/pch.c
1 file changed, 3 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Christian Walter: Looks good to me, approved

diff --git a/src/soc/intel/xeon_sp/pch.c b/src/soc/intel/xeon_sp/pch.c
index 5427952..2b35223 100644
--- a/src/soc/intel/xeon_sp/pch.c
+++ b/src/soc/intel/xeon_sp/pch.c
@@ -39,6 +39,9 @@
    reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1;
    pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32);
    pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8);
+
+	reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL);
+	pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK);
 }

void bootblock_pch_init(void)

Change in coreboot[master]: soc/intel/xeon_sp: Lock down DMICTL

[Marc Jones (Code Review)]

Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMICTL
......................................................................

Patch Set 10:

This breaks TiogaPass/ SKX FSP

Calling FspMemoryInit: 0xff03c488
        0xfe80fce0: raminit_upd
        0xfe8110c0: &hob_list_ptr
POST: 0x92
ASSERT [SiInitPreMem] c:\fsp_ipclean\PurleySktPkg\SouthClusterLbg\Library\PeiDxeSmmPchCycleDecodingLib\PchCycleDecodingLib.c(75): ((BOOLEAN)(0==1)

Change in coreboot[master]: soc/intel/xeon_sp: Lock down DMICTL

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47449 )

Change subject: soc/intel/xeon_sp: Lock down DMICTL
......................................................................

Patch Set 10:
> Patch Set 10:
>
> This breaks TiogaPass/ SKX FSP
>
> Calling FspMemoryInit: 0xff03c488
>         0xfe80fce0: raminit_upd
>         0xfe8110c0: &hob_list_ptr
> POST: 0x92
> ASSERT [SiInitPreMem] c:\fsp_ipclean\PurleySktPkg\SouthClusterLbg\Library\PeiDxeSmmPchCycleDecodingLib\PchCycleDecodingLib.c(75): ((BOOLEAN)(0==1)

Thanks! Ours was not booting either and we don't get FSP debug output! It indeed makes little sense to do this in the bootblock. I'll move it.