Change in coreboot[master]: cpu/intel: Core 2 Duo E8200 is not model_6fx

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Core 2 Duo E8200 is not model_6fx
......................................................................

cpu/intel: Core 2 Duo E8200 is not model_6fx

With a CPUID of 10676, it is clearly model_1067x... Wait, it's already
there, but the comment is wrong. There isn't any CPU called Core 2 Solo.

Change-Id: Ia4b73537805e2a8fa9e28bde76aa20a524f8f873
Signed-off-by: Angel Pons 
---
M src/cpu/intel/model_1067x/model_1067x_init.c
M src/cpu/intel/model_6fx/model_6fx_init.c
2 files changed, 1 insertion(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44247/1

diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c
index cd774d3..ecdb8cf 100644
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
@@ -292,7 +292,7 @@
 };

static const struct cpu_device_id cpu_table[] = {
-	{ X86_VENDOR_INTEL, 0x10676 }, /* Intel Core 2 Solo/Core Duo */
+	{ X86_VENDOR_INTEL, 0x10676 }, /* Core 2 Duo E8200 */
    { X86_VENDOR_INTEL, 0x10677 },
    { X86_VENDOR_INTEL, 0x1067A },
    { 0, 0 },
diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c
index e2755f8..d0987b4 100644
--- a/src/cpu/intel/model_6fx/model_6fx_init.c
+++ b/src/cpu/intel/model_6fx/model_6fx_init.c
@@ -146,7 +146,6 @@
    { X86_VENDOR_INTEL, 0x06fb }, /* Intel Core 2 Solo/Core Duo */
    { X86_VENDOR_INTEL, 0x06fd }, /* Intel Core 2 Solo/Core Duo */
    { X86_VENDOR_INTEL, 0x10661 }, /* Intel Core 2 Celeron Conroe-L */
-	{ X86_VENDOR_INTEL, 0x10676 }, /* Core2 Duo E8200 */
    { 0, 0 },
 };

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Core 2 Duo E8200 is not model_6fx
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/44247/1/src/cpu/intel/model_1067x/m... 
File src/cpu/intel/model_1067x/model_1067x_init.c:

https://review.coreboot.org/c/coreboot/+/44247/1/src/cpu/intel/model_1067x/m... 
PS1, Line 295: /* Core 2 Duo E8200 */
is this the only CPU with that CPUID signature? Maybe just remove the comment?

[Angel Pons (Code Review)]

Hello Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: cpu/intel: Core 2 Duo E8200 is not model_6fx
......................................................................

cpu/intel: Core 2 Duo E8200 is not model_6fx

With a CPUID of 10676, it is clearly model_1067x... Wait, it's already
there, but the comment is wrong. There isn't any CPU called Core 2 Solo.

