Change in coreboot[master]: soc/intel/cannonlake: Modify CSE's PCI BAR address

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

soc/intel/cannonlake: Modify CSE's PCI BAR address

The patch modifies CSE's PCI BAR address to 0xfed1a000. Currently coreboot
uses 0xfeda2000 as a PCI BAR address for CSE device while FSP-M uses
0xfed1a000. So, after FSP-M call, CSE's BAR address is overridden with
0xfed1a000. This causes HECI command transactions fail between FSP-M call
and postcar.

TEST=Verified sending HECI commands before and after FSP-M call on hatch.

Signed-off-by: Sridhar Siricilla 
Change-Id: I371cb658a96f5d580faff32ffab013cb6e6c492c
---
M src/soc/intel/cannonlake/include/soc/iomap.h
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/44211/1

diff --git a/src/soc/intel/cannonlake/include/soc/iomap.h b/src/soc/intel/cannonlake/include/soc/iomap.h
index dc07089..9a58c65 100644
--- a/src/soc/intel/cannonlake/include/soc/iomap.h
+++ b/src/soc/intel/cannonlake/include/soc/iomap.h
@@ -52,7 +52,11 @@

#define GPIO_BASE_SIZE		0x10000

+#if CONFIG(SOC_INTEL_COMETLAKE)
+#define HECI1_BASE_ADDRESS	0xfed1a000
+#else
 #define HECI1_BASE_ADDRESS	0xfeda2000
+#endif

/* PTT registers */
 #define PTT_TXT_BASE_ADDRESS	0xfed30800

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG@12 
PS1, Line 12:  fail
"to fail"

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG@14 
PS1, Line 14: 
I am guessing you need this for puff? In that case can you please add BRANCH=puff?

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG@14 
PS1, Line 14:
> I am guessing you need this for puff? In that case can you please add BRANCH=puff?

The change will be useful for Puff. But, the change is still applicable for all CML platforms even though CB romstage(except FSP-M) doesn't deal with CSE.

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG@14 
PS1, Line 14:
> The change will be useful for Puff. […]

Understood. For any changes that are meant to land in firmware branch, we add the tag BRANCH= so that the team can use it to ensure the required changes are cherry-picked to that branch.

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Jamie Ryu, Tim Wawrzynczak, Rizwan Qureshi, Paul Menzel, Edward O'Callaghan, Subrata Banik, Sowmya V, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

soc/intel/cannonlake: Modify CSE's PCI BAR address

The patch modifies CSE's PCI BAR address to 0xfed1a000. Currently coreboot
uses 0xfeda2000 as a PCI BAR address for CSE device while FSP-M uses
0xfed1a000. So, after FSP-M call, CSE's BAR address is overridden with
0xfed1a000. This causes HECI command transactions to fail between FSP-M
call and postcar.

BRANCH=master
TEST=Verified sending HECI commands before and after FSP-M call on hatch.

Signed-off-by: Sridhar Siricilla 
Change-Id: I371cb658a96f5d580faff32ffab013cb6e6c492c
---
M src/soc/intel/cannonlake/include/soc/iomap.h
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/44211/2

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG@12 
PS1, Line 12:  fail
> "to fail"

Ack

https://review.coreboot.org/c/coreboot/+/44211/1//COMMIT_MSG@14 
PS1, Line 14:
> Understood. […]

Sure, I will add the tag BRANCH= to commit messages.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 2:

What about setting the Heci1BarAddress UPD?

[Sridhar Siricilla (Code Review)]

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Modify CSE's PCI BAR address
......................................................................

Patch Set 2:
> Patch Set 2:
>
> What about setting the Heci1BarAddress UPD?

Configuring the Heci1BarAddress UPD will close the gap between coreboot & FSP-M. 
I will push the change. Thanks

Change in coreboot[master]: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Jamie Ryu, Tim Wawrzynczak, Rizwan Qureshi, Paul Menzel, Edward O'Callaghan, Subrata Banik, Sowmya V, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress
......................................................................

soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

The patch sets FSP-M UPD Heci1BarAddress to avoid disconnect between
coreboot and FSP-M. Currently coreboot uses 0xfeda2000 as a PCI BAR address
for CSE device while FSP-M uses 0xfed1a000. So, after FSP-M call, CSE's BAR
address is overridden with 0xfed1a000. This causes HECI command
transactions to fail between FSP-M call and postcar.

BRANCH=puff
TEST=Verified sending HECI commands before and after FSP-M call on hatch.

Signed-off-by: Sridhar Siricilla 
Change-Id: I371cb658a96f5d580faff32ffab013cb6e6c492c
---
M src/soc/intel/cannonlake/romstage/fsp_params.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/44211/3

Change in coreboot[master]: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

[Sridhar Siricilla (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Jamie Ryu, Tim Wawrzynczak, Rizwan Qureshi, Paul Menzel, Edward O'Callaghan, Subrata Banik, Sowmya V, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress
......................................................................

soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

The patch sets FSP-M UPD Heci1BarAddress to avoid disconnect between
coreboot and FSP-M. Currently coreboot uses 0xfeda2000 as a PCI BAR address
for CSE device while FSP-M uses 0xfed1a000. So, after FSP-M call, CSE's BAR
address is overridden with 0xfed1a000. This causes HECI transactions to
fail between FSP-M call and postcar.

BRANCH=puff
TEST=Verified sending HECI commands before and after FSP-M call on hatch.

Signed-off-by: Sridhar Siricilla 
Change-Id: I371cb658a96f5d580faff32ffab013cb6e6c492c
---
M src/soc/intel/cannonlake/romstage/fsp_params.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/44211/4

Change in coreboot[master]: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress
......................................................................

Patch Set 4: Code-Review+2

Change in coreboot[master]: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44211 )

Change subject: soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress
......................................................................

soc/intel/cannonlake: Set FSP-M UPD Heci1BarAddress

The patch sets FSP-M UPD Heci1BarAddress to avoid disconnect between
coreboot and FSP-M. Currently coreboot uses 0xfeda2000 as a PCI BAR address
for CSE device while FSP-M uses 0xfed1a000. So, after FSP-M call, CSE's BAR
address is overridden with 0xfed1a000. This causes HECI transactions to
fail between FSP-M call and postcar.

BRANCH=puff
TEST=Verified sending HECI commands before and after FSP-M call on hatch.

Signed-off-by: Sridhar Siricilla 
Change-Id: I371cb658a96f5d580faff32ffab013cb6e6c492c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44211
Tested-by: build bot (Jenkins) 
Reviewed-by: Tim Wawrzynczak 
---
M src/soc/intel/cannonlake/romstage/fsp_params.c
1 file changed, 3 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/soc/intel/cannonlake/romstage/fsp_params.c b/src/soc/intel/cannonlake/romstage/fsp_params.c
index 37f4d50..ac42e00 100644
--- a/src/soc/intel/cannonlake/romstage/fsp_params.c
+++ b/src/soc/intel/cannonlake/romstage/fsp_params.c
@@ -157,6 +157,9 @@
    /* Configure VT-d */
    tconfig->VtdDisable = 0;

+	/* Set HECI1 PCI BAR address */
+	m_cfg->Heci1BarAddress = HECI1_BASE_ADDRESS;
+
    mainboard_memory_init_params(mupd);
 }