coreboot-gerrit September 2019

coreboot-gerrit@coreboot.org

1 participants
1442 discussions

Change in coreboot[master]: soc/intel/common/timer: Make TSC frequency calculation dynamically
by Patrick Georgi (Code Review) 02 Sep '19

02 Sep '19

1 0

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

02 Sep '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 11: (2 comments) https://review.coreboot.org/c/coreboot/+/35148/9//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35148/9//COMMIT_MSG@16 PS9, Line 16: On SKL/KBL/CML CPUID.15h.ecx = nominal core crystal clock = 0 Hz > Thanks. I didn't get the relation between the two paragraphs and it's much clearer to me now. […] yes Patrick, correct https://review.coreboot.org/c/coreboot/+/35148/10/src/soc/intel/common/bloc… File src/soc/intel/common/block/timer/timer.c: https://review.coreboot.org/c/coreboot/+/35148/10/src/soc/intel/common/bloc… PS10, Line 124: calculate > We don't calculate anything in that function, but (according to comments) only return the bus freque […] 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: 11 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: Duncan Laurie <dlaurie(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-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Comment-Date: Mon, 02 Sep 2019 14:13:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: comment

1 0

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

02 Sep '19

Hello Aaron Durbin, Patrick Rudolph, Meera Ravindranath, Lean Sheng Tan, Aamir Bohra, Wonkyu Kim, Maulik V Vaghela, Duncan Laurie, 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 (#11). 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. On SKL/KBL/CML CPUID.15h.ecx = nominal core crystal clock = 0 Hz hence we had to use static table to calculate crystal clock. Recommendation is to make use of CPUID.16h where crystal clock frequency was not reported by CPUID.15h to calculate the 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, 76 insertions(+), 117 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35148/11 -- 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: 11 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: Duncan Laurie <dlaurie(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-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: soc/intel/quark: Remove variable set but not used
by HAOUAS Elyes (Code Review) 02 Sep '19

02 Sep '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32912 Change subject: soc/intel/quark: Remove variable set but not used ...................................................................... soc/intel/quark: Remove variable set but not used Change-Id: I09292c2776309982cfb4d72012991bf7725b75fb Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/soc/intel/quark/i2c.c 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/32912/1 diff --git a/src/soc/intel/quark/i2c.c b/src/soc/intel/quark/i2c.c index 595c818..49ebaa2 100644 --- a/src/soc/intel/quark/i2c.c +++ b/src/soc/intel/quark/i2c.c @@ -117,7 +117,6 @@ int bytes_transferred; uint32_t cmd; int fifo_bytes; - uint8_t junk; I2C_REGS *regs; uint32_t status; @@ -128,7 +127,7 @@ /* Empty the FIFO */ status = regs->ic_status; while (status & IC_STATUS_RFNE) { - junk = (uint8_t)regs->ic_data_cmd; + (uint8_t)regs->ic_data_cmd; status = regs->ic_status; } -- To view, visit https://review.coreboot.org/c/coreboot/+/32912 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I09292c2776309982cfb4d72012991bf7725b75fb Gerrit-Change-Number: 32912 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

2 8

Change in coreboot[master]: device/i2c_bus: Add i2c_dev_readb_at_word()
by Angel Pons (Code Review) 02 Sep '19

02 Sep '19

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29478 ) Change subject: device/i2c_bus: Add i2c_dev_readb_at_word() ...................................................................... Patch Set 8: (1 comment) https://review.coreboot.org/c/coreboot/+/29478/8/src/device/i2c_bus.c File src/device/i2c_bus.c: https://review.coreboot.org/c/coreboot/+/29478/8/src/device/i2c_bus.c@189 PS8, Line 189: transfer(busdev, msg, ARRAY_SIZE(msg)); > Avoid multiple line dereference - prefer 'busdev->ops->ops_i2c_bus->transfer' O_o -- To view, visit https://review.coreboot.org/c/coreboot/+/29478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7516f3e5d9aca362c2b340aa5627d91510c09412 Gerrit-Change-Number: 29478 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 02 Sep 2019 14:06:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/timer: Make TSC frequency calculation dynamically
by Patrick Georgi (Code Review) 02 Sep '19

02 Sep '19

Patrick Georgi 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 10: (2 comments) https://review.coreboot.org/c/coreboot/+/35148/9//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35148/9//COMMIT_MSG@16 PS9, Line 16: On SKL/KBL/CML CPUID.15h.ecx = nominal core crystal clock = 0 Hz > isn't that i have added in above line […] Thanks. I didn't get the relation between the two paragraphs and it's much clearer to me now. Just to make sure I understand correctly: Normally 15h is used for the crystal clock. SKL/KBL/CML report 0 there, so 16h is used instead (in favor to using a static table like the code did so far). Correct? https://review.coreboot.org/c/coreboot/+/35148/10/src/soc/intel/common/bloc… File src/soc/intel/common/block/timer/timer.c: https://review.coreboot.org/c/coreboot/+/35148/10/src/soc/intel/common/bloc… PS10, Line 124: calculate We don't calculate anything in that function, but (according to comments) only return the bus frequency. That might be enough, but then the comment should say so. -- 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: 10 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: Duncan Laurie <dlaurie(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-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Comment-Date: Mon, 02 Sep 2019 14:05:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: comment

1 0

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

02 Sep '19

Hello Aaron Durbin, Patrick Rudolph, Meera Ravindranath, Lean Sheng Tan, Aamir Bohra, Wonkyu Kim, Maulik V Vaghela, Duncan Laurie, 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 (#10). 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. On SKL/KBL/CML CPUID.15h.ecx = nominal core crystal clock = 0 Hz hence we had to use static table to calculate crystal clock. Recommendation is to make use of CPUID.16h where crystal clock frequency was not reported by CPUID.15h to calculate the 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, 77 insertions(+), 117 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35148/10 -- 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: 10 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: Duncan Laurie <dlaurie(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-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newpatchset

1 0

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

02 Sep '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 9: (1 comment) https://review.coreboot.org/c/coreboot/+/35148/9//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35148/9//COMMIT_MSG@16 PS9, Line 16: On SKL/KBL/CML CPUID.15h.ecx = nominal core crystal clock = 0 Hz > so how are they handled now? isn't that i have added in above line Recommendation is to make use of CPUID.16h where crystal clock frequency was not reported by CPUID.15h to calculate the crystal clock. -- 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: 9 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: Duncan Laurie <dlaurie(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-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Comment-Date: Mon, 02 Sep 2019 13:44:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device/i2c_bus: Add i2c_dev_readb_at_word()
by Nico Huber (Code Review) 02 Sep '19

02 Sep '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29478 ) Change subject: device/i2c_bus: Add i2c_dev_readb_at_word() ...................................................................... Patch Set 8: (2 comments) https://review.coreboot.org/c/coreboot/+/29478/8//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/29478/8//COMMIT_MSG@10 PS8, Line 10: chip (for larger EEPROM parts). not true (anymore?) https://review.coreboot.org/c/coreboot/+/29478/8/src/include/device/i2c_bus… File src/include/device/i2c_bus.h: https://review.coreboot.org/c/coreboot/+/29478/8/src/include/device/i2c_bus… PS8, Line 99: uint8_t *buf, size_t len, uint16_t off); Not sure how it happened, but this signature, the function name and the commit message are out of sync. As this function doesn't read a single byte but a string of bytes, it should be named i2c_dev_read_at_word or maybe i2c_dev_read_at16 ? -- To view, visit https://review.coreboot.org/c/coreboot/+/29478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7516f3e5d9aca362c2b340aa5627d91510c09412 Gerrit-Change-Number: 29478 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 02 Sep 2019 11:12:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: ec/kontron/kempld: Select DRIVERS_UART_8250IO
by Thomas Heijligen (Code Review) 02 Sep '19

02 Sep '19

Hello Nico Huber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/31366 to review the following change. Change subject: ec/kontron/kempld: Select DRIVERS_UART_8250IO ...................................................................... ec/kontron/kempld: Select DRIVERS_UART_8250IO Change-Id: I1d0a46b6e4fc3aea403e2adce987de30703358c7 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M src/ec/kontron/kempld/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/31366/1 diff --git a/src/ec/kontron/kempld/Kconfig b/src/ec/kontron/kempld/Kconfig index 647bd12..08401c9 100644 --- a/src/ec/kontron/kempld/Kconfig +++ b/src/ec/kontron/kempld/Kconfig @@ -1,5 +1,6 @@ config EC_KONTRON_KEMPLD bool + select DRIVERS_UART_8250IO help Driver for Kontron's express module programmable logic device used on their COMexpress modules. -- To view, visit https://review.coreboot.org/c/coreboot/+/31366 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d0a46b6e4fc3aea403e2adce987de30703358c7 Gerrit-Change-Number: 31366 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

3 3

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2019