Change in ...coreboot[master]: nb/intel/i945: Use macro instead of magic number

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

HAOUAS Elyes (Code Review)

18 Jan 2019 18 Jan '19

2:39 p.m.

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30993

Change subject: nb/intel/i945: Use macro instead of magic number ......................................................................

nb/intel/i945: Use macro instead of magic number

Aslo add comments on Read/Write-Once bits. ULBA register is 64bits, so maybe there is a typo on RCBA(0x114) and it should be 0x11b insted.

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M 3rdparty/blobs M src/northbridge/intel/i945/early_init.c 2 files changed, 8 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/1

diff --git a/3rdparty/blobs b/3rdparty/blobs index 16058e5..998982d 160000 --- a/3rdparty/blobs +++ b/3rdparty/blobs @@ -1 +1 @@ -Subproject commit 16058e552279b4884b1f671e7a78752d28abd1cc +Subproject commit 998982d714a08481bb4944f456929be06b5e696f diff --git a/src/northbridge/intel/i945/early_init.c b/src/northbridge/intel/i945/early_init.c index 6bfa1c7..70b8152 100644 --- a/src/northbridge/intel/i945/early_init.c +++ b/src/northbridge/intel/i945/early_init.c @@ -883,10 +883,13 @@

static void ich7_setup_root_complex_topology(void) { - RCBA32(0x104) = 0x00000802; - RCBA32(0x110) = 0x00000001; - RCBA32(0x114) = 0x00000000; - RCBA32(0x118) = 0x00000000; + /* FIXME: ESD [23:16] is R/WO */ + RCBA32(ESD) = 0x00000802; + /* FIXME: ULD [31:16] is R/WO */ + RCBA32(ULD) = 0x00000001; + /* FIXME: ULBA [63:0] is R/WO */ + RCBA32(0x114) = 0x00000000; // RCBA32(0x11b) ? + RCBA32(ULBA) = 0x00000000; }

static void ich7_setup_pci_express(void)

Attachments:

attachment.htm (text/html — 3.7 KB)

Show replies by date

HAOUAS Elyes (Code Review)

18 Jan 18 Jan

2:40 p.m.

Hello Patrick Rudolph,

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

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

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

Change subject: nb/intel/i945: Use macro instead of magic number ......................................................................

nb/intel/i945: Use macro instead of magic number

Aslo add comments on Read/Write-Once bits. ULBA register is 64bits, so maybe there is a typo on RCBA(0x114) and it should be 0x11b insted. (Please see ICH7 datasheet page 271).

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 7 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/2

HAOUAS Elyes (Code Review)

3:27 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30993 )

Change subject: nb/intel/i945: Use macro instead of magic number ......................................................................

Patch Set 2:

(4 comments)

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... File src/northbridge/intel/i945/early_init.c:

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... PS2, Line 349: 810 this RO and reserved bits. and inany case , it will never happen as it is already done at ich7_setup_root_complex_topology function. (this RCBA register is R/WO)

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... PS2, Line 371: RCBA32(ESD) |= (2 << 16); : : RCBA32(ULD) |= (1 << 24) | (1 << 16); : : RCBA32(ULBA) = (uintptr_t)DEFAULT_DMIBAR; this will never happen the registers are R/WO and we already did it at function ich7_setup_root_complex_topology()

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... PS2, Line 887: 802 this RO and reserved. so is this write needed ?

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... PS2, Line 889: 1 this write do not make sense this bit is RO if I'm not wrong

HAOUAS Elyes (Code Review)

3:43 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30993 )

Change subject: nb/intel/i945: Use macro instead of magic number ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... File src/northbridge/intel/i945/early_init.c:

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... PS2, Line 350: RCBA32(RP1D) = 0x01000003 [23:16] is R/WO . this write is not needed and it will prevent as to write RCBA32(RP1D) |= (2 << 16); (line 377)

https://review.coreboot.org/#/c/30993/2/src/northbridge/intel/i945/early_ini... PS2, Line 351: RCBA32(RP2D) = 0x02000002; : RCBA32(RP3D) = 0x03000002; : RCBA32(RP4D) = 0x04000002; : RCBA32(HDD) = 0x0f000003; : RCBA32(RP5D) = 0x05000002; same ...

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 18 Jan 2019 15:43:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

5:13 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

Hello Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

nb/intel/i945: Don't rewrite R/WO RCBA registers

Element Self Description register (ESD) [23:16] is R/WO,so let write the ESD.CID when we start ich7_setup_root_complex_topology This value is also used to programme the R/WO 'Target Component ID' registers of RPxD and HHD. Once is done, no need to rewrite on them as they become RO(or reserved). (For more information, please see ICH7 datasheet page 271).

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/3

HAOUAS Elyes (Code Review)

