Change in coreboot[master]: cpu/intel/model_6xx: Unify all Slot 1 CPUs
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... cpu/intel/model_6xx: Unify all Slot 1 CPUs Also remove duplicated CPU IDs across two files. This shaves off 64 bytes from the resulting coreboot image for the Asus P2B-DS. Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Signed-off-by: Angel Pons <th3fanbus@gmail.com> --- D src/cpu/intel/model_65x_67x/Makefile.inc D src/cpu/intel/model_65x_67x/model_65x_67x_init.c D src/cpu/intel/model_68x_6bx/Makefile.inc D src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c M src/cpu/intel/model_6xx/Makefile.inc M src/cpu/intel/model_6xx/model_6xx_init.c M src/cpu/intel/slot_1/Makefile.inc 7 files changed, 34 insertions(+), 118 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/44241/1 diff --git a/src/cpu/intel/model_65x_67x/Makefile.inc b/src/cpu/intel/model_65x_67x/Makefile.inc deleted file mode 100644 index 3b78c4e..0000000 --- a/src/cpu/intel/model_65x_67x/Makefile.inc +++ /dev/null @@ -1,6 +0,0 @@ -## SPDX-License-Identifier: GPL-2.0-or-later - -ramstage-y += model_65x_67x_init.c - -cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-05-*) -cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-07-*) diff --git a/src/cpu/intel/model_65x_67x/model_65x_67x_init.c b/src/cpu/intel/model_65x_67x/model_65x_67x_init.c deleted file mode 100644 index 86950e4..0000000 --- a/src/cpu/intel/model_65x_67x/model_65x_67x_init.c +++ /dev/null @@ -1,49 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <device/device.h> -#include <cpu/cpu.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/lapic.h> -#include <cpu/intel/microcode.h> -#include <cpu/x86/cache.h> -#include <cpu/intel/l2_cache.h> - -static void model_65x_67x_init(struct device *cpu) -{ - /* Update the microcode */ - intel_update_microcode_from_cbfs(); - - /* Initialize L2 cache */ - p6_configure_l2_cache(); - - /* Turn on caching if we haven't already */ - x86_enable_cache(); - - /* Setup MTRRs */ - x86_setup_mtrrs(); - x86_mtrr_check(); - - /* Enable the local CPU APICs */ - setup_lapic(); -} - -static struct device_operations cpu_dev_ops = { - .init = model_65x_67x_init, -}; - -static const struct cpu_device_id cpu_table[] = { - { X86_VENDOR_INTEL, 0x0650 }, /* PII/Celeron, dA0/mdA0/A0 */ - { X86_VENDOR_INTEL, 0x0651 }, /* PII/Celeron, dA1/A1 */ - { X86_VENDOR_INTEL, 0x0652 }, /* PII/Celeron/Xeon, dB0/mdB0/B0 */ - { X86_VENDOR_INTEL, 0x0653 }, /* PII/Xeon, dB1/B1 */ - - { X86_VENDOR_INTEL, 0x0671 }, - { X86_VENDOR_INTEL, 0x0672 }, /* PIII, kB0 */ - { X86_VENDOR_INTEL, 0x0673 }, /* PIII, kC0 */ - { 0, 0 }, -}; - -static const struct cpu_driver driver __cpu_driver = { - .ops = &cpu_dev_ops, - .id_table = cpu_table, -}; diff --git a/src/cpu/intel/model_68x_6bx/Makefile.inc b/src/cpu/intel/model_68x_6bx/Makefile.inc deleted file mode 100644 index b12f4d0..0000000 --- a/src/cpu/intel/model_68x_6bx/Makefile.inc +++ /dev/null @@ -1,7 +0,0 @@ -## SPDX-License-Identifier: GPL-2.0-or-later - -ramstage-y += model_68x_6bx_init.c -subdirs-y += ../../x86/name - -cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-08-*) -cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-0b-*) diff --git a/src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c b/src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c deleted file mode 100644 index 06ac099..0000000 --- a/src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c +++ /dev/null @@ -1,54 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <console/console.h> -#include <device/device.h> -#include <cpu/cpu.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/lapic.h> -#include <cpu/intel/microcode.h> -#include <cpu/x86/cache.h> -#include <cpu/x86/name.h> - -static void model_68x_6bx_init(struct device *cpu) -{ - char processor_name[49]; - - /* Update the microcode */ - intel_update_microcode_from_cbfs(); - - /* Turn on caching if we haven't already */ - x86_enable_cache(); - - /* Print processor name */ - fill_processor_name(processor_name); - printk(BIOS_INFO, "CPU: %s.\n", processor_name); - - /* Setup MTRRs */ - x86_setup_mtrrs(); - x86_mtrr_check(); - - /* Enable the local CPU APICs */ - setup_lapic(); -} - -static struct device_operations cpu_dev_ops = { - .init = model_68x_6bx_init, -}; - -static const struct cpu_device_id cpu_table[] = { - { X86_VENDOR_INTEL, 0x0680 }, - { X86_VENDOR_INTEL, 0x0681 }, /* PIII, cA2/cA2c/A2/BA2/PA2/MA2 */ - { X86_VENDOR_INTEL, 0x0683 }, /* PIII/Celeron, cB0/cB0c/B0/BB0/PB0/MB0*/ - { X86_VENDOR_INTEL, 0x0686 }, /* PIII/Celeron, cC0/C0/BC0/PC0/MC0 */ - { X86_VENDOR_INTEL, 0x068a }, /* PIII/Celeron, cD0/D0/BD0/PD0 */ - - { X86_VENDOR_INTEL, 0x06b1 }, /* Pentium III/Celeron, tA1/A1/FPA1 */ - { X86_VENDOR_INTEL, 0x06b4 }, /* Pentium III, tB1/FPB1 */ - - { 0, 0 }, -}; - -static const struct cpu_driver driver __cpu_driver = { - .ops = &cpu_dev_ops, - .id_table = cpu_table, -}; diff --git a/src/cpu/intel/model_6xx/Makefile.inc b/src/cpu/intel/model_6xx/Makefile.inc index b2f87ab..83d3dc0 100644 --- a/src/cpu/intel/model_6xx/Makefile.inc +++ b/src/cpu/intel/model_6xx/Makefile.inc @@ -1,3 +1,9 @@ ramstage-y += model_6xx_init.c +subdirs-y += ../../x86/name + +cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-05-*) cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-06-*) +cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-07-*) +cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-08-*) +cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-0b-*) diff --git a/src/cpu/intel/model_6xx/model_6xx_init.c b/src/cpu/intel/model_6xx/model_6xx_init.c index 8ebc7ee..b3019c3 100644 --- a/src/cpu/intel/model_6xx/model_6xx_init.c +++ b/src/cpu/intel/model_6xx/model_6xx_init.c @@ -1,20 +1,35 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <console/console.h> #include <device/device.h> #include <cpu/cpu.h> #include <cpu/x86/mtrr.h> #include <cpu/x86/lapic.h> #include <cpu/intel/microcode.h> #include <cpu/x86/cache.h> +#include <cpu/intel/l2_cache.h> +#include <cpu/x86/name.h> static void model_6xx_init(struct device *dev) { /* Update the microcode */ intel_update_microcode_from_cbfs(); + /* Initialize off-die L2 cache */ + if ((dev->device & 0x0ff0) == 0x0650 || (dev->device & 0x0ff0) == 0x0670) + p6_configure_l2_cache(); + /* Turn on caching if we haven't already */ x86_enable_cache(); + /* Print processor name */ + if ((dev->device & 0x0ff0) == 0x0680 || (dev->device & 0x0ff0) == 0x06b0) { + char processor_name[49]; + + fill_processor_name(processor_name); + printk(BIOS_INFO, "CPU: %s.\n", processor_name); + } + /* Setup MTRRs */ x86_setup_mtrrs(); x86_mtrr_check(); @@ -37,10 +52,19 @@ { X86_VENDOR_INTEL, 0x0633 }, /* PII, C0 */ { X86_VENDOR_INTEL, 0x0634 }, /* PII, C1 */ + { X86_VENDOR_INTEL, 0x0650 }, /* PII/Celeron, dA0/mdA0/A0 */ + { X86_VENDOR_INTEL, 0x0651 }, /* PII/Celeron, dA1/A1 */ + { X86_VENDOR_INTEL, 0x0652 }, /* PII/Celeron/Xeon, dB0/mdB0/B0 */ + { X86_VENDOR_INTEL, 0x0653 }, /* PII/Xeon, dB1/B1 */ + { X86_VENDOR_INTEL, 0x0660 }, /* Celeron, A0 */ { X86_VENDOR_INTEL, 0x0665 }, /* Celeron, B0 */ { X86_VENDOR_INTEL, 0x066a }, /* PII, mdxA0/dmmA0 + others */ + { X86_VENDOR_INTEL, 0x0671 }, + { X86_VENDOR_INTEL, 0x0672 }, /* PIII, kB0 */ + { X86_VENDOR_INTEL, 0x0673 }, /* PIII, kC0 */ + { X86_VENDOR_INTEL, 0x0680 }, { X86_VENDOR_INTEL, 0x0681 }, /* PIII, cA2/cA2c/A2/BA2/PA2/MA2 */ { X86_VENDOR_INTEL, 0x0683 }, /* PIII/Celeron, cB0/cB0c/B0/BB0/PB0/MB0*/ @@ -50,6 +74,10 @@ { X86_VENDOR_INTEL, 0x06a0 }, /* PIII, A0 */ { X86_VENDOR_INTEL, 0x06a1 }, /* PIII, A1 */ { X86_VENDOR_INTEL, 0x06a4 }, /* PIII, B0 */ + + { X86_VENDOR_INTEL, 0x06b1 }, /* Pentium III/Celeron, tA1/A1/FPA1 */ + { X86_VENDOR_INTEL, 0x06b4 }, /* Pentium III, tB1/FPB1 */ + { 0, 0 }, }; diff --git a/src/cpu/intel/slot_1/Makefile.inc b/src/cpu/intel/slot_1/Makefile.inc index 593e585..dbdb7c3 100644 --- a/src/cpu/intel/slot_1/Makefile.inc +++ b/src/cpu/intel/slot_1/Makefile.inc @@ -3,8 +3,6 @@ ramstage-y += slot_1.c ramstage-y += l2_cache.c subdirs-y += ../model_6xx -subdirs-y += ../model_65x_67x -subdirs-y += ../model_68x_6bx subdirs-y += ../microcode bootblock-y += ../car/p3/cache_as_ram.S -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: newchange
Hello Keith Hui, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44241 to look at the new patch set (#2). Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... cpu/intel/model_6xx: Unify all Slot 1 CPUs Also remove duplicated CPU IDs across two files. This shaves off 64 bytes from the resulting coreboot image for the Asus P2B-DS. Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Signed-off-by: Angel Pons <th3fanbus@gmail.com> --- D src/cpu/intel/model_65x_67x/Makefile.inc D src/cpu/intel/model_65x_67x/model_65x_67x_init.c D src/cpu/intel/model_68x_6bx/Makefile.inc D src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c M src/cpu/intel/model_6xx/Makefile.inc M src/cpu/intel/model_6xx/model_6xx_init.c M src/cpu/intel/slot_1/Makefile.inc 7 files changed, 34 insertions(+), 118 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/44241/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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
Hello Keith Hui, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44241 to look at the new patch set (#3). Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... cpu/intel/model_6xx: Unify all Slot 1 CPUs Also remove duplicated CPU IDs across two files. This shaves off 64 bytes from the resulting coreboot image for the Asus P2B-DS. Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Signed-off-by: Angel Pons <th3fanbus@gmail.com> --- D src/cpu/intel/model_65x_67x/Makefile.inc D src/cpu/intel/model_65x_67x/model_65x_67x_init.c D src/cpu/intel/model_68x_6bx/Makefile.inc D src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c M src/cpu/intel/model_6xx/Makefile.inc M src/cpu/intel/model_6xx/model_6xx_init.c M src/cpu/intel/slot_1/Makefile.inc 7 files changed, 67 insertions(+), 153 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/44241/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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
Hello Keith Hui, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44241 to look at the new patch set (#4). Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... cpu/intel/model_6xx: Unify all Slot 1 CPUs Also remove duplicated CPU IDs across two files. This shaves off 64 bytes from the resulting coreboot image for the Asus P2B-DS. There may be duplicated URLs now. This is handled in a later commit. Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Signed-off-by: Angel Pons <th3fanbus@gmail.com> --- D src/cpu/intel/model_65x_67x/Makefile.inc D src/cpu/intel/model_65x_67x/model_65x_67x_init.c D src/cpu/intel/model_68x_6bx/Makefile.inc D src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c M src/cpu/intel/model_6xx/Makefile.inc M src/cpu/intel/model_6xx/model_6xx_init.c M src/cpu/intel/slot_1/Makefile.inc 7 files changed, 67 insertions(+), 153 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/44241/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 4: (1 comment) you duplicated links ... https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... File src/cpu/intel/model_6xx/model_6xx_init.c: https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... PS4, Line 71: download.intel.com/design/PentiumII/specupdt/24333749.pdf duplicated -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 15:48:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... File src/cpu/intel/model_6xx/model_6xx_init.c: https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... PS4, Line 71: download.intel.com/design/PentiumII/specupdt/24333749.pdf
duplicated see commit message.
-- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 15:50:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... File src/cpu/intel/model_6xx/model_6xx_init.c: https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... PS4, Line 26: if ((dev->device & 0x0ff0) == 0x0680 || (dev->device & 0x0ff0) == 0x06b0) { : char processor_name[49]; : : fill_processor_name(processor_name); : printk(BIOS_INFO, "CPU: %s.\n", processor_name); : } I don't think it makes sense to print the processor name on just these CPU's. One could just check in fill_processor_name() if CPUID.EAX=0x80000004 is supported and change the return type to report fail/success. -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 15:53:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... File src/cpu/intel/model_6xx/model_6xx_init.c: https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... PS4, Line 26: if ((dev->device & 0x0ff0) == 0x0680 || (dev->device & 0x0ff0) == 0x06b0) { : char processor_name[49]; : : fill_processor_name(processor_name); : printk(BIOS_INFO, "CPU: %s.\n", processor_name); : }
I don't think it makes sense to print the processor name on just these CPU's. […] I did it this way to preserve the behavior of the original code with this commit. I can ofc change this in a follow-up. Sounds good?
-- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 15:55:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... File src/cpu/intel/model_6xx/model_6xx_init.c: https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... PS4, Line 71: download.intel.com/design/PentiumII/specupdt/24333749.pdf
see commit message. oops, sorry
-- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 15:56:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 4: (1 comment) It might also be worth stating in the commit message that it assumes the only users of the CPU's will be slot_1 (which supports all of these?) and that further distinction should not be needed. https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... File src/cpu/intel/model_6xx/model_6xx_init.c: https://review.coreboot.org/c/coreboot/+/44241/4/src/cpu/intel/model_6xx/mod... PS4, Line 26: if ((dev->device & 0x0ff0) == 0x0680 || (dev->device & 0x0ff0) == 0x06b0) { : char processor_name[49]; : : fill_processor_name(processor_name); : printk(BIOS_INFO, "CPU: %s.\n", processor_name); : }
I did it this way to preserve the behavior of the original code with this commit. I can ofc change this in a follow-up. Sounds good?
sure! -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 16:00:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
Hello Keith Hui, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44241 to look at the new patch set (#5). Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... cpu/intel/model_6xx: Unify all Slot 1 CPUs These CPUs have very similar initialization sequences. Since only Slot 1 currently uses them, we might as well group them together. This patch aims to preserve the behavior of the original code, while reducing the amount of redundant boilerplate to a minimum. Also remove duplicated CPU IDs across two files. This shaves off 64 bytes from the resulting coreboot image for the Asus P2B-DS. There may be duplicated URLs now. This is handled in a later commit. Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Signed-off-by: Angel Pons <th3fanbus@gmail.com> --- D src/cpu/intel/model_65x_67x/Makefile.inc D src/cpu/intel/model_65x_67x/model_65x_67x_init.c D src/cpu/intel/model_68x_6bx/Makefile.inc D src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c M src/cpu/intel/model_6xx/Makefile.inc M src/cpu/intel/model_6xx/model_6xx_init.c M src/cpu/intel/slot_1/Makefile.inc 7 files changed, 67 insertions(+), 153 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/44241/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 5:
Patch Set 4:
(1 comment)
It might also be worth stating in the commit message that it assumes the only users of the CPU's will be slot_1 (which supports all of these?) and that further distinction should not be needed.
Done. Several of these CPUs aren't Slot 1: Pentium Pro is Socket 8, and later Pentium 3 are Socket 370. However... Slotkets! https://en.wikipedia.org/wiki/Slotket -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 16:10:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Patch Set 5:
Patch Set 5:
Patch Set 4:
(1 comment)
It might also be worth stating in the commit message that it assumes the only users of the CPU's will be slot_1 (which supports all of these?) and that further distinction should not be needed.
Done. Several of these CPUs aren't Slot 1: Pentium Pro is Socket 8, and later Pentium 3 are Socket 370. However... Slotkets! https://en.wikipedia.org/wiki/Slotket
Also, there's Socket 370 mainboards with the i440BX northbridge: https://en.wikipedia.org/wiki/ABIT_BP6 -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Aug 2020 16:11:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello Keith Hui, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44241 to look at the new patch set (#6). Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... cpu/intel/model_6xx: Unify all Slot 1 CPUs Also remove duplicated CPU IDs across two files. This shaves off 64 bytes from the resulting coreboot image for the Asus P2B-DS. There may be duplicated URLs now. This is handled in a later commit. Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Signed-off-by: Angel Pons <th3fanbus@gmail.com> --- D src/cpu/intel/model_65x_67x/Makefile.inc D src/cpu/intel/model_65x_67x/model_65x_67x_init.c D src/cpu/intel/model_68x_6bx/Makefile.inc D src/cpu/intel/model_68x_6bx/model_68x_6bx_init.c M src/cpu/intel/model_6xx/Makefile.inc M src/cpu/intel/model_6xx/model_6xx_init.c M src/cpu/intel/slot_1/Makefile.inc 7 files changed, 67 insertions(+), 153 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/44241/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 6 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44241 ) Change subject: cpu/intel/model_6xx: Unify all Slot 1 CPUs ...................................................................... Abandoned Sorry, I ran out of patience and energy to care about these changes any longer. -- To view, visit https://review.coreboot.org/c/coreboot/+/44241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22092d97c244263eec7e65abbfe2bbeb58680fc8 Gerrit-Change-Number: 44241 Gerrit-PatchSet: 7 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> 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: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: abandon
participants (3)
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
HAOUAS Elyes (Code Review)