Change in ...coreboot[master]: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33194

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable

Change-Id: Ia88f3315a9dc3365d0acc13ed887e7c596c97c91
Signed-off-by: Subrata Banik 
---
M src/mainboard/google/hatch/variants/baseboard/devicetree.cb
M src/mainboard/google/sarien/variants/arcada/devicetree.cb
M src/mainboard/google/sarien/variants/sarien/devicetree.cb
3 files changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/33194/1

diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
index 9d10cac..fba8d96 100644
--- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
@@ -28,7 +28,7 @@
    # Enable System Agent dynamic frequency
    register "SaGv" = "SaGv_Enabled"
    # Enable heci communication
-	register "HeciEnabled" = "1"
+	register "HeciEnabled" = "0"
    # Enable Speed Shift Technology support
    register "speed_shift_enable" = "1"
    # Enable S0ix
diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb
index 767df1f..ce960a7 100644
--- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb
+++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb
@@ -15,7 +15,7 @@

# FSP configuration
    register "SaGv" = "SaGv_Enabled"
-	register "HeciEnabled" = "1"
+	register "HeciEnabled" = "0"
    register "SataSalpSupport" = "1"
    register "SataMode" = "Sata_AHCI"
    register "SataPortsEnable[2]" = "1"
diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb
index c96423c..739a849 100644
--- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb
+++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb
@@ -15,7 +15,7 @@

# FSP configuration
    register "SaGv" = "SaGv_Enabled"
-	register "HeciEnabled" = "1"
+	register "HeciEnabled" = "0"
    register "SataSalpSupport" = "1"
    register "SataMode" = "Sata_AHCI"
    register "SataPortsEnable[0]" = "1"

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

Patch Set 6:

IIUC, this has to go with a particular FSP version. Is that correct?

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33194 )

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

Patch Set 6:
> Patch Set 6:
>
> IIUC, this has to go with a particular FSP version. Is that correct?

i guess this CL can go as well, it won't create any harm. HECI device will still show in pci bus although hecienable=0 because pch_disable_heci() if !hecienable and HECI_DISABLE_IN_SMM is set.

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

Patch Set 6:
> Patch Set 6:
> > Patch Set 6:
> >
> > IIUC, this has to go with a particular FSP version. Is that correct?
>
> i guess this CL can go as well, it won't create any harm. HECI device will still show in pci bus although hecienable=0 because pch_disable_heci() if !hecienable and HECI_DISABLE_IN_SMM is set.

Aah yes.

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

Patch Set 6: Code-Review+2

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/#/c/33194/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33194/6//COMMIT_MSG@7 
PS6, Line 7: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
Can you please add a comment indicating that this is not actually disabling Heci and it requires some more changes to cannonlake SoC code to set the right UPDs.

[Subrata Banik (Code Review)]

Hello Aaron Durbin, Duncan Laurie, Rizwan Qureshi, Bora Guvendik, Shelley Chen, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable

This patch is not actually disabling HECI1 as it requires a dedicated FSP UPD
for WHL/CML SoC code to set this HECI1 chip config.

Change-Id: Ia88f3315a9dc3365d0acc13ed887e7c596c97c91
Signed-off-by: Subrata Banik 
---
M src/mainboard/google/hatch/variants/baseboard/devicetree.cb
M src/mainboard/google/sarien/variants/arcada/devicetree.cb
M src/mainboard/google/sarien/variants/sarien/devicetree.cb
3 files changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/33194/7

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33194 )

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/#/c/33194/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33194/6//COMMIT_MSG@7 
PS6, Line 7: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
> Can you please add a comment indicating that this is not actually disabling Heci and it requires som […]

Done

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33194 )

Change subject: mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable
......................................................................

mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable

This patch is not actually disabling HECI1 as it requires a dedicated FSP UPD
for WHL/CML SoC code to set this HECI1 chip config.

Change-Id: Ia88f3315a9dc3365d0acc13ed887e7c596c97c91
Signed-off-by: Subrata Banik 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33194
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/mainboard/google/hatch/variants/baseboard/devicetree.cb
M src/mainboard/google/sarien/variants/arcada/devicetree.cb
M src/mainboard/google/sarien/variants/sarien/devicetree.cb
3 files changed, 3 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
index 61b712d..112c279 100644
--- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
@@ -28,7 +28,7 @@
    # Enable System Agent dynamic frequency
    register "SaGv" = "SaGv_Enabled"
    # Enable heci communication
-	register "HeciEnabled" = "1"
+	register "HeciEnabled" = "0"
    # Enable Speed Shift Technology support
    register "speed_shift_enable" = "1"
    # Enable S0ix
diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb
index 767df1f..ce960a7 100644
--- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb
+++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb
@@ -15,7 +15,7 @@

# FSP configuration
    register "SaGv" = "SaGv_Enabled"
-	register "HeciEnabled" = "1"
+	register "HeciEnabled" = "0"
    register "SataSalpSupport" = "1"
    register "SataMode" = "Sata_AHCI"
    register "SataPortsEnable[2]" = "1"
diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb
index c96423c..739a849 100644
--- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb
+++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb
@@ -15,7 +15,7 @@

# FSP configuration
    register "SaGv" = "SaGv_Enabled"
-	register "HeciEnabled" = "1"
+	register "HeciEnabled" = "0"
    register "SataSalpSupport" = "1"
    register "SataMode" = "Sata_AHCI"
    register "SataPortsEnable[0]" = "1"