Change-Id: Ia4b73537805e2a8fa9e28bde76aa20a524f8f873
Signed-off-by: Angel Pons 
---
M src/cpu/intel/model_1067x/model_1067x_init.c
M src/cpu/intel/model_6fx/model_6fx_init.c
2 files changed, 1 insertion(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44247/2

[Angel Pons (Code Review)]

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

Change subject: cpu/intel: Core 2 Duo E8200 is not model_6fx
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44247/1/src/cpu/intel/model_1067x/m... 
File src/cpu/intel/model_1067x/model_1067x_init.c:

https://review.coreboot.org/c/coreboot/+/44247/1/src/cpu/intel/model_1067x/m... 
PS1, Line 295: /* Core 2 Duo E8200 */
> is this the only CPU with that CPUID signature? Maybe just remove the comment?

No, done

[Paul Menzel (Code Review)]

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

Change subject: cpu/intel: Core 2 Duo E8200 is not model_6fx
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44247/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44247/2//COMMIT_MSG@7 
PS2, Line 7: cpu/intel: Core 2 Duo E8200 is not model_6fx
Please make it a statement about the action, and not problem.
> Remove incorrect Core 2 Duo E8200 entry and comments

[Angel Pons (Code Review)]

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

Change subject: cpu/intel: Core 2 Duo E8200 is not model_6fx
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44247/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44247/2//COMMIT_MSG@7 
PS2, Line 7: cpu/intel: Core 2 Duo E8200 is not model_6fx
> Please make it a statement about the action, and not problem. […]

IMHO, that's longer but doesn't summarize the problem as well.

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

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

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

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

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

With a CPUID of 10676, it is clearly model_1067x... Wait, it's already
there, but the comment is wrong. There isn't any CPU called Core 2 Solo.

Change-Id: Ia4b73537805e2a8fa9e28bde76aa20a524f8f873
Signed-off-by: Angel Pons 
---
M src/cpu/intel/model_1067x/model_1067x_init.c
M src/cpu/intel/model_6fx/model_6fx_init.c
2 files changed, 1 insertion(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44247/3

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44247/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44247/2//COMMIT_MSG@7 
PS2, Line 7: cpu/intel: Core 2 Duo E8200 is not model_6fx
> IMHO, that's longer but doesn't summarize the problem as well.

Done

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[HAOUAS Elyes (Code Review)]

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Patch Set 3:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/44247/3//COMMIT_MSG@10 
PS3, Line 10: There isn't any CPU called Core 2 Solo
https://ark.intel.com/content/www/fr/fr/ark/products/37133/intel-core-2-solo...

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

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

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

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

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

With a CPUID of 10676, it is clearly model_1067x... Wait, it's already
there, but the comment is wrong. This ID isn't for Core Duo CPUs.

Change-Id: Ia4b73537805e2a8fa9e28bde76aa20a524f8f873
Signed-off-by: Angel Pons 
---
M src/cpu/intel/model_1067x/model_1067x_init.c
M src/cpu/intel/model_6fx/model_6fx_init.c
2 files changed, 1 insertion(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44247/4

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Patch Set 4:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/44247/3//COMMIT_MSG@10 
PS3, Line 10: There isn't any CPU called Core 2 Solo
> https://ark.intel. […]

Ack

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Abandoned

Sorry, I ran out of patience and energy to care about these changes any longer.

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

Angel Pons has restored this change. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Restored

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Removed Verified+1 by build bot (Jenkins)

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Paul Menzel (Code Review)]

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Patch Set 6: Code-Review+2

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Patch Set 6: Code-Review+2

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

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

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

Patch Set 6:

Thank you!

Change in coreboot[master]: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44247 )

Change subject: cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx
......................................................................

cpu/intel: Remove Core 2 Duo E8200 CPUID from model_6fx

With a CPUID of 10676, it is clearly model_1067x... Wait, it's already
there, but the comment is wrong. This ID isn't for Core Duo CPUs.

Change-Id: Ia4b73537805e2a8fa9e28bde76aa20a524f8f873
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44247
Tested-by: build bot (Jenkins) 
Reviewed-by: Paul Menzel 
Reviewed-by: Arthur Heymans 
---
M src/cpu/intel/model_1067x/model_1067x_init.c
M src/cpu/intel/model_6fx/model_6fx_init.c
2 files changed, 1 insertion(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, approved
  Arthur Heymans: Looks good to me, approved

diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c
index cd774d3..6553f29 100644
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
@@ -292,7 +292,7 @@
 };

static const struct cpu_device_id cpu_table[] = {
-	{ X86_VENDOR_INTEL, 0x10676 }, /* Intel Core 2 Solo/Core Duo */
+	{ X86_VENDOR_INTEL, 0x10676 },
    { X86_VENDOR_INTEL, 0x10677 },
    { X86_VENDOR_INTEL, 0x1067A },
    { 0, 0 },
diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c
index e2755f8..d0987b4 100644
--- a/src/cpu/intel/model_6fx/model_6fx_init.c
+++ b/src/cpu/intel/model_6fx/model_6fx_init.c
@@ -146,7 +146,6 @@
    { X86_VENDOR_INTEL, 0x06fb }, /* Intel Core 2 Solo/Core Duo */
    { X86_VENDOR_INTEL, 0x06fd }, /* Intel Core 2 Solo/Core Duo */
    { X86_VENDOR_INTEL, 0x10661 }, /* Intel Core 2 Celeron Conroe-L */
-	{ X86_VENDOR_INTEL, 0x10676 }, /* Core2 Duo E8200 */
    { 0, 0 },
 };