coreboot-gerrit August 2019

coreboot-gerrit@coreboot.org

1 participants
1942 discussions

Change in coreboot[master]: soc/intel/common/timer: Make TSC frequency calculation dynamically
by Wonkyu Kim (Code Review) 29 Aug '19

29 Aug '19

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35148 ) Change subject: soc/intel/common/timer: Make TSC frequency calculation dynamically ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/35148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If660a4b8d12e54b39252bce62bcc0ffcc967f5da Gerrit-Change-Number: 35148 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 29 Aug 2019 18:49:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Documentation/acpi: Add new document on adding ACPI devices to device...
by Tim Wawrzynczak (Code Review) 29 Aug '19

29 Aug '19

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35080 ) Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ...................................................................... Documentation/acpi: Add new document on adding ACPI devices to devicetree Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- A Documentation/acpi/devicetree.md 1 file changed, 191 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/1 diff --git a/Documentation/acpi/devicetree.md b/Documentation/acpi/devicetree.md new file mode 100644 index 0000000..fdc36f1 --- /dev/null +++ b/Documentation/acpi/devicetree.md @@ -0,0 +1,191 @@ +# Adding new devices to a device tree + +## Introduction + +ACPI exposes a platform-independent interface for operating systems to +perform power management and other platform-level functions. Some +operating systems also use ACPI to enumerate devices that are not immediately +discoverable, such as those behind I2C or SPI busses (in contrast to +PCI). This document discusses the way that coreboot uses the concept +of a "device tree" to expose devices to the OS. + +## Devicetree and overridetree (if applicable) + +For mainboards that are organized around a "reference board" or "baseboard" +model (see _src/mainboard/google/octopus_ or _...hatch_ for examples), there is +typically a devicetree.cb file that all boards share, and any differences +for a specific board ("variant") are captured in the overridetree.cb file. Any settings +changed in the overridetree take precedence over those in the main devicetree. +Note that not all mainboards will have the devicetree/overridetree distinction, +and may only have a devicetree.cb file. Or you can always just write the AML code +yourself. + +## Device drivers + +Let's take a look at an example entry from _src/mainboard/google/hatch/variant/hatch/overridetree.cb_: + +``` +device pci 15.0 on + chip drivers/i2c/generic + register "hid" = ""ELAN0000"" + register "desc" = ""ELAN Touchpad"" + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_A21_IRQ)" + register "wake" = "GPE0_DW0_21" + device i2c 15 on end + end +end # I2C #0 +``` + +When this entry is processed during ramstage, it will create a device in the +ACPI SSDT table (all devices in devicetrees end up in the SSDT table). The +ACPI generation routines in coreboot actually generate the raw bytecode that +represents the device's structure, but looking at AML code is easier to +understand; see below for what the disassembled bytecode looks like: + +``` +Scope (\_SB.PCI0.I2C0) +{ + Device (D015) + { + Name (_HID, "ELAN0000") // _HID: Hardware ID + Name (_UID, Zero) // _UID: Unique ID + Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name + Method (_STA, 0, NotSerialized) // _STA: Status + { + Return (0x0F) + } + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + I2cSerialBusV2 (0x0015, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\\_SB.PCI0.I2C0", + 0x00, ResourceConsumer, , Exclusive, ) + Interrupt (ResourceConsumer, Edge, ActiveLow, ExclusiveAndWake, ,, ) + { + 0x0000002D, + } + }) + Name (_S0W, 0x04) // _S0W: S0 Device Wake State + Name (_PRW, Package (0x02) // _PRW: Power Resources for Wake + { + 0x15, + 0x03 + }) + } +} +``` + +You can see it generates _HID, _UID, _DDN, _STA, _CRS, _S0W, and _PRW +names/methods in the Device's scope. Let's take a look at how the devicetree +language corresponds to the "generated AML". + +First, note this: + +``` + chip drivers/i2c/generic +``` + +This means that the device driver we're using has a corresponding structure, +located at _src/drivers/i2c/generic/chip.h_, named **struct drivers_i2c_generic_config** +and it contains many properties you can specify to be included in the ACPI +table. + +## Diving into the above example: + +### hid + +``` + register "hid" = ""ELAN0000"" +``` + +This corresponds to **const char *hid** in the struct. +In the ACPI AML, it translates to: + +``` + Name (_HID, "ELAN0000") // _HID: Hardware ID +``` + +under the Device. + +### desc + +``` + register "desc" = ""ELAN Touchpad"" +``` + +corresponds to **const char *desc** and in AML: + +``` + Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name +``` + +### irq + +It also adds the interrupt, which comes from: + +``` + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_A21_IRQ)" +``` + +(Note that the ACPI_IRQ_WAKE_EDGE_LOW macro informs the platform that the GPIO +will be routed through SCI (ACPI's System Control Interrupt) for use as a wake +source). + +### wake + +The last register is: + +``` + register "wake" = "GPE0_DW0_21" +``` + +which indicates that the method of waking the system using the touchpad will +be through a GPE, #21 associated with DW0 (which is setup in devicetree.cb +from this example). + +The last bit of the definition of that device includes: + +``` + device i2c 15 on end +``` + +which means it's an I2C device, with 7-bit address 0x15, and the device +is "on", meaning it will be exposed in the ACPI table. + +### Other auto-generated names + +### _S0W + _S0W indicates the deepest S0 sleep state that this device can wake itself +from, which in this case is 4, representing _D3cold_. + +### _PRW + _PRW indicates the power resources and events required for wake. There are no +dependent power resources, but the GPE (GPE0_DW0_21) is mentioned here (0x15), +as well as the deepest sleep state that supports waking the system (3), which +is S3. + +### _STA +The _STA method is generated automatically, and its values, 0xF, indicates the +following: + + Bit [0] – Set if the device is present. + Bit [1] – Set if the device is enabled and decoding its resources. + Bit [2] – Set if the device should be shown in the UI. + Bit [3] – Set if the device is functioning properly (cleared if device failed its diagnostics). + +### _CRS +The _CRS (current resources) method is generated automatically, as the driver +knows it is an I2C controller, and so specifies how to configure the controller +for proper operation with the touchpad. + +``` +Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings +{ + I2cSerialBusV2 (0x0015, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\\_SB.PCI0.I2C0", + 0x00, ResourceConsumer, , Exclusive, ) +``` + +## Notes + + - **All fields that are left unspecified in the devicetree are initialized to zero.** + - **All devices in devicetrees end up in the SSDT table, and are generated in coreboot's ramstage** -- To view, visit https://review.coreboot.org/c/coreboot/+/35080 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Gerrit-Change-Number: 35080 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

