Change in coreboot[master]: intel/i945: Call fixup_i945_errata() only for mobile version

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

HAOUAS Elyes (Code Review)

25 Nov 2019 25 Nov '19

1:09 a.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

intel/i945: Call fixup_i945_errata() only for mobile version

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

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/37188/1

diff --git a/src/northbridge/intel/i945/romstage.c b/src/northbridge/intel/i945/romstage.c index 4795881..131fa96 100644 --- a/src/northbridge/intel/i945/romstage.c +++ b/src/northbridge/intel/i945/romstage.c @@ -79,7 +79,8 @@ mainboard_late_rcba_config();

/* Chipset Errata! */ - fixup_i945_errata(); + if (CONFIG(NORTHBRIDGE_INTEL_SUBTYPE_I945GM)) + fixup_i945_errata();

/* Initialize the internal PCIe links before we go into stage2 */ i945_late_initialization(s3resume);

Attachments:

attachment.htm (text/html — 2.4 KB)

Show replies by date

Patrick Georgi (Code Review)

10 Mar 10 Mar

3:41 p.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/37188/1/src/northbridge/intel/i945/... File src/northbridge/intel/i945/romstage.c:

https://review.coreboot.org/c/coreboot/+/37188/1/src/northbridge/intel/i945/... PS1, Line 82: if (CONFIG(NORTHBRIDGE_INTEL_SUBTYPE_I945GM)) Please add the source stating that the errata are only for 945GM and rename the function to fixup_i945gm_errata()

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 10 Mar 2020 20:41:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

4:24 p.m.

Hello build bot (Jenkins), Patrick Georgi, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

intel/i945: Call fixup_i945_errata() only for mobile version

Per Mobile Intel ® 945 Express Chipset Family - Specification Update Document Number: 309220-013 (page 15) , the power saving optimization Erratum is for Mobile Intel ® 945 Express Chipset family. So rename it to fixup_i945gm_errata() and apply only for I945GM.

Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/errata.c M src/northbridge/intel/i945/raminit.h M src/northbridge/intel/i945/romstage.c 3 files changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/37188/2

HAOUAS Elyes (Code Review)

4:25 p.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

Patch Set 2:

(1 comment)

Thx

https://review.coreboot.org/c/coreboot/+/37188/1/src/northbridge/intel/i945/... File src/northbridge/intel/i945/romstage.c:

https://review.coreboot.org/c/coreboot/+/37188/1/src/northbridge/intel/i945/... PS1, Line 82: if (CONFIG(NORTHBRIDGE_INTEL_SUBTYPE_I945GM))

...

Please add the source stating that the errata are only for 945GM and rename the function to fixup_i9 […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 10 Mar 2020 21:25:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

4:25 p.m.

Hello build bot (Jenkins), Patrick Georgi, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

intel/i945: Call fixup_i945_errata() only for mobile version

Per Mobile Intel ® 945 Express Chipset Family - Specification Update Document Number: 309220-013 (page 15), the power saving optimization Erratum is for Mobile Intel ® 945 Express Chipset family. So rename it to fixup_i945gm_errata() and apply only for I945GM.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/37188/3

Angel Pons (Code Review)

4:45 p.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

Patch Set 3: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 3 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 10 Mar 2020 21:45:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

4:54 p.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37188/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37188/3//COMMIT_MSG@13 PS3, Line 13: How did you test this?

Please mention the function renaming.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 3 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 10 Mar 2020 21:54:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

4:56 p.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37188/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37188/3//COMMIT_MSG@13 PS3, Line 13:

...

How did you test this? […]

Probably not tested on mobile (should be unaffected) and maybe tested on the ECS 945G-M4. But in any case, given that desktop support was bolted on, I would expect that it's correct.

About the function renaming, isn't it mentioned already?

So rename it to fixup_i945gm_errata() and apply only for I945GM.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 3 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 10 Mar 2020 21:56:18 +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)

11 Mar 11 Mar

1:08 a.m.

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37188/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37188/3//COMMIT_MSG@13 PS3, Line 13:

...

Probably not tested on mobile (should be unaffected) and maybe tested on the ECS 945G-M4. […]

indeed, tested on 945G-M4. there is no code modification for mobile version.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 3 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 11 Mar 2020 06:08:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

1:11 a.m.

Hello build bot (Jenkins), Patrick Georgi, Angel Pons, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

intel/i945: Call fixup_i945_errata() only for mobile version

Per Mobile Intel ® 945 Express Chipset Family - Specification Update Document Number: 309220-013 (page 15), the power saving optimization Erratum is for Mobile Intel ® 945 Express Chipset family.

So rename 'fixup_i945_errata()' to 'fixup_i945gm_errata()' and apply that function only for I945GM.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/37188/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 4 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

9:25 a.m.

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37188 )

Change subject: intel/i945: Call fixup_i945_errata() only for mobile version ......................................................................

intel/i945: Call fixup_i945_errata() only for mobile version

Per Mobile Intel ® 945 Express Chipset Family - Specification Update Document Number: 309220-013 (page 15), the power saving optimization Erratum is for Mobile Intel ® 945 Express Chipset family.

So rename 'fixup_i945_errata()' to 'fixup_i945gm_errata()' and apply that function only for I945GM.

Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/37188 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/i945/errata.c M src/northbridge/intel/i945/raminit.h M src/northbridge/intel/i945/romstage.c 3 files changed, 4 insertions(+), 3 deletions(-)

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

diff --git a/src/northbridge/intel/i945/errata.c b/src/northbridge/intel/i945/errata.c index 2b9b941..4d8b999 100644 --- a/src/northbridge/intel/i945/errata.c +++ b/src/northbridge/intel/i945/errata.c @@ -17,7 +17,7 @@ #include "i945.h" #include "raminit.h"

-int fixup_i945_errata(void) +int fixup_i945gm_errata(void) { u32 reg32;

diff --git a/src/northbridge/intel/i945/raminit.h b/src/northbridge/intel/i945/raminit.h index d417169..26a1f50 100644 --- a/src/northbridge/intel/i945/raminit.h +++ b/src/northbridge/intel/i945/raminit.h @@ -67,5 +67,5 @@

void receive_enable_adjust(struct sys_info *sysinfo); void sdram_initialize(int boot_path, const u8 *sdram_addresses); -int fixup_i945_errata(void); +int fixup_i945gm_errata(void); #endif /* RAMINIT_H */ diff --git a/src/northbridge/intel/i945/romstage.c b/src/northbridge/intel/i945/romstage.c index 6274e09..2333b7d 100644 --- a/src/northbridge/intel/i945/romstage.c +++ b/src/northbridge/intel/i945/romstage.c @@ -76,7 +76,8 @@ mainboard_late_rcba_config();

/* Chipset Errata! */ - fixup_i945_errata(); + if (CONFIG(NORTHBRIDGE_INTEL_SUBTYPE_I945GM)) + fixup_i945gm_errata();

/* Initialize the internal PCIe links before we go into stage2 */ i945_late_initialization(s3resume);

-- To view, visit https://review.coreboot.org/c/coreboot/+/37188 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2656021b791061b4c22c0b252656a340de76ae5e Gerrit-Change-Number: 37188 Gerrit-PatchSet: 5 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: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1830

days inactive

1937

days old

coreboot-gerrit@coreboot.org

10 comments

4 participants

tags (0)

participants (4)

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