5:27 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

Hello Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

nb/intel/i945: Don't rewrite R/WO RCBA registers

Element Self Description register (ESD) [23:16] is R/WO,so let write the ESD.CID when we start ich7_setup_root_complex_topology This value is also used to programme the R/WO 'Target Component ID' registers of RPxD and HHD. Once it is done, no need to rewrite on them as they become RO. (For more information, please see ICH7 datasheet page 271).

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/4

Paul Menzel (Code Review)

21 Jan 21 Jan

3:16 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

Patch Set 5: Code-Review+1

(1 comment)

Can you test this somehow?

https://review.coreboot.org/#/c/30993/5//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/30993/5//COMMIT_MSG@11 PS5, Line 11: programme program

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Mon, 21 Jan 2019 15:16:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

4:10 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

nb/intel/i945: Don't rewrite R/WO RCBA registers

Element Self Description register (ESD) [23:16] is R/WO,so let write the ESD.CID when we start ich7_setup_root_complex_topology This value is also used to program the R/WO 'Target Component ID' registers of RPxD and HHD. Once it is done, no need to rewrite on them as they become RO. (For more information, please see ICH7 datasheet page 271).

Tested on 945G-M4: befor this change, writing on those registers have no effect. The check is done using printk before and after writing. With this change, those R/WO got the "right" values.

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 6 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

4:14 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30993 )

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

Patch Set 6:

(3 comments)

https://review.coreboot.org/#/c/30993/6/src/northbridge/intel/i945/early_ini... File src/northbridge/intel/i945/early_init.c:

https://review.coreboot.org/#/c/30993/6/src/northbridge/intel/i945/early_ini... PS6, Line 349: : : : : : : here printk before and after the "write" shows that no thing happen. the "write" have no effect.

https://review.coreboot.org/#/c/30993/6/src/northbridge/intel/i945/early_ini... PS6, Line 371: : : : : : : : : : : : : here also, using printk, we can see that the "write" have no effect, as those registers are R/WO and we already write on them

https://review.coreboot.org/#/c/30993/6/src/northbridge/intel/i945/early_ini... PS6, Line 867: RCBA32(ESD) |= (2 << 16); : : RCBA32(ULD) |= (1 << 24) | (1 << 16); : : RCBA32(ULBA) = (uintptr_t)DEFAULT_DMIBAR; : /* Write ESD.CID to TCID */ : RCBA32(RP1D) |= (2 << 16); : RCBA32(RP2D) |= (2 << 16); : RCBA32(RP3D) |= (2 << 16); : RCBA32(RP4D) |= (2 << 16); : RCBA32(HDD) |= (2 << 16); : RCBA32(RP5D) |= (2 << 16); : RCBA32(RP6D) |= (2 << 16); here using printk, we can clearly see that we write on those registers

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 6 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Mon, 21 Jan 2019 16:14:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

5:57 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

nb/intel/i945: Don't rewrite R/WO RCBA registers

Element Self Description register (ESD) [23:16] is R/WO, so let write the ESD.CID when we start ich7_setup_root_complex_topology. This value is also used to program the R/WO 'Target Component ID' registers of RPxD and HHD. Once it is done, no need to rewrite on them as they become RO. (For more information, please see ICH7 datasheet page 271).

Tested on 945G-M4: befor this change, writing on those registers have no effect. The check is done using printk before and after writing. With this change, those R/WO got the "right" values.

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/7

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 7 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

26 Jan 26 Jan

4:55 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

nb/intel/i945: Don't rewrite R/WO RCBA registers

Tested on 945G-M4: befor this change, writing on those registers have no effect. The check is done using printk before and after writing. With this change, those R/WO got the "right" values. Here is the values: (witout this patch --> with this patch): ESD: 0x0104: 0x00000802 --> 0x0104: 0x00020802 ULD: 0x0110: 0x00000001 --> 0x0110: 0x01010001 ULBA: 0x0118: 0x00000000 --> 0x0118: 0xfed18000 RP1D: 0x0120: 0x01000003 --> 0x0120: 0x01020003 RP2D: 0x0130: 0x02000003 --> 0x0130: 0x02020003 RP3D: 0x0140: 0x03000002 --> 0x0140: 0x03020002 RP4D: 0x0150: 0x04000002 --> 0x0150: 0x04020002 HDD: 0x0160: 0x0f000002 --> 0x0160: 0x0f020002 RP5D: 0x0170: 0x05000002 --> 0x0170: 0x05020002 RP6D: 0x0180: 0x06000002 --> 0x0180: 0x06020002

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/10

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 10 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

4:55 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Don't rewrite R/WO RCBA registers

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Don't rewrite R/WO RCBA registers ......................................................................

nb/intel/i945: Don't rewrite R/WO RCBA registers

