coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: sb/amd/cimx/sb800: Separate a section from fch.asl
by Kyösti Mälkki (Code Review) 11 Nov '21

11 Nov '21

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59178 ) Change subject: sb/amd/cimx/sb800: Separate a section from fch.asl ...................................................................... sb/amd/cimx/sb800: Separate a section from fch.asl The section is the same and at root scope. Change-Id: I3b3ff2fddc7d4db09903151bcb92e3e1b5dc7d69 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/mainboard/lippert/frontrunner-af/dsdt.asl M src/southbridge/amd/cimx/sb800/acpi/fch.asl A src/southbridge/amd/cimx/sb800/acpi/misc_io.asl 3 files changed, 173 insertions(+), 338 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/59178/1 diff --git a/src/mainboard/lippert/frontrunner-af/dsdt.asl b/src/mainboard/lippert/frontrunner-af/dsdt.asl index e35a70b..70a9b2e 100644 --- a/src/mainboard/lippert/frontrunner-af/dsdt.asl +++ b/src/mainboard/lippert/frontrunner-af/dsdt.asl @@ -37,174 +37,7 @@ } } /* End _SB scope */ - /* Client Management index/data registers */ - OperationRegion(CMT, SystemIO, 0x00000C50, 0x00000002) - Field(CMT, ByteAcc, NoLock, Preserve) { - CMTI, 8, - /* Client Management Data register */ - G64E, 1, - G64O, 1, - G32O, 2, - , 2, - GPSL, 2, - } - - /* GPM Port register */ - OperationRegion(GPT, SystemIO, 0x00000C52, 0x00000001) - Field(GPT, ByteAcc, NoLock, Preserve) { - GPB0,1, - GPB1,1, - GPB2,1, - GPB3,1, - GPB4,1, - GPB5,1, - GPB6,1, - GPB7,1, - } - - /* Flash ROM program enable register */ - OperationRegion(FRE, SystemIO, 0x00000C6F, 0x00000001) - Field(FRE, ByteAcc, NoLock, Preserve) { - , 0x00000006, - FLRE, 0x00000001, - } - - /* PM2 index/data registers */ - OperationRegion(PM2R, SystemIO, 0x00000CD0, 0x00000002) - Field(PM2R, ByteAcc, NoLock, Preserve) { - PM2I, 0x00000008, - PM2D, 0x00000008, - } - - /* Power Management I/O registers, TODO:PMIO is quite different in SB800. */ - OperationRegion(PIOR, SystemIO, 0x00000CD6, 0x00000002) - Field(PIOR, ByteAcc, NoLock, Preserve) { - PIOI, 0x00000008, - PIOD, 0x00000008, - } - IndexField (PIOI, PIOD, ByteAcc, NoLock, Preserve) { - , 1, /* MiscControl */ - T1EE, 1, - T2EE, 1, - Offset(0x01), /* MiscStatus */ - , 1, - T1E, 1, - T2E, 1, - Offset(0x04), /* SmiWakeUpEventEnable3 */ - , 7, - SSEN, 1, - Offset(0x07), /* SmiWakeUpEventStatus3 */ - , 7, - CSSM, 1, - Offset(0x10), /* AcpiEnable */ - , 6, - PWDE, 1, - Offset(0x1C), /* ProgramIoEnable */ - , 3, - MKME, 1, - IO3E, 1, - IO2E, 1, - IO1E, 1, - IO0E, 1, - Offset(0x1D), /* IOMonitorStatus */ - , 3, - MKMS, 1, - IO3S, 1, - IO2S, 1, - IO1S, 1, - IO0S,1, - Offset(0x20), /* AcpiPmEvtBlk. TODO: should be 0x60 */ - APEB, 16, - Offset(0x36), /* GEvtLevelConfig */ - , 6, - ELC6, 1, - ELC7, 1, - Offset(0x37), /* GPMLevelConfig0 */ - , 3, - PLC0, 1, - PLC1, 1, - PLC2, 1, - PLC3, 1, - PLC8, 1, - Offset(0x38), /* GPMLevelConfig1 */ - , 1, - PLC4, 1, - PLC5, 1, - , 1, - PLC6, 1, - PLC7, 1, - Offset(0x3B), /* PMEStatus1 */ - GP0S, 1, - GM4S, 1, - GM5S, 1, - APS, 1, - GM6S, 1, - GM7S, 1, - GP2S, 1, - STSS, 1, - Offset(0x55), /* SoftPciRst */ - SPRE, 1, - , 1, - , 1, - PNAT, 1, - PWMK, 1, - PWNS, 1, - - Offset(0x65), /* UsbPMControl */ - , 4, - URRE, 1, - Offset(0x68), /* MiscEnable68 */ - , 3, - TMTE, 1, - , 1, - Offset(0x92), /* GEVENTIN */ - , 7, - E7IS, 1, - Offset(0x96), /* GPM98IN */ - G8IS, 1, - G9IS, 1, - Offset(0x9A), /* EnhanceControl */ - ,7, - HPDE, 1, - Offset(0xA8), /* PIO7654Enable */ - IO4E, 1, - IO5E, 1, - IO6E, 1, - IO7E, 1, - Offset(0xA9), /* PIO7654Status */ - IO4S, 1, - IO5S, 1, - IO6S, 1, - IO7S, 1, - } - - /* PM1 Event Block - * First word is PM1_Status, Second word is PM1_Enable - */ - OperationRegion(P1EB, SystemIO, APEB, 0x04) - Field(P1EB, ByteAcc, NoLock, Preserve) { - TMST, 1, - , 3, - BMST, 1, - GBST, 1, - Offset(0x01), - PBST, 1, - , 1, - RTST, 1, - , 3, - PWST, 1, - SPWS, 1, - Offset(0x02), - TMEN, 1, - , 4, - GBEN, 1, - Offset(0x03), - PBEN, 1, - , 1, - RTEN, 1, - , 3, - PWDA, 1, - } + #include <southbridge/amd/cimx/sb800/acpi/misc_io.asl> #include "acpi/routing.asl" diff --git a/src/southbridge/amd/cimx/sb800/acpi/fch.asl b/src/southbridge/amd/cimx/sb800/acpi/fch.asl index 88fbf7b..48247c7 100644 --- a/src/southbridge/amd/cimx/sb800/acpi/fch.asl +++ b/src/southbridge/amd/cimx/sb800/acpi/fch.asl @@ -164,176 +164,7 @@ } /* End Method(_SB._INI) */ Scope(\){ - /* Client Management index/data registers */ - OperationRegion(CMT, SystemIO, 0x00000C50, 0x00000002) - Field(CMT, ByteAcc, NoLock, Preserve) { - CMTI, 8, - /* Client Management Data register */ - G64E, 1, - G64O, 1, - G32O, 2, - , 2, - GPSL, 2, - } - /* GPM Port register */ - OperationRegion(GPT, SystemIO, 0x00000C52, 0x00000001) - Field(GPT, ByteAcc, NoLock, Preserve) { - GPB0,1, - GPB1,1, - GPB2,1, - GPB3,1, - GPB4,1, - GPB5,1, - GPB6,1, - GPB7,1, - } + #include "misc_io.asl" - /* Flash ROM program enable register */ - OperationRegion(FRE, SystemIO, 0x00000C6F, 0x00000001) - Field(FRE, ByteAcc, NoLock, Preserve) { - , 0x00000006, - FLRE, 0x00000001, - } - - /* PM2 index/data registers */ - OperationRegion(PM2R, SystemIO, 0x00000CD0, 0x00000002) - Field(PM2R, ByteAcc, NoLock, Preserve) { - PM2I, 0x00000008, - PM2D, 0x00000008, - } - - /* Power Management I/O registers, TODO:PMIO is quite different in SB800. */ - OperationRegion(PIOR, SystemIO, 0x00000CD6, 0x00000002) - Field(PIOR, ByteAcc, NoLock, Preserve) { - PIOI, 0x00000008, - PIOD, 0x00000008, - } - IndexField (PIOI, PIOD, ByteAcc, NoLock, Preserve) { - , 1, /* MiscControl */ - T1EE, 1, - T2EE, 1, - Offset(0x01), /* MiscStatus */ - , 1, - T1E, 1, - T2E, 1, - Offset(0x04), /* SmiWakeUpEventEnable3 */ - , 7, - SSEN, 1, - Offset(0x07), /* SmiWakeUpEventStatus3 */ - , 7, - CSSM, 1, - Offset(0x10), /* AcpiEnable */ - , 6, - PWDE, 1, - Offset(0x1C), /* ProgramIoEnable */ - , 3, - MKME, 1, - IO3E, 1, - IO2E, 1, - IO1E, 1, - IO0E, 1, - Offset(0x1D), /* IOMonitorStatus */ - , 3, - MKMS, 1, - IO3S, 1, - IO2S, 1, - IO1S, 1, - IO0S,1, - Offset(0x20), /* AcpiPmEvtBlk. TODO: should be 0x60 */ - APEB, 16, - Offset(0x36), /* GEvtLevelConfig */ - , 6, - ELC6, 1, - ELC7, 1, - Offset(0x37), /* GPMLevelConfig0 */ - , 3, - PLC0, 1, - PLC1, 1, - PLC2, 1, - PLC3, 1, - PLC8, 1, - Offset(0x38), /* GPMLevelConfig1 */ - , 1, - PLC4, 1, - PLC5, 1, - , 1, - PLC6, 1, - PLC7, 1, - Offset(0x3B), /* PMEStatus1 */ - GP0S, 1, - GM4S, 1, - GM5S, 1, - APS, 1, - GM6S, 1, - GM7S, 1, - GP2S, 1, - STSS, 1, - Offset(0x55), /* SoftPciRst */ - SPRE, 1, - , 1, - , 1, - PNAT, 1, - PWMK, 1, - PWNS, 1, - - /* Offset(0x61), */ /* Options_1 */ - /* ,7, */ - /* R617,1, */ - - Offset(0x65), /* UsbPMControl */ - , 4, - URRE, 1, - Offset(0x68), /* MiscEnable68 */ - , 3, - TMTE, 1, - , 1, - Offset(0x92), /* GEVENTIN */ - , 7, - E7IS, 1, - Offset(0x96), /* GPM98IN */ - G8IS, 1, - G9IS, 1, - Offset(0x9A), /* EnhanceControl */ - ,7, - HPDE, 1, - Offset(0xA8), /* PIO7654Enable */ - IO4E, 1, - IO5E, 1, - IO6E, 1, - IO7E, 1, - Offset(0xA9), /* PIO7654Status */ - IO4S, 1, - IO5S, 1, - IO6S, 1, - IO7S, 1, - } - - /* PM1 Event Block - * First word is PM1_Status, Second word is PM1_Enable - */ - OperationRegion(P1EB, SystemIO, APEB, 0x04) - Field(P1EB, ByteAcc, NoLock, Preserve) { - TMST, 1, - , 3, - BMST, 1, - GBST, 1, - Offset(0x01), - PBST, 1, - , 1, - RTST, 1, - , 3, - PWST, 1, - SPWS, 1, - Offset(0x02), - TMEN, 1, - , 4, - GBEN, 1, - Offset(0x03), - PBEN, 1, - , 1, - RTEN, 1, - , 3, - PWDA, 1, - } } diff --git a/src/southbridge/amd/cimx/sb800/acpi/misc_io.asl b/src/southbridge/amd/cimx/sb800/acpi/misc_io.asl new file mode 100644 index 0000000..b8296fb --- /dev/null +++ b/src/southbridge/amd/cimx/sb800/acpi/misc_io.asl @@ -0,0 +1,171 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + + +/* Client Management index/data registers */ +OperationRegion(CMT, SystemIO, 0x00000C50, 0x00000002) + Field(CMT, ByteAcc, NoLock, Preserve) { + CMTI, 8, + /* Client Management Data register */ + G64E, 1, + G64O, 1, + G32O, 2, + , 2, + GPSL, 2, +} + +/* GPM Port register */ +OperationRegion(GPT, SystemIO, 0x00000C52, 0x00000001) + Field(GPT, ByteAcc, NoLock, Preserve) { + GPB0,1, + GPB1,1, + GPB2,1, + GPB3,1, + GPB4,1, + GPB5,1, + GPB6,1, + GPB7,1, +} + +/* Flash ROM program enable register */ +OperationRegion(FRE, SystemIO, 0x00000C6F, 0x00000001) + Field(FRE, ByteAcc, NoLock, Preserve) { + , 0x00000006, + FLRE, 0x00000001, +} + +/* PM2 index/data registers */ +OperationRegion(PM2R, SystemIO, 0x00000CD0, 0x00000002) + Field(PM2R, ByteAcc, NoLock, Preserve) { + PM2I, 0x00000008, + PM2D, 0x00000008, +} + +/* Power Management I/O registers, TODO:PMIO is quite different in SB800. */ +OperationRegion(PIOR, SystemIO, 0x00000CD6, 0x00000002) + Field(PIOR, ByteAcc, NoLock, Preserve) { + PIOI, 0x00000008, + PIOD, 0x00000008, +} +IndexField (PIOI, PIOD, ByteAcc, NoLock, Preserve) { + , 1, /* MiscControl */ + T1EE, 1, + T2EE, 1, + Offset(0x01), /* MiscStatus */ + , 1, + T1E, 1, + T2E, 1, + Offset(0x04), /* SmiWakeUpEventEnable3 */ + , 7, + SSEN, 1, + Offset(0x07), /* SmiWakeUpEventStatus3 */ + , 7, + CSSM, 1, + Offset(0x10), /* AcpiEnable */ + , 6, + PWDE, 1, + Offset(0x1C), /* ProgramIoEnable */ + , 3, + MKME, 1, + IO3E, 1, + IO2E, 1, + IO1E, 1, + IO0E, 1, + Offset(0x1D), /* IOMonitorStatus */ + , 3, + MKMS, 1, + IO3S, 1, + IO2S, 1, + IO1S, 1, + IO0S,1, + Offset(0x20), /* AcpiPmEvtBlk. TODO: should be 0x60 */ + APEB, 16, + Offset(0x36), /* GEvtLevelConfig */ + , 6, + ELC6, 1, + ELC7, 1, + Offset(0x37), /* GPMLevelConfig0 */ + , 3, + PLC0, 1, + PLC1, 1, + PLC2, 1, + PLC3, 1, + PLC8, 1, + Offset(0x38), /* GPMLevelConfig1 */ + , 1, + PLC4, 1, + PLC5, 1, + , 1, + PLC6, 1, + PLC7, 1, + Offset(0x3B), /* PMEStatus1 */ + GP0S, 1, + GM4S, 1, + GM5S, 1, + APS, 1, + GM6S, 1, + GM7S, 1, + GP2S, 1, + STSS, 1, + Offset(0x55), /* SoftPciRst */ + SPRE, 1, + , 1, + , 1, + PNAT, 1, + PWMK, 1, + PWNS, 1, + + Offset(0x65), /* UsbPMControl */ + , 4, + URRE, 1, + Offset(0x68), /* MiscEnable68 */ + , 3, + TMTE, 1, + , 1, + Offset(0x92), /* GEVENTIN */ + , 7, + E7IS, 1, + Offset(0x96), /* GPM98IN */ + G8IS, 1, + G9IS, 1, + Offset(0x9A), /* EnhanceControl */ + ,7, + HPDE, 1, + Offset(0xA8), /* PIO7654Enable */ + IO4E, 1, + IO5E, 1, + IO6E, 1, + IO7E, 1, + Offset(0xA9), /* PIO7654Status */ + IO4S, 1, + IO5S, 1, + IO6S, 1, + IO7S, 1, +} + +/* PM1 Event Block +* First word is PM1_Status, Second word is PM1_Enable +*/ +OperationRegion(P1EB, SystemIO, APEB, 0x04) + Field(P1EB, ByteAcc, NoLock, Preserve) { + TMST, 1, + , 3, + BMST, 1, + GBST, 1, + Offset(0x01), + PBST, 1, + , 1, + RTST, 1, + , 3, + PWST, 1, + SPWS, 1, + Offset(0x02), + TMEN, 1, + , 4, + GBEN, 1, + Offset(0x03), + PBEN, 1, + , 1, + RTEN, 1, + , 3, + PWDA, 1, +} -- To view, visit https://review.coreboot.org/c/coreboot/+/59178 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b3ff2fddc7d4db09903151bcb92e3e1b5dc7d69 Gerrit-Change-Number: 59178 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [WIP] mb/lippert/frontrunner-af: Use common cpu/ and nb/ ASL files
by Kyösti Mälkki (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Kyösti Mälkki. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50658 to look at the new patch set (#3). Change subject: [WIP] mb/lippert/frontrunner-af: Use common cpu/ and nb/ ASL files ...................................................................... [WIP] mb/lippert/frontrunner-af: Use common cpu/ and nb/ ASL files There no quad-core CPU models with fam4, \_SB.C002 and .C003 get removed from ASL. Change-Id: I96df5b3f93c2dd6a05d5693069b991ca01f71d73 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/mainboard/lippert/frontrunner-af/dsdt.asl 1 file changed, 4 insertions(+), 112 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/50658/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/50658 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I96df5b3f93c2dd6a05d5693069b991ca01f71d73 Gerrit-Change-Number: 50658 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/lippert/frontrunner-af: Use common cimx/sb800 ASL
by Kyösti Mälkki (Code Review) 11 Nov '21

11 Nov '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50657 to look at the new patch set (#3). Change subject: mb/lippert/frontrunner-af: Use common cimx/sb800 ASL ...................................................................... mb/lippert/frontrunner-af: Use common cimx/sb800 ASL Change-Id: Ia65b1873f1d184b8b8c64a61a26820ae0900437d Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/mainboard/lippert/frontrunner-af/dsdt.asl M src/southbridge/amd/cimx/sb800/acpi/lpc.asl 2 files changed, 3 insertions(+), 125 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/50657/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/50657 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia65b1873f1d184b8b8c64a61a26820ae0900437d Gerrit-Change-Number: 50657 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/wifi/generic: fix package_size to align with WLAN driver
by EricR Lai (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Sugnan Prabhu S, Tim Wawrzynczak, Paul Menzel, Matt Chen, Subrata Banik. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58951 ) Change subject: drivers/wifi/generic: fix package_size to align with WLAN driver ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: sar_limits->sar->dsar_set_count = CONFIG_DSAR_SET_NUM; config DSAR_SET_NUM hex "Number of SAR sets when D-SAR is enabled" default 0x3 Anything different here? -- To view, visit https://review.coreboot.org/c/coreboot/+/58951 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I18e7d5f658bbf42b7eeed3da330508f14b86c0f8 Gerrit-Change-Number: 58951 Gerrit-PatchSet: 6 Gerrit-Owner: Matt Chen <matt.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Matt Chen <matt.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.corp-partner.google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Matt Chen <matt.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Thu, 11 Nov 2021 09:18:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/agesa/romstage.c: Move timestamp and console init up
by Kyösti Mälkki (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Arthur Heymans, Paul Menzel, Angel Pons, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55066 ) Change subject: drivers/intel/agesa/romstage.c: Move timestamp and console init up ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55066/comment/183610cc_23bf58bf PS1, Line 8: > Ping Immediate follow-up removes them from this file to common romstage. So it's nice those functions are made to appear first here in separate commit. -- To view, visit https://review.coreboot.org/c/coreboot/+/55066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I26a37f9384a581a8a750efccc2100a5c6a6f0f85 Gerrit-Change-Number: 55066 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 11 Nov 2021 09:17:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86: Refactor the SMBIOS type 17 write function
by Subrata Banik (Code Review) 11 Nov '21

11 Nov '21

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56628 ) Change subject: arch/x86: Refactor the SMBIOS type 17 write function ...................................................................... arch/x86: Refactor the SMBIOS type 17 write function List of changes: 1. Create Module Type macros as per Memory Type (i.e. DDR2/DDR3/DDR4/DDR5/LPDDR4/LPDDR5) and fix compilation issue due to renaming of existing macros due to scoping the Memory Type. 2. Use dedicated Memory Type and Module type for `Form Factor` and `TypeDetail` conversion using `get_spd_info()` function. 3. Create a new API (convert_form_factor_to_module_type()) for `Form Factor` to 'Module type' conversion as per `Memory Type`. 4. Add new argument as `Memory Type` to smbios_form_factor_to_spd_mod_type() so that it can internally call convert_form_factor_to_module_type() for `Module Type` conversion. 5. Update `test_smbios_form_factor_to_spd_mod_type()` to accommodate different memory types. 6. Skip fixed module type to form factor conversion using DDR2 SPD4 specification (inside dimm_info_fill()). Refer to datasheet SPD4.1.2.M-1 for LPDDRx and SPD4.1.2.L-3 for DDRx. BUG=b:194659789 TEST=Refer to dmidecode -t 17 output as below: Without this code change: Handle 0x0012, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 16 bits Data Width: 16 bits Size: 2048 MB Form Factor: Unknown .... With this code change: Handle 0x0012, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 16 bits Data Width: 16 bits Size: 2048 MB Form Factor: Row Of Chips .... Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56628 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jakub Czapiga <jacz(a)semihalf.com> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/arch/x86/smbios.c M src/device/dram/ddr3.c M src/device/dram/ddr4.c M src/device/dram/spd.c M src/include/device/dram/spd.h M src/include/dimm_info_util.h M src/include/spd.h M src/lib/dimm_info_util.c M src/mainboard/scaleway/tagada/ramstage.c M src/northbridge/intel/haswell/haswell_mrc/raminit.c M src/soc/amd/common/fsp/dmi.c M src/soc/amd/common/pi/amd_late_init.c M src/soc/intel/common/smbios.c M tests/lib/Makefile.inc M tests/lib/dimm_info_util-test.c 15 files changed, 404 insertions(+), 107 deletions(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved Jakub Czapiga: Looks good to me, but someone else must approve diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index c05f905..c48ce86 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -224,6 +224,9 @@ unsigned long *current, int *handle, int type16_handle) { + struct spd_info info; + get_spd_info(dimm->ddr_type, dimm->mod_type, &info); + struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, sizeof(*t), *handle); @@ -244,24 +247,7 @@ } t->data_width = 8 * (1 << (dimm->bus_width & 0x7)); t->total_width = t->data_width + 8 * ((dimm->bus_width & 0x18) >> 3); - - switch (dimm->mod_type) { - case SPD_RDIMM: - case SPD_MINI_RDIMM: - t->form_factor = MEMORY_FORMFACTOR_RIMM; - break; - case SPD_UDIMM: - case SPD_MICRO_DIMM: - case SPD_MINI_UDIMM: - t->form_factor = MEMORY_FORMFACTOR_DIMM; - break; - case SPD_SODIMM: - t->form_factor = MEMORY_FORMFACTOR_SODIMM; - break; - default: - t->form_factor = MEMORY_FORMFACTOR_UNKNOWN; - break; - } + t->form_factor = info.form_factor; smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t); smbios_fill_dimm_serial_number(dimm, t); @@ -278,19 +264,8 @@ t->maximum_voltage = dimm->vdd_voltage; /* Fill in type detail */ - switch (dimm->mod_type) { - case SPD_RDIMM: - case SPD_MINI_RDIMM: - t->type_detail = MEMORY_TYPE_DETAIL_REGISTERED; - break; - case SPD_UDIMM: - case SPD_MINI_UDIMM: - t->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED; - break; - default: - t->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; - break; - } + t->type_detail = info.type_detail; + /* Synchronous = 1 */ t->type_detail |= MEMORY_TYPE_DETAIL_SYNCHRONOUS; /* no handle for error information */ diff --git a/src/device/dram/ddr3.c b/src/device/dram/ddr3.c index 0a32d02..b99730d 100644 --- a/src/device/dram/ddr3.c +++ b/src/device/dram/ddr3.c @@ -545,19 +545,19 @@ switch (info->dimm_type) { case SPD_DDR3_DIMM_TYPE_SO_DIMM: - dimm->mod_type = SPD_SODIMM; + dimm->mod_type = DDR3_SPD_SODIMM; break; case SPD_DDR3_DIMM_TYPE_72B_SO_CDIMM: - dimm->mod_type = SPD_72B_SO_CDIMM; + dimm->mod_type = DDR3_SPD_72B_SO_CDIMM; break; case SPD_DDR3_DIMM_TYPE_72B_SO_RDIMM: - dimm->mod_type = SPD_72B_SO_RDIMM; + dimm->mod_type = DDR3_SPD_72B_SO_RDIMM; break; case SPD_DDR3_DIMM_TYPE_UDIMM: - dimm->mod_type = SPD_UDIMM; + dimm->mod_type = DDR3_SPD_UDIMM; break; case SPD_DDR3_DIMM_TYPE_RDIMM: - dimm->mod_type = SPD_RDIMM; + dimm->mod_type = DDR3_SPD_RDIMM; break; case SPD_DDR3_DIMM_TYPE_UNDEFINED: default: diff --git a/src/device/dram/ddr4.c b/src/device/dram/ddr4.c index c5a8d13..eea5f0a 100644 --- a/src/device/dram/ddr4.c +++ b/src/device/dram/ddr4.c @@ -299,16 +299,16 @@ switch (info->dimm_type) { case SPD_DDR4_DIMM_TYPE_SO_DIMM: - dimm->mod_type = SPD_SODIMM; + dimm->mod_type = DDR4_SPD_SODIMM; break; case SPD_DDR4_DIMM_TYPE_72B_SO_RDIMM: - dimm->mod_type = SPD_72B_SO_RDIMM; + dimm->mod_type = DDR4_SPD_72B_SO_RDIMM; break; case SPD_DDR4_DIMM_TYPE_UDIMM: - dimm->mod_type = SPD_UDIMM; + dimm->mod_type = DDR4_SPD_UDIMM; break; case SPD_DDR4_DIMM_TYPE_RDIMM: - dimm->mod_type = SPD_RDIMM; + dimm->mod_type = DDR4_SPD_RDIMM; break; default: dimm->mod_type = SPD_UNDEFINED; diff --git a/src/device/dram/spd.c b/src/device/dram/spd.c index 0b2dd49..11808e2 100644 --- a/src/device/dram/spd.c +++ b/src/device/dram/spd.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #include <device/dram/spd.h> +#include <spd.h> const char *spd_manufacturer_name(const uint16_t mod_id) { @@ -38,3 +39,219 @@ return NULL; } } + +static void convert_default_module_type_to_spd_info(struct spd_info *info) +{ + info->form_factor = MEMORY_FORMFACTOR_UNKNOWN; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; +} + +static void convert_ddr2_module_type_to_spd_info(enum ddr2_module_type module_type, + struct spd_info *info) +{ + switch (module_type) { + case DDR2_SPD_RDIMM: + case DDR2_SPD_MINI_RDIMM: + info->form_factor = MEMORY_FORMFACTOR_RIMM; + info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED; + break; + case DDR2_SPD_UDIMM: + case DDR2_SPD_MINI_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_DIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED; + break; + case DDR2_SPD_MICRO_DIMM: + info->form_factor = MEMORY_FORMFACTOR_DIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + case DDR2_SPD_SODIMM: + info->form_factor = MEMORY_FORMFACTOR_SODIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + default: + convert_default_module_type_to_spd_info(info); + break; + } +} + +static void convert_ddr3_module_type_to_spd_info(enum ddr3_module_type module_type, + struct spd_info *info) +{ + switch (module_type) { + case DDR3_SPD_RDIMM: + case DDR3_SPD_MINI_RDIMM: + info->form_factor = MEMORY_FORMFACTOR_RIMM; + info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED; + break; + case DDR3_SPD_UDIMM: + case DDR3_SPD_MINI_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_DIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED; + break; + case DDR3_SPD_MICRO_DIMM: + info->form_factor = MEMORY_FORMFACTOR_DIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + case DDR3_SPD_SODIMM: + case DDR3_SPD_72B_SO_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_SODIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + default: + convert_default_module_type_to_spd_info(info); + break; + } +} + +static void convert_ddr4_module_type_to_spd_info(enum ddr4_module_type module_type, + struct spd_info *info) +{ + switch (module_type) { + case DDR4_SPD_RDIMM: + case DDR4_SPD_MINI_RDIMM: + info->form_factor = MEMORY_FORMFACTOR_RIMM; + info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED; + break; + case DDR4_SPD_UDIMM: + case DDR4_SPD_MINI_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_DIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED; + break; + case DDR4_SPD_SODIMM: + case DDR4_SPD_72B_SO_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_SODIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + default: + convert_default_module_type_to_spd_info(info); + break; + } +} + +static void convert_ddr5_module_type_to_spd_info(enum ddr5_module_type module_type, + struct spd_info *info) +{ + switch (module_type) { + case DDR5_SPD_RDIMM: + case DDR5_SPD_MINI_RDIMM: + info->form_factor = MEMORY_FORMFACTOR_RIMM; + info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED; + break; + case DDR5_SPD_UDIMM: + case DDR5_SPD_MINI_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_DIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED; + break; + case DDR5_SPD_SODIMM: + case DDR5_SPD_72B_SO_UDIMM: + info->form_factor = MEMORY_FORMFACTOR_SODIMM; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + case DDR5_SPD_2DPC: + info->form_factor = MEMORY_FORMFACTOR_PROPRIETARY_CARD; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + default: + convert_default_module_type_to_spd_info(info); + break; + } +} + +static void convert_lpx_module_type_to_spd_info(enum lpx_module_type module_type, + struct spd_info *info) +{ + switch (module_type) { + case LPX_SPD_NONDIMM: + info->form_factor = MEMORY_FORMFACTOR_ROC; + info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN; + break; + default: + convert_default_module_type_to_spd_info(info); + break; + } +} + +void get_spd_info(smbios_memory_type memory_type, uint8_t module_type, struct spd_info *info) +{ + switch (memory_type) { + case MEMORY_TYPE_DDR2: + convert_ddr2_module_type_to_spd_info(module_type, info); + break; + case MEMORY_TYPE_DDR3: + convert_ddr3_module_type_to_spd_info(module_type, info); + break; + case MEMORY_TYPE_DDR4: + convert_ddr4_module_type_to_spd_info(module_type, info); + break; + case MEMORY_TYPE_DDR5: + convert_ddr5_module_type_to_spd_info(module_type, info); + break; + case MEMORY_TYPE_LPDDR3: + case MEMORY_TYPE_LPDDR4: + case MEMORY_TYPE_LPDDR5: + convert_lpx_module_type_to_spd_info(module_type, info); + break; + default: + convert_default_module_type_to_spd_info(info); + break; + } +} + +static uint8_t convert_default_form_factor_to_module_type(void) +{ + return SPD_UNDEFINED; +} + +static uint8_t convert_ddrx_form_factor_to_module_type(smbios_memory_type memory_type, + smbios_memory_form_factor form_factor) +{ + uint8_t module_type; + + switch (form_factor) { + case MEMORY_FORMFACTOR_DIMM: + return DDR2_SPD_UDIMM; + case MEMORY_FORMFACTOR_RIMM: + return DDR2_SPD_RDIMM; + case MEMORY_FORMFACTOR_SODIMM: + module_type = (memory_type == MEMORY_TYPE_DDR2) ? DDR2_SPD_SODIMM + : DDR3_SPD_SODIMM; + return module_type; + default: + return convert_default_form_factor_to_module_type(); + } +} + +static uint8_t convert_lpx_form_factor_to_module_type(smbios_memory_form_factor form_factor) +{ + switch (form_factor) { + case MEMORY_FORMFACTOR_ROC: + return LPX_SPD_NONDIMM; + default: + return convert_default_form_factor_to_module_type(); + } +} + +uint8_t convert_form_factor_to_module_type(smbios_memory_type memory_type, + smbios_memory_form_factor form_factor) +{ + uint8_t module_type; + + switch (memory_type) { + case MEMORY_TYPE_DDR2: + case MEMORY_TYPE_DDR3: + case MEMORY_TYPE_DDR4: + case MEMORY_TYPE_DDR5: + module_type = convert_ddrx_form_factor_to_module_type(memory_type, form_factor); + break; + case MEMORY_TYPE_LPDDR3: + case MEMORY_TYPE_LPDDR4: + case MEMORY_TYPE_LPDDR5: + module_type = convert_lpx_form_factor_to_module_type(form_factor); + break; + default: + module_type = convert_default_form_factor_to_module_type(); + break; + } + + return module_type; +} diff --git a/src/include/device/dram/spd.h b/src/include/device/dram/spd.h index c677f4c..1a86ea3 100644 --- a/src/include/device/dram/spd.h +++ b/src/include/device/dram/spd.h @@ -3,8 +3,18 @@ #ifndef DEVICE_DRAM_SPD_H #define DEVICE_DRAM_SPD_H +#include <smbios.h> #include <types.h> const char *spd_manufacturer_name(const uint16_t mod_id); +struct spd_info { + uint16_t type_detail; + uint8_t form_factor; +}; + +void get_spd_info(smbios_memory_type memory_type, uint8_t module_type, struct spd_info *info); +uint8_t convert_form_factor_to_module_type(smbios_memory_type memory_type, + smbios_memory_form_factor form_factor); + #endif /* DEVICE_DRAM_SPD_H */ diff --git a/src/include/dimm_info_util.h b/src/include/dimm_info_util.h index 04e9336..e7285d5 100644 --- a/src/include/dimm_info_util.h +++ b/src/include/dimm_info_util.h @@ -28,7 +28,7 @@ * * Use this when setting dimm_info.mod_type. */ -uint8_t -smbios_form_factor_to_spd_mod_type(smbios_memory_form_factor form_factor); +uint8_t smbios_form_factor_to_spd_mod_type(smbios_memory_type memory_type, + smbios_memory_form_factor form_factor); #endif diff --git a/src/include/spd.h b/src/include/spd.h index af3072e..8493d40 100644 --- a/src/include/spd.h +++ b/src/include/spd.h @@ -197,18 +197,70 @@ #define MODULE_BUFFERED 1 #define MODULE_REGISTERED 2 -/* Byte 3: Module type information */ #define SPD_UNDEFINED 0x00 -#define SPD_RDIMM 0x01 -#define SPD_UDIMM 0x02 -#define SPD_SODIMM 0x04 -#define SPD_72B_SO_CDIMM 0x06 -#define SPD_72B_SO_RDIMM 0x07 -#define SPD_MICRO_DIMM 0x08 -#define SPD_MINI_RDIMM 0x10 -#define SPD_MINI_UDIMM 0x20 - #define SPD_ECC_8BIT (1<<3) #define SPD_ECC_8BIT_LP5_DDR5 (1<<4) +/* Byte 3: Module type information */ +enum ddr2_module_type { + DDR2_SPD_RDIMM = 0x01, + DDR2_SPD_UDIMM = 0x02, + DDR2_SPD_SODIMM = 0x04, + DDR2_SPD_72B_SO_CDIMM = 0x06, + DDR2_SPD_72B_SO_RDIMM = 0x07, + DDR2_SPD_MICRO_DIMM = 0x08, + DDR2_SPD_MINI_RDIMM = 0x10, + DDR2_SPD_MINI_UDIMM = 0x20, +}; + +enum ddr3_module_type { + DDR3_SPD_RDIMM = 0x01, + DDR3_SPD_UDIMM = 0x02, + DDR3_SPD_SODIMM = 0x03, + DDR3_SPD_MICRO_DIMM = 0x04, + DDR3_SPD_MINI_RDIMM = 0x05, + DDR3_SPD_MINI_UDIMM = 0x06, + DDR3_SPD_MINI_CDIMM = 0x07, + DDR3_SPD_72B_SO_UDIMM = 0x08, + DDR3_SPD_72B_SO_RDIMM = 0x09, + DDR3_SPD_72B_SO_CDIMM = 0x0a, + DDR3_SPD_LRDIMM = 0x0b, + DDR3_SPD_16B_SO_DIMM = 0x0c, + DDR3_SPD_32B_SO_RDIMM = 0x0d, +}; + +enum ddr4_module_type { + DDR4_SPD_RDIMM = 0x01, + DDR4_SPD_UDIMM = 0x02, + DDR4_SPD_SODIMM = 0x03, + DDR4_SPD_LRDIMM = 0x04, + DDR4_SPD_MINI_RDIMM = 0x05, + DDR4_SPD_MINI_UDIMM = 0x06, + DDR4_SPD_72B_SO_UDIMM = 0x08, + DDR4_SPD_72B_SO_RDIMM = 0x09, + DDR4_SPD_16B_SO_DIMM = 0x0c, + DDR4_SPD_32B_SO_RDIMM = 0x0d, +}; + +enum ddr5_module_type { + DDR5_SPD_RDIMM = 0x01, + DDR5_SPD_UDIMM = 0x02, + DDR5_SPD_SODIMM = 0x03, + DDR5_SPD_LRDIMM = 0x04, + DDR5_SPD_MINI_RDIMM = 0x05, + DDR5_SPD_MINI_UDIMM = 0x06, + DDR5_SPD_72B_SO_UDIMM = 0x08, + DDR5_SPD_72B_SO_RDIMM = 0x09, + DDR5_SPD_SOLDERED_DOWN = 0x0b, + DDR5_SPD_16B_SO_DIMM = 0x0c, + DDR5_SPD_32B_SO_RDIMM = 0x0d, + DDR5_SPD_1DPC = 0x0e, + DDR5_SPD_2DPC = 0x0f, +}; + +enum lpx_module_type { + LPX_SPD_LPDIMM = 0x07, + LPX_SPD_NONDIMM = 0x0e, +}; + #endif diff --git a/src/lib/dimm_info_util.c b/src/lib/dimm_info_util.c index 0e7cc12..3507366 100644 --- a/src/lib/dimm_info_util.c +++ b/src/lib/dimm_info_util.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <device/dram/spd.h> #include <dimm_info_util.h> #include <smbios.h> #include <spd.h> @@ -72,18 +73,8 @@ return memory_size; } -uint8_t -smbios_form_factor_to_spd_mod_type(smbios_memory_form_factor form_factor) +uint8_t smbios_form_factor_to_spd_mod_type(smbios_memory_type memory_type, + smbios_memory_form_factor form_factor) { - /* This switch reverses the switch in smbios.c */ - switch (form_factor) { - case MEMORY_FORMFACTOR_DIMM: - return SPD_UDIMM; - case MEMORY_FORMFACTOR_RIMM: - return SPD_RDIMM; - case MEMORY_FORMFACTOR_SODIMM: - return SPD_SODIMM; - default: - return SPD_UNDEFINED; - } + return convert_form_factor_to_module_type(memory_type, form_factor); } diff --git a/src/mainboard/scaleway/tagada/ramstage.c b/src/mainboard/scaleway/tagada/ramstage.c index 6d5d3ff..bb0a385 100644 --- a/src/mainboard/scaleway/tagada/ramstage.c +++ b/src/mainboard/scaleway/tagada/ramstage.c @@ -83,5 +83,5 @@ int channel, int dimm, int index) { /* Mainboard only has DDR4 DIMM slots */ - mem_info->dimm[index].mod_type = SPD_UDIMM; + mem_info->dimm[index].mod_type = DDR4_SPD_UDIMM; } diff --git a/src/northbridge/intel/haswell/haswell_mrc/raminit.c b/src/northbridge/intel/haswell/haswell_mrc/raminit.c index f71d117..e46eb57 100644 --- a/src/northbridge/intel/haswell/haswell_mrc/raminit.c +++ b/src/northbridge/intel/haswell/haswell_mrc/raminit.c @@ -260,7 +260,7 @@ dimm->mod_id = (pei_data->spd_data[index][SPD_DIMM_MOD_ID2] << 8) | (pei_data->spd_data[index][SPD_DIMM_MOD_ID1] & 0xff); - dimm->mod_type = SPD_SODIMM; + dimm->mod_type = DDR3_SPD_SODIMM; dimm->bus_width = MEMORY_BUS_WIDTH_64; dimm_cnt++; } diff --git a/src/soc/amd/common/fsp/dmi.c b/src/soc/amd/common/fsp/dmi.c index 223cce1..b96b047 100644 --- a/src/soc/amd/common/fsp/dmi.c +++ b/src/soc/amd/common/fsp/dmi.c @@ -52,7 +52,8 @@ dimm->rank_per_dimm = dmi17->Attributes; - dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor); + dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->MemoryType, + dmi17->FormFactor); dimm->bus_width = smbios_bus_width_to_spd_width(dmi17->MemoryType, dmi17->TotalWidth, dmi17->DataWidth); diff --git a/src/soc/amd/common/pi/amd_late_init.c b/src/soc/amd/common/pi/amd_late_init.c index 3714404..4d2250b 100644 --- a/src/soc/amd/common/pi/amd_late_init.c +++ b/src/soc/amd/common/pi/amd_late_init.c @@ -36,7 +36,8 @@ dimm->rank_per_dimm = dmi17->Attributes; - dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor); + dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->MemoryType, + dmi17->FormFactor); dimm->bus_width = smbios_bus_width_to_spd_width(dmi17->MemoryType, dmi17->TotalWidth, dmi17->DataWidth); diff --git a/src/soc/intel/common/smbios.c b/src/soc/intel/common/smbios.c index 213be65..4aca022 100644 --- a/src/soc/intel/common/smbios.c +++ b/src/soc/intel/common/smbios.c @@ -18,28 +18,7 @@ bool ecc_support, u16 mod_id, u8 mod_type) { dimm->mod_id = mod_id; - /* Translate to DDR2 module type field that SMBIOS code expects. */ - switch (mod_type) { - case SPD_DDR3_DIMM_TYPE_SO_DIMM: - dimm->mod_type = SPD_SODIMM; - break; - case SPD_DDR3_DIMM_TYPE_72B_SO_CDIMM: - dimm->mod_type = SPD_72B_SO_CDIMM; - break; - case SPD_DDR3_DIMM_TYPE_72B_SO_RDIMM: - dimm->mod_type = SPD_72B_SO_RDIMM; - break; - case SPD_DDR3_DIMM_TYPE_UDIMM: - dimm->mod_type = SPD_UDIMM; - break; - case SPD_DDR3_DIMM_TYPE_RDIMM: - dimm->mod_type = SPD_RDIMM; - break; - case SPD_DDR3_DIMM_TYPE_UNDEFINED: - default: - dimm->mod_type = SPD_UNDEFINED; - break; - } + dimm->mod_type = mod_type; dimm->dimm_size = dimm_capacity; dimm->ddr_type = ddr_type; dimm->ddr_frequency = frequency; diff --git a/tests/lib/Makefile.inc b/tests/lib/Makefile.inc index 0aba0f5..0aea0ce 100644 --- a/tests/lib/Makefile.inc +++ b/tests/lib/Makefile.inc @@ -138,6 +138,7 @@ bootmem-test-srcs += src/lib/memrange.c dimm_info_util-test-srcs += tests/lib/dimm_info_util-test.c +dimm_info_util-test-srcs += src/device/dram/spd.c dimm_info_util-test-srcs += src/lib/dimm_info_util.c dimm_info_util-test-srcs += tests/stubs/console.c diff --git a/tests/lib/dimm_info_util-test.c b/tests/lib/dimm_info_util-test.c index 25800d9..cfe7176 100644 --- a/tests/lib/dimm_info_util-test.c +++ b/tests/lib/dimm_info_util-test.c @@ -4,6 +4,8 @@ #include <spd.h> #include <tests/test.h> +#define MAX_ALLOWED_MODULE_TYPE 3 + static void test_smbios_bus_width_to_spd_width_parametrized(smbios_memory_type ddr_type) { /* Non-ECC variants */ @@ -86,22 +88,8 @@ assert_int_equal(memory_size, smbios_memory_size_to_mib(memory_size, 694735)); } -static void test_smbios_form_factor_to_spd_mod_type(void **state) +static void test_smbios_form_factor_to_spd_mod_type_ddr(smbios_memory_type memory_type) { - /* Form factors defined in coreboot */ - const LargestIntegralType udimm_allowed[] = { - SPD_UDIMM, SPD_MICRO_DIMM, SPD_MINI_UDIMM, - }; - assert_in_set(smbios_form_factor_to_spd_mod_type(MEMORY_FORMFACTOR_DIMM), - udimm_allowed, ARRAY_SIZE(udimm_allowed)); - - const LargestIntegralType rdimm_allowed[] = { SPD_RDIMM, SPD_MINI_RDIMM }; - assert_in_set(smbios_form_factor_to_spd_mod_type(MEMORY_FORMFACTOR_RIMM), - rdimm_allowed, ARRAY_SIZE(rdimm_allowed)); - - assert_int_equal(SPD_SODIMM, - smbios_form_factor_to_spd_mod_type(MEMORY_FORMFACTOR_SODIMM)); - const smbios_memory_form_factor undefined_factors[] = { MEMORY_FORMFACTOR_OTHER, MEMORY_FORMFACTOR_UNKNOWN, @@ -119,10 +107,92 @@ }; for (int i = 0; i < ARRAY_SIZE(undefined_factors); ++i) { assert_int_equal(SPD_UNDEFINED, - smbios_form_factor_to_spd_mod_type(undefined_factors[i])); + smbios_form_factor_to_spd_mod_type(memory_type, + undefined_factors[i])); } } +static void test_smbios_form_factor_to_spd_mod_type_ddrx_parametrized( + smbios_memory_type memory_type, + const LargestIntegralType udimm_allowed[], + const LargestIntegralType rdimm_allowed[], + LargestIntegralType expected_module_type) +{ + print_message("%s(%d)\n", __func__, memory_type); + + assert_in_set(smbios_form_factor_to_spd_mod_type(memory_type, MEMORY_FORMFACTOR_DIMM), + udimm_allowed, MAX_ALLOWED_MODULE_TYPE); + + assert_in_set(smbios_form_factor_to_spd_mod_type(memory_type, MEMORY_FORMFACTOR_RIMM), + rdimm_allowed, MAX_ALLOWED_MODULE_TYPE); + + assert_int_equal(expected_module_type, smbios_form_factor_to_spd_mod_type(memory_type, + MEMORY_FORMFACTOR_SODIMM)); + + test_smbios_form_factor_to_spd_mod_type_ddr(memory_type); +} + +static void test_smbios_form_factor_to_spd_mod_type_lpddrx(smbios_memory_type memory_type) +{ + print_message("%s(%d)\n", __func__, memory_type); + /* Form factors defined in coreboot */ + assert_int_equal(LPX_SPD_NONDIMM, smbios_form_factor_to_spd_mod_type(memory_type, + MEMORY_FORMFACTOR_ROC)); +} + +static void test_smbios_form_factor_to_spd_mod_type(void **state) +{ + const struct smbios_form_factor_test_info_ddrx { + smbios_memory_type memory_type; + const LargestIntegralType udimm_allowed[MAX_ALLOWED_MODULE_TYPE]; + const LargestIntegralType rdimm_allowed[MAX_ALLOWED_MODULE_TYPE]; + LargestIntegralType expected_module_type; + } ddrx_info[] = { + { + .memory_type = MEMORY_TYPE_DDR2, + .udimm_allowed = { DDR2_SPD_UDIMM, DDR2_SPD_MICRO_DIMM, + DDR2_SPD_MINI_UDIMM }, + .rdimm_allowed = { DDR2_SPD_RDIMM, DDR2_SPD_MINI_RDIMM }, + .expected_module_type = DDR2_SPD_SODIMM, + }, + { + .memory_type = MEMORY_TYPE_DDR3, + .udimm_allowed = { DDR3_SPD_UDIMM, DDR3_SPD_MICRO_DIMM, + DDR3_SPD_MINI_UDIMM }, + .rdimm_allowed = { DDR3_SPD_RDIMM, DDR3_SPD_MINI_RDIMM }, + .expected_module_type = DDR3_SPD_SODIMM, + }, + { + .memory_type = MEMORY_TYPE_DDR4, + .udimm_allowed = { DDR4_SPD_UDIMM, DDR4_SPD_MINI_UDIMM }, + .rdimm_allowed = { DDR4_SPD_RDIMM, DDR4_SPD_MINI_RDIMM }, + .expected_module_type = DDR4_SPD_SODIMM, + }, + { + .memory_type = MEMORY_TYPE_DDR5, + .udimm_allowed = { DDR5_SPD_UDIMM, DDR5_SPD_MINI_UDIMM }, + .rdimm_allowed = { DDR5_SPD_RDIMM, DDR5_SPD_MINI_RDIMM }, + .expected_module_type = DDR5_SPD_SODIMM + }, + }; + + /* Test for DDRx DIMM Modules */ + for (int i = 0; i < ARRAY_SIZE(ddrx_info); i++) + test_smbios_form_factor_to_spd_mod_type_ddrx_parametrized( + ddrx_info[i].memory_type, ddrx_info[i].udimm_allowed, + ddrx_info[i].rdimm_allowed, ddrx_info[i].expected_module_type); + + smbios_memory_type lpddrx_memory_type[] = { + MEMORY_TYPE_LPDDR3, + MEMORY_TYPE_LPDDR4, + MEMORY_TYPE_LPDDR5, + }; + + /* Test for Lpddrx DIMM Modules */ + for (int i = 0; i < ARRAY_SIZE(lpddrx_memory_type); i++) + test_smbios_form_factor_to_spd_mod_type_lpddrx(lpddrx_memory_type[i]); +} + int main(void) { const struct CMUnitTest tests[] = { 25 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/56628 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50 Gerrit-Change-Number: 56628 Gerrit-PatchSet: 27 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: drivers/intel/agesa/romstage.c: Move timestamp and console init up
by Angel Pons (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Arthur Heymans, Paul Menzel, Kyösti Mälkki, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55066 ) Change subject: drivers/intel/agesa/romstage.c: Move timestamp and console init up ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/55066/comment/ae5a528f_3715224c PS1, Line 8: > Probably, but I'm not sure even after reading the commit message's body. Maybe if there was one... […] Ping Commit Message: https://review.coreboot.org/c/coreboot/+/55066/comment/538e3509_0024f09a PS2, Line 7: intel > this should be amd Ping -- To view, visit https://review.coreboot.org/c/coreboot/+/55066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I26a37f9384a581a8a750efccc2100a5c6a6f0f85 Gerrit-Change-Number: 55066 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 11 Nov 2021 09:08:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/amd/agesa/romstage.c: Remove lapic_id check
by Angel Pons (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Arthur Heymans, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55065 ) Change subject: drivers/amd/agesa/romstage.c: Remove lapic_id check ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55065/comment/24635c81_46977224 PS3, Line 9: The APs don't execute this codepath but ap_romstage_main(). So, if I understand correctly, `romstage_main()` is only executed by the BSP. Does the BSP always have an initial LAPIC ID of 0? I guess so, otherwise the original code would have problems. -- To view, visit https://review.coreboot.org/c/coreboot/+/55065 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If884001bc8c5363efbbf00422a9a700896318f7b Gerrit-Change-Number: 55065 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 11 Nov 2021 09:07:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/taeko: Fix PCIE gen 4 cannot enter S0ix issue
by Joey Peng (Code Review) 11 Nov '21

11 Nov '21

Joey Peng has removed Tim Wawrzynczak from this change. ( https://review.coreboot.org/c/coreboot/+/59176 ) Change subject: mb/google/brya/var/taeko: Fix PCIE gen 4 cannot enter S0ix issue ...................................................................... Removed reviewer Tim Wawrzynczak. -- To view, visit https://review.coreboot.org/c/coreboot/+/59176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8b6cbdfe4cff8360317089edc2d0f94873355d78 Gerrit-Change-Number: 59176 Gerrit-PatchSet: 1 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: deleteReviewer

1 0

Change in coreboot[master]: mb/google/brya/var/taeko: Fix PCIE gen 4 cannot enter S0ix issue
by Joey Peng (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Tim Wawrzynczak. Joey Peng has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59176 ) Change subject: mb/google/brya/var/taeko: Fix PCIE gen 4 cannot enter S0ix issue ...................................................................... mb/google/brya/var/taeko: Fix PCIE gen 4 cannot enter S0ix issue Fix PCIE gen 4 cannot enter S0ix issue BUG=b:205920263 TEST=emerge-brya coreboot and verify that DUT can enter S0ix 15 times Signed-off-by: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Change-Id: I8b6cbdfe4cff8360317089edc2d0f94873355d78 --- M src/mainboard/google/brya/variants/taeko/overridetree.cb 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/59176/1 diff --git a/src/mainboard/google/brya/variants/taeko/overridetree.cb b/src/mainboard/google/brya/variants/taeko/overridetree.cb index d6d7c9b..7a797dc 100644 --- a/src/mainboard/google/brya/variants/taeko/overridetree.cb +++ b/src/mainboard/google/brya/variants/taeko/overridetree.cb @@ -222,6 +222,12 @@ end end device ref pcie4_0 on + chip soc/intel/common/block/pcie/rtd3 + register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D11)" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_B4)" + register "srcclk_pin" = "0" + device generic 0 on end + end # Enable CPU PCIE RP 1 using CLK 0 register "cpu_pcie_rp[CPU_RP(1)]" = "{ .clk_req = 0, -- To view, visit https://review.coreboot.org/c/coreboot/+/59176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8b6cbdfe4cff8360317089edc2d0f94873355d78 Gerrit-Change-Number: 59176 Gerrit-PatchSet: 1 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021