Change in ...coreboot[master]: nb/intel/i945: Fix typo on DMIBAR32(0x334)

List overview All Threads
Download

newer

older

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

HAOUAS Elyes (Code Review)

24 Jan 2019 24 Jan '19

7:48 a.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

nb/intel/i945: Fix typo on DMIBAR32(0x334)

Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/early_init.c 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/31063/1

diff --git a/src/northbridge/intel/i945/early_init.c b/src/northbridge/intel/i945/early_init.c index f11e612..cc7b1ef 100644 --- a/src/northbridge/intel/i945/early_init.c +++ b/src/northbridge/intel/i945/early_init.c @@ -513,7 +513,7 @@ DMIBAR32(0x314) = DMIBAR32(0x314); DMIBAR32(0x324) = DMIBAR32(0x324); DMIBAR32(0x328) = DMIBAR32(0x328); - DMIBAR32(0x338) = DMIBAR32(0x334); + DMIBAR32(0x334) = DMIBAR32(0x334); DMIBAR32(0x338) = DMIBAR32(0x338);

if (i945_silicon_revision() == 1 && (MCHBAR8(DFT_STRAP1) & (1 << 5))) {

Attachments:

attachment.htm (text/html — 2.5 KB)

Show replies by date

Nico Huber (Code Review)

26 Jan 26 Jan

9:43 a.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1: Code-Review+1

lol

Well, it obviously is wrong. But after 10 years + of testing why risk to break something?

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 12:43:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

9:45 a.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1:

...

lol

Well, it obviously is wrong. But after 10 years + of testing why risk to break something?

Actually we don't know if the code is wrong or the absence of a comment that would explain the intention. I first thought, the comment above says that we write-back what we read, but it doesn't go into that much detail.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 12:45:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

11:41 a.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1: Code-Review+1

lol

It would be wise to test this thoroughly. If it does indeed break something, I would definitely leave a comment regarding the matter.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 14:41:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

12:24 p.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1:

Elyes, you could also dump the values of these registers before writing to them. Maybe both have the same value anyway. Then we could be sure that this doesn't break anything.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 15:24:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

1:36 p.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/#/c/31063/1/src/northbridge/intel/i945/early_ini... PS1, Line 512: : : : : : lol

I've dropped all of them and still getting the same values tested on 945G-M4 + inteltool (with and without all of those lines): 0x0308: 0x00060018 0x0314: 0x00060000 0x0324: 0x00030018 0x0324: 0x00030018 0x0328: 0x00030000 0x0334: 0x00040008 0x0338: 0x00040000

I also have the same values with or without this patch, so for me, they are not WO but RO registers ...

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 16:36:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

1:46 p.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1: Code-Review+2

(1 comment)

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

https://review.coreboot.org/#/c/31063/1/src/northbridge/intel/i945/early_ini... PS1, Line 512: : : : : :

...

lol […]

Or maybe just bit 3 at 0x338 is RO. That's all we know.

Oh, wait did you remove all power in between? otherwise the values of a previous boot might still stick... not likely as 0x334 and 0x338 differ, though.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 16:46:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

2:09 p.m.

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

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/#/c/31063/1/src/northbridge/intel/i945/early_ini... PS1, Line 512: : : : : :

...

Or maybe just bit 3 at 0x338 is RO. That's all we know. […]

yes, I've removed all power and switched on without power in between. still getting the same values:) 0x0308: 0x00060018 0x0314: 0x00060000 0x0324: 0x00030018 0x0328: 0x00030000 0x0334: 0x00040008 0x0338: 0x00040000

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 26 Jan 2019 17:09:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Nico Huber (Code Review)

27 Jan 27 Jan

7:40 a.m.

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31063 )

Change subject: nb/intel/i945: Fix typo on DMIBAR32(0x334) ......................................................................

nb/intel/i945: Fix typo on DMIBAR32(0x334)

Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/31063 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de --- M src/northbridge/intel/i945/early_init.c 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve

diff --git a/src/northbridge/intel/i945/early_init.c b/src/northbridge/intel/i945/early_init.c index 763300a..b82812e 100644 --- a/src/northbridge/intel/i945/early_init.c +++ b/src/northbridge/intel/i945/early_init.c @@ -530,7 +530,7 @@ DMIBAR32(0x314) = DMIBAR32(0x314); DMIBAR32(0x324) = DMIBAR32(0x324); DMIBAR32(0x328) = DMIBAR32(0x328); - DMIBAR32(0x338) = DMIBAR32(0x334); + DMIBAR32(0x334) = DMIBAR32(0x334); DMIBAR32(0x338) = DMIBAR32(0x338);

if (i945_silicon_revision() == 1 && (MCHBAR8(DFT_STRAP1) & (1 << 5))) {

-- To view, visit https://review.coreboot.org/c/coreboot/+/31063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib894c24bc787c6c211da26dca78bcd330ded6681 Gerrit-Change-Number: 31063 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

2275

days inactive

2278

days old

coreboot-gerrit@coreboot.org

8 comments

3 participants

tags (0)

participants (3)

Angel Pons (Code Review)
HAOUAS Elyes (Code Review)
Nico Huber (Code Review)