7 19

Change in coreboot[master]: src/soc/intel/common/itss: Add support to get IRQ configuration for P...
by Aamir Bohra (Code Review) 29 Aug '19

29 Aug '19

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34089 ) Change subject: src/soc/intel/common/itss: Add support to get IRQ configuration for PCI devices ...................................................................... Patch Set 18: (2 comments) https://review.coreboot.org/c/coreboot/+/34089/18/src/soc/intel/common/bloc… File src/soc/intel/common/block/itss/irq.c: https://review.coreboot.org/c/coreboot/+/34089/18/src/soc/intel/common/bloc… PS18, Line 41: > Is there a concern there could be more than 8 LPSS devices requiring IRQ assignment? Maybe we need […] it can extend beyond the 8 IRQ assingments. But, it will conflict with the GPIO IRQs(23-119) and the conflicting GPIOs should not use interrupt trigger via IOAPIC. I can print warning to notify. https://review.coreboot.org/c/coreboot/+/34089/18/src/soc/intel/common/bloc… PS18, Line 99: index > To me, exiting early if index becomes >= num_entries shows the error a little more clearly. […] ok, I can add a return in else here. -- To view, visit https://review.coreboot.org/c/coreboot/+/34089 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib7066432ff5f0d7017ac5a44922ca69f07da9556 Gerrit-Change-Number: 34089 Gerrit-PatchSet: 18 Gerrit-Owner: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Aug 2019 17:26:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[4.10_branch]: soc/intel/fsp_broadwell_de: Fix use of config_of()
by Martin Roth (Code Review) 29 Aug '19

29 Aug '19

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35045 ) Change subject: soc/intel/fsp_broadwell_de: Fix use of config_of() ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/35045 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: 4.10_branch Gerrit-Change-Id: I96d423720fbe67c067373436ad250edf37939e99 Gerrit-Change-Number: 35045 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 29 Aug 2019 14:49:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/timer: Make TSC frequency calculation dynamically
by Subrata Banik (Code Review) 29 Aug '19

29 Aug '19

Hello Aaron Durbin, Patrick Rudolph, Meera Ravindranath, Lean Sheng Tan, Aamir Bohra, Wonkyu Kim, Maulik V Vaghela, V Sowmya, build bot (Jenkins), Furquan Shaikh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35148 to look at the new patch set (#4). Change subject: soc/intel/common/timer: Make TSC frequency calculation dynamically ...................................................................... soc/intel/common/timer: Make TSC frequency calculation dynamically tsc_freq_mhz() had a static table of Intel CPU families and crystal clock, but it is possible to calculate the crystal clock speed dynamically, and this is preferred over hardcoded table. Recommendation is to make use of CPUID.16h where crystal clock frequency was not reported by CPUID.15h to calculate the crystal clock. On SKL/KBL/CML CPUID.15h.ecx = nominal core crystal clock = 0 Hz hence we had to use static table to calculate crystal clock. BUG=b:139798422, b:129839774 TEST=Able to build and boot KBL/CML/ICL. Change-Id: If660a4b8d12e54b39252bce62bcc0ffcc967f5da Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- D src/arch/x86/include/arch/intel-family.h M src/soc/intel/common/block/timer/timer.c 2 files changed, 40 insertions(+), 112 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35148/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/35148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If660a4b8d12e54b39252bce62bcc0ffcc967f5da Gerrit-Change-Number: 35148 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/timer: Make TSC frequency calculation dynamically
by Subrata Banik (Code Review) 29 Aug '19

29 Aug '19

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35148 ) Change subject: soc/intel/common/timer: Make TSC frequency calculation dynamically ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/c/coreboot/+/35148/3/src/soc/intel/common/block… File src/soc/intel/common/block/timer/timer.c: https://review.coreboot.org/c/coreboot/+/35148/3/src/soc/intel/common/block… PS3, Line 50: * EAX Bit 31-0 : An unsignd integer which has the processor base frequency > unsigned Done https://review.coreboot.org/c/coreboot/+/35148/3/src/soc/intel/common/block… PS3, Line 67: return 0; > Returning 0 from this function should be forbidden. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/35148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If660a4b8d12e54b39252bce62bcc0ffcc967f5da Gerrit-Change-Number: 35148 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 29 Aug 2019 14:29:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86: Fix clearing .bss section
by Aaron Durbin (Code Review) 29 Aug '19