Tested on 945G-M4: befor this change, writing on those registers have no effect. The check is done using printk before and after writing. With this change, those R/WO got the "right" values. Here is the values: (without this patch --> with this patch): ESD: 0x0104: 0x00000802 --> 0x0104: 0x00020802 ULD: 0x0110: 0x00000001 --> 0x0110: 0x01010001 ULBA: 0x0118: 0x00000000 --> 0x0118: 0xfed18000 RP1D: 0x0120: 0x01000003 --> 0x0120: 0x01020003 RP2D: 0x0130: 0x02000003 --> 0x0130: 0x02020003 RP3D: 0x0140: 0x03000002 --> 0x0140: 0x03020002 RP4D: 0x0150: 0x04000002 --> 0x0150: 0x04020002 HDD: 0x0160: 0x0f000002 --> 0x0160: 0x0f020002 RP5D: 0x0170: 0x05000002 --> 0x0170: 0x05020002 RP6D: 0x0180: 0x06000002 --> 0x0180: 0x06020002

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/11

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 11 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

5:27 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

nb/intel/i945: Fix ich7_setup_root_complex_topology

Tested on 945G-M4: befor this change, writing on those registers have no effect. The check is done using printk before and after writing. With this change, those R/WO got the "right" values. i.e., We can see RCBA32(ULBA) is now equal to (uintptr_t)DEFAULT_DMIBAR. Here is the values: (without this patch --> with this patch): ESD: 0x0104: 0x00000802 --> 0x0104: 0x00020802 ULD: 0x0110: 0x00000001 --> 0x0110: 0x01010001 ULBA: 0x0118: 0x00000000 --> 0x0118: 0xfed18000 RP1D: 0x0120: 0x01000003 --> 0x0120: 0x01020003 RP2D: 0x0130: 0x02000003 --> 0x0130: 0x02020003 RP3D: 0x0140: 0x03000002 --> 0x0140: 0x03020002 RP4D: 0x0150: 0x04000002 --> 0x0150: 0x04020002 HDD: 0x0160: 0x0f000002 --> 0x0160: 0x0f020002 RP5D: 0x0170: 0x05000002 --> 0x0170: 0x05020002 RP6D: 0x0180: 0x06000002 --> 0x0180: 0x06020002

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/12

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 12 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

Paul Menzel (Code Review)

28 Jan 28 Jan

12:17 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

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

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

Patch Set 13: Code-Review+1

(4 comments)

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@14 PS13, Line 14: (For more information, please see ICH7 datasheet page 271). Please move the dot into the brackets: ….).

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@17 PS13, Line 17: befor Before

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@17 PS13, Line 17: have had

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@19 PS13, Line 19: got get

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 13 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Mon, 28 Jan 2019 12:17:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

12:27 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

nb/intel/i945: Fix ich7_setup_root_complex_topology

Tested on 945G-M4: before this change, writing on those registers had no effect. The check is done using printk before and after writing. With this change, those R/WO get the "right" values. i.e., We can see RCBA32(ULBA) is now equal to (uintptr_t)DEFAULT_DMIBAR. Here is the values: (without this patch --> with this patch): ESD: 0x0104: 0x00000802 --> 0x0104: 0x00020802 ULD: 0x0110: 0x00000001 --> 0x0110: 0x01010001 ULBA: 0x0118: 0x00000000 --> 0x0118: 0xfed18000 RP1D: 0x0120: 0x01000003 --> 0x0120: 0x01020003 RP2D: 0x0130: 0x02000003 --> 0x0130: 0x02020003 RP3D: 0x0140: 0x03000002 --> 0x0140: 0x03020002 RP4D: 0x0150: 0x04000002 --> 0x0150: 0x04020002 HDD: 0x0160: 0x0f000002 --> 0x0160: 0x0f020002 RP5D: 0x0170: 0x05000002 --> 0x0170: 0x05020002 RP6D: 0x0180: 0x06000002 --> 0x0180: 0x06020002

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/14

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 14 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

12:28 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30993 )

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

Patch Set 13:

(4 comments)

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@14 PS13, Line 14: (For more information, please see ICH7 datasheet page 271).

...

Please move the dot into the brackets: ….).

Done

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@17 PS13, Line 17: have

...

had

Done

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@17 PS13, Line 17: befor

...

Before

Done

https://review.coreboot.org/#/c/30993/13//COMMIT_MSG@19 PS13, Line 19: got

...

get

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 13 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Mon, 28 Jan 2019 12:28:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

27 Feb 27 Feb

12:59 p.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

nb/intel/i945: Fix ich7_setup_root_complex_topology

Tested done on 945G-M4 using printk before and after writing. Before this change, writing on those registers had no effect: ESD: 0x0104: 0x00000802 ULD: 0x0110: 0x00000001 ULBA: 0x0118: 0x00000000 RP1D: 0x0120: 0x01000003 RP2D: 0x0130: 0x02000003 RP3D: 0x0140: 0x03000002 RP4D: 0x0150: 0x04000002 HDD: 0x0160: 0x0f000002 RP5D: 0x0170: 0x05000002 RP6D: 0x0180: 0x06000002

Using this patche, those R/WO get the "right" values. i.e., We can see RCBA32(ULBA) is now equal to (uintptr_t)DEFAULT_DMIBAR. ESD: 0x0104: 0x00020802 ULD: 0x0110: 0x01010001 ULBA: 0x0118: 0xfed18000 RP1D: 0x0120: 0x01020003 RP2D: 0x0130: 0x02020003 RP3D: 0x0140: 0x03020002 RP4D: 0x0150: 0x04020002 HDD: 0x0160: 0x0f020002 RP5D: 0x0170: 0x05020002 RP6D: 0x0180: 0x06020002

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/30993/16

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 16 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

11 Apr 11 Apr

11:32 a.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30993 )

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

Patch Set 16: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 16 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-Comment-Date: Thu, 11 Apr 2019 11:32:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

11:32 a.m.

New subject: Change in ...coreboot[master]: nb/intel/i945: Fix ich7_setup_root_complex_topology

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30993 )

Change subject: nb/intel/i945: Fix ich7_setup_root_complex_topology ......................................................................

nb/intel/i945: Fix ich7_setup_root_complex_topology

Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/30993 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/i945/early_init.c 1 file changed, 15 insertions(+), 25 deletions(-)

Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved

diff --git a/src/northbridge/intel/i945/early_init.c b/src/northbridge/intel/i945/early_init.c index 08dd675..0c92c4b 100644 --- a/src/northbridge/intel/i945/early_init.c +++ b/src/northbridge/intel/i945/early_init.c @@ -341,13 +341,6 @@

RCBA32(V0CTL) = 0x80000001; RCBA32(V1CAP) = 0x03128010; - RCBA32(ESD) = 0x00000810; - RCBA32(RP1D) = 0x01000003; - RCBA32(RP2D) = 0x02000002; - RCBA32(RP3D) = 0x03000002; - RCBA32(RP4D) = 0x04000002; - RCBA32(HDD) = 0x0f000003; - RCBA32(RP5D) = 0x05000002;

pci_write_config16(PCI_DEV(0, 0x1c, 0), 0x42, 0x0141); pci_write_config16(PCI_DEV(0, 0x1c, 4), 0x42, 0x0141); @@ -361,20 +354,6 @@ reg32 |= (0x40 << 1) | (4 << 17) | (1 << 24) | (1 << 31); RCBA32(V1CTL) = reg32;

- RCBA32(ESD) |= (2 << 16); - - RCBA32(ULD) |= (1 << 24) | (1 << 16); - - RCBA32(ULBA) = (uintptr_t)DEFAULT_DMIBAR; - - RCBA32(RP1D) |= (2 << 16); - RCBA32(RP2D) |= (2 << 16); - RCBA32(RP3D) |= (2 << 16); - RCBA32(RP4D) |= (2 << 16); - RCBA32(HDD) |= (2 << 16); - RCBA32(RP5D) |= (2 << 16); - RCBA32(RP6D) |= (2 << 16); - RCBA32(LCAP) |= (3 << 10); }

@@ -871,10 +850,21 @@

static void ich7_setup_root_complex_topology(void) { - RCBA32(0x104) = 0x00000802; - RCBA32(0x110) = 0x00000001; - RCBA32(0x114) = 0x00000000; - RCBA32(0x118) = 0x00000000; + /* Write the R/WO registers */ + + RCBA32(ESD) |= (2 << 16); + + RCBA32(ULD) |= (1 << 24) | (1 << 16); + + RCBA32(ULBA) = (uintptr_t)DEFAULT_DMIBAR; + /* Write ESD.CID to TCID */ + RCBA32(RP1D) |= (2 << 16); + RCBA32(RP2D) |= (2 << 16); + RCBA32(RP3D) |= (2 << 16); + RCBA32(RP4D) |= (2 << 16); + RCBA32(HDD) |= (2 << 16); + RCBA32(RP5D) |= (2 << 16); + RCBA32(RP6D) |= (2 << 16); }

static void ich7_setup_pci_express(void)

-- To view, visit https://review.coreboot.org/c/coreboot/+/30993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3 Gerrit-Change-Number: 30993 Gerrit-PatchSet: 17 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: merged

1864

days inactive

1947

days old

coreboot-gerrit@coreboot.org

18 comments

3 participants

tags (0)

participants (3)

HAOUAS Elyes (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)