29 Aug '19

Aaron Durbin has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/35139 ) Change subject: arch/x86: Fix clearing .bss section ...................................................................... arch/x86: Fix clearing .bss section Using stosl clears 4 bytes at a time. Change-Id: Ie54fcfcb7e5a2a5a88d988476aa69b2a163e919c Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35139 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> --- M src/arch/x86/assembly_entry.S 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Aaron Durbin: Looks good to me, approved Marshall Dawson: Looks good to me, approved diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S index 0ac59ed..bdecebe 100644 --- a/src/arch/x86/assembly_entry.S +++ b/src/arch/x86/assembly_entry.S @@ -41,6 +41,7 @@ movl $(_car_global_end), %ecx movl $(_car_global_start), %edi sub %edi, %ecx + shrl $2, %ecx rep stosl #if ((ENV_VERSTAGE && CONFIG(VERSTAGE_DEBUG_SPINLOOP)) \ -- To view, visit https://review.coreboot.org/c/coreboot/+/35139 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie54fcfcb7e5a2a5a88d988476aa69b2a163e919c Gerrit-Change-Number: 35139 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: arch/x86: remove weak car_stage_entry() symbol
by Aaron Durbin (Code Review) 29 Aug '19

29 Aug '19

Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35135 ) Change subject: arch/x86: remove weak car_stage_entry() symbol ...................................................................... arch/x86: remove weak car_stage_entry() symbol Many (if not all) platforms have moved to using consistent constructs where a weak car_stage_entry() is no longer necessary to avoid the previous combinations of many different boot flows. Now it's just causing issues so remove it. Change-Id: I7e7897c0609aac8eef96a08bb789374b2403956d Signed-off-by: Aaron Durbin <adurbin(a)chromium.org> --- M src/arch/x86/assembly_entry.S 1 file changed, 1 insertion(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/35135/1 diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S index 4ead9ea..0ac59ed 100644 --- a/src/arch/x86/assembly_entry.S +++ b/src/arch/x86/assembly_entry.S @@ -60,9 +60,7 @@ #endif call car_stage_entry -/* This is here for linking purposes. */ -.weak car_stage_entry -car_stage_entry: + /* Expect to never return. */ 1: jmp 1b -- To view, visit https://review.coreboot.org/c/coreboot/+/35135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e7897c0609aac8eef96a08bb789374b2403956d Gerrit-Change-Number: 35135 Gerrit-PatchSet: 1 Gerrit-Owner: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: newchange

3 7

Change in coreboot[master]: arch/x86: Fix clearing .bss section
by Marshall Dawson (Code Review) 29 Aug '19

29 Aug '19

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35139 ) Change subject: arch/x86: Fix clearing .bss section ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/35139 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie54fcfcb7e5a2a5a88d988476aa69b2a163e919c Gerrit-Change-Number: 35139 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 29 Aug 2019 13:58:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/timer: Make TSC frequency calculation dynamically
by Kyösti Mälkki (Code Review) 29 Aug '19

29 Aug '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35148 ) Change subject: soc/intel/common/timer: Make TSC frequency calculation dynamically ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/c/coreboot/+/35148/3/src/soc/intel/common/block… File src/soc/intel/common/block/timer/timer.c: https://review.coreboot.org/c/coreboot/+/35148/3/src/soc/intel/common/block… PS3, Line 50: * EAX Bit 31-0 : An unsignd integer which has the processor base frequency unsigned https://review.coreboot.org/c/coreboot/+/35148/3/src/soc/intel/common/block… PS3, Line 67: return 0; Returning 0 from this function should be forbidden. IMHO it is better to put an assert() here and return some "reasonable but large" clockrate. Reason for this is the value is/may be used as a divisor (like done in tsc/delay_tsc.c) and will result in divide-by-zero exception at some non-obvious location. Larger frequency here causes udelay() to last longer than it should, but it's better making things slower than faster. See CB:31430 -- To view, visit https://review.coreboot.org/c/coreboot/+/35148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If660a4b8d12e54b39252bce62bcc0ffcc967f5da Gerrit-Change-Number: 35148 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 29 Aug 2019 13:40